On Mon, Dec 14, 2009 at 02:08:44PM -0800, Ping wrote: > On Sun, Dec 13, 2009 at 9:38 PM, Peter Hutterer > <[email protected]>wrote: > > it does, a bit anyway. > > the device-specific features are the enabling of touch, etc. based on the > > device's capabilities, right? (like the hunk below). > > > > AIUI, these capabilities don't change during the runtime of a device. if > > so, > > usbWcmInit is the wrong place to put the checks, it's called every time the > > device is opened and that seems a bit excessive. > > > I see your point. We can easily move this to wcmValidateDevice.c for USB. > But it would be a challenge for ISDV4 devices since the driver may not get > the accurate feature until it talks to the device directly in Wacom mode. > Anyway, I made a clean, I think :), patch. ISDV4 would need more work > later. But this patch covers the ISDV4 devices that we see in the market > now. > > > > (also, it looks like this > > is the cause for the touch property being overwritten when you resume from > > suspend :) > > > > Unfortunately, your "kill two birds with one stone" plan did not work :). > The property resume issue stays even with the new patch. In fact, if you > hardcode both wcmTouch and wcmTouchDefault to 1 without changing it anywhere > else in the driver, wcmTouch got changed to 0 after the system resumes. I > am running a 64 bit system. Maybe that makes a difference? Something fishy > in InitWcmAtom.
doh. I'll have a look at it. > I think what we need here is something similar to the EvdevProbe() function > > - a function that opens the device, checks the capabilities and sets the > > required flags so that xf86WcmDevOpen and it's childs can be reduced to > > little more than fd = open(device). > > > > Let me know if my solution is acceptable or not. It is only called once now > if that was your major concern. > From 5df49f641cb331646f6cbfb0975a9661f23e7a4e Mon Sep 17 00:00:00 2001 > From: Ping Cheng <[email protected]> > Date: Mon, 14 Dec 2009 13:04:04 -0800 > Subject: [PATCH] Set Touch and Gesture options during the configuration > > We moved the touch and gesture option check from usbWcmInit > to wcmValidateDevice.c since usbWcmInit gets called every time > the device is opened, which is unnecessary for option check > in most cases. > > The Tablet PC Button option should also be dealt in the same way. > But it requires extra defines to make it clean, which will affect > more files in the driver. We'll do it in a separate patch later. > > Signed-off-by: Ping Cheng <[email protected]> > --- > src/wcmConfig.c | 37 ++++++++++++++++++++++++++++++++++++- > src/wcmISDV4.c | 16 ---------------- > src/wcmUSB.c | 25 +------------------------ > src/wcmValidateDevice.c | 7 ------- > 4 files changed, 37 insertions(+), 48 deletions(-) > > diff --git a/src/wcmConfig.c b/src/wcmConfig.c > index 9a51596..cf7f2ab 100644 > --- a/src/wcmConfig.c > +++ b/src/wcmConfig.c > @@ -37,6 +37,7 @@ extern void wcmHotplugOthers(LocalDevicePtr local, unsigned > long* keys); > extern int wcmDeviceTypeKeys(LocalDevicePtr local, unsigned long* keys); > > static int xf86WcmAllocate(LocalDevicePtr local, char* name, int flag); > +static void wcmDeviceSpecCommonOptions(LocalDevicePtr local, unsigned long* > keys); > > > /***************************************************************************** > * xf86WcmAllocate -- > @@ -312,6 +313,37 @@ static Bool xf86WcmMatchDevice(LocalDevicePtr pMatch, > LocalDevicePtr pLocal) > return 0; > } > > +/* retrieve the specific options for the device */ > +static void wcmDeviceSpecCommonOptions(LocalDevicePtr local, unsigned long* > keys) > +{ > + WacomDevicePtr priv = (WacomDevicePtr)local->private; > + WacomCommonPtr common = priv->common; > + > + /* a single touch device */ > + if (ISBITSET (keys, BTN_TOOL_DOUBLETAP)) > + { > + /* TouchDefault was off for all devices > + * except when touch is supported */ > + common->wcmTouchDefault = 1; > + } > + > + /* 2FG touch device */ > + if (ISBITSET (keys, BTN_TOOL_TRIPLETAP)) > + { > + /* GestureDefault was off for all devices > + * except when multi-touch is supported */ > + common->wcmGestureDefault = 1; > + } > + > + /* check if touch was turned off in xorg.conf */ > + common->wcmTouch = xf86SetBoolOption(local->options, "Touch", > + common->wcmTouchDefault); > + > + /* Touch gesture applies to the whole tablet */ > + common->wcmGesture = xf86SetBoolOption(local->options, "Gesture", > + common->wcmGestureDefault); > +} I don't quite understand why we'd need a separate function for this. is this future-proofing it for the ISDV4 support you mentioned above? if not, we could just add those few lines to wcmParseOptions, couldn't we? > /* xf86WcmInit - called for each input devices with the driver set to > * "wacom" */ > static LocalDevicePtr xf86WcmInit(InputDriverPtr drv, IDevPtr dev, int flags) > @@ -384,7 +416,10 @@ static LocalDevicePtr xf86WcmInit(InputDriverPtr drv, > IDevPtr dev, int flags) > } > } > > - /* Process the common options. */ > + /* update device specific common options */ > + wcmDeviceSpecCommonOptions(local, keys); > + > + /* Process the general common options. */ > xf86ProcessCommonOptions(local, local->options); > if (!wcmParseOptions(local)) > goto SetupProc_fail; if wcmDeviceSpecCommonOptions is neeed (see above hunk): any reason we can't call wcmDeviceSpecCommonOptions from wcmParseOptions? That'd give us a single entry point into the option parsing code. also, IIRC parsing options before xf86ProcessCommonOptions can lead to funny results. rest of the patch looks good though. Cheers, Peter ------------------------------------------------------------------------------ Return on Information: Google Enterprise Search pays you back Get the facts. http://p.sf.net/sfu/google-dev2dev _______________________________________________ Linuxwacom-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
