[PATCH 1/2] kdrive: add protocol mouse option

[Peter Hutterer]

On Wed, May 27, 2009 at 07:04:16PM +0200, Olivier Blin wrote:
> Peter Hutterer <peter.hutterer at who-t.net> writes:
> 
> > On Tue, May 26, 2009 at 03:30:08PM +0200, Olivier Blin wrote:
> >> kdrive probes a lot of PS/2 protocols for the mouse device, which
> >> makes the mouse unusable for some seconds after X startup.
> >> This new "protocol" option allows forcing the mouse protocol.
> >> It can be used this way:
> >> Xfbdev -mouse mouse,,protocol=ps/2 -keybd keyboard
> >> 
> >> Signed-off-by: Olivier Blin <blino at mandriva.com>
> >> ---
> >>  hw/kdrive/linux/mouse.c |    4 +++-
> >>  hw/kdrive/src/kdrive.h  |    1 +
> >>  hw/kdrive/src/kinput.c  |    3 +++
> >>  3 files changed, 7 insertions(+), 1 deletions(-)
> >> 
> >> diff --git a/hw/kdrive/linux/mouse.c b/hw/kdrive/linux/mouse.c
> >> index 02214b3..253da26 100644
> >> --- a/hw/kdrive/linux/mouse.c
> >> +++ b/hw/kdrive/linux/mouse.c
> >> @@ -961,7 +961,9 @@ MouseInit (KdPointerInfo *pi)
> >>      km = (Kmouse *) xalloc (sizeof (Kmouse));
> >>      if (km) {
> >>          km->iob.avail = km->iob.used = 0;
> >> -        MouseFirstProtocol(km, "exps/2");
> >> +        MouseFirstProtocol(km, pi->force_protocol ? pi->force_protocol : "exps/2");
> >> +        if (pi->force_protocol) 
> >> +                km->state = MouseWorking;
> >>          km->i_prot = 0;
> >>          km->tty = isatty (fd);
> >>          km->iob.fd = -1;
> >> diff --git a/hw/kdrive/src/kdrive.h b/hw/kdrive/src/kdrive.h
> >> index c60559a..c025144 100644
> >> --- a/hw/kdrive/src/kdrive.h
> >> +++ b/hw/kdrive/src/kdrive.h
> >> @@ -220,6 +220,7 @@ struct _KdPointerInfo {
> >>      DeviceIntPtr          dixdev;
> >>      char                  *name;
> >>      char                  *path;
> >> +    char                  *force_protocol;
> >>      InputOption           *options;
> >>      int                   inputClass;
> >>  
> >> diff --git a/hw/kdrive/src/kinput.c b/hw/kdrive/src/kinput.c
> >> index 0d216a9..5f82ba4 100644
> >> --- a/hw/kdrive/src/kinput.c
> >> +++ b/hw/kdrive/src/kinput.c
> >> @@ -1166,6 +1166,8 @@ KdParsePointerOptions (KdPointerInfo *pi)
> >>              pi->transformCoordinates = FALSE;
> >>          else if (!strcasecmp (option->key, "device"))
> >>              pi->path = strdup(option->value);
> >> +        else if (!strcasecmp (option->key, "protocol"))
> >> +            pi->force_protocol = strdup(option->value);
> >>          else
> >>              ErrorF("Pointer option key (%s) of value (%s) not assigned!\n", 
> >>                      option->key, option->value);
> >> @@ -1186,6 +1188,7 @@ KdParsePointer (char *arg)
> >>          return NULL;
> >>      pi->emulateMiddleButton = kdEmulateMiddleButton;
> >>      pi->transformCoordinates = !kdRawPointerCoordinates;
> >> +    pi->force_protocol = NULL;
> >>      pi->nButtons = 5; /* XXX should not be hardcoded */
> >>      pi->inputClass = KD_MOUSE;
> >>  
> >> -- 
> >> 1.6.2.4
> >
> > wouldn't it be better to name the option "protocol" instead of
> > "force_protocol" to reflect the cmdline option?
> 
> I used the "force_" prefix mainly to indicate it is optional, but it's
> ok to just name it protocol (note that "device" option and pi->path have
> the same issue, if we want to be picky).
> Do you want me to resend the patch using "protocol" as field name?

yes. I think it is the better option. We don't need to show in the name that
it's optional, it's common that an unset option defaults to some
auto-detected value. 

force_protocol is IMO even a bit misleading here, it would indicate that
you're overriding or enforcing a protocol on the device even if the protocol
is not supported. When in fact I guess that by using a wrong protocol the
device simply won't work, right?

Either way, amend with s/force_protocol/protocol/ and we're all good.

Cheers,
  Peter