Re: [Linuxwacom-devel] [PATCH 1/4 v2] Remove channel duplication code for generic devices

2012-12-17 Thread Chris Bagwell
On Sun, Dec 16, 2012 at 10:34 PM, Ping Cheng  wrote:

> Hi Chris,
>
> Thank you for your reply and your comments. Most importantly, thank you
> for your time.
>
> The patch does not have to be merged. Let's figure out if the duplicated
> channel is necessary or not.
>
>
> On Sun, Dec 16, 2012 at 2:56 PM, Chris Bagwell wrote:
>
>>
>> On Thu, Dec 13, 2012 at 2:16 PM, Ping Cheng  wrote:
>>
>>> We use true MT protocol for MT devices in kernel now. This code
>>> was introduced to deal with ABS_TOOL_*TAP events loss issue. It
>>> is uncessary any more. And its existence makes it hard to support
>>> generic PAD cleanly.
>>>
>>>
>> I can't think of any negative effects removing these will do with
>> protocol 4 and generic devices with MT events using the latest input-wacom
>> drivers.  There is the theoritical issue the code is solving so if its
>> possible to solve what ever the bug is without deleting this then that
>> would be my preference.
>>
>> Since I don't know exact issue your seeing, I can't really comment on if
>> its a good idea to delete this.
>>
>> If you do decide to delete this logic
>>
>
> We can keep this logic. But it does not provide the information required
> for generic device any more.
>

After discussing 3/4, I see now we need some form of this patch.  We can
either delete this logic or keep it *but* save off device_type and
proximity and restore it after the structure copy.  Maybe need serial # to.

I'm OK with either solution since I have no known issue it solves... except
for next comment.


> For P5 devices, we do not use it anyway.
>
> The theoritical issues only happen:
>
> if incoming tool lands on outgoing tool's axes, As you mentioned we deal
> with absolute values, this case is hard to happen, plus ignoring the first
> motion events would avoid this issue;
>
> if user pressing a button while bringing tool out of prox, then pressing
> the same button while bringing it back in. In this case, we want to send
> the last button up since we do not know if the tool is going to come back
> or not.
>
> In both cases, duplicated channel is unnecessary.
>
>
> then I'd say we official do not support the *old* 2 finger drivers and so
>> I'd delete the Protocol 4 DOUBLETAP and TRIPLETAP logic in wcmUSB.c and
>> wcmValidateDevice.c as part of this patch as well.
>>
>
> Those TAP code are used by ISDV4 touch devices, which I can not afford to
> delete.
>
>
>> Touchpad's using old drivers but without this code become close to
>> un-usable with all the cursor jumping that occurs.
>>
>
> Which old driver are we talking about, in input-wacom or in old kernel
> releases? Which versions?
>
> The current 2.6.30 kernel driver for bamboo in input-wacom is un-usable
> with the current xf86-input-wacom. We have to use xf86-input-synaptics. I
> thought that was on purpose.
>


Oh yeah, I got part of that wrong.  We must keep DOUBLETAP logic.  Its the
TRIPLETAP that I'm worried about.  If any of the old drivers are used that
sends TRIPLETAP then it needs the logic that this patch is attempting to
delete to be somewhat usable (meaning your cursor doesn't constantly make
wild jumps).

I'd prefer to get rid of TRIPLETAP support in xf86-input-wacom *even* *if*
older kernels may be sending it since 1) it improves code readability, 2)
TRIPLETAP has known issues that xf86-input-wacom could never fully work
around and 3) removing code from xf86-input-wacom doesn't break working
with those older kernel drivers, it just removes second finger touch
support which you've mentioned is already broke anyways in current
xf86-input-wacom.

If you want to keep this version of patch (deleting logic), do you mind
also deleting lines 304-307 in wcmValidateDevice.c and 1343-1357 in
wcmUSB.c?  That should be all thats needed to remove support for old style
TRIPLETAP.

Chris



> Ping
>
>
>
>>
>> Chris
>>
>>
>> Signed-off-by: Ping Cheng 
>>> Acked-by: Jason Gerecke 
>>> ---
>>> v2: only patch 2/4 has code change from Jason's comments. To ease the
>>> review
>>> and merge effort, I updated all to v2 with acked-by and reviewed-by tags.
>>> ---
>>>  src/wcmUSB.c |   34 ++
>>>  1 file changed, 2 insertions(+), 32 deletions(-)
>>>
>>> diff --git a/src/wcmUSB.c b/src/wcmUSB.c
>>> index e192489..4b5f53b 100644
>>> --- a/src/wcmUSB.c
>>> +++ b/src/wcmUSB.c
>>> @@ -37,7 +37,6 @@ typedef struct {
>>> Bool wcmPenTouch;
>>> Bool wcmUseMT;
>>> int wcmMTChannel;
>>> -   int wcmPrevChannel;
>>> int wcmEventCnt;
>>> struct input_event wcmEvents[MAX_USB_EVENTS];
>>> int nbuttons;/* total number of buttons */
>>> @@ -1601,11 +1600,8 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
>>> return;
>>> }
>>>
>>> -   /* Protocol 5 devices have some complications related to
>>> DUALINPUT
>>> -* support and can not use below logic to recover from input
>>> -* event filtering.  Instead, just live with occasional dro

[Linuxwacom-devel] [PATCH 1/4 v2] Remove channel duplication code for generic devices

2012-12-16 Thread Ping Cheng
On Sunday, December 16, 2012, Peter Hutterer wrote:

> On Thu, Dec 13, 2012 at 10:19:09PM -0800, Ping Cheng wrote:
> > On Thursday, December 13, 2012, Peter Hutterer wrote:
> >
> > > On Thu, Dec 13, 2012 at 12:16:52PM -0800, Ping Cheng wrote:
> > > > We use true MT protocol for MT devices in kernel now. This code
> > > > was introduced to deal with ABS_TOOL_*TAP events loss issue. It
> > > > is uncessary any more. And its existence makes it hard to support
> > > > generic PAD cleanly.
> > > >
> > > > Signed-off-by: Ping Cheng >
> > > > Acked-by: Jason Gerecke >
> > > > ---
> > > > v2: only patch 2/4 has code change from Jason's comments. To ease the
> > > review
> > > > and merge effort, I updated all to v2 with acked-by and reviewed-by
> tags.
> > >
> > > I honestly don't know this code well enough to do more than a cursory
> > > review (on all 4 patches), but is there any chance we can have
> test-cases
> > > for this? or does it rely on kernel versions too much?
> >
> >
> > Yes, it relies on the kernel version and the device (Bamboo2, the 2FG
> touch
> > series).
> >
> > How much chance do we have for a user to run kernel 2.6.30 with our
> latest
> > X driver which does not support X servers older than 1.7?
>
> I personally don't care about 2.6.30, but I do about 2.6.32 (for obvious
> reasons :).


2.6.32 is important to me for the same reason. TBH, RHEL6 is one
of the systems I have to test ...

Like it or not, we are in the same boat ;-).

Ping


> Note that the kernel is somewhat of a special case too, you can
> run latest userspace on an old kernel, so in theory. we can't run an older
> X
> server with the new driver, but i'm sure we can run on most 2.6 kernels.
>
> Cheers,
>Peter
>
> > > > ---
> > > >  src/wcmUSB.c |   34 ++
> > > >  1 file changed, 2 insertions(+), 32 deletions(-)
> > > >
> > > > diff --git a/src/wcmUSB.c b/src/wcmUSB.c
> > > > index e192489..4b5f53b 100644
> > > > --- a/src/wcmUSB.c
> > > > +++ b/src/wcmUSB.c
> > > > @@ -37,7 +37,6 @@ typedef struct {
> > > >   Bool wcmPenTouch;
> > > >   Bool wcmUseMT;
> > > >   int wcmMTChannel;
> > > > - int wcmPrevChannel;
> > > >   int wcmEventCnt;
> > > >   struct input_event wcmEvents[MAX_USB_EVENTS];
> > > >   int nbuttons;/* total number of buttons */
> > > > @@ -1601,11 +1600,8 @@ static void usbDispatchEvents(InputInfoPtr
> pInfo)
> > > >   return;
> > > >   }
> > > >
> > > > - /* Protocol 5 devices have some complications related to
> DUALINPUT
> > > > -  * support and can not use below logic to recover from input
> > > > -  * event filtering.  Instead, just live with occasional dropped
> > > > -  * event.  Since tools are dynamically assigned a channel #,
> the
> > > > -  * structure must be initialized to known starting values
> > > > + /* Protocol 5 tools are dynamically assigned with channel
> numbers.
> > > > +  * The structure must be initialized to known starting values
> > > >* when first entering proximity to discard invalid data.
> > > >*/
> > > >   if (common->wcmProtocolLevel == WCM_PROTOCOL_5)
> > > > @@ -1614,32 +1610,6 @@ static void usbDispatchEvents(InputInfoPtr
> pInfo)
> > > >   memset(&common->wcmChannel[channel],0,
> > > >  sizeof(WacomChannel));
> > > >   }
> > > > - else
> > > > - {
> > > > - /* Because of linux input filtering, each switch to a
> new
> > > > -  * tool is required to have its initial values match
> values
> > > > -  * of previous tool.
> > > > -  *
> > > > -  * For normal case, all tools are in channel 0 and so
> > > > -  * no issue.  Protocol 4 2FGT devices split between
> > > > -  * two channels though and so need to copy data between
> > > > -  * channels to prevent loss of events; which could
> > > > -  * lead to cursor jumps.
> > > > -  *
> > > > -  * PAD device is special.  It shares no events
> > > > -  * with other channels and is always in proximity.
> > > > -  * So it requires no copying of data from other
> > > > -  * channels.
> > > > -  */
> > > > - if (private->wcmPrevChannel != channel &&
> > > > - channel != PAD_CHANNEL &&
> > > > - private->wcmPrevChannel != PAD_CHANNEL)
> > > > - {
> > > > - common->wcmChannel[channel].work =
> > > > -
> > > common->wcmChannel[private->wcmPrevChannel].work;
> > > > -
--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue d

Re: [Linuxwacom-devel] [PATCH 1/4 v2] Remove channel duplication code for generic devices

2012-12-16 Thread Peter Hutterer
On Thu, Dec 13, 2012 at 10:19:09PM -0800, Ping Cheng wrote:
> On Thursday, December 13, 2012, Peter Hutterer wrote:
> 
> > On Thu, Dec 13, 2012 at 12:16:52PM -0800, Ping Cheng wrote:
> > > We use true MT protocol for MT devices in kernel now. This code
> > > was introduced to deal with ABS_TOOL_*TAP events loss issue. It
> > > is uncessary any more. And its existence makes it hard to support
> > > generic PAD cleanly.
> > >
> > > Signed-off-by: Ping Cheng >
> > > Acked-by: Jason Gerecke >
> > > ---
> > > v2: only patch 2/4 has code change from Jason's comments. To ease the
> > review
> > > and merge effort, I updated all to v2 with acked-by and reviewed-by tags.
> >
> > I honestly don't know this code well enough to do more than a cursory
> > review (on all 4 patches), but is there any chance we can have test-cases
> > for this? or does it rely on kernel versions too much?
> 
> 
> Yes, it relies on the kernel version and the device (Bamboo2, the 2FG touch
> series).
> 
> How much chance do we have for a user to run kernel 2.6.30 with our latest
> X driver which does not support X servers older than 1.7?

I personally don't care about 2.6.30, but I do about 2.6.32 (for obvious
reasons :). Note that the kernel is somewhat of a special case too, you can
run latest userspace on an old kernel, so in theory. we can't run an older X
server with the new driver, but i'm sure we can run on most 2.6 kernels.

Cheers,
   Peter

> > > ---
> > >  src/wcmUSB.c |   34 ++
> > >  1 file changed, 2 insertions(+), 32 deletions(-)
> > >
> > > diff --git a/src/wcmUSB.c b/src/wcmUSB.c
> > > index e192489..4b5f53b 100644
> > > --- a/src/wcmUSB.c
> > > +++ b/src/wcmUSB.c
> > > @@ -37,7 +37,6 @@ typedef struct {
> > >   Bool wcmPenTouch;
> > >   Bool wcmUseMT;
> > >   int wcmMTChannel;
> > > - int wcmPrevChannel;
> > >   int wcmEventCnt;
> > >   struct input_event wcmEvents[MAX_USB_EVENTS];
> > >   int nbuttons;/* total number of buttons */
> > > @@ -1601,11 +1600,8 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
> > >   return;
> > >   }
> > >
> > > - /* Protocol 5 devices have some complications related to DUALINPUT
> > > -  * support and can not use below logic to recover from input
> > > -  * event filtering.  Instead, just live with occasional dropped
> > > -  * event.  Since tools are dynamically assigned a channel #, the
> > > -  * structure must be initialized to known starting values
> > > + /* Protocol 5 tools are dynamically assigned with channel numbers.
> > > +  * The structure must be initialized to known starting values
> > >* when first entering proximity to discard invalid data.
> > >*/
> > >   if (common->wcmProtocolLevel == WCM_PROTOCOL_5)
> > > @@ -1614,32 +1610,6 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
> > >   memset(&common->wcmChannel[channel],0,
> > >  sizeof(WacomChannel));
> > >   }
> > > - else
> > > - {
> > > - /* Because of linux input filtering, each switch to a new
> > > -  * tool is required to have its initial values match values
> > > -  * of previous tool.
> > > -  *
> > > -  * For normal case, all tools are in channel 0 and so
> > > -  * no issue.  Protocol 4 2FGT devices split between
> > > -  * two channels though and so need to copy data between
> > > -  * channels to prevent loss of events; which could
> > > -  * lead to cursor jumps.
> > > -  *
> > > -  * PAD device is special.  It shares no events
> > > -  * with other channels and is always in proximity.
> > > -  * So it requires no copying of data from other
> > > -  * channels.
> > > -  */
> > > - if (private->wcmPrevChannel != channel &&
> > > - channel != PAD_CHANNEL &&
> > > - private->wcmPrevChannel != PAD_CHANNEL)
> > > - {
> > > - common->wcmChannel[channel].work =
> > > -
> > common->wcmChannel[private->wcmPrevChannel].work;
> > > - private->wcmPrevChannel = channel;
> > > - }
> > > - }
> > >
> > >   ds = &common->wcmChannel[channel].work;
> > >   dslast = common->wcmChannel[channel].valid.state;
> > > --
> > > 1.7.10.4
> > >
> >

--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https:/

Re: [Linuxwacom-devel] [PATCH 1/4 v2] Remove channel duplication code for generic devices

2012-12-16 Thread Ping Cheng
Hi Chris,

Thank you for your reply and your comments. Most importantly, thank you for
your time.

The patch does not have to be merged. Let's figure out if the duplicated
channel is necessary or not.


On Sun, Dec 16, 2012 at 2:56 PM, Chris Bagwell  wrote:

>
> On Thu, Dec 13, 2012 at 2:16 PM, Ping Cheng  wrote:
>
>> We use true MT protocol for MT devices in kernel now. This code
>> was introduced to deal with ABS_TOOL_*TAP events loss issue. It
>> is uncessary any more. And its existence makes it hard to support
>> generic PAD cleanly.
>>
>>
> I can't think of any negative effects removing these will do with protocol
> 4 and generic devices with MT events using the latest input-wacom drivers.
> There is the theoritical issue the code is solving so if its possible to
> solve what ever the bug is without deleting this then that would be my
> preference.
>
> Since I don't know exact issue your seeing, I can't really comment on if
> its a good idea to delete this.
>
> If you do decide to delete this logic
>

We can keep this logic. But it does not provide the information required
for generic device any more.

For P5 devices, we do not use it anyway.

The theoritical issues only happen:

if incoming tool lands on outgoing tool's axes, As you mentioned we deal
with absolute values, this case is hard to happen, plus ignoring the first
motion events would avoid this issue;

if user pressing a button while bringing tool out of prox, then pressing
the same button while bringing it back in. In this case, we want to send
the last button up since we do not know if the tool is going to come back
or not.

In both cases, duplicated channel is unnecessary.


then I'd say we official do not support the *old* 2 finger drivers and so
> I'd delete the Protocol 4 DOUBLETAP and TRIPLETAP logic in wcmUSB.c and
> wcmValidateDevice.c as part of this patch as well.
>

Those TAP code are used by ISDV4 touch devices, which I can not afford to
delete.


> Touchpad's using old drivers but without this code become close to
> un-usable with all the cursor jumping that occurs.
>

Which old driver are we talking about, in input-wacom or in old kernel
releases? Which versions?

The current 2.6.30 kernel driver for bamboo in input-wacom is un-usable
with the current xf86-input-wacom. We have to use xf86-input-synaptics. I
thought that was on purpose.

Ping



>
> Chris
>
>
> Signed-off-by: Ping Cheng 
>> Acked-by: Jason Gerecke 
>> ---
>> v2: only patch 2/4 has code change from Jason's comments. To ease the
>> review
>> and merge effort, I updated all to v2 with acked-by and reviewed-by tags.
>> ---
>>  src/wcmUSB.c |   34 ++
>>  1 file changed, 2 insertions(+), 32 deletions(-)
>>
>> diff --git a/src/wcmUSB.c b/src/wcmUSB.c
>> index e192489..4b5f53b 100644
>> --- a/src/wcmUSB.c
>> +++ b/src/wcmUSB.c
>> @@ -37,7 +37,6 @@ typedef struct {
>> Bool wcmPenTouch;
>> Bool wcmUseMT;
>> int wcmMTChannel;
>> -   int wcmPrevChannel;
>> int wcmEventCnt;
>> struct input_event wcmEvents[MAX_USB_EVENTS];
>> int nbuttons;/* total number of buttons */
>> @@ -1601,11 +1600,8 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
>> return;
>> }
>>
>> -   /* Protocol 5 devices have some complications related to DUALINPUT
>> -* support and can not use below logic to recover from input
>> -* event filtering.  Instead, just live with occasional dropped
>> -* event.  Since tools are dynamically assigned a channel #, the
>> -* structure must be initialized to known starting values
>> +   /* Protocol 5 tools are dynamically assigned with channel numbers.
>> +* The structure must be initialized to known starting values
>>  * when first entering proximity to discard invalid data.
>>  */
>> if (common->wcmProtocolLevel == WCM_PROTOCOL_5)
>> @@ -1614,32 +1610,6 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
>> memset(&common->wcmChannel[channel],0,
>>sizeof(WacomChannel));
>> }
>> -   else
>> -   {
>> -   /* Because of linux input filtering, each switch to a new
>> -* tool is required to have its initial values match
>> values
>> -* of previous tool.
>> -*
>> -* For normal case, all tools are in channel 0 and so
>> -* no issue.  Protocol 4 2FGT devices split between
>> -* two channels though and so need to copy data between
>> -* channels to prevent loss of events; which could
>> -* lead to cursor jumps.
>> -*
>> -* PAD device is special.  It shares no events
>> -* with other channels and is always in proximity.
>> -* So it requires no copying of data from other
>> -* channels.
>> -

Re: [Linuxwacom-devel] [PATCH 1/4 v2] Remove channel duplication code for generic devices

2012-12-16 Thread Chris Bagwell
On Thu, Dec 13, 2012 at 2:16 PM, Ping Cheng  wrote:

> We use true MT protocol for MT devices in kernel now. This code
> was introduced to deal with ABS_TOOL_*TAP events loss issue. It
> is uncessary any more. And its existence makes it hard to support
> generic PAD cleanly.
>
>
I can't think of any negative effects removing these will do with protocol
4 and generic devices with MT events using the latest input-wacom drivers.
There is the theoritical issue the code is solving so if its possible to
solve what ever the bug is without deleting this then that would be my
preference.

Since I don't know exact issue your seeing, I can't really comment on if
its a good idea to delete this.

If you do decide to delete this logic then I'd say we official do not
support the *old* 2 finger drivers and so I'd delete the Protocol 4
DOUBLETAP and TRIPLETAP logic in wcmUSB.c and  wcmValidateDevice.c as part
of this patch as well.  Touchpad's using old drivers but without this code
become close to un-usable with all the cursor jumping that occurs.

Chris


Signed-off-by: Ping Cheng 
> Acked-by: Jason Gerecke 
> ---
> v2: only patch 2/4 has code change from Jason's comments. To ease the
> review
> and merge effort, I updated all to v2 with acked-by and reviewed-by tags.
> ---
>  src/wcmUSB.c |   34 ++
>  1 file changed, 2 insertions(+), 32 deletions(-)
>
> diff --git a/src/wcmUSB.c b/src/wcmUSB.c
> index e192489..4b5f53b 100644
> --- a/src/wcmUSB.c
> +++ b/src/wcmUSB.c
> @@ -37,7 +37,6 @@ typedef struct {
> Bool wcmPenTouch;
> Bool wcmUseMT;
> int wcmMTChannel;
> -   int wcmPrevChannel;
> int wcmEventCnt;
> struct input_event wcmEvents[MAX_USB_EVENTS];
> int nbuttons;/* total number of buttons */
> @@ -1601,11 +1600,8 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
> return;
> }
>
> -   /* Protocol 5 devices have some complications related to DUALINPUT
> -* support and can not use below logic to recover from input
> -* event filtering.  Instead, just live with occasional dropped
> -* event.  Since tools are dynamically assigned a channel #, the
> -* structure must be initialized to known starting values
> +   /* Protocol 5 tools are dynamically assigned with channel numbers.
> +* The structure must be initialized to known starting values
>  * when first entering proximity to discard invalid data.
>  */
> if (common->wcmProtocolLevel == WCM_PROTOCOL_5)
> @@ -1614,32 +1610,6 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
> memset(&common->wcmChannel[channel],0,
>sizeof(WacomChannel));
> }
> -   else
> -   {
> -   /* Because of linux input filtering, each switch to a new
> -* tool is required to have its initial values match values
> -* of previous tool.
> -*
> -* For normal case, all tools are in channel 0 and so
> -* no issue.  Protocol 4 2FGT devices split between
> -* two channels though and so need to copy data between
> -* channels to prevent loss of events; which could
> -* lead to cursor jumps.
> -*
> -* PAD device is special.  It shares no events
> -* with other channels and is always in proximity.
> -* So it requires no copying of data from other
> -* channels.
> -*/
> -   if (private->wcmPrevChannel != channel &&
> -   channel != PAD_CHANNEL &&
> -   private->wcmPrevChannel != PAD_CHANNEL)
> -   {
> -   common->wcmChannel[channel].work =
> -
> common->wcmChannel[private->wcmPrevChannel].work;
> -   private->wcmPrevChannel = channel;
> -   }
> -   }
>
> ds = &common->wcmChannel[channel].work;
> dslast = common->wcmChannel[channel].valid.state;
> --
> 1.7.10.4
>
>
>
> --
> LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
> Remotely access PCs and mobile devices and provide instant support
> Improve your efficiency, and focus on delivering more value-add services
> Discover what IT Professionals Know. Rescue delivers
> http://p.sf.net/sfu/logmein_12329d2d
> ___
> Linuxwacom-devel mailing list
> Linuxwacom-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
>
--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delive

Re: [Linuxwacom-devel] [PATCH 1/4 v2] Remove channel duplication code for generic devices

2012-12-13 Thread Ping Cheng
On Thursday, December 13, 2012, Peter Hutterer wrote:

> On Thu, Dec 13, 2012 at 12:16:52PM -0800, Ping Cheng wrote:
> > We use true MT protocol for MT devices in kernel now. This code
> > was introduced to deal with ABS_TOOL_*TAP events loss issue. It
> > is uncessary any more. And its existence makes it hard to support
> > generic PAD cleanly.
> >
> > Signed-off-by: Ping Cheng >
> > Acked-by: Jason Gerecke >
> > ---
> > v2: only patch 2/4 has code change from Jason's comments. To ease the
> review
> > and merge effort, I updated all to v2 with acked-by and reviewed-by tags.
>
> I honestly don't know this code well enough to do more than a cursory
> review (on all 4 patches), but is there any chance we can have test-cases
> for this? or does it rely on kernel versions too much?


Yes, it relies on the kernel version and the device (Bamboo2, the 2FG touch
series).

How much chance do we have for a user to run kernel 2.6.30 with our latest
X driver which does not support X servers older than 1.7?

Ping


> Cheers,
>Peter
>
>
> > ---
> >  src/wcmUSB.c |   34 ++
> >  1 file changed, 2 insertions(+), 32 deletions(-)
> >
> > diff --git a/src/wcmUSB.c b/src/wcmUSB.c
> > index e192489..4b5f53b 100644
> > --- a/src/wcmUSB.c
> > +++ b/src/wcmUSB.c
> > @@ -37,7 +37,6 @@ typedef struct {
> >   Bool wcmPenTouch;
> >   Bool wcmUseMT;
> >   int wcmMTChannel;
> > - int wcmPrevChannel;
> >   int wcmEventCnt;
> >   struct input_event wcmEvents[MAX_USB_EVENTS];
> >   int nbuttons;/* total number of buttons */
> > @@ -1601,11 +1600,8 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
> >   return;
> >   }
> >
> > - /* Protocol 5 devices have some complications related to DUALINPUT
> > -  * support and can not use below logic to recover from input
> > -  * event filtering.  Instead, just live with occasional dropped
> > -  * event.  Since tools are dynamically assigned a channel #, the
> > -  * structure must be initialized to known starting values
> > + /* Protocol 5 tools are dynamically assigned with channel numbers.
> > +  * The structure must be initialized to known starting values
> >* when first entering proximity to discard invalid data.
> >*/
> >   if (common->wcmProtocolLevel == WCM_PROTOCOL_5)
> > @@ -1614,32 +1610,6 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
> >   memset(&common->wcmChannel[channel],0,
> >  sizeof(WacomChannel));
> >   }
> > - else
> > - {
> > - /* Because of linux input filtering, each switch to a new
> > -  * tool is required to have its initial values match values
> > -  * of previous tool.
> > -  *
> > -  * For normal case, all tools are in channel 0 and so
> > -  * no issue.  Protocol 4 2FGT devices split between
> > -  * two channels though and so need to copy data between
> > -  * channels to prevent loss of events; which could
> > -  * lead to cursor jumps.
> > -  *
> > -  * PAD device is special.  It shares no events
> > -  * with other channels and is always in proximity.
> > -  * So it requires no copying of data from other
> > -  * channels.
> > -  */
> > - if (private->wcmPrevChannel != channel &&
> > - channel != PAD_CHANNEL &&
> > - private->wcmPrevChannel != PAD_CHANNEL)
> > - {
> > - common->wcmChannel[channel].work =
> > -
> common->wcmChannel[private->wcmPrevChannel].work;
> > - private->wcmPrevChannel = channel;
> > - }
> > - }
> >
> >   ds = &common->wcmChannel[channel].work;
> >   dslast = common->wcmChannel[channel].valid.state;
> > --
> > 1.7.10.4
> >
>
--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel


Re: [Linuxwacom-devel] [PATCH 1/4 v2] Remove channel duplication code for generic devices

2012-12-13 Thread Peter Hutterer
On Thu, Dec 13, 2012 at 12:16:52PM -0800, Ping Cheng wrote:
> We use true MT protocol for MT devices in kernel now. This code
> was introduced to deal with ABS_TOOL_*TAP events loss issue. It
> is uncessary any more. And its existence makes it hard to support
> generic PAD cleanly.
> 
> Signed-off-by: Ping Cheng 
> Acked-by: Jason Gerecke 
> ---
> v2: only patch 2/4 has code change from Jason's comments. To ease the review
> and merge effort, I updated all to v2 with acked-by and reviewed-by tags.

I honestly don't know this code well enough to do more than a cursory
review (on all 4 patches), but is there any chance we can have test-cases
for this? or does it rely on kernel versions too much?

Cheers,
   Peter


> ---
>  src/wcmUSB.c |   34 ++
>  1 file changed, 2 insertions(+), 32 deletions(-)
> 
> diff --git a/src/wcmUSB.c b/src/wcmUSB.c
> index e192489..4b5f53b 100644
> --- a/src/wcmUSB.c
> +++ b/src/wcmUSB.c
> @@ -37,7 +37,6 @@ typedef struct {
>   Bool wcmPenTouch;
>   Bool wcmUseMT;
>   int wcmMTChannel;
> - int wcmPrevChannel;
>   int wcmEventCnt;
>   struct input_event wcmEvents[MAX_USB_EVENTS];
>   int nbuttons;/* total number of buttons */
> @@ -1601,11 +1600,8 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
>   return;
>   }
>  
> - /* Protocol 5 devices have some complications related to DUALINPUT
> -  * support and can not use below logic to recover from input
> -  * event filtering.  Instead, just live with occasional dropped
> -  * event.  Since tools are dynamically assigned a channel #, the
> -  * structure must be initialized to known starting values
> + /* Protocol 5 tools are dynamically assigned with channel numbers.
> +  * The structure must be initialized to known starting values
>* when first entering proximity to discard invalid data.
>*/
>   if (common->wcmProtocolLevel == WCM_PROTOCOL_5)
> @@ -1614,32 +1610,6 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
>   memset(&common->wcmChannel[channel],0,
>  sizeof(WacomChannel));
>   }
> - else
> - {
> - /* Because of linux input filtering, each switch to a new
> -  * tool is required to have its initial values match values
> -  * of previous tool.
> -  *
> -  * For normal case, all tools are in channel 0 and so
> -  * no issue.  Protocol 4 2FGT devices split between
> -  * two channels though and so need to copy data between
> -  * channels to prevent loss of events; which could
> -  * lead to cursor jumps.
> -  *
> -  * PAD device is special.  It shares no events
> -  * with other channels and is always in proximity.
> -  * So it requires no copying of data from other
> -  * channels.
> -  */
> - if (private->wcmPrevChannel != channel &&
> - channel != PAD_CHANNEL &&
> - private->wcmPrevChannel != PAD_CHANNEL)
> - {
> - common->wcmChannel[channel].work =
> - 
> common->wcmChannel[private->wcmPrevChannel].work;
> - private->wcmPrevChannel = channel;
> - }
> - }
>  
>   ds = &common->wcmChannel[channel].work;
>   dslast = common->wcmChannel[channel].valid.state;
> -- 
> 1.7.10.4
> 

--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel


[Linuxwacom-devel] [PATCH 1/4 v2] Remove channel duplication code for generic devices

2012-12-13 Thread Ping Cheng
We use true MT protocol for MT devices in kernel now. This code
was introduced to deal with ABS_TOOL_*TAP events loss issue. It
is uncessary any more. And its existence makes it hard to support
generic PAD cleanly.

Signed-off-by: Ping Cheng 
Acked-by: Jason Gerecke 
---
v2: only patch 2/4 has code change from Jason's comments. To ease the review
and merge effort, I updated all to v2 with acked-by and reviewed-by tags.
---
 src/wcmUSB.c |   34 ++
 1 file changed, 2 insertions(+), 32 deletions(-)

diff --git a/src/wcmUSB.c b/src/wcmUSB.c
index e192489..4b5f53b 100644
--- a/src/wcmUSB.c
+++ b/src/wcmUSB.c
@@ -37,7 +37,6 @@ typedef struct {
Bool wcmPenTouch;
Bool wcmUseMT;
int wcmMTChannel;
-   int wcmPrevChannel;
int wcmEventCnt;
struct input_event wcmEvents[MAX_USB_EVENTS];
int nbuttons;/* total number of buttons */
@@ -1601,11 +1600,8 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
return;
}
 
-   /* Protocol 5 devices have some complications related to DUALINPUT
-* support and can not use below logic to recover from input
-* event filtering.  Instead, just live with occasional dropped
-* event.  Since tools are dynamically assigned a channel #, the
-* structure must be initialized to known starting values
+   /* Protocol 5 tools are dynamically assigned with channel numbers.
+* The structure must be initialized to known starting values
 * when first entering proximity to discard invalid data.
 */
if (common->wcmProtocolLevel == WCM_PROTOCOL_5)
@@ -1614,32 +1610,6 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
memset(&common->wcmChannel[channel],0,
   sizeof(WacomChannel));
}
-   else
-   {
-   /* Because of linux input filtering, each switch to a new
-* tool is required to have its initial values match values
-* of previous tool.
-*
-* For normal case, all tools are in channel 0 and so
-* no issue.  Protocol 4 2FGT devices split between
-* two channels though and so need to copy data between
-* channels to prevent loss of events; which could
-* lead to cursor jumps.
-*
-* PAD device is special.  It shares no events
-* with other channels and is always in proximity.
-* So it requires no copying of data from other
-* channels.
-*/
-   if (private->wcmPrevChannel != channel &&
-   channel != PAD_CHANNEL &&
-   private->wcmPrevChannel != PAD_CHANNEL)
-   {
-   common->wcmChannel[channel].work =
-   
common->wcmChannel[private->wcmPrevChannel].work;
-   private->wcmPrevChannel = channel;
-   }
-   }
 
ds = &common->wcmChannel[channel].work;
dslast = common->wcmChannel[channel].valid.state;
-- 
1.7.10.4


--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel