[PATCH synaptics 7/8] Rewrite mechanisn detect Protocol and Device
Peter Hutterer
peter.hutterer at who-t.net
Sun Feb 27 19:36:23 PST 2011
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.Shadchin at 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
>
More information about the xorg-devel
mailing list