On Wed, Mar 17, 2010 at 05:57:33PM -0700, Ping Cheng wrote:
> From: Ping Cheng <[email protected]>
> Date: Wed, 17 Mar 2010 17:43:53 -0700
> Subject: [PATCH 1/2] Make keys into wcmKeys
> 
> Tool type and other device/tool specific characters can be retrieved
> through the supported events, defined in keys, from the kernel. Make
> this variable an attribute of common so we only need to retrieve it
> once in wcmPreInit
> 
> Ping Cheng <[email protected]>

Signed-off-by, I assume? ;)

The patch makes sense and I like it. One thing seems to be missing though -
the purpose is to only retrieve the keys once and then store it in wcmCommon.
As I read the flow, it's still retrieved for each dependent device,
overwriting the original bitmask. Shouldn't there be some check in place?

> ---
>  src/wcmConfig.c         |   13 ++++++-------
>  src/wcmISDV4.c          |   20 +++++++++++---------
>  src/wcmUSB.c            |   28 +++++++++++-----------------
>  src/wcmValidateDevice.c |   30 +++++++++++++++---------------
>  src/xf86Wacom.h         |   16 ++++++++--------
>  src/xf86WacomDefs.h     |    3 ++-
>  6 files changed, 53 insertions(+), 57 deletions(-)
> 
> diff --git a/src/wcmConfig.c b/src/wcmConfig.c
> index 0bc0436..8f0eadb 100644
> --- a/src/wcmConfig.c
> +++ b/src/wcmConfig.c
> @@ -1,6 +1,6 @@
>  /*
>   * Copyright 1995-2002 by Frederic Lepied, France. <[email protected]>
> - * Copyright 2002-2009 by Ping Cheng, Wacom. <[email protected]>
> + * Copyright 2002-2010 by Ping Cheng, Wacom. <[email protected]>

I noticed that some copyrights are Wacom, others are Wacom Technology. Maybe
you want to fix this with another patch? (might want to consult the lawyers
first :)

>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License
> @@ -333,7 +333,6 @@ static LocalDevicePtr wcmPreInit(InputDriverPtr drv, 
> IDevPtr dev, int flags)
>       const char*     type;
>       char*           device;
>       int             need_hotplug = 0;
> -     unsigned long   keys[NBITS(KEY_MAX)];
>       int             tablet_id = 0;
> 
>       gWacomModule.wcmDrv = drv;
> @@ -384,11 +383,11 @@ static LocalDevicePtr wcmPreInit(InputDriverPtr drv, 
> IDevPtr dev, int flags)
>       }
> 
>       /* initialize supported keys */
> -     wcmDeviceTypeKeys(local, keys, &tablet_id);
> -     need_hotplug = wcmNeedAutoHotplug(local, &type, keys);
> +     wcmDeviceTypeKeys(local, &tablet_id);
> +     need_hotplug = wcmNeedAutoHotplug(local, &type);
> 
>       /* check if the type is valid for those don't need hotplug */
> -     if(!need_hotplug && !wcmIsAValidType(type, keys))
> +     if(!need_hotplug && !wcmIsAValidType(local, type))
>               goto SetupProc_fail;
> 
>       /* check if the same device file has been added already */
> @@ -413,7 +412,7 @@ static LocalDevicePtr wcmPreInit(InputDriverPtr drv, 
> IDevPtr dev, int flags)
> 
>       /* Process the common options. */
>       xf86ProcessCommonOptions(local, local->options);
> -     if (!wcmParseOptions(local, keys))
> +     if (!wcmParseOptions(local))
>               goto SetupProc_fail;
> 
>       /* mark the device configured */
> @@ -422,7 +421,7 @@ static LocalDevicePtr wcmPreInit(InputDriverPtr drv, 
> IDevPtr dev, int flags)
>       if (need_hotplug)
>       {
>               priv->isParent = 1;
> -             wcmHotplugOthers(local, keys);
> +             wcmHotplugOthers(local);
>       }
> 
>       if (local->fd != -1)
> diff --git a/src/wcmISDV4.c b/src/wcmISDV4.c
> index 8d96216..e10816e 100644
> --- a/src/wcmISDV4.c
> +++ b/src/wcmISDV4.c
> @@ -1,6 +1,6 @@
>  /*
>   * Copyright 1995-2002 by Frederic Lepied, France. <[email protected]>
> - * Copyright 2002-2009 by Ping Cheng, Wacom Technology. <[email protected]>
> + * Copyright 2002-2010 by Ping Cheng, Wacom Technology. <[email protected]>
>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License
> @@ -633,12 +633,14 @@ static int wcmWaitForTablet(LocalDevicePtr local, char* 
> answer, int size)
>   * device ID. This matching only works for wacom devices (serial ID of
>   * WACf), all others are simply assumed to be pen + erasor.
>   */
> -int isdv4ProbeKeys(LocalDevicePtr local, unsigned long *keys)
> +int isdv4ProbeKeys(LocalDevicePtr local)
>  {
>       int id, i;
>       int tablet_id = 0;
>       struct serial_struct tmp;
>       const char *device = xf86SetStrOption(local->options, "Device", NULL);
> +     WacomDevicePtr  priv = (WacomDevicePtr)local->private;
> +     WacomCommonPtr  common = priv->common;
> 
>       if (ioctl(local->fd, TIOCGSERIAL, &tmp) < 0)
>               return 0;
> @@ -667,23 +669,23 @@ int isdv4ProbeKeys(LocalDevicePtr local, unsigned long 
> *keys)
> 
>       /* we have tried memset. it doesn't work */
>       for (i=0; i<NBITS(KEY_MAX); i++)
> -             keys[i] = 0;
> +             common->wcmKeys[i] = 0;
> 
>       /* default to penabled */
> -     keys[LONG(BTN_TOOL_PEN)] |= BIT(BTN_TOOL_PEN);
> -     keys[LONG(BTN_TOOL_RUBBER)] |= BIT(BTN_TOOL_RUBBER);
> +     common->wcmKeys[LONG(BTN_TOOL_PEN)] |= BIT(BTN_TOOL_PEN);
> +     common->wcmKeys[LONG(BTN_TOOL_RUBBER)] |= BIT(BTN_TOOL_RUBBER);
> 
>       /* id < 0x008 are only penabled */
>       if (id > 0x007)
> -             keys[LONG(BTN_TOOL_DOUBLETAP)] |= BIT(BTN_TOOL_DOUBLETAP);
> +             common->wcmKeys[LONG(BTN_TOOL_DOUBLETAP)] |= 
> BIT(BTN_TOOL_DOUBLETAP);
>       if (id > 0x0a)
> -             keys[LONG(BTN_TOOL_TRIPLETAP)] |= BIT(BTN_TOOL_TRIPLETAP);
> +             common->wcmKeys[LONG(BTN_TOOL_TRIPLETAP)] |= 
> BIT(BTN_TOOL_TRIPLETAP);
> 
>       /* no pen 2FGT */
>       if (id == 0x010)
>       {
> -             keys[LONG(BTN_TOOL_PEN)] &= ~BIT(BTN_TOOL_PEN);
> -             keys[LONG(BTN_TOOL_RUBBER)] &= ~BIT(BTN_TOOL_RUBBER);
> +             common->wcmKeys[LONG(BTN_TOOL_PEN)] &= ~BIT(BTN_TOOL_PEN);
> +             common->wcmKeys[LONG(BTN_TOOL_RUBBER)] &= ~BIT(BTN_TOOL_RUBBER);
>       }
> 
>       /* 0x9a and 0x9f are only detected by communicating
> diff --git a/src/wcmUSB.c b/src/wcmUSB.c
> index 7532551..503052a 100644
> --- a/src/wcmUSB.c
> +++ b/src/wcmUSB.c
> @@ -1,6 +1,6 @@
>  /*
>   * Copyright 1995-2002 by Frederic Lepied, France. <[email protected]>
> - * Copyright 2002-2009 by Ping Cheng, Wacom Technology. <[email protected]>    
>         
> + * Copyright 2002-2009 by Ping Cheng, Wacom Technology.
> <[email protected]>

I think this should be 2010.

Cheers,
  Peter

>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License
> @@ -429,7 +429,6 @@ static Bool usbWcmInit(LocalDevicePtr local, char* id, 
> float *version)
>  {
>       int i;
>       struct input_id sID;
> -     unsigned long keys[NBITS(KEY_MAX)] = {0};
>       WacomDevicePtr priv = (WacomDevicePtr)local->private;
>       WacomCommonPtr common = priv->common;
> 
> @@ -440,13 +439,6 @@ static Bool usbWcmInit(LocalDevicePtr local, char* id, 
> float *version)
>       ioctl(local->fd, EVIOCGID, &sID);
>       ioctl(local->fd, EVIOCGNAME(sizeof(id)), id);
> 
> -     /* retrieve tool type, device type and buttons from the kernel */
> -     if (ioctl(local->fd, EVIOCGBIT(EV_KEY,sizeof(keys)),keys) < 0)
> -     {
> -             xf86Msg(X_ERROR, "%s: unable to ioctl key bits.\n", 
> local->name);
> -             return FALSE;
> -     }
> -
>       /* vendor is wacom */
>       if (sID.vendor == WACOM_VENDOR_ID)
>       {
> @@ -482,18 +474,18 @@ static Bool usbWcmInit(LocalDevicePtr local, char* id, 
> float *version)
>        * BTN_LEFT and BTN_RIGHT, which are always fixed. */
>       common->npadkeys = 0;
>       for (i = 0; i < sizeof (padkey_codes) / sizeof (padkey_codes [0]); i++)
> -             if (ISBITSET (keys, padkey_codes [i]))
> +             if (ISBITSET (common->wcmKeys, padkey_codes [i]))
>                       common->padkey_code [common->npadkeys++] = padkey_codes 
> [i];
> 
> -     if (ISBITSET (keys, BTN_TASK))
> +     if (ISBITSET (common->wcmKeys, BTN_TASK))
>               common->nbuttons = 10;
> -     else if (ISBITSET (keys, BTN_BACK))
> +     else if (ISBITSET (common->wcmKeys, BTN_BACK))
>               common->nbuttons = 9;
> -     else if (ISBITSET (keys, BTN_FORWARD))
> +     else if (ISBITSET (common->wcmKeys, BTN_FORWARD))
>               common->nbuttons = 8;
> -     else if (ISBITSET (keys, BTN_EXTRA))
> +     else if (ISBITSET (common->wcmKeys, BTN_EXTRA))
>               common->nbuttons = 7;
> -     else if (ISBITSET (keys, BTN_SIDE))
> +     else if (ISBITSET (common->wcmKeys, BTN_SIDE))
>               common->nbuttons = 6;
>       else
>               common->nbuttons = 5;
> @@ -1066,12 +1058,14 @@ static void usbParseChannel(LocalDevicePtr local, int 
> channel)
>   * on success or 0 on failure.
>   * For USB devices, we simply copy the information the kernel gives us.
>   */
> -int usbProbeKeys(LocalDevicePtr local, unsigned long *keys)
> +int usbProbeKeys(LocalDevicePtr local)
>  {
>       struct input_id wacom_id;
> +     WacomDevicePtr  priv = (WacomDevicePtr)local->private;
> +     WacomCommonPtr  common = priv->common;
> 
>       if (ioctl(local->fd, EVIOCGBIT(EV_KEY, (sizeof(unsigned long)
> -                                             * NBITS(KEY_MAX))), keys) < 0)
> +                                             * NBITS(KEY_MAX))), 
> common->wcmKeys) < 0)
>       {
>               xf86Msg(X_ERROR, "%s: wcmDeviceTypeKeys unable to "
>                               "ioctl USB key bits.\n", local->name);
> diff --git a/src/wcmValidateDevice.c b/src/wcmValidateDevice.c
> index 9597d61..5e49193 100644
> --- a/src/wcmValidateDevice.c
> +++ b/src/wcmValidateDevice.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright 2009 by Ping Cheng, Wacom. <[email protected]>
> + * Copyright 2009 - 2010 by Ping Cheng, Wacom. <[email protected]>
>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License
> @@ -134,9 +134,11 @@ static struct
>  };
> 
>  /* validate tool type for device/product */
> -Bool wcmIsAValidType(const char* type, unsigned long* keys)
> +Bool wcmIsAValidType(LocalDevicePtr local, const char* type)
>  {
>       int j, ret = FALSE;
> +     WacomDevicePtr priv = (WacomDevicePtr)local->private;
> +     WacomCommonPtr common = priv->common;
> 
>       if (!type)
>               return FALSE;
> @@ -145,7 +147,7 @@ Bool wcmIsAValidType(const char* type, unsigned long* 
> keys)
>       for (j = 0; j < ARRAY_SIZE(wcmType); j++)
>       {
>               if (!strcmp(wcmType[j].type, type))
> -                     if (ISBITSET (keys, wcmType[j].tool))
> +                     if (ISBITSET (common->wcmKeys, wcmType[j].tool))
>                       {
>                               ret = TRUE;
>                               break;
> @@ -155,15 +157,14 @@ Bool wcmIsAValidType(const char* type, unsigned long* 
> keys)
>  }
> 
>  /* Choose valid types according to device ID. */
> -int wcmDeviceTypeKeys(LocalDevicePtr local, unsigned long* keys,
> -                   int* tablet_id)
> +int wcmDeviceTypeKeys(LocalDevicePtr local, int* tablet_id)
>  {
>       int ret = 1;
> 
>       /* serial ISDV4 devices */
> -     *tablet_id = isdv4ProbeKeys(local, keys);
> +     *tablet_id = isdv4ProbeKeys(local);
>       if (!*tablet_id) /* USB devices */
> -             *tablet_id = usbProbeKeys(local, keys);
> +             *tablet_id = usbProbeKeys(local);
> 
>       return ret;
>  }
> @@ -235,7 +236,7 @@ static void wcmHotplug(LocalDevicePtr local, const char 
> *type)
>       wcmFreeInputOpts(input_options);
>  }
> 
> -void wcmHotplugOthers(LocalDevicePtr local, unsigned long* keys)
> +void wcmHotplugOthers(LocalDevicePtr local)
>  {
>       int i, skip = 1;
>       char*           device;
> @@ -246,7 +247,7 @@ void wcmHotplugOthers(LocalDevicePtr local, unsigned 
> long* keys)
>           * need to start at the second one */
>       for (i = 0; i < ARRAY_SIZE(wcmType); i++)
>       {
> -             if (wcmIsAValidType(wcmType[i].type, keys))
> +             if (wcmIsAValidType(local, wcmType[i].type))
>               {
>                       if (skip)
>                               skip = 0;
> @@ -266,8 +267,7 @@ void wcmHotplugOthers(LocalDevicePtr local, unsigned 
> long* keys)
>   * This changes the source to _driver/wacom, all auto-hotplugged devices
>   * will have the same source.
>   */
> -int wcmNeedAutoHotplug(LocalDevicePtr local, const char **type,
> -             unsigned long* keys)
> +int wcmNeedAutoHotplug(LocalDevicePtr local, const char **type)
>  {
>       char *source = xf86CheckStrOption(local->options, "_source", "");
>       int i;
> @@ -282,7 +282,7 @@ int wcmNeedAutoHotplug(LocalDevicePtr local, const char 
> **type,
>        * for our device */
>       for (i = 0; i < ARRAY_SIZE(wcmType); i++)
>       {
> -             if (wcmIsAValidType(wcmType[i].type, keys))
> +             if (wcmIsAValidType(local, wcmType[i].type))
>               {
>                       *type = strdup(wcmType[i].type);
>                       break;
> @@ -301,7 +301,7 @@ int wcmNeedAutoHotplug(LocalDevicePtr local, const char 
> **type,
>       return 1;
>  }
> 
> -int wcmParseOptions(LocalDevicePtr local, unsigned long* keys)
> +int wcmParseOptions(LocalDevicePtr local)
>  {
>       WacomDevicePtr  priv = (WacomDevicePtr)local->private;
>       WacomCommonPtr  common = priv->common;
> @@ -493,7 +493,7 @@ int wcmParseOptions(LocalDevicePtr local, unsigned long* 
> keys)
>                                                        
> common->wcmTPCButtonDefault);
> 
>       /* a single touch device */
> -     if (ISBITSET (keys, BTN_TOOL_DOUBLETAP))
> +     if (ISBITSET (common->wcmKeys, BTN_TOOL_DOUBLETAP))
>       {
>               /* TouchDefault was off for all devices
>                * except when touch is supported */
> @@ -501,7 +501,7 @@ int wcmParseOptions(LocalDevicePtr local, unsigned long* 
> keys)
>       }
> 
>       /* 2FG touch device */
> -     if (ISBITSET (keys, BTN_TOOL_TRIPLETAP))
> +     if (ISBITSET (common->wcmKeys, BTN_TOOL_TRIPLETAP))
>       {
>               /* GestureDefault was off for all devices
>                * except when multi-touch is supported */
> diff --git a/src/xf86Wacom.h b/src/xf86Wacom.h
> index 6303c27..c2038c7 100644
> --- a/src/xf86Wacom.h
> +++ b/src/xf86Wacom.h
> @@ -1,6 +1,6 @@
>  /*
>   * Copyright 1995-2002 by Frederic Lepied, France. <[email protected]>
> - * Copyright 2002-2009 by Ping Cheng, Wacom Technology. <[email protected]>
> + * Copyright 2002-2010 by Ping Cheng, Wacom Technology. <[email protected]>
>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License
> @@ -148,18 +148,18 @@ Bool wcmAreaListOverlap(WacomToolAreaPtr area, 
> WacomToolAreaPtr list);
>  void wcmMappingFactor(LocalDevicePtr local);
> 
>  /* validation */
> -extern Bool wcmIsAValidType(const char* type, unsigned long* keys);
> +extern Bool wcmIsAValidType(LocalDevicePtr local, const char* type);
>  extern Bool wcmIsWacomDevice (char* fname);
>  extern int wcmIsDuplicate(char* device, LocalDevicePtr local);
> -extern int wcmDeviceTypeKeys(LocalDevicePtr local, unsigned long*
> keys, int* tablet_id);
> +extern int wcmDeviceTypeKeys(LocalDevicePtr local, int* tablet_id);
> 
>  /* hotplug */
> -extern int wcmNeedAutoHotplug(LocalDevicePtr local, const char
> **type, unsigned long* keys);
> -extern void wcmHotplugOthers(LocalDevicePtr local, unsigned long* keys);
> +extern int wcmNeedAutoHotplug(LocalDevicePtr local, const char **type);
> +extern void wcmHotplugOthers(LocalDevicePtr local);
>  extern int wcmAutoProbeDevice(LocalDevicePtr local);
> 
>  /* setup */
> -extern int wcmParseOptions(LocalDevicePtr local, unsigned long* keys);
> +extern int wcmParseOptions(LocalDevicePtr local);
>  extern void wcmInitialCoordinates(LocalDevicePtr local, int axes);
>  extern void wcmInitialScreens(LocalDevicePtr local);
>  extern void wcmInitialScreens(LocalDevicePtr local);
> @@ -187,8 +187,8 @@ extern void InitWcmDeviceProperties(LocalDevicePtr local);
>  #endif
> 
>  /* Device probing */
> -int isdv4ProbeKeys(LocalDevicePtr local, unsigned long *keys);
> -int usbProbeKeys(LocalDevicePtr local, unsigned long *keys);
> +int isdv4ProbeKeys(LocalDevicePtr local);
> +int usbProbeKeys(LocalDevicePtr local);
> 
>  
> /****************************************************************************/
>  #endif /* __XF86WACOM_H */
> diff --git a/src/xf86WacomDefs.h b/src/xf86WacomDefs.h
> index 3a9a89a..3af8a3f 100644
> --- a/src/xf86WacomDefs.h
> +++ b/src/xf86WacomDefs.h
> @@ -1,6 +1,6 @@
>  /*
>   * Copyright 1995-2002 by Frederic Lepied, France. <[email protected]>
> - * Copyright 2002-2009 by Ping Cheng, Wacom Technology. <[email protected]>
> + * Copyright 2002-2010 by Ping Cheng, Wacom Technology. <[email protected]>
>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License
> @@ -352,6 +352,7 @@ struct _WacomCommonRec
>       int tablet_id;               /* USB tablet ID */
>       int fd;                      /* file descriptor to tablet */
>       int fd_refs;                 /* number of references to fd; if =0,
> fd is invalid */
> +     unsigned long wcmKeys[NBITS(KEY_MAX)]; /* supported tool types for
> the device */
> 
>       /* These values are in tablet coordinates */
>       int wcmMaxX;                 /* tablet max X value */
> -- 
> 1.6.6.1


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Reply via email to