Message ID | 20180418150445.9805-1-andr2000@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 18/04/18 17:04, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > It is now only possible to control if multi-touch virtual device > is created or not (via the corresponding XenStore entries), > but keyboard and pointer devices are always created. Why don't you want to go that route for keyboard and mouse, too? Or does this really make no sense? > In some cases this is not desirable. For example, if virtual > keyboard device is exposed to Android then the latter won't > automatically show on-screen keyboard as it expects that a > physical keyboard device can be used for typing. > > Make it possible to configure which virtual devices are created > with module parameters: > - no_ptr_dev=1 if no pointer device needs to be created > - no_kbd_dev=1 if no keyboard device needs to be created > Keep old behavior by default. > > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > Suggested-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com> > Tested-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com> > --- > drivers/input/misc/xen-kbdfront.c | 159 +++++++++++++++++------------- > 1 file changed, 92 insertions(+), 67 deletions(-) > > diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c > index d91f3b1c5375..a3306aad40b0 100644 > --- a/drivers/input/misc/xen-kbdfront.c > +++ b/drivers/input/misc/xen-kbdfront.c > @@ -51,6 +51,16 @@ module_param_array(ptr_size, int, NULL, 0444); > MODULE_PARM_DESC(ptr_size, > "Pointing device width, height in pixels (default 800,600)"); > > +static unsigned int no_ptr_dev; > +module_param(no_ptr_dev, uint, 0); Use type invbool instead? > +MODULE_PARM_DESC(no_ptr_dev, > + "If set then no virtual pointing device exposed to the guest"); > + > +static unsigned int no_kbd_dev; > +module_param(no_kbd_dev, uint, 0); invbool? Juergen -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/19/2018 02:25 PM, Juergen Gross wrote: > On 18/04/18 17:04, Oleksandr Andrushchenko wrote: >> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> >> It is now only possible to control if multi-touch virtual device >> is created or not (via the corresponding XenStore entries), >> but keyboard and pointer devices are always created. > Why don't you want to go that route for keyboard and mouse, too? > Or does this really make no sense? Well, I would prefer not to touch anything outside Linux and this driver. And these settings seem to be implementation specific. So, this is why introduce Linux module parameters and don't extend the kbdif protocol. >> In some cases this is not desirable. For example, if virtual >> keyboard device is exposed to Android then the latter won't >> automatically show on-screen keyboard as it expects that a >> physical keyboard device can be used for typing. >> >> Make it possible to configure which virtual devices are created >> with module parameters: >> - no_ptr_dev=1 if no pointer device needs to be created >> - no_kbd_dev=1 if no keyboard device needs to be created >> Keep old behavior by default. >> >> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> Suggested-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com> >> Tested-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com> >> --- >> drivers/input/misc/xen-kbdfront.c | 159 +++++++++++++++++------------- >> 1 file changed, 92 insertions(+), 67 deletions(-) >> >> diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c >> index d91f3b1c5375..a3306aad40b0 100644 >> --- a/drivers/input/misc/xen-kbdfront.c >> +++ b/drivers/input/misc/xen-kbdfront.c >> @@ -51,6 +51,16 @@ module_param_array(ptr_size, int, NULL, 0444); >> MODULE_PARM_DESC(ptr_size, >> "Pointing device width, height in pixels (default 800,600)"); >> >> +static unsigned int no_ptr_dev; >> +module_param(no_ptr_dev, uint, 0); > Use type invbool instead? Hm, better bool then? invbool will require parameter name change to something like "with_ptr_dev" which might confuse, e.g. default was to go with pointer device, now we have with_ptr_dev module parameter: do I now need to set it to preserve the old behavior? The answer is no (because of invbool), but you have to dig for it. Will bool work for you? >> +MODULE_PARM_DESC(no_ptr_dev, >> + "If set then no virtual pointing device exposed to the guest"); >> + >> +static unsigned int no_kbd_dev; >> +module_param(no_kbd_dev, uint, 0); > invbool? > > > Juergen -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 19/04/18 13:44, Oleksandr Andrushchenko wrote: > On 04/19/2018 02:25 PM, Juergen Gross wrote: >> On 18/04/18 17:04, Oleksandr Andrushchenko wrote: >>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>> >>> It is now only possible to control if multi-touch virtual device >>> is created or not (via the corresponding XenStore entries), >>> but keyboard and pointer devices are always created. >> Why don't you want to go that route for keyboard and mouse, too? >> Or does this really make no sense? > Well, I would prefer not to touch anything outside Linux and > this driver. And these settings seem to be implementation specific. > So, this is why introduce Linux module parameters and don't extend > the kbdif protocol. >>> In some cases this is not desirable. For example, if virtual >>> keyboard device is exposed to Android then the latter won't >>> automatically show on-screen keyboard as it expects that a >>> physical keyboard device can be used for typing. >>> >>> Make it possible to configure which virtual devices are created >>> with module parameters: >>> - no_ptr_dev=1 if no pointer device needs to be created >>> - no_kbd_dev=1 if no keyboard device needs to be created >>> Keep old behavior by default. >>> >>> Signed-off-by: Oleksandr Andrushchenko >>> <oleksandr_andrushchenko@epam.com> >>> Suggested-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com> >>> Tested-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com> >>> --- >>> drivers/input/misc/xen-kbdfront.c | 159 +++++++++++++++++------------- >>> 1 file changed, 92 insertions(+), 67 deletions(-) >>> >>> diff --git a/drivers/input/misc/xen-kbdfront.c >>> b/drivers/input/misc/xen-kbdfront.c >>> index d91f3b1c5375..a3306aad40b0 100644 >>> --- a/drivers/input/misc/xen-kbdfront.c >>> +++ b/drivers/input/misc/xen-kbdfront.c >>> @@ -51,6 +51,16 @@ module_param_array(ptr_size, int, NULL, 0444); >>> MODULE_PARM_DESC(ptr_size, >>> "Pointing device width, height in pixels (default 800,600)"); >>> +static unsigned int no_ptr_dev; >>> +module_param(no_ptr_dev, uint, 0); >> Use type invbool instead? > Hm, better bool then? invbool will require parameter name change to > something like "with_ptr_dev" which might confuse, e.g. > default was to go with pointer device, now we have with_ptr_dev > module parameter: do I now need to set it to preserve the old behavior? > The answer is no (because of invbool), but you have to dig for it. > > Will bool work for you? As long as the default won't change from today: yes. Juergen -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 19, 2018 at 02:44:19PM +0300, Oleksandr Andrushchenko wrote: > On 04/19/2018 02:25 PM, Juergen Gross wrote: > > On 18/04/18 17:04, Oleksandr Andrushchenko wrote: > > > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > > > > > It is now only possible to control if multi-touch virtual device > > > is created or not (via the corresponding XenStore entries), > > > but keyboard and pointer devices are always created. > > Why don't you want to go that route for keyboard and mouse, too? > > Or does this really make no sense? > Well, I would prefer not to touch anything outside Linux and > this driver. And these settings seem to be implementation specific. > So, this is why introduce Linux module parameters and don't extend > the kbdif protocol. Why do you consider this implementation specific? How other guests decide to forego creation of relative pointer device or keyboard-like device? You already have "features" for absolute pointing device and multitouch, so please extend the protocol properly so you indeed do not code something implementation-specific (i.e. module parameters). Thanks.
On 04/23/2018 09:53 PM, Dmitry Torokhov wrote: > On Thu, Apr 19, 2018 at 02:44:19PM +0300, Oleksandr Andrushchenko wrote: >> On 04/19/2018 02:25 PM, Juergen Gross wrote: >>> On 18/04/18 17:04, Oleksandr Andrushchenko wrote: >>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>>> >>>> It is now only possible to control if multi-touch virtual device >>>> is created or not (via the corresponding XenStore entries), >>>> but keyboard and pointer devices are always created. >>> Why don't you want to go that route for keyboard and mouse, too? >>> Or does this really make no sense? >> Well, I would prefer not to touch anything outside Linux and >> this driver. And these settings seem to be implementation specific. >> So, this is why introduce Linux module parameters and don't extend >> the kbdif protocol. > Why do you consider this implementation specific? How other guests > decide to forego creation of relative pointer device or keyboard-like > device? > > You already have "features" for absolute pointing device and multitouch, > so please extend the protocol properly so you indeed do not code > something implementation-specific (i.e. module parameters). Ok, but in order to preserve the default behavior, e.g. pointer and keyboard devices are always created now, I'll have to have reverse features in the protocol: - feature-no-pointer - feature-no-keyboard The above may be set as a part of frontend's configuration and if missed are considered to be set to false. > > Thanks. > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 24, 2018 at 08:55:19AM +0300, Oleksandr Andrushchenko wrote: > On 04/23/2018 09:53 PM, Dmitry Torokhov wrote: > > On Thu, Apr 19, 2018 at 02:44:19PM +0300, Oleksandr Andrushchenko wrote: > > > On 04/19/2018 02:25 PM, Juergen Gross wrote: > > > > On 18/04/18 17:04, Oleksandr Andrushchenko wrote: > > > > > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > > > > > > > > > It is now only possible to control if multi-touch virtual device > > > > > is created or not (via the corresponding XenStore entries), > > > > > but keyboard and pointer devices are always created. > > > > Why don't you want to go that route for keyboard and mouse, too? > > > > Or does this really make no sense? > > > Well, I would prefer not to touch anything outside Linux and > > > this driver. And these settings seem to be implementation specific. > > > So, this is why introduce Linux module parameters and don't extend > > > the kbdif protocol. > > Why do you consider this implementation specific? How other guests > > decide to forego creation of relative pointer device or keyboard-like > > device? > > > > You already have "features" for absolute pointing device and multitouch, > > so please extend the protocol properly so you indeed do not code > > something implementation-specific (i.e. module parameters). > Ok, but in order to preserve the default behavior, e.g. > pointer and keyboard devices are always created now, I'll have > to have reverse features in the protocol: > - feature-no-pointer > - feature-no-keyboard > The above may be set as a part of frontend's configuration and > if missed are considered to be set to false. I think you can have them as "feature-pointer" and "feature-keyboard" (no negation), but assume not present considered enabled. I.e. kbd = xenbus_read_unsigned(..., XENKBD_FIELD_FEAT_KEYBOARD, 1); if (kbd) { ... } Thanks.
On 04/26/2018 10:16 PM, Dmitry Torokhov wrote: > On Tue, Apr 24, 2018 at 08:55:19AM +0300, Oleksandr Andrushchenko wrote: >> On 04/23/2018 09:53 PM, Dmitry Torokhov wrote: >>> On Thu, Apr 19, 2018 at 02:44:19PM +0300, Oleksandr Andrushchenko wrote: >>>> On 04/19/2018 02:25 PM, Juergen Gross wrote: >>>>> On 18/04/18 17:04, Oleksandr Andrushchenko wrote: >>>>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>>>>> >>>>>> It is now only possible to control if multi-touch virtual device >>>>>> is created or not (via the corresponding XenStore entries), >>>>>> but keyboard and pointer devices are always created. >>>>> Why don't you want to go that route for keyboard and mouse, too? >>>>> Or does this really make no sense? >>>> Well, I would prefer not to touch anything outside Linux and >>>> this driver. And these settings seem to be implementation specific. >>>> So, this is why introduce Linux module parameters and don't extend >>>> the kbdif protocol. >>> Why do you consider this implementation specific? How other guests >>> decide to forego creation of relative pointer device or keyboard-like >>> device? >>> >>> You already have "features" for absolute pointing device and multitouch, >>> so please extend the protocol properly so you indeed do not code >>> something implementation-specific (i.e. module parameters). >> Ok, but in order to preserve the default behavior, e.g. >> pointer and keyboard devices are always created now, I'll have >> to have reverse features in the protocol: >> - feature-no-pointer >> - feature-no-keyboard >> The above may be set as a part of frontend's configuration and >> if missed are considered to be set to false. > I think you can have them as "feature-pointer" and "feature-keyboard" > (no negation), but assume not present considered enabled. I.e. > > kbd = xenbus_read_unsigned(..., XENKBD_FIELD_FEAT_KEYBOARD, 1); > if (kbd) { > ... Thank you for your comments, could you please take a look at the patch [1] where I am trying to change the corresponding Xen protocol to fit the requirements? As we agreed I have to change the protocol first, so this patch is no longer valid > } > > Thanks. Thank you, Oleksandr [1] https://www.spinics.net/lists/linux-input/msg56094.html -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c index d91f3b1c5375..a3306aad40b0 100644 --- a/drivers/input/misc/xen-kbdfront.c +++ b/drivers/input/misc/xen-kbdfront.c @@ -51,6 +51,16 @@ module_param_array(ptr_size, int, NULL, 0444); MODULE_PARM_DESC(ptr_size, "Pointing device width, height in pixels (default 800,600)"); +static unsigned int no_ptr_dev; +module_param(no_ptr_dev, uint, 0); +MODULE_PARM_DESC(no_ptr_dev, + "If set then no virtual pointing device exposed to the guest"); + +static unsigned int no_kbd_dev; +module_param(no_kbd_dev, uint, 0); +MODULE_PARM_DESC(no_kbd_dev, + "If set then no virtual keyboard device exposed to the guest"); + static int xenkbd_remove(struct xenbus_device *); static int xenkbd_connect_backend(struct xenbus_device *, struct xenkbd_info *); static void xenkbd_disconnect_backend(struct xenkbd_info *); @@ -63,6 +73,9 @@ static void xenkbd_disconnect_backend(struct xenkbd_info *); static void xenkbd_handle_motion_event(struct xenkbd_info *info, struct xenkbd_motion *motion) { + if (unlikely(!info->ptr)) + return; + input_report_rel(info->ptr, REL_X, motion->rel_x); input_report_rel(info->ptr, REL_Y, motion->rel_y); if (motion->rel_z) @@ -73,6 +86,9 @@ static void xenkbd_handle_motion_event(struct xenkbd_info *info, static void xenkbd_handle_position_event(struct xenkbd_info *info, struct xenkbd_position *pos) { + if (unlikely(!info->ptr)) + return; + input_report_abs(info->ptr, ABS_X, pos->abs_x); input_report_abs(info->ptr, ABS_Y, pos->abs_y); if (pos->rel_z) @@ -97,6 +113,9 @@ static void xenkbd_handle_key_event(struct xenkbd_info *info, return; } + if (unlikely(!dev)) + return; + input_event(dev, EV_KEY, key->keycode, value); input_sync(dev); } @@ -192,7 +211,7 @@ static int xenkbd_probe(struct xenbus_device *dev, const struct xenbus_device_id *id) { int ret, i; - unsigned int abs, touch; + unsigned int touch; struct xenkbd_info *info; struct input_dev *kbd, *ptr, *mtouch; @@ -211,24 +230,6 @@ static int xenkbd_probe(struct xenbus_device *dev, if (!info->page) goto error_nomem; - /* Set input abs params to match backend screen res */ - abs = xenbus_read_unsigned(dev->otherend, - XENKBD_FIELD_FEAT_ABS_POINTER, 0); - ptr_size[KPARAM_X] = xenbus_read_unsigned(dev->otherend, - XENKBD_FIELD_WIDTH, - ptr_size[KPARAM_X]); - ptr_size[KPARAM_Y] = xenbus_read_unsigned(dev->otherend, - XENKBD_FIELD_HEIGHT, - ptr_size[KPARAM_Y]); - if (abs) { - ret = xenbus_write(XBT_NIL, dev->nodename, - XENKBD_FIELD_REQ_ABS_POINTER, "1"); - if (ret) { - pr_warn("xenkbd: can't request abs-pointer\n"); - abs = 0; - } - } - touch = xenbus_read_unsigned(dev->nodename, XENKBD_FIELD_FEAT_MTOUCH, 0); if (touch) { @@ -241,60 +242,84 @@ static int xenkbd_probe(struct xenbus_device *dev, } /* keyboard */ - kbd = input_allocate_device(); - if (!kbd) - goto error_nomem; - kbd->name = "Xen Virtual Keyboard"; - kbd->phys = info->phys; - kbd->id.bustype = BUS_PCI; - kbd->id.vendor = 0x5853; - kbd->id.product = 0xffff; - - __set_bit(EV_KEY, kbd->evbit); - for (i = KEY_ESC; i < KEY_UNKNOWN; i++) - __set_bit(i, kbd->keybit); - for (i = KEY_OK; i < KEY_MAX; i++) - __set_bit(i, kbd->keybit); - - ret = input_register_device(kbd); - if (ret) { - input_free_device(kbd); - xenbus_dev_fatal(dev, ret, "input_register_device(kbd)"); - goto error; + if (!no_kbd_dev) { + kbd = input_allocate_device(); + if (!kbd) + goto error_nomem; + kbd->name = "Xen Virtual Keyboard"; + kbd->phys = info->phys; + kbd->id.bustype = BUS_PCI; + kbd->id.vendor = 0x5853; + kbd->id.product = 0xffff; + + __set_bit(EV_KEY, kbd->evbit); + for (i = KEY_ESC; i < KEY_UNKNOWN; i++) + __set_bit(i, kbd->keybit); + for (i = KEY_OK; i < KEY_MAX; i++) + __set_bit(i, kbd->keybit); + + ret = input_register_device(kbd); + if (ret) { + input_free_device(kbd); + xenbus_dev_fatal(dev, ret, "input_register_device(kbd)"); + goto error; + } + info->kbd = kbd; } - info->kbd = kbd; /* pointing device */ - ptr = input_allocate_device(); - if (!ptr) - goto error_nomem; - ptr->name = "Xen Virtual Pointer"; - ptr->phys = info->phys; - ptr->id.bustype = BUS_PCI; - ptr->id.vendor = 0x5853; - ptr->id.product = 0xfffe; - - if (abs) { - __set_bit(EV_ABS, ptr->evbit); - input_set_abs_params(ptr, ABS_X, 0, ptr_size[KPARAM_X], 0, 0); - input_set_abs_params(ptr, ABS_Y, 0, ptr_size[KPARAM_Y], 0, 0); - } else { - input_set_capability(ptr, EV_REL, REL_X); - input_set_capability(ptr, EV_REL, REL_Y); - } - input_set_capability(ptr, EV_REL, REL_WHEEL); + if (!no_ptr_dev) { + unsigned int abs; + + /* Set input abs params to match backend screen res */ + abs = xenbus_read_unsigned(dev->otherend, + XENKBD_FIELD_FEAT_ABS_POINTER, 0); + ptr_size[KPARAM_X] = xenbus_read_unsigned(dev->otherend, + XENKBD_FIELD_WIDTH, + ptr_size[KPARAM_X]); + ptr_size[KPARAM_Y] = xenbus_read_unsigned(dev->otherend, + XENKBD_FIELD_HEIGHT, + ptr_size[KPARAM_Y]); + if (abs) { + ret = xenbus_write(XBT_NIL, dev->nodename, + XENKBD_FIELD_REQ_ABS_POINTER, "1"); + if (ret) { + pr_warn("xenkbd: can't request abs-pointer\n"); + abs = 0; + } + } - __set_bit(EV_KEY, ptr->evbit); - for (i = BTN_LEFT; i <= BTN_TASK; i++) - __set_bit(i, ptr->keybit); + ptr = input_allocate_device(); + if (!ptr) + goto error_nomem; + ptr->name = "Xen Virtual Pointer"; + ptr->phys = info->phys; + ptr->id.bustype = BUS_PCI; + ptr->id.vendor = 0x5853; + ptr->id.product = 0xfffe; + + if (abs) { + __set_bit(EV_ABS, ptr->evbit); + input_set_abs_params(ptr, ABS_X, 0, ptr_size[KPARAM_X], 0, 0); + input_set_abs_params(ptr, ABS_Y, 0, ptr_size[KPARAM_Y], 0, 0); + } else { + input_set_capability(ptr, EV_REL, REL_X); + input_set_capability(ptr, EV_REL, REL_Y); + } + input_set_capability(ptr, EV_REL, REL_WHEEL); - ret = input_register_device(ptr); - if (ret) { - input_free_device(ptr); - xenbus_dev_fatal(dev, ret, "input_register_device(ptr)"); - goto error; + __set_bit(EV_KEY, ptr->evbit); + for (i = BTN_LEFT; i <= BTN_TASK; i++) + __set_bit(i, ptr->keybit); + + ret = input_register_device(ptr); + if (ret) { + input_free_device(ptr); + xenbus_dev_fatal(dev, ret, "input_register_device(ptr)"); + goto error; + } + info->ptr = ptr; } - info->ptr = ptr; /* multi-touch device */ if (touch) {