typo in subject "Rewrite mechanisn _to_ detect ..." please describe in the commit message what you did here. in the future, it'll be much easier to understand why a line was changed and this commit isn't as straightforward as removing dead code.
On Sun, Feb 27, 2011 at 01:11:50AM +0500, Alexandr Shadchin wrote: > It is now easier to add new backends > > Signed-off-by: Alexandr Shadchin <alexandr.shadc...@gmail.com> > --- > src/alpscomm.c | 8 +----- > src/ps2comm.c | 8 +----- > src/psmcomm.c | 7 +---- > src/synaptics.c | 80 > ++++++++++++++++++++----------------------------------- > src/synproto.h | 16 +++------- > 5 files changed, 37 insertions(+), 82 deletions(-) > > diff --git a/src/alpscomm.c b/src/alpscomm.c > index 3872f5c..dc76655 100644 > --- a/src/alpscomm.c > +++ b/src/alpscomm.c > @@ -220,17 +220,11 @@ ALPSReadHwState(InputInfoPtr pInfo, > return TRUE; > } > > -static Bool > -ALPSAutoDevProbe(InputInfoPtr pInfo) > -{ > - return FALSE; > -} > - > struct SynapticsProtocolOperations alps_proto_operations = { > NULL, > NULL, > ALPSQueryHardware, > ALPSReadHwState, > - ALPSAutoDevProbe, > + NULL, > NULL > }; > diff --git a/src/ps2comm.c b/src/ps2comm.c > index b676ddc..92665c8 100644 > --- a/src/ps2comm.c > +++ b/src/ps2comm.c > @@ -660,17 +660,11 @@ PS2ReadHwState(InputInfoPtr pInfo, > return PS2ReadHwStatePriv(pInfo, &psaux_proto_operations, comm, hwRet); > } > > -static Bool > -PS2AutoDevProbe(InputInfoPtr pInfo) > -{ > - return FALSE; > -} > - > struct SynapticsProtocolOperations psaux_proto_operations = { > NULL, > PS2DeviceOffHook, > PS2QueryHardware, > PS2ReadHwState, > - PS2AutoDevProbe, > + NULL, > NULL > }; > diff --git a/src/psmcomm.c b/src/psmcomm.c > index ea8cf88..8d633bd 100644 > --- a/src/psmcomm.c > +++ b/src/psmcomm.c > @@ -162,16 +162,11 @@ PSMReadHwState(InputInfoPtr pInfo, > return PS2ReadHwStatePriv(pInfo, &psm_proto_operations, comm, hwRet); > } > > -static Bool PSMAutoDevProbe(InputInfoPtr pInfo) > -{ > - return FALSE; > -} > - > struct SynapticsProtocolOperations psm_proto_operations = { > NULL, > NULL, > PSMQueryHardware, > PSMReadHwState, > - PSMAutoDevProbe, > + NULL, > NULL > }; > diff --git a/src/synaptics.c b/src/synaptics.c > index 1a559a2..8819798 100644 > --- a/src/synaptics.c > +++ b/src/synaptics.c > @@ -135,6 +135,18 @@ void InitDeviceProperties(InputInfoPtr pInfo); > int SetProperty(DeviceIntPtr dev, Atom property, XIPropertyValuePtr prop, > BOOL checkonly); > > +const static SynapticsProtocolRec protocols[] = { > + {"psaux", &psaux_proto_operations}, > + {"alps", &alps_proto_operations}, > +#ifdef BUILD_PSMCOMM > + {"psm", &psm_proto_operations}, > +#endif > +#ifdef BUILD_EVENTCOMM > + {"event", &event_proto_operations}, > +#endif > + {NULL, NULL} > +}; event is the most common backend these days, moving it up would save precious nanoseconds ;) > + > InputDriverRec SYNAPTICS = { > 1, > "synaptics", > @@ -177,61 +189,23 @@ _X_EXPORT XF86ModuleData synapticsModuleData = { > static void > SetDeviceAndProtocol(InputInfoPtr pInfo) > { > - char *str_par, *device; > SynapticsPrivate *priv = pInfo->private; > - enum SynapticsProtocol proto = SYN_PROTO_PSAUX; > + char *proto, *device; > + int i; > > + proto = xf86SetStrOption(pInfo->options, "Protocol", NULL); > device = xf86SetStrOption(pInfo->options, "Device", NULL); > - if (!device) { > - device = xf86SetStrOption(pInfo->options, "Path", NULL); > - if (device) { > - pInfo->options = > - xf86ReplaceStrOption(pInfo->options, "Device", device); > - } > - } > - if (device && strstr(device, "/dev/input/event")) { > -#ifdef BUILD_EVENTCOMM > - proto = SYN_PROTO_EVENT; > -#endif > - } else { > - str_par = xf86FindOptionValue(pInfo->options, "Protocol"); > - if (str_par && !strcmp(str_par, "psaux")) { > - /* Already set up */ > -#ifdef BUILD_EVENTCOMM > - } else if (str_par && !strcmp(str_par, "event")) { > - proto = SYN_PROTO_EVENT; > -#endif /* BUILD_EVENTCOMM */ > -#ifdef BUILD_PSMCOMM > - } else if (str_par && !strcmp(str_par, "psm")) { > - proto = SYN_PROTO_PSM; > -#endif /* BUILD_PSMCOMM */ > - } else if (str_par && !strcmp(str_par, "alps")) { > - proto = SYN_PROTO_ALPS; > - } else { /* default to auto-dev */ > -#ifdef BUILD_EVENTCOMM > - if (!device && event_proto_operations.AutoDevProbe(pInfo)) > - proto = SYN_PROTO_EVENT; > -#endif > - } > - } > - switch (proto) { > - case SYN_PROTO_PSAUX: > - priv->proto_ops = &psaux_proto_operations; > - break; > -#ifdef BUILD_EVENTCOMM > - case SYN_PROTO_EVENT: > - priv->proto_ops = &event_proto_operations; > - break; > -#endif /* BUILD_EVENTCOMM */ > -#ifdef BUILD_PSMCOMM > - case SYN_PROTO_PSM: > - priv->proto_ops = &psm_proto_operations; > - break; > -#endif /* BUILD_PSMCOMM */ > - case SYN_PROTO_ALPS: > - priv->proto_ops = &alps_proto_operations; > - break; > + for (i = 0; protocols[i].name; i++) { > + if (proto && device && !strcmp(proto, protocols[i].name)) > + break; > + if (protocols[i].proto_ops->AutoDevProbe && > + protocols[i].proto_ops->AutoDevProbe(pInfo)) > + break; this needs a comment. looking at the old code it's understandable why you have the device != NULL check in there but this isn't quite as clear when just looking at the new code. just by reshuffling the two conditions you make this clearer: if (!device && AutoDevProve && AutoDevProbe(pInfo)) break; else if (proto && !strcmp(proto, name)) break; > } > + free(proto); > + free(device); > + > + priv->proto_ops = protocols[i].proto_ops; > } > > /* > @@ -656,6 +630,10 @@ SynapticsPreInit(InputDriverPtr drv, InputInfoPtr pInfo, > int flags) > > /* may change pInfo->options */ > SetDeviceAndProtocol(pInfo); > + if (priv->proto_ops == NULL) { > + xf86Msg(X_ERROR, "Synaptics driver unable to detect protocol\n"); > + goto SetupProc_fail; > + } > > /* open the touchpad device */ > pInfo->fd = xf86OpenSerial(pInfo->options); > diff --git a/src/synproto.h b/src/synproto.h > index 4b37df0..9c25428 100644 > --- a/src/synproto.h > +++ b/src/synproto.h > @@ -67,17 +67,6 @@ struct CommData { > Bool threeFingers; > }; > > -enum SynapticsProtocol { > - SYN_PROTO_PSAUX, /* Raw psaux device */ > -#ifdef BUILD_EVENTCOMM > - SYN_PROTO_EVENT, /* Linux kernel event interface */ > -#endif /* BUILD_EVENTCOMM */ > -#ifdef BUILD_PSMCOMM > - SYN_PROTO_PSM, /* FreeBSD psm driver */ > -#endif /* BUILD_PSMCOMM */ > - SYN_PROTO_ALPS /* ALPS touchpad protocol */ > -}; > - > struct _SynapticsParameters; > > struct SynapticsProtocolOperations { > @@ -90,6 +79,11 @@ struct SynapticsProtocolOperations { > void (*ReadDevDimensions)(InputInfoPtr pInfo); > }; > > +typedef struct { > + const char *name; > + struct SynapticsProtocolOperations *proto_ops; > +} SynapticsProtocolRec; > + this isn't used anywhere else, so you could just move that up to where protocols[] is defined. Cheers, Peter > extern struct SynapticsProtocolOperations psaux_proto_operations; > #ifdef BUILD_EVENTCOMM > extern struct SynapticsProtocolOperations event_proto_operations; > -- > 1.7.3.5 > _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel