[patch] add module parameter to provide hardware-calibrated coordinate data for MicroTouch/3M touchscreens

[patch] add module parameter to provide hardware-calibrated coordinate data for MicroTouch/3M touchscreens

[Dan Streetman]

Back in mtouchusb, I added the "raw_coordinates" module parameter to
make it possible to get the hw-calib data:
http://marc.info/?l=linux-usb-devel&m=110968309531306&w=2

When mtouchusb was integrated into the usbtouchscreen driver, that
ability wasn't carried over - it reports only the raw coordinates
again.

This patch restores the ability to get hardware-calibrated coordinate
data, and defaults to the raw data so existing systems with a
microtouch touchscreen won't see any changes.

Note that the Y axis is inverted as reported by the touchscreen, so
this reverses the Y min and max (0xffff for the min, 0 for the max).
This works fine on my system using the X evdev driver.

(I'm sending this from gmail which hopefully won't mangle the
patch...I tested it and it seems ok)

Signed-off-by: Dan Streetman <ddstreet-EkmVulN54Sk@public.gmane.org>


--- a/drivers/input/touchscreen/usbtouchscreen.c	2008-11-12
13:06:23.000000000 -0500
+++ b/drivers/input/touchscreen/usbtouchscreen.c	2008-11-12
13:07:44.000000000 -0500
@@ -60,6 +60,10 @@ static int swap_xy;
 module_param(swap_xy, bool, 0644);
 MODULE_PARM_DESC(swap_xy, "If set X and Y axes are swapped.");

+static int hwcalib_xy = 0;
+module_param(hwcalib_xy, bool, 0444);
+MODULE_PARM_DESC(hwcalib_xy, "If set hw-calibrated X/Y are used if available");
+
 /* device specifc data/functions */
 struct usbtouch_usb;
 struct usbtouch_device_info {
@@ -260,8 +264,13 @@ static int panjit_read_data(struct usbto

 static int mtouch_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
 {
-	dev->x = (pkt[8] << 8) | pkt[7];
-	dev->y = (pkt[10] << 8) | pkt[9];
+	if (hwcalib_xy) {
+		dev->x = (pkt[4] << 8) | pkt[3];
+		dev->y = (pkt[6] << 8) | pkt[5];
+	} else {
+		dev->x = (pkt[8] << 8) | pkt[7];
+		dev->y = (pkt[10] << 8) | pkt[9];
+	}
 	dev->touch = (pkt[2] & 0x40) ? 1 : 0;

 	return 1;
@@ -294,6 +303,12 @@ static int mtouch_init(struct usbtouch_u
 			return ret;
 	}

+	// Default min/max xy are the raw values, override if using hw-calib
+	if (hwcalib_xy) {
+		input_set_abs_params(usbtouch->input, ABS_X, 0, 0xffff, 0, 0);
+		input_set_abs_params(usbtouch->input, ABS_Y, 0xffff, 0, 0, 0);
+	}
+
 	return 0;
 }
 #endif
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] add module parameter to provide hardware-calibrated coordinate data for MicroTouch/3M touchscreens

[Greg KH]

On Thu, Nov 13, 2008 at 03:22:52PM -0500, Dan Streetman wrote:
> Back in mtouchusb, I added the "raw_coordinates" module parameter to
> make it possible to get the hw-calib data:
> http://marc.info/?l=linux-usb-devel&m=110968309531306&w=2
> 
> When mtouchusb was integrated into the usbtouchscreen driver, that
> ability wasn't carried over - it reports only the raw coordinates
> again.
> 
> This patch restores the ability to get hardware-calibrated coordinate
> data, and defaults to the raw data so existing systems with a
> microtouch touchscreen won't see any changes.
> 
> Note that the Y axis is inverted as reported by the touchscreen, so
> this reverses the Y min and max (0xffff for the min, 0 for the max).
> This works fine on my system using the X evdev driver.
> 
> (I'm sending this from gmail which hopefully won't mangle the
> patch...I tested it and it seems ok)
> 
> Signed-off-by: Dan Streetman <ddstreet-EkmVulN54Sk@public.gmane.org>
> 
> 
> --- a/drivers/input/touchscreen/usbtouchscreen.c	2008-11-12
> 13:06:23.000000000 -0500
> +++ b/drivers/input/touchscreen/usbtouchscreen.c	2008-11-12
> 13:07:44.000000000 -0500

Your patch is line-wrapped :(

> @@ -60,6 +60,10 @@ static int swap_xy;
>  module_param(swap_xy, bool, 0644);
>  MODULE_PARM_DESC(swap_xy, "If set X and Y axes are swapped.");
> 
> +static int hwcalib_xy = 0;
> +module_param(hwcalib_xy, bool, 0444);
> +MODULE_PARM_DESC(hwcalib_xy, "If set hw-calibrated X/Y are used if available");

Should this variable be allowed to be changed at run time?  That might
be useful if the code is built into the system.  If so, please change
the permissions on the module_param() call.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] add module parameter to provide hardware-calibrated coordinate data for MicroTouch/3M touchscreens

[Dan Streetman]

[-- Attachment #1: Type: text/plain, Size: 2930 bytes --]


On Thu, Nov 13, 2008 at 3:31 PM, Greg KH <greg-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> wrote:
 > On Thu, Nov 13, 2008 at 03:22:52PM -0500, Dan Streetman wrote:
 >>
 >> --- a/drivers/input/touchscreen/usbtouchscreen.c      2008-11-12
 >> 13:06:23.000000000 -0500
 >> +++ b/drivers/input/touchscreen/usbtouchscreen.c      2008-11-12
 >> 13:07:44.000000000 -0500
 >
 > Your patch is line-wrapped :(

Stupid gmail! :(
I'm trying now with thunderbird but I'm not sure if it has problems also or not.  I'm attaching the patch as well just in case :(

 >
 >> @@ -60,6 +60,10 @@ static int swap_xy;
 >>  module_param(swap_xy, bool, 0644);
 >>  MODULE_PARM_DESC(swap_xy, "If set X and Y axes are swapped.");
 >>
 >> +static int hwcalib_xy = 0;
 >> +module_param(hwcalib_xy, bool, 0444);
 >> +MODULE_PARM_DESC(hwcalib_xy, "If set hw-calibrated X/Y are used if available");
 >
 > Should this variable be allowed to be changed at run time?  That might
 > be useful if the code is built into the system.  If so, please change
 > the permissions on the module_param() call.

I set it as read only because I had to override the min/max coordinate values in the mtouch_init() function
if the hw-calibrated coordinates were being used, so if someone changed the value after a screen
was already connected, it would start reporting hw-calibrated coordinates but the min/max values
would still be the raw min/max...however, for people wanting to change the value and then hotplug
their touchscreens (to reset the min/max values), it would be useful.  Below is the patch with the
permissions adjusted.


--- a/drivers/input/touchscreen/usbtouchscreen.c	2008-11-12 13:06:23.000000000 -0500
+++ b/drivers/input/touchscreen/usbtouchscreen.c	2008-11-12 13:07:44.000000000 -0500
@@ -60,6 +60,10 @@ static int swap_xy;
  module_param(swap_xy, bool, 0644);
  MODULE_PARM_DESC(swap_xy, "If set X and Y axes are swapped.");

+static int hwcalib_xy = 0;
+module_param(hwcalib_xy, bool, 0644);
+MODULE_PARM_DESC(hwcalib_xy, "If set hw-calibrated X/Y are used if available");
+
  /* device specifc data/functions */
  struct usbtouch_usb;
  struct usbtouch_device_info {
@@ -260,8 +264,13 @@ static int panjit_read_data(struct usbto

  static int mtouch_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
  {
-	dev->x = (pkt[8] << 8) | pkt[7];
-	dev->y = (pkt[10] << 8) | pkt[9];
+	if (hwcalib_xy) {
+		dev->x = (pkt[4] << 8) | pkt[3];
+		dev->y = (pkt[6] << 8) | pkt[5];
+	} else {
+		dev->x = (pkt[8] << 8) | pkt[7];
+		dev->y = (pkt[10] << 8) | pkt[9];
+	}
  	dev->touch = (pkt[2] & 0x40) ? 1 : 0;

  	return 1;
@@ -294,6 +303,12 @@ static int mtouch_init(struct usbtouch_u
  			return ret;
  	}

+	// Default min/max xy are the raw values, override if using hw-calib
+	if (hwcalib_xy) {
+		input_set_abs_params(usbtouch->input, ABS_X, 0, 0xffff, 0, 0);
+		input_set_abs_params(usbtouch->input, ABS_Y, 0xffff, 0, 0, 0);
+	}
+
  	return 0;
  }
  #endif

[-- Attachment #2: patch-usbtouchscreen-hwcalib --]
[-- Type: text/plain, Size: 1334 bytes --]

--- a/drivers/input/touchscreen/usbtouchscreen.c	2008-11-12 13:06:23.000000000 -0500
+++ b/drivers/input/touchscreen/usbtouchscreen.c	2008-11-12 13:07:44.000000000 -0500
@@ -60,6 +60,10 @@ static int swap_xy;
 module_param(swap_xy, bool, 0644);
 MODULE_PARM_DESC(swap_xy, "If set X and Y axes are swapped.");
 
+static int hwcalib_xy = 0;
+module_param(hwcalib_xy, bool, 0644);
+MODULE_PARM_DESC(hwcalib_xy, "If set hw-calibrated X/Y are used if available");
+
 /* device specifc data/functions */
 struct usbtouch_usb;
 struct usbtouch_device_info {
@@ -260,8 +264,13 @@ static int panjit_read_data(struct usbto
 
 static int mtouch_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
 {
-	dev->x = (pkt[8] << 8) | pkt[7];
-	dev->y = (pkt[10] << 8) | pkt[9];
+	if (hwcalib_xy) {
+		dev->x = (pkt[4] << 8) | pkt[3];
+		dev->y = (pkt[6] << 8) | pkt[5];
+	} else {
+		dev->x = (pkt[8] << 8) | pkt[7];
+		dev->y = (pkt[10] << 8) | pkt[9];
+	}
 	dev->touch = (pkt[2] & 0x40) ? 1 : 0;
 
 	return 1;
@@ -294,6 +303,12 @@ static int mtouch_init(struct usbtouch_u
 			return ret;
 	}
 
+	// Default min/max xy are the raw values, override if using hw-calib
+	if (hwcalib_xy) {
+		input_set_abs_params(usbtouch->input, ABS_X, 0, 0xffff, 0, 0);
+		input_set_abs_params(usbtouch->input, ABS_Y, 0xffff, 0, 0, 0);
+	}
+
 	return 0;
 }
 #endif

Re: [patch] add module parameter to provide hardware-calibrated coordinate data for MicroTouch/3M touchscreens

[Alexey Klimov]

On Fri, Nov 14, 2008 at 2:25 AM, Dan Streetman <ddstreet@ieee.org> wrote:
>
> On Thu, Nov 13, 2008 at 3:31 PM, Greg KH <greg@kroah.com> wrote:
>> On Thu, Nov 13, 2008 at 03:22:52PM -0500, Dan Streetman wrote:
>>>
>>> --- a/drivers/input/touchscreen/usbtouchscreen.c      2008-11-12
>>> 13:06:23.000000000 -0500
>>> +++ b/drivers/input/touchscreen/usbtouchscreen.c      2008-11-12
>>> 13:07:44.000000000 -0500
>>
>> Your patch is line-wrapped :(
>
> Stupid gmail! :(
> I'm trying now with thunderbird but I'm not sure if it has problems also or
> not.  I'm attaching the patch as well just in case :(

<snip>

Hello, all
Dan, may i get you small advice ? There is file in
linux/Documentation/email-clients.txt that describes e-mail clients
and what you should do to send patch inlined. May be it will be
helpful to you :)
(Sorry for interruption)

-- 
Best regards, Klimov Alexey

Re: [patch] add module parameter to provide hardware-calibrated coordinate data for MicroTouch/3M touchscreens

[Greg KH]

On Thu, Nov 13, 2008 at 06:25:39PM -0500, Dan Streetman wrote:
>
> On Thu, Nov 13, 2008 at 3:31 PM, Greg KH <greg@kroah.com> wrote:
> > On Thu, Nov 13, 2008 at 03:22:52PM -0500, Dan Streetman wrote:
> >>
> >> --- a/drivers/input/touchscreen/usbtouchscreen.c      2008-11-12
> >> 13:06:23.000000000 -0500
> >> +++ b/drivers/input/touchscreen/usbtouchscreen.c      2008-11-12
> >> 13:07:44.000000000 -0500
> >
> > Your patch is line-wrapped :(
>
> Stupid gmail! :(
> I'm trying now with thunderbird but I'm not sure if it has problems also or 
> not.  I'm attaching the patch as well just in case :(

Still linewrapped (the attached version isn't but then you have 2 copies
of the same patch in one email, which our tools do not like...)

> >> @@ -60,6 +60,10 @@ static int swap_xy;
> >>  module_param(swap_xy, bool, 0644);
> >>  MODULE_PARM_DESC(swap_xy, "If set X and Y axes are swapped.");
> >>
> >> +static int hwcalib_xy = 0;
> >> +module_param(hwcalib_xy, bool, 0444);
> >> +MODULE_PARM_DESC(hwcalib_xy, "If set hw-calibrated X/Y are used if 
> available");
> >
> > Should this variable be allowed to be changed at run time?  That might
> > be useful if the code is built into the system.  If so, please change
> > the permissions on the module_param() call.
>
> I set it as read only because I had to override the min/max coordinate 
> values in the mtouch_init() function
> if the hw-calibrated coordinates were being used, so if someone changed the 
> value after a screen
> was already connected, it would start reporting hw-calibrated coordinates 
> but the min/max values
> would still be the raw min/max...however, for people wanting to change the 
> value and then hotplug
> their touchscreens (to reset the min/max values), it would be useful.  
> Below is the patch with the
> permissions adjusted.

This kind of implies that you want this setting on a per-device basis,
right?  Why not make it attached to the individual device instead of a
system-wide option?

thanks,

greg k-h

Re: [patch] add module parameter to provide hardware-calibrated coordinate data for MicroTouch/3M touchscreens

[Dan Streetman]

On Thu, Nov 13, 2008 at 7:03 PM, Greg KH <greg-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> wrote:
>
> On Thu, Nov 13, 2008 at 06:25:39PM -0500, Dan Streetman wrote:
> >
> > On Thu, Nov 13, 2008 at 3:31 PM, Greg KH <greg-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> wrote:
> > > On Thu, Nov 13, 2008 at 03:22:52PM -0500, Dan Streetman wrote:
> > >>
>
> Still linewrapped (the attached version isn't but then you have 2 copies
> of the same patch in one email, which our tools do not like...)

Ok, back to (al)pine it is then for patch submissions :)

I can't believe it's been so long since I used pine or sent a patch in...

>
> > >> @@ -60,6 +60,10 @@ static int swap_xy;
> > >>  module_param(swap_xy, bool, 0644);
> > >>  MODULE_PARM_DESC(swap_xy, "If set X and Y axes are swapped.");
> > >>
> > >> +static int hwcalib_xy = 0;
> > >> +module_param(hwcalib_xy, bool, 0444);
> > >> +MODULE_PARM_DESC(hwcalib_xy, "If set hw-calibrated X/Y are used if
> > available");
> > >
> > > Should this variable be allowed to be changed at run time?  That might
> > > be useful if the code is built into the system.  If so, please change
> > > the permissions on the module_param() call.
> >
> > I set it as read only because I had to override the min/max coordinate
> > values in the mtouch_init() function
> > if the hw-calibrated coordinates were being used, so if someone changed the
> > value after a screen
> > was already connected, it would start reporting hw-calibrated coordinates
> > but the min/max values
> > would still be the raw min/max...however, for people wanting to change the
> > value and then hotplug
> > their touchscreens (to reset the min/max values), it would be useful.
> > Below is the patch with the
> > permissions adjusted.
>
> This kind of implies that you want this setting on a per-device basis,
> right?  Why not make it attached to the individual device instead of a
> system-wide option?

Oh no, I don't think it is useful on a per-device basis; I was only concerned 
about changing the value after a touchscreen had been connected.

I hadn't intended to get into the details of whether I think the raw coordinate 
data or hardware-calibrated coordinate data is better, but I guess I might as 
well :)

My opinion is that there is no benefit in using the raw coordinate data, at all.  
All the advantages are with the hardware-calibrated coordinate data:

1) The hw-calibrated data has a higher resolution, of 0xffff on each axis; the 
raw coordinate data has a resolution of 0x4000 on each axis.  No benefit to the 
raw data here.

2) The raw coordinates will never match the screen position, regardless of 
whether the screen is hardware-calibrated.  The hw-calibrated coordinates will 
match exactly the screen position if the screen is hw-calbrated.

3) With capacitive and resistive touchscreens, the raw coordinate values "drift" 
over time.  So occasionally, some calibration has to be done.  If the raw data 
is being used, then software calibration has to be done; if the hw-calibrated 
data is being used, then *either* software calibration or hardware calibration 
can be done.

4) Backwards compatibility is broken if we switch, however.  Anyone who has 
their system set up with a Microtouch touchscreen with software calibration will 
have to re-perform the software calibration one time.

I'll briefly explain my understanding of what is involved from the user end with 
both approaches.  Etiher way, the usbtouchscreen module creates an evdev node 
with ABS_X and ABS_Y coordinates, and it reports the min and max of those 
coordinate ranges as 0 to 0x4000 for the raw data or 0 to 0xffff if changed to 
use the hw-calib data.  The xorg.conf file has a InputDevice section added to 
use the "evdev" driver.  The X evdev driver reads the evdev node's reported 
minimum and maximum ABS_X and ABS_Y values.  It uses those values to map all 
incoming coordinate points into a screen position.  When using the raw 
coordinate data, those min and max values *do not* correspond to the actual min 
and max values that will be sent when touching the corners of the screen, 
specifically because it is raw data - the actual min and max values will vary 
per touchscreen, and over time with "drift".  So the user *has* to manually find 
the actual min and max X and Y values and put that into the xorg.conf file so X 
will know what the correct values to use are.  However, when using the 
hardware-calibrated coordinates, if the touchscreen is correctly hw-calibrated, 
the min and max values of 0-0xffff do exactly match the values reported when 
touching the corners of the screen.  So there is no need to put any min/max 
values manually into the xorg.conf file.  In fact, with a hardware-calibrated 
touchscreen and a simple, generic evdev InputDevice section int the xorg.conf 
file, you can just plug the touchscreen in and it will immediately and fully 
automatically work with the cursor directly under your finger.

Now, I realize I've been saying "with a hw-calibrated touchscreen".  So how is 
it hw-calibrated?  Right now, either by using one fresh from manufacturing, or 
by using another system that does have a hw-calibration program (e.g. Windows).  
But, Microtouch does publish the full spec for these devices including how to 
hardware-calibrate them, and it is actually rather easy.  Part of my "next 
steps" is to add a simple way to perform hardware calibration on these 
touchscreens in Linux.

But I want to point out, that besides the one-time need to change any 
existing user's software calibration values, there is no difference from a 
software-calibration point of view between the raw coordinates and the 
hardware-calibration coordinates.  You can still use "touchcal" to get the 
actual min/max values and manually put that into your xorg.conf file.  Even if 
the touchscreen is not hardware-calibrated, it will work fine.


So, what I really think the patch should do is not add a module parameter, but 
simply change the values over to use the hw-calibrated coordinate data, and 
ignore the raw coordinate data.  Does everyone agree?


>
> thanks,
>
> greg k-h


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] add module parameter to provide hardware-calibrated coordinate data for MicroTouch/3M touchscreens

[Vojtech Pavlik]

On Thu, Nov 13, 2008 at 03:22:52PM -0500, Dan Streetman wrote:

> Note that the Y axis is inverted as reported by the touchscreen, so
> this reverses the Y min and max (0xffff for the min, 0 for the max).
> This works fine on my system using the X evdev driver.

> +	// Default min/max xy are the raw values, override if using hw-calib
> +	if (hwcalib_xy) {
> +		input_set_abs_params(usbtouch->input, ABS_X, 0, 0xffff, 0, 0);
> +		input_set_abs_params(usbtouch->input, ABS_Y, 0xffff, 0, 0, 0);

While it does work with the X evdev, please don't do it, and instead do
the inversion before reporting ABS_Y. Linux input devices should follow
the (USB) HID specification of axis direction and stick to it. The
behaviour with reversed min/max is undefined.

-- 
Vojtech Pavlik
Director SuSE Labs

Re: [patch] add module parameter to provide hardware-calibrated coordinate data for MicroTouch/3M touchscreens

[Dan Streetman]

Ok - I'll update the patch to invert Y before reporting.

On the issue of whether to use a module param to send raw coordinates
or hw-calibrated coordinates, or to just change the code to report the
hw-calibrated coordinates instead of the raw coordinates, what do you
think?

On Fri, Nov 14, 2008 at 4:31 PM, Vojtech Pavlik <vojtech@suse.cz> wrote:
> On Thu, Nov 13, 2008 at 03:22:52PM -0500, Dan Streetman wrote:
>
>> Note that the Y axis is inverted as reported by the touchscreen, so
>> this reverses the Y min and max (0xffff for the min, 0 for the max).
>> This works fine on my system using the X evdev driver.
>
>> +     // Default min/max xy are the raw values, override if using hw-calib
>> +     if (hwcalib_xy) {
>> +             input_set_abs_params(usbtouch->input, ABS_X, 0, 0xffff, 0, 0);
>> +             input_set_abs_params(usbtouch->input, ABS_Y, 0xffff, 0, 0, 0);
>
> While it does work with the X evdev, please don't do it, and instead do
> the inversion before reporting ABS_Y. Linux input devices should follow
> the (USB) HID specification of axis direction and stick to it. The
> behaviour with reversed min/max is undefined.
>
> --
> Vojtech Pavlik
> Director SuSE Labs
>

Re: [patch] add module parameter to provide hardware-calibrated coordinate data for MicroTouch/3M touchscreens

[Vojtech Pavlik]

On Fri, Nov 14, 2008 at 04:46:08PM -0500, Dan Streetman wrote:
> Ok - I'll update the patch to invert Y before reporting.
> 
> On the issue of whether to use a module param to send raw coordinates
> or hw-calibrated coordinates, or to just change the code to report the
> hw-calibrated coordinates instead of the raw coordinates, what do you
> think?

I'm in favor for the option, because of:

	* backwards compatibility 
	* the Linux input standard is unprocessed coordinate data
	  (but after compensating for all hardware quirks)
	* most touchscreens can't do HW calibration anyway, and we
	  benefit from common SW calibration code, eg in tslib

Actually, I don't see much value in the hw-calibrated data, but I see it
could be useful in some cases, so I'm still fine with adding the
possibility to use that mode.

Vojtech

> 
> On Fri, Nov 14, 2008 at 4:31 PM, Vojtech Pavlik <vojtech@suse.cz> wrote:
> > On Thu, Nov 13, 2008 at 03:22:52PM -0500, Dan Streetman wrote:
> >
> >> Note that the Y axis is inverted as reported by the touchscreen, so
> >> this reverses the Y min and max (0xffff for the min, 0 for the max).
> >> This works fine on my system using the X evdev driver.
> >
> >> +     // Default min/max xy are the raw values, override if using hw-calib
> >> +     if (hwcalib_xy) {
> >> +             input_set_abs_params(usbtouch->input, ABS_X, 0, 0xffff, 0, 0);
> >> +             input_set_abs_params(usbtouch->input, ABS_Y, 0xffff, 0, 0, 0);
> >
> > While it does work with the X evdev, please don't do it, and instead do
> > the inversion before reporting ABS_Y. Linux input devices should follow
> > the (USB) HID specification of axis direction and stick to it. The
> > behaviour with reversed min/max is undefined.
> >
> > --
> > Vojtech Pavlik
> > Director SuSE Labs
> >
> 

-- 
Vojtech Pavlik
Director SuSE Labs

Re: [patch] add module parameter to provide hardware-calibrated coordinate data for MicroTouch/3M touchscreens

[Dan Streetman]

On Sat, Nov 15, 2008 at 4:08 PM, Vojtech Pavlik <vojtech@suse.cz> wrote:
> On Fri, Nov 14, 2008 at 04:46:08PM -0500, Dan Streetman wrote:
>> Ok - I'll update the patch to invert Y before reporting.
>>
>> On the issue of whether to use a module param to send raw coordinates
>> or hw-calibrated coordinates, or to just change the code to report the
>> hw-calibrated coordinates instead of the raw coordinates, what do you
>> think?
>
> I'm in favor for the option, because of:
>
>        * backwards compatibility
>        * the Linux input standard is unprocessed coordinate data
>          (but after compensating for all hardware quirks)
>        * most touchscreens can't do HW calibration anyway, and we
>          benefit from common SW calibration code, eg in tslib
>
> Actually, I don't see much value in the hw-calibrated data, but I see it
> could be useful in some cases, so I'm still fine with adding the
> possibility to use that mode.

Just to point it out again because I think it's important...userspace can treat 
the hardware-calibrated coordinates *exactly the same* as the raw coordinates if 
using software calibration such as tslib.  So like I said there is not really a 
benefit there of using the raw coordinates, except for one-time backwards 
compatibility.  This is why I don't see the point in reporting the raw 
coordinates by default instead of the hw-calibrated coordinates - I think the 
module parameter is fine to let the user pick which one they want, but it should 
default to the hw-calibrated coordinates.

But, that's just my opinion :)  So here is the updated patch with the Y 
inversion and normal non-inverted Y min/max, the module parameter permissions 
set to 0644, and default to raw.


This patch adds a module parameter to report either the raw coordinate data or 
the hardware-calibrated coordinate data.  The default is set to the raw 
coordinates for backwards compatibilty.

Signed-off-by: Dan Streetman <ddstreet@ieee.org>



--- a/drivers/input/touchscreen/usbtouchscreen.c	2008-11-17 10:51:30.000000000 -0500
+++ b/drivers/input/touchscreen/usbtouchscreen.c	2008-11-17 10:51:45.000000000 -0500
@@ -60,6 +60,10 @@ static int swap_xy;
 module_param(swap_xy, bool, 0644);
 MODULE_PARM_DESC(swap_xy, "If set X and Y axes are swapped.");
 
+static int hwcalib_xy = 0;
+module_param(hwcalib_xy, bool, 0644);
+MODULE_PARM_DESC(hwcalib_xy, "If set hw-calibrated X/Y are used if available");
+
 /* device specifc data/functions */
 struct usbtouch_usb;
 struct usbtouch_device_info {
@@ -260,8 +264,13 @@ static int panjit_read_data(struct usbto
 
 static int mtouch_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
 {
-	dev->x = (pkt[8] << 8) | pkt[7];
-	dev->y = (pkt[10] << 8) | pkt[9];
+	if (hwcalib_xy) {
+		dev->x = (pkt[4] << 8) | pkt[3];
+		dev->y = 0xffff - ((pkt[6] << 8) | pkt[5]); // Invert Y
+	} else {
+		dev->x = (pkt[8] << 8) | pkt[7];
+		dev->y = (pkt[10] << 8) | pkt[9];
+	}
 	dev->touch = (pkt[2] & 0x40) ? 1 : 0;
 
 	return 1;
@@ -294,6 +303,12 @@ static int mtouch_init(struct usbtouch_u
 			return ret;
 	}
 
+	// Default min/max xy are the raw values, override if using hw-calib
+	if (hwcalib_xy) {
+		input_set_abs_params(usbtouch->input, ABS_X, 0, 0xffff, 0, 0);
+		input_set_abs_params(usbtouch->input, ABS_Y, 0, 0xffff, 0, 0);
+	}
+
 	return 0;
 }
 #endif

Re: [patch] add module parameter to provide hardware-calibrated coordinate data for MicroTouch/3M touchscreens

[Dan Streetman]

Ping, in case my last email didn't go through.

Vojtech and Greg, is this patch ok?

------



This patch adds a module parameter to report either the raw coordinate data or 
the hardware-calibrated coordinate data.  The default is set to the raw 
coordinates for backwards compatibilty.

Signed-off-by: Dan Streetman <ddstreet-EkmVulN54Sk@public.gmane.org>



--- a/drivers/input/touchscreen/usbtouchscreen.c	2008-11-17 10:51:30.000000000 -0500
+++ b/drivers/input/touchscreen/usbtouchscreen.c	2008-11-17 10:51:45.000000000 -0500
@@ -60,6 +60,10 @@ static int swap_xy;
 module_param(swap_xy, bool, 0644);
 MODULE_PARM_DESC(swap_xy, "If set X and Y axes are swapped.");
 
+static int hwcalib_xy = 0;
+module_param(hwcalib_xy, bool, 0644);
+MODULE_PARM_DESC(hwcalib_xy, "If set hw-calibrated X/Y are used if available");
+
 /* device specifc data/functions */
 struct usbtouch_usb;
 struct usbtouch_device_info {
@@ -260,8 +264,13 @@ static int panjit_read_data(struct usbto
 
 static int mtouch_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
 {
-	dev->x = (pkt[8] << 8) | pkt[7];
-	dev->y = (pkt[10] << 8) | pkt[9];
+	if (hwcalib_xy) {
+		dev->x = (pkt[4] << 8) | pkt[3];
+		dev->y = 0xffff - ((pkt[6] << 8) | pkt[5]); // Invert Y
+	} else {
+		dev->x = (pkt[8] << 8) | pkt[7];
+		dev->y = (pkt[10] << 8) | pkt[9];
+	}
 	dev->touch = (pkt[2] & 0x40) ? 1 : 0;
 
 	return 1;
@@ -294,6 +303,12 @@ static int mtouch_init(struct usbtouch_u
 			return ret;
 	}
 
+	// Default min/max xy are the raw values, override if using hw-calib
+	if (hwcalib_xy) {
+		input_set_abs_params(usbtouch->input, ABS_X, 0, 0xffff, 0, 0);
+		input_set_abs_params(usbtouch->input, ABS_Y, 0, 0xffff, 0, 0);
+	}
+
 	return 0;
 }
 #endif
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] add module parameter to provide hardware-calibrated coordinate data for MicroTouch/3M touchscreens

[Jiri Kosina]

On Mon, 24 Nov 2008, Dan Streetman wrote:

> This patch adds a module parameter to report either the raw coordinate data or 
> the hardware-calibrated coordinate data.  The default is set to the raw 
> coordinates for backwards compatibilty.

Please don't forget to include input subsystem maitainer (Dmitry Torokhov) 
when sending patches to code residing in drivers/input. Added.

> @@ -294,6 +303,12 @@ static int mtouch_init(struct usbtouch_u
>  			return ret;
>  	}
>  
> +	// Default min/max xy are the raw values, override if using hw-calib

Please use style of comments that is in compliance with kernel coding 
style.

Otherwise I think that the patch is fine.

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] add module parameter to provide hardware-calibrated coordinate data for MicroTouch/3M touchscreens

[Dan Streetman]

On Tue, Nov 25, 2008 at 10:27 AM, Jiri Kosina <jkosina-AlSwsSmVLrQ@public.gmane.org> wrote:
> On Mon, 24 Nov 2008, Dan Streetman wrote:
>
>> +     // Default min/max xy are the raw values, override if using hw-calib
>
> Please use style of comments that is in compliance with kernel coding
> style.

Oops, ok.  Updated patch below.




This patch adds a module parameter to report either the raw coordinate data or
the hardware-calibrated coordinate data.  The default is set to the raw
coordinates for backwards compatibilty.

Signed-off-by: Dan Streetman <ddstreet-EkmVulN54Sk@public.gmane.org>


--- a/drivers/input/touchscreen/usbtouchscreen.c	2008-11-17 10:51:30.000000000 -0500
+++ b/drivers/input/touchscreen/usbtouchscreen.c	2008-11-17 10:51:45.000000000 -0500
@@ -60,6 +60,10 @@ static int swap_xy;
 module_param(swap_xy, bool, 0644);
 MODULE_PARM_DESC(swap_xy, "If set X and Y axes are swapped.");
 
+static int hwcalib_xy = 0;
+module_param(hwcalib_xy, bool, 0644);
+MODULE_PARM_DESC(hwcalib_xy, "If set hw-calibrated X/Y are used if available");
+
 /* device specifc data/functions */
 struct usbtouch_usb;
 struct usbtouch_device_info {
@@ -260,8 +264,13 @@ static int panjit_read_data(struct usbto
 
 static int mtouch_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
 {
-	dev->x = (pkt[8] << 8) | pkt[7];
-	dev->y = (pkt[10] << 8) | pkt[9];
+	if (hwcalib_xy) {
+		dev->x = (pkt[4] << 8) | pkt[3];
+		dev->y = 0xffff - ((pkt[6] << 8) | pkt[5]);
+	} else {
+		dev->x = (pkt[8] << 8) | pkt[7];
+		dev->y = (pkt[10] << 8) | pkt[9];
+	}
 	dev->touch = (pkt[2] & 0x40) ? 1 : 0;
 
 	return 1;
@@ -294,6 +303,12 @@ static int mtouch_init(struct usbtouch_u
 			return ret;
 	}
 
+	/* Default min/max xy are the raw values, override if using hw-calib */
+	if (hwcalib_xy) {
+		input_set_abs_params(usbtouch->input, ABS_X, 0, 0xffff, 0, 0);
+		input_set_abs_params(usbtouch->input, ABS_Y, 0, 0xffff, 0, 0);
+	}
+
 	return 0;
 }
 #endif
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] add module parameter to provide hardware-calibrated coordinate data for MicroTouch/3M touchscreens

[Dan Streetman]

Ping.

Dmitry, does the patch look ok?

On Tue, Nov 25, 2008 at 11:29 AM, Dan Streetman <ddstreet@ieee.org> wrote:
>
> On Tue, Nov 25, 2008 at 10:27 AM, Jiri Kosina <jkosina@suse.cz> wrote:
>> On Mon, 24 Nov 2008, Dan Streetman wrote:
>>
>>> +     // Default min/max xy are the raw values, override if using hw-calib
>>
>> Please use style of comments that is in compliance with kernel coding
>> style.
>
> Oops, ok.  Updated patch below.
>
>
>
>
> This patch adds a module parameter to report either the raw coordinate data or
> the hardware-calibrated coordinate data.  The default is set to the raw
> coordinates for backwards compatibilty.
>
> Signed-off-by: Dan Streetman <ddstreet@ieee.org>
>
>
> --- a/drivers/input/touchscreen/usbtouchscreen.c        2008-11-17 10:51:30.000000000 -0500
> +++ b/drivers/input/touchscreen/usbtouchscreen.c        2008-11-17 10:51:45.000000000 -0500
> @@ -60,6 +60,10 @@ static int swap_xy;
>  module_param(swap_xy, bool, 0644);
>  MODULE_PARM_DESC(swap_xy, "If set X and Y axes are swapped.");
>
> +static int hwcalib_xy = 0;
> +module_param(hwcalib_xy, bool, 0644);
> +MODULE_PARM_DESC(hwcalib_xy, "If set hw-calibrated X/Y are used if available");
> +
>  /* device specifc data/functions */
>  struct usbtouch_usb;
>  struct usbtouch_device_info {
> @@ -260,8 +264,13 @@ static int panjit_read_data(struct usbto
>
>  static int mtouch_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
>  {
> -       dev->x = (pkt[8] << 8) | pkt[7];
> -       dev->y = (pkt[10] << 8) | pkt[9];
> +       if (hwcalib_xy) {
> +               dev->x = (pkt[4] << 8) | pkt[3];
> +               dev->y = 0xffff - ((pkt[6] << 8) | pkt[5]);
> +       } else {
> +               dev->x = (pkt[8] << 8) | pkt[7];
> +               dev->y = (pkt[10] << 8) | pkt[9];
> +       }
>        dev->touch = (pkt[2] & 0x40) ? 1 : 0;
>
>        return 1;
> @@ -294,6 +303,12 @@ static int mtouch_init(struct usbtouch_u
>                        return ret;
>        }
>
> +       /* Default min/max xy are the raw values, override if using hw-calib */
> +       if (hwcalib_xy) {
> +               input_set_abs_params(usbtouch->input, ABS_X, 0, 0xffff, 0, 0);
> +               input_set_abs_params(usbtouch->input, ABS_Y, 0, 0xffff, 0, 0);
> +       }
> +
>        return 0;
>  }
>  #endif
> --
> 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] add module parameter to provide hardware-calibrated coordinate data for MicroTouch/3M touchscreens

[Dan Streetman]

Ping.  Any comments on this?  Is it ok to get it into the kernel?

On Tue, Dec 2, 2008 at 2:35 PM, Dan Streetman <ddstreet@ieee.org> wrote:
> Ping.
>
> Dmitry, does the patch look ok?
>
> On Tue, Nov 25, 2008 at 11:29 AM, Dan Streetman <ddstreet@ieee.org> wrote:
>>
>> On Tue, Nov 25, 2008 at 10:27 AM, Jiri Kosina <jkosina@suse.cz> wrote:
>>> On Mon, 24 Nov 2008, Dan Streetman wrote:
>>>
>>>> +     // Default min/max xy are the raw values, override if using hw-calib
>>>
>>> Please use style of comments that is in compliance with kernel coding
>>> style.
>>
>> Oops, ok.  Updated patch below.
>>
>>
>>
>>
>> This patch adds a module parameter to report either the raw coordinate data or
>> the hardware-calibrated coordinate data.  The default is set to the raw
>> coordinates for backwards compatibilty.
>>
>> Signed-off-by: Dan Streetman <ddstreet@ieee.org>
>>
>>
>> --- a/drivers/input/touchscreen/usbtouchscreen.c        2008-11-17 10:51:30.000000000 -0500
>> +++ b/drivers/input/touchscreen/usbtouchscreen.c        2008-11-17 10:51:45.000000000 -0500
>> @@ -60,6 +60,10 @@ static int swap_xy;
>>  module_param(swap_xy, bool, 0644);
>>  MODULE_PARM_DESC(swap_xy, "If set X and Y axes are swapped.");
>>
>> +static int hwcalib_xy = 0;
>> +module_param(hwcalib_xy, bool, 0644);
>> +MODULE_PARM_DESC(hwcalib_xy, "If set hw-calibrated X/Y are used if available");
>> +
>>  /* device specifc data/functions */
>>  struct usbtouch_usb;
>>  struct usbtouch_device_info {
>> @@ -260,8 +264,13 @@ static int panjit_read_data(struct usbto
>>
>>  static int mtouch_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
>>  {
>> -       dev->x = (pkt[8] << 8) | pkt[7];
>> -       dev->y = (pkt[10] << 8) | pkt[9];
>> +       if (hwcalib_xy) {
>> +               dev->x = (pkt[4] << 8) | pkt[3];
>> +               dev->y = 0xffff - ((pkt[6] << 8) | pkt[5]);
>> +       } else {
>> +               dev->x = (pkt[8] << 8) | pkt[7];
>> +               dev->y = (pkt[10] << 8) | pkt[9];
>> +       }
>>        dev->touch = (pkt[2] & 0x40) ? 1 : 0;
>>
>>        return 1;
>> @@ -294,6 +303,12 @@ static int mtouch_init(struct usbtouch_u
>>                        return ret;
>>        }
>>
>> +       /* Default min/max xy are the raw values, override if using hw-calib */
>> +       if (hwcalib_xy) {
>> +               input_set_abs_params(usbtouch->input, ABS_X, 0, 0xffff, 0, 0);
>> +               input_set_abs_params(usbtouch->input, ABS_Y, 0, 0xffff, 0, 0);
>> +       }
>> +
>>        return 0;
>>  }
>>  #endif
>> --
>> 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] add module parameter to provide hardware-calibrated coordinate data for MicroTouch/3M touchscreens

[Jiri Kosina]

On Tue, 25 Nov 2008, Dan Streetman wrote:

> On Tue, Nov 25, 2008 at 10:27 AM, Jiri Kosina <jkosina-AlSwsSmVLrQ@public.gmane.org> wrote:
> > On Mon, 24 Nov 2008, Dan Streetman wrote:
> >
> >> +     // Default min/max xy are the raw values, override if using hw-calib
> >
> > Please use style of comments that is in compliance with kernel coding
> > style.
> 
> Oops, ok.  Updated patch below.
> 
> 
> 
> 
> This patch adds a module parameter to report either the raw coordinate data or
> the hardware-calibrated coordinate data.  The default is set to the raw
> coordinates for backwards compatibilty.
> 
> Signed-off-by: Dan Streetman <ddstreet-EkmVulN54Sk@public.gmane.org>
> 
> 
> --- a/drivers/input/touchscreen/usbtouchscreen.c	2008-11-17 10:51:30.000000000 -0500
> +++ b/drivers/input/touchscreen/usbtouchscreen.c	2008-11-17 10:51:45.000000000 -0500
> @@ -60,6 +60,10 @@ static int swap_xy;
>  module_param(swap_xy, bool, 0644);
>  MODULE_PARM_DESC(swap_xy, "If set X and Y axes are swapped.");
>  
> +static int hwcalib_xy = 0;
> +module_param(hwcalib_xy, bool, 0644);
> +MODULE_PARM_DESC(hwcalib_xy, "If set hw-calibrated X/Y are used if available");
> +
>  /* device specifc data/functions */
>  struct usbtouch_usb;
>  struct usbtouch_device_info {
> @@ -260,8 +264,13 @@ static int panjit_read_data(struct usbto
>  
>  static int mtouch_read_data(struct usbtouch_usb *dev, unsigned char *pkt)
>  {
> -	dev->x = (pkt[8] << 8) | pkt[7];
> -	dev->y = (pkt[10] << 8) | pkt[9];
> +	if (hwcalib_xy) {
> +		dev->x = (pkt[4] << 8) | pkt[3];
> +		dev->y = 0xffff - ((pkt[6] << 8) | pkt[5]);
> +	} else {
> +		dev->x = (pkt[8] << 8) | pkt[7];
> +		dev->y = (pkt[10] << 8) | pkt[9];
> +	}
>  	dev->touch = (pkt[2] & 0x40) ? 1 : 0;
>  
>  	return 1;
> @@ -294,6 +303,12 @@ static int mtouch_init(struct usbtouch_u
>  			return ret;
>  	}
>  
> +	/* Default min/max xy are the raw values, override if using hw-calib */
> +	if (hwcalib_xy) {
> +		input_set_abs_params(usbtouch->input, ABS_X, 0, 0xffff, 0, 0);
> +		input_set_abs_params(usbtouch->input, ABS_Y, 0, 0xffff, 0, 0);
> +	}
> +
>  	return 0;
>  }
>  #endif

[ just making sure that this one didn't get lost in the noise ]

Dmitry, the patch above looks sane to me. Would you consider merging it 
(ideally still for 2.6.29) please?

Thanks,

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] add module parameter to provide hardware-calibrated coordinate data for MicroTouch/3M touchscreens

[Dmitry Torokhov]

On Thu, Jan 08, 2009 at 01:52:41AM +0100, Jiri Kosina wrote:
> 
> [ just making sure that this one didn't get lost in the noise ]
> 
> Dmitry, the patch above looks sane to me. Would you consider merging it 
> (ideally still for 2.6.29) please?
> 

Applied and will be in my next pull request.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html