| Message ID | 20180419133934.31306-1-andr2000@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Juergen, Jason, Dmitry any comment on this? Thank you, Oleksandr On 04/19/2018 04:39 PM, 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. > 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: > - provide no_ptr_dev if no pointer device needs to be created > - provide no_kbd_dev 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> > --- > Changes since v1: > - changed module parameters from uint to bool > > 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..d8cca212f737 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 bool no_ptr_dev; > +module_param(no_ptr_dev, bool, 0); > +MODULE_PARM_DESC(no_ptr_dev, > + "If set then no virtual pointing device exposed to the guest"); > + > +static bool no_kbd_dev; > +module_param(no_kbd_dev, bool, 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) { -- 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 23/04/18 10:02, Oleksandr Andrushchenko wrote: > Juergen, Jason, Dmitry > any comment on this? Oleksandr, please give us some time. I can't speak for others, but I am not sitting here idling and hoping that some work (e.g. patches to review) might appear. I have a lot of other stuff to do and will respond when I find some time to look at your patches. Pinging others on Monday when having sent out the patch only on Thursday is rather unfriendly. 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/23/2018 11:23 AM, Juergen Gross wrote: > On 23/04/18 10:02, Oleksandr Andrushchenko wrote: >> Juergen, Jason, Dmitry >> any comment on this? > Oleksandr, please give us some time. I can't speak for others, but I am > not sitting here idling and hoping that some work (e.g. patches to > review) might appear. > > I have a lot of other stuff to do and will respond when I find some time > to look at your patches. > > Pinging others on Monday when having sent out the patch only on Thursday > is rather unfriendly. Really sorry about this, my bad. I had an impression that the only change we need to discuss was uint -> bool change which seemed rather trivial > > 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
Hi, Oleksandr. On Thu, Apr 19, 2018 at 9:39 AM, Oleksandr Andrushchenko <andr2000@gmail.com> wrote: <snip> > @@ -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) { My earlier suggestion on the option name was aimed at replacing the above with: if (kbd_dev) { I find it easier to read then the double negative !no_kbd_dev. But it's only used once, so it's not a big deal either way. <snip> > > - __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); Just noticed these lines now exceed 80 columns. Otherwise it's just code motion and fine by me. Regards, Jason -- 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 Mon, Apr 23, 2018 at 11:29:45AM +0300, Oleksandr Andrushchenko wrote: > On 04/23/2018 11:23 AM, Juergen Gross wrote: > > On 23/04/18 10:02, Oleksandr Andrushchenko wrote: > > > Juergen, Jason, Dmitry > > > any comment on this? > > Oleksandr, please give us some time. I can't speak for others, but I am > > not sitting here idling and hoping that some work (e.g. patches to > > review) might appear. > > > > I have a lot of other stuff to do and will respond when I find some time > > to look at your patches. > > > > Pinging others on Monday when having sent out the patch only on Thursday > > is rather unfriendly. > Really sorry about this, my bad. > I had an impression that the only change we need to > discuss was uint -> bool change which seemed rather trivial I am sorry I did not respond to the previous version until today, but I do not think that we should be extending module parameters for this. Protocol features are already there for absolute pointers and multi-touch, keyboard and relative pointers should use the same mechanism. Thanks.
On 04/23/2018 09:55 PM, Dmitry Torokhov wrote: > On Mon, Apr 23, 2018 at 11:29:45AM +0300, Oleksandr Andrushchenko wrote: >> On 04/23/2018 11:23 AM, Juergen Gross wrote: >>> On 23/04/18 10:02, Oleksandr Andrushchenko wrote: >>>> Juergen, Jason, Dmitry >>>> any comment on this? >>> Oleksandr, please give us some time. I can't speak for others, but I am >>> not sitting here idling and hoping that some work (e.g. patches to >>> review) might appear. >>> >>> I have a lot of other stuff to do and will respond when I find some time >>> to look at your patches. >>> >>> Pinging others on Monday when having sent out the patch only on Thursday >>> is rather unfriendly. >> Really sorry about this, my bad. >> I had an impression that the only change we need to >> discuss was uint -> bool change which seemed rather trivial > I am sorry I did not respond to the previous version until today, but I > do not think that we should be extending module parameters for this. > Protocol features are already there for absolute pointers and > multi-touch, keyboard and relative pointers should use the same > mechanism. Ok, then I'll start from extending the protocol > 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
diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c index d91f3b1c5375..d8cca212f737 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 bool no_ptr_dev; +module_param(no_ptr_dev, bool, 0); +MODULE_PARM_DESC(no_ptr_dev, + "If set then no virtual pointing device exposed to the guest"); + +static bool no_kbd_dev; +module_param(no_kbd_dev, bool, 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) {