All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Input: elantech - correct x, y value range for v2 hardware
@ 2011-08-15  3:51 JJ Ding
  2011-08-15  5:25 ` Daniel Kurtz
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: JJ Ding @ 2011-08-15  3:51 UTC (permalink / raw)
  To: linux-input
  Cc: Tom Lin, Aaron Huang, Dmitry Torokhov, Éric Piel,
	Daniel Kurtz, JJ Ding

x, y values are actually 12-bit long. Also update protocol document to reflect
the change.

Signed-off-by: JJ Ding <jj_ding@emc.com.tw>
---
Hello List,

We are working on adding support for newer elantech touchpad, which we will
submit shortly. Along the way I found this error in current code.
This is a simple patch to correct x, y value ranges in v2 hardware.
Please review, thank you.

 Documentation/input/elantech.txt |    8 ++++----
 drivers/input/mouse/elantech.c   |    8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/input/elantech.txt b/Documentation/input/elantech.txt
index db798af..bce9941 100644
--- a/Documentation/input/elantech.txt
+++ b/Documentation/input/elantech.txt
@@ -389,14 +389,14 @@ byte 0:
 byte 1:
 
    bit   7   6   5   4   3   2   1   0
-	 p7  p6  p5  p4  .  x10 x9  x8
+	 p7  p6  p5  p4 x11 x10 x9  x8
 
 byte 2:
 
    bit   7   6   5   4   3   2   1   0
 	 x7  x6  x5  x4  x3  x2  x1  x0
 
-         x10..x0 = absolute x value (horizontal)
+         x11..x0 = absolute x value (horizontal)
 
 byte 3:
 
@@ -420,7 +420,7 @@ byte 3:
 byte 4:
 
    bit   7   6   5   4   3   2   1   0
-        p3  p1  p2  p0   .   .  y9  y8
+        p3  p1  p2  p0  y11 y10 y9  y8
 
 	 p7..p0 = pressure (not EF113)
 
@@ -429,7 +429,7 @@ byte 5:
    bit   7   6   5   4   3   2   1   0
         y7  y6  y5  y4  y3  y2  y1  y0
 
-         y9..y0 = absolute y value (vertical)
+         y11..y0 = absolute y value (vertical)
 
 
 4.2.2 Two finger touch
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 3250356..da161da 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -290,15 +290,15 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
 		/* pass through... */
 	case 1:
 		/*
-		 * byte 1:  .   .   .   .   .  x10 x9  x8
+		 * byte 1:  .   .   .   .  x11 x10 x9  x8
 		 * byte 2: x7  x6  x5  x4  x4  x2  x1  x0
 		 */
-		x1 = ((packet[1] & 0x07) << 8) | packet[2];
+		x1 = ((packet[1] & 0x0f) << 8) | packet[2];
 		/*
-		 * byte 4:  .   .   .   .   .   .  y9  y8
+		 * byte 4:  .   .   .   .  y11 y10 y9  y8
 		 * byte 5: y7  y6  y5  y4  y3  y2  y1  y0
 		 */
-		y1 = ETP_YMAX_V2 - (((packet[4] & 0x03) << 8) | packet[5]);
+		y1 = ETP_YMAX_V2 - (((packet[4] & 0x0f) << 8) | packet[5]);
 
 		input_report_abs(dev, ABS_X, x1);
 		input_report_abs(dev, ABS_Y, y1);
-- 
1.7.4.1


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

* Re: [PATCH] Input: elantech - correct x, y value range for v2 hardware
  2011-08-15  3:51 [PATCH] Input: elantech - correct x, y value range for v2 hardware JJ Ding
@ 2011-08-15  5:25 ` Daniel Kurtz
  2011-08-15 12:45 ` Éric Piel
  2011-08-16 11:11 ` JJ Ding
  2 siblings, 0 replies; 10+ messages in thread
From: Daniel Kurtz @ 2011-08-15  5:25 UTC (permalink / raw)
  To: JJ Ding
  Cc: linux-input, Tom Lin, Aaron Huang, Dmitry Torokhov, Éric Piel

Hi JJ,

On Mon, Aug 15, 2011 at 11:51 AM, JJ Ding <jj_ding@emc.com.tw> wrote:
>
> x, y values are actually 12-bit long. Also update protocol document to reflect
> the change.
>
> Signed-off-by: JJ Ding <jj_ding@emc.com.tw>

Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>

> ---
> Hello List,
>
> We are working on adding support for newer elantech touchpad, which we will
> submit shortly. Along the way I found this error in current code.
> This is a simple patch to correct x, y value ranges in v2 hardware.
> Please review, thank you.
>
>  Documentation/input/elantech.txt |    8 ++++----
>  drivers/input/mouse/elantech.c   |    8 ++++----
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/input/elantech.txt b/Documentation/input/elantech.txt
> index db798af..bce9941 100644
> --- a/Documentation/input/elantech.txt
> +++ b/Documentation/input/elantech.txt
> @@ -389,14 +389,14 @@ byte 0:
>  byte 1:
>
>    bit   7   6   5   4   3   2   1   0
> -        p7  p6  p5  p4  .  x10 x9  x8
> +        p7  p6  p5  p4 x11 x10 x9  x8
>
>  byte 2:
>
>    bit   7   6   5   4   3   2   1   0
>         x7  x6  x5  x4  x3  x2  x1  x0
>
> -         x10..x0 = absolute x value (horizontal)
> +         x11..x0 = absolute x value (horizontal)
>
>  byte 3:
>
> @@ -420,7 +420,7 @@ byte 3:
>  byte 4:
>
>    bit   7   6   5   4   3   2   1   0
> -        p3  p1  p2  p0   .   .  y9  y8
> +        p3  p1  p2  p0  y11 y10 y9  y8
>
>         p7..p0 = pressure (not EF113)
>
> @@ -429,7 +429,7 @@ byte 5:
>    bit   7   6   5   4   3   2   1   0
>         y7  y6  y5  y4  y3  y2  y1  y0
>
> -         y9..y0 = absolute y value (vertical)
> +         y11..y0 = absolute y value (vertical)
>
>
>  4.2.2 Two finger touch
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index 3250356..da161da 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -290,15 +290,15 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
>                /* pass through... */
>        case 1:
>                /*
> -                * byte 1:  .   .   .   .   .  x10 x9  x8
> +                * byte 1:  .   .   .   .  x11 x10 x9  x8
>                 * byte 2: x7  x6  x5  x4  x4  x2  x1  x0
>                 */
> -               x1 = ((packet[1] & 0x07) << 8) | packet[2];
> +               x1 = ((packet[1] & 0x0f) << 8) | packet[2];
>                /*
> -                * byte 4:  .   .   .   .   .   .  y9  y8
> +                * byte 4:  .   .   .   .  y11 y10 y9  y8
>                 * byte 5: y7  y6  y5  y4  y3  y2  y1  y0
>                 */
> -               y1 = ETP_YMAX_V2 - (((packet[4] & 0x03) << 8) | packet[5]);
> +               y1 = ETP_YMAX_V2 - (((packet[4] & 0x0f) << 8) | packet[5]);
>
>                input_report_abs(dev, ABS_X, x1);
>                input_report_abs(dev, ABS_Y, y1);
> --
> 1.7.4.1
>

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

* Re: [PATCH] Input: elantech - correct x, y value range for v2 hardware
  2011-08-15  3:51 [PATCH] Input: elantech - correct x, y value range for v2 hardware JJ Ding
  2011-08-15  5:25 ` Daniel Kurtz
@ 2011-08-15 12:45 ` Éric Piel
  2011-08-16  1:12   ` JJ Ding
  2011-08-16 11:11 ` JJ Ding
  2 siblings, 1 reply; 10+ messages in thread
From: Éric Piel @ 2011-08-15 12:45 UTC (permalink / raw)
  To: JJ Ding, Dmitry Torokhov
  Cc: linux-input, Tom Lin, Aaron Huang, Daniel Kurtz, Jocelyn Heuzé

On 15-08-11 05:51, JJ Ding wrote:
> x, y values are actually 12-bit long. Also update protocol document to reflect
> the change.
>
> Signed-off-by: JJ Ding<jj_ding@emc.com.tw>
> ---
> Hello List,
>
> We are working on adding support for newer elantech touchpad, which we will
> submit shortly. Along the way I found this error in current code.
> This is a simple patch to correct x, y value ranges in v2 hardware.
> Please review, thank you.
Hello,

I'm glad to hear Elantech is planning to contribute directly to the 
Linux driver!

In your patch, be careful to the Y axis. ETP_YMAX_V2 is actually set to 
760. Compared to the 4095 that can be reached, there might be overflow.

Do all the v2 hardwares have the same resolution (in which case we can 
leave 760, or 768)? Or is there a way to query their resolution? 
Otherwise, we could just put ETP_YMAX_V2 = 4095, and ETP_YMIN_V2 = (4095 
- 768), to be sure we cannot have an overflow. I think that's what is 
done in the synaptics driver.

Dmitry, would this seems the correct way to set min/max in 
input_set_abs_params()?

Cheers,
Éric


>
>   Documentation/input/elantech.txt |    8 ++++----
>   drivers/input/mouse/elantech.c   |    8 ++++----
>   2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/input/elantech.txt b/Documentation/input/elantech.txt
> index db798af..bce9941 100644
> --- a/Documentation/input/elantech.txt
> +++ b/Documentation/input/elantech.txt
> @@ -389,14 +389,14 @@ byte 0:
>   byte 1:
>
>      bit   7   6   5   4   3   2   1   0
> -	 p7  p6  p5  p4  .  x10 x9  x8
> +	 p7  p6  p5  p4 x11 x10 x9  x8
>
>   byte 2:
>
>      bit   7   6   5   4   3   2   1   0
>   	 x7  x6  x5  x4  x3  x2  x1  x0
>
> -         x10..x0 = absolute x value (horizontal)
> +         x11..x0 = absolute x value (horizontal)
>
>   byte 3:
>
> @@ -420,7 +420,7 @@ byte 3:
>   byte 4:
>
>      bit   7   6   5   4   3   2   1   0
> -        p3  p1  p2  p0   .   .  y9  y8
> +        p3  p1  p2  p0  y11 y10 y9  y8
>
>   	 p7..p0 = pressure (not EF113)
>
> @@ -429,7 +429,7 @@ byte 5:
>      bit   7   6   5   4   3   2   1   0
>           y7  y6  y5  y4  y3  y2  y1  y0
>
> -         y9..y0 = absolute y value (vertical)
> +         y11..y0 = absolute y value (vertical)
>
>
>   4.2.2 Two finger touch
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index 3250356..da161da 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -290,15 +290,15 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
>   		/* pass through... */
>   	case 1:
>   		/*
> -		 * byte 1:  .   .   .   .   .  x10 x9  x8
> +		 * byte 1:  .   .   .   .  x11 x10 x9  x8
>   		 * byte 2: x7  x6  x5  x4  x4  x2  x1  x0
>   		 */
> -		x1 = ((packet[1]&  0x07)<<  8) | packet[2];
> +		x1 = ((packet[1]&  0x0f)<<  8) | packet[2];
>   		/*
> -		 * byte 4:  .   .   .   .   .   .  y9  y8
> +		 * byte 4:  .   .   .   .  y11 y10 y9  y8
>   		 * byte 5: y7  y6  y5  y4  y3  y2  y1  y0
>   		 */
> -		y1 = ETP_YMAX_V2 - (((packet[4]&  0x03)<<  8) | packet[5]);
> +		y1 = ETP_YMAX_V2 - (((packet[4]&  0x0f)<<  8) | packet[5]);
>
>   		input_report_abs(dev, ABS_X, x1);
>   		input_report_abs(dev, ABS_Y, y1);

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

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

* Re: [PATCH] Input: elantech - correct x, y value range for v2 hardware
  2011-08-15 12:45 ` Éric Piel
@ 2011-08-16  1:12   ` JJ Ding
  0 siblings, 0 replies; 10+ messages in thread
From: JJ Ding @ 2011-08-16  1:12 UTC (permalink / raw)
  To: Éric Piel, Dmitry Torokhov
  Cc: linux-input, Tom Lin, Aaron Huang, Daniel Kurtz, Jocelyn Heuzé

Hi Éric,

On Mon, 15 Aug 2011 14:45:24 +0200, Éric Piel <E.A.B.Piel@tudelft.nl> wrote:
> On 15-08-11 05:51, JJ Ding wrote:
> > x, y values are actually 12-bit long. Also update protocol document to reflect
> > the change.
> >
> > Signed-off-by: JJ Ding<jj_ding@emc.com.tw>
> > ---
> > Hello List,
> >
> > We are working on adding support for newer elantech touchpad, which we will
> > submit shortly. Along the way I found this error in current code.
> > This is a simple patch to correct x, y value ranges in v2 hardware.
> > Please review, thank you.
> Hello,
> 
> I'm glad to hear Elantech is planning to contribute directly to the 
> Linux driver!
I am glad, too. :)
> 
> In your patch, be careful to the Y axis. ETP_YMAX_V2 is actually set to 
> 760. Compared to the 4095 that can be reached, there might be overflow.
> 
> Do all the v2 hardwares have the same resolution (in which case we can 
> leave 760, or 768)? Or is there a way to query their resolution? 
Newer hardware does have a way to query max x, y ranges. We are going to
submit such a patch, along with other newer hardware support code.
I personally hope to submit the patchset in a few days.
> Otherwise, we could just put ETP_YMAX_V2 = 4095, and ETP_YMIN_V2 = (4095 
> - 768), to be sure we cannot have an overflow. I think that's what is 
> done in the synaptics driver.
> 
> Dmitry, would this seems the correct way to set min/max in 
> input_set_abs_params()?
> 
> Cheers,
> Éric
> 
> 
> >
> >   Documentation/input/elantech.txt |    8 ++++----
> >   drivers/input/mouse/elantech.c   |    8 ++++----
> >   2 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/input/elantech.txt b/Documentation/input/elantech.txt
> > index db798af..bce9941 100644
> > --- a/Documentation/input/elantech.txt
> > +++ b/Documentation/input/elantech.txt
> > @@ -389,14 +389,14 @@ byte 0:
> >   byte 1:
> >
> >      bit   7   6   5   4   3   2   1   0
> > -	 p7  p6  p5  p4  .  x10 x9  x8
> > +	 p7  p6  p5  p4 x11 x10 x9  x8
> >
> >   byte 2:
> >
> >      bit   7   6   5   4   3   2   1   0
> >   	 x7  x6  x5  x4  x3  x2  x1  x0
> >
> > -         x10..x0 = absolute x value (horizontal)
> > +         x11..x0 = absolute x value (horizontal)
> >
> >   byte 3:
> >
> > @@ -420,7 +420,7 @@ byte 3:
> >   byte 4:
> >
> >      bit   7   6   5   4   3   2   1   0
> > -        p3  p1  p2  p0   .   .  y9  y8
> > +        p3  p1  p2  p0  y11 y10 y9  y8
> >
> >   	 p7..p0 = pressure (not EF113)
> >
> > @@ -429,7 +429,7 @@ byte 5:
> >      bit   7   6   5   4   3   2   1   0
> >           y7  y6  y5  y4  y3  y2  y1  y0
> >
> > -         y9..y0 = absolute y value (vertical)
> > +         y11..y0 = absolute y value (vertical)
> >
> >
> >   4.2.2 Two finger touch
> > diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> > index 3250356..da161da 100644
> > --- a/drivers/input/mouse/elantech.c
> > +++ b/drivers/input/mouse/elantech.c
> > @@ -290,15 +290,15 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
> >   		/* pass through... */
> >   	case 1:
> >   		/*
> > -		 * byte 1:  .   .   .   .   .  x10 x9  x8
> > +		 * byte 1:  .   .   .   .  x11 x10 x9  x8
> >   		 * byte 2: x7  x6  x5  x4  x4  x2  x1  x0
> >   		 */
> > -		x1 = ((packet[1]&  0x07)<<  8) | packet[2];
> > +		x1 = ((packet[1]&  0x0f)<<  8) | packet[2];
> >   		/*
> > -		 * byte 4:  .   .   .   .   .   .  y9  y8
> > +		 * byte 4:  .   .   .   .  y11 y10 y9  y8
> >   		 * byte 5: y7  y6  y5  y4  y3  y2  y1  y0
> >   		 */
> > -		y1 = ETP_YMAX_V2 - (((packet[4]&  0x03)<<  8) | packet[5]);
> > +		y1 = ETP_YMAX_V2 - (((packet[4]&  0x0f)<<  8) | packet[5]);
> >
> >   		input_report_abs(dev, ABS_X, x1);
> >   		input_report_abs(dev, ABS_Y, y1);
> 
> --
> 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

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

* Re: [PATCH] Input: elantech - correct x, y value range for v2 hardware
  2011-08-15  3:51 [PATCH] Input: elantech - correct x, y value range for v2 hardware JJ Ding
  2011-08-15  5:25 ` Daniel Kurtz
  2011-08-15 12:45 ` Éric Piel
@ 2011-08-16 11:11 ` JJ Ding
  2011-08-16 21:50   ` Éric Piel
  2 siblings, 1 reply; 10+ messages in thread
From: JJ Ding @ 2011-08-16 11:11 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Tom Lin, Aaron Huang, Éric Piel, Daniel Kurtz

Hi Dmitry,

What do you think about this patch?
This is really a simple correction, I am wondering if it's OK to push
this to 3.1?

On Mon, 15 Aug 2011 11:51:35 +0800, JJ Ding <jj_ding@emc.com.tw> wrote:
> x, y values are actually 12-bit long. Also update protocol document to reflect
> the change.
> 
> Signed-off-by: JJ Ding <jj_ding@emc.com.tw>
> ---
> Hello List,
> 
> We are working on adding support for newer elantech touchpad, which we will
> submit shortly. Along the way I found this error in current code.
> This is a simple patch to correct x, y value ranges in v2 hardware.
> Please review, thank you.
> 
>  Documentation/input/elantech.txt |    8 ++++----
>  drivers/input/mouse/elantech.c   |    8 ++++----
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/input/elantech.txt b/Documentation/input/elantech.txt
> index db798af..bce9941 100644
> --- a/Documentation/input/elantech.txt
> +++ b/Documentation/input/elantech.txt
> @@ -389,14 +389,14 @@ byte 0:
>  byte 1:
>  
>     bit   7   6   5   4   3   2   1   0
> -	 p7  p6  p5  p4  .  x10 x9  x8
> +	 p7  p6  p5  p4 x11 x10 x9  x8
>  
>  byte 2:
>  
>     bit   7   6   5   4   3   2   1   0
>  	 x7  x6  x5  x4  x3  x2  x1  x0
>  
> -         x10..x0 = absolute x value (horizontal)
> +         x11..x0 = absolute x value (horizontal)
>  
>  byte 3:
>  
> @@ -420,7 +420,7 @@ byte 3:
>  byte 4:
>  
>     bit   7   6   5   4   3   2   1   0
> -        p3  p1  p2  p0   .   .  y9  y8
> +        p3  p1  p2  p0  y11 y10 y9  y8
>  
>  	 p7..p0 = pressure (not EF113)
>  
> @@ -429,7 +429,7 @@ byte 5:
>     bit   7   6   5   4   3   2   1   0
>          y7  y6  y5  y4  y3  y2  y1  y0
>  
> -         y9..y0 = absolute y value (vertical)
> +         y11..y0 = absolute y value (vertical)
>  
>  
>  4.2.2 Two finger touch
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index 3250356..da161da 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -290,15 +290,15 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
>  		/* pass through... */
>  	case 1:
>  		/*
> -		 * byte 1:  .   .   .   .   .  x10 x9  x8
> +		 * byte 1:  .   .   .   .  x11 x10 x9  x8
>  		 * byte 2: x7  x6  x5  x4  x4  x2  x1  x0
>  		 */
> -		x1 = ((packet[1] & 0x07) << 8) | packet[2];
> +		x1 = ((packet[1] & 0x0f) << 8) | packet[2];
>  		/*
> -		 * byte 4:  .   .   .   .   .   .  y9  y8
> +		 * byte 4:  .   .   .   .  y11 y10 y9  y8
>  		 * byte 5: y7  y6  y5  y4  y3  y2  y1  y0
>  		 */
> -		y1 = ETP_YMAX_V2 - (((packet[4] & 0x03) << 8) | packet[5]);
> +		y1 = ETP_YMAX_V2 - (((packet[4] & 0x0f) << 8) | packet[5]);
>  
>  		input_report_abs(dev, ABS_X, x1);
>  		input_report_abs(dev, ABS_Y, y1);
> -- 
> 1.7.4.1
> 

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

* Re: [PATCH] Input: elantech - correct x, y value range for v2 hardware
  2011-08-16 11:11 ` JJ Ding
@ 2011-08-16 21:50   ` Éric Piel
  2011-08-17  1:28     ` JJ Ding
  0 siblings, 1 reply; 10+ messages in thread
From: Éric Piel @ 2011-08-16 21:50 UTC (permalink / raw)
  To: JJ Ding; +Cc: Dmitry Torokhov, linux-input, Tom Lin, Aaron Huang, Daniel Kurtz

On 16-08-11 13:11, JJ Ding wrote:
> Hi Dmitry,
>
> What do you think about this patch?
> This is really a simple correction, I am wondering if it's OK to push
> this to 3.1?
Hi JJ Ding,
The merge window of 3.1 is already closed, so only patches which fix 
bugs are accepted. Is there any hardware already available for which 
this patch solves a bug? That would be a great incentive to push it :-)

Éric

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

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

* Re: [PATCH] Input: elantech - correct x, y value range for v2 hardware
  2011-08-16 21:50   ` Éric Piel
@ 2011-08-17  1:28     ` JJ Ding
  2011-08-17  6:43       ` Dmitry Torokhov
  0 siblings, 1 reply; 10+ messages in thread
From: JJ Ding @ 2011-08-17  1:28 UTC (permalink / raw)
  To: Éric Piel
  Cc: Dmitry Torokhov, linux-input, Tom Lin, Aaron Huang, Daniel Kurtz

Hi Éric,

Thanks for the reply.

On Tue, 16 Aug 2011 23:50:47 +0200, Éric Piel <E.A.B.Piel@tudelft.nl> wrote:
> On 16-08-11 13:11, JJ Ding wrote:
> > Hi Dmitry,
> >
> > What do you think about this patch?
> > This is really a simple correction, I am wondering if it's OK to push
> > this to 3.1?
> Hi JJ Ding,
> The merge window of 3.1 is already closed, so only patches which fix 
> bugs are accepted. Is there any hardware already available for which 
> this patch solves a bug? That would be a great incentive to push it
> > :-)
As far as I know, this doesn't fix a bug. I just want to make sure the
driver and what the firmware sends are consistent.

Thank you for telling me this. I will try this again with newer hardware
support patches, aiming for 3.2 inclusion.

jj

> Éric
> 
--
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

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

* Re: [PATCH] Input: elantech - correct x, y value range for v2 hardware
  2011-08-17  1:28     ` JJ Ding
@ 2011-08-17  6:43       ` Dmitry Torokhov
  2011-08-17  9:53         ` Éric Piel
  0 siblings, 1 reply; 10+ messages in thread
From: Dmitry Torokhov @ 2011-08-17  6:43 UTC (permalink / raw)
  To: JJ Ding; +Cc: Éric Piel, linux-input, Tom Lin, Aaron Huang, Daniel Kurtz

Hi JJ,

On Wed, Aug 17, 2011 at 09:28:04AM +0800, JJ Ding wrote:
> Hi Éric,
> 
> Thanks for the reply.
> 
> On Tue, 16 Aug 2011 23:50:47 +0200, Éric Piel <E.A.B.Piel@tudelft.nl> wrote:
> > On 16-08-11 13:11, JJ Ding wrote:
> > > Hi Dmitry,
> > >
> > > What do you think about this patch?
> > > This is really a simple correction, I am wondering if it's OK to push
> > > this to 3.1?
> > Hi JJ Ding,
> > The merge window of 3.1 is already closed, so only patches which fix 
> > bugs are accepted. Is there any hardware already available for which 
> > this patch solves a bug? That would be a great incentive to push it
> > > :-)
> As far as I know, this doesn't fix a bug. I just want to make sure the
> driver and what the firmware sends are consistent.

As Éric mentioned, simply extending range to 12 bits is dangerous
because we might cause overflows. Blindly increasing ETP_YMAX_V2 is not
a good idea either as userspace would expect much larger device and
reports would only cover fraction of its surface. We need to separate
ETP_YMAX_V2 as absolute maximum from reported coordinates range, like it
is done in synaptics driver.

> 
> Thank you for telling me this. I will try this again with newer hardware
> support patches, aiming for 3.2 inclusion.

Yes, if hardware currently supported by mainline version of the driver
is not affected it is better to queue the patch[es] for 3.2. BTW, that
means that you should not wait till 3.2 merge window opens as we'll need
time to review the patches and get them into my 'next' branch.

Thanks.

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

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

* Re: [PATCH] Input: elantech - correct x, y value range for v2 hardware
  2011-08-17  6:43       ` Dmitry Torokhov
@ 2011-08-17  9:53         ` Éric Piel
  2011-08-17 16:37           ` Dmitry Torokhov
  0 siblings, 1 reply; 10+ messages in thread
From: Éric Piel @ 2011-08-17  9:53 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: JJ Ding, linux-input, Tom Lin, Aaron Huang, Daniel Kurtz

Op 17-08-11 08:43, Dmitry Torokhov schreef:
> Hi JJ,
>
>> As far as I know, this doesn't fix a bug. I just want to make sure the
>> driver and what the firmware sends are consistent.
>
> As Éric mentioned, simply extending range to 12 bits is dangerous
> because we might cause overflows. Blindly increasing ETP_YMAX_V2 is not
> a good idea either as userspace would expect much larger device and
> reports would only cover fraction of its surface. We need to separate
> ETP_YMAX_V2 as absolute maximum from reported coordinates range, like it
> is done in synaptics driver.
Hi Dmitry,
Concerning this point, I was wondering if it'd be ok to only send 
negative values? Such as min/max are between -768 and 0. So that, if the 
hardware actually has a bigger resolution, it can go down to -4095 
without any overflow? It sounds the simplest way to report the 
coordinate for a device which has an opposite axis. Or is it safer to 
stay in the positive numbers, with min/max between 3328 and 4095, and in 
case of bigger resolution the values go down to 0?

Cheers,
Éric
--
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

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

* Re: [PATCH] Input: elantech - correct x, y value range for v2 hardware
  2011-08-17  9:53         ` Éric Piel
@ 2011-08-17 16:37           ` Dmitry Torokhov
  0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2011-08-17 16:37 UTC (permalink / raw)
  To: Éric Piel; +Cc: JJ Ding, linux-input, Tom Lin, Aaron Huang, Daniel Kurtz

On Wed, Aug 17, 2011 at 11:53:55AM +0200, Éric Piel wrote:
> Op 17-08-11 08:43, Dmitry Torokhov schreef:
> >Hi JJ,
> >
> >>As far as I know, this doesn't fix a bug. I just want to make sure the
> >>driver and what the firmware sends are consistent.
> >
> >As Éric mentioned, simply extending range to 12 bits is dangerous
> >because we might cause overflows. Blindly increasing ETP_YMAX_V2 is not
> >a good idea either as userspace would expect much larger device and
> >reports would only cover fraction of its surface. We need to separate
> >ETP_YMAX_V2 as absolute maximum from reported coordinates range, like it
> >is done in synaptics driver.
> Hi Dmitry,
> Concerning this point, I was wondering if it'd be ok to only send
> negative values? Such as min/max are between -768 and 0. So that, if
> the hardware actually has a bigger resolution, it can go down to
> -4095 without any overflow?

Theoretically it should be possible, at least nothing in evdev protocol
says that absolute coordinates shoudl be positive. It is certainly not
true for joystick-like devvices. But I am concerned about users (as
in applications/drivers) getting negative coordinates for
touchpad/touchscreen like devices. I believe first reaction is to expect
non-negative values.

> It sounds the simplest way to report the
> coordinate for a device which has an opposite axis. Or is it safer
> to stay in the positive numbers, with min/max between 3328 and 4095,
> and in case of bigger resolution the values go down to 0?

I think this is the best way. EVIOCGABS gives the "window" into the
expected range of coordinates emitted by the device, so as long as we
keep the scale (i.e. not reporting min/max as 0-4096 while actual
coordinates go only 0-768) it should work fine.

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

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

end of thread, other threads:[~2011-08-17 16:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-15  3:51 [PATCH] Input: elantech - correct x, y value range for v2 hardware JJ Ding
2011-08-15  5:25 ` Daniel Kurtz
2011-08-15 12:45 ` Éric Piel
2011-08-16  1:12   ` JJ Ding
2011-08-16 11:11 ` JJ Ding
2011-08-16 21:50   ` Éric Piel
2011-08-17  1:28     ` JJ Ding
2011-08-17  6:43       ` Dmitry Torokhov
2011-08-17  9:53         ` Éric Piel
2011-08-17 16:37           ` Dmitry Torokhov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.