LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v2] Input: xen-kbdfront - allow better run-time configuration
@ 2018-04-19 13:39 Oleksandr Andrushchenko
  2018-04-23  8:02 ` Oleksandr Andrushchenko
  2018-04-23 18:40 ` [Xen-devel] " Jason Andryuk
  0 siblings, 2 replies; 7+ messages in thread
From: Oleksandr Andrushchenko @ 2018-04-19 13:39 UTC (permalink / raw)
  To: xen-devel, linux-input, linux-kernel, dmitry.torokhov, jgross,
	lyan, boris.ostrovsky
  Cc: andr2000, andrii_chepurnyi, Oleksandr Andrushchenko

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) {
-- 
2.17.0

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] Input: xen-kbdfront - allow better run-time configuration
  2018-04-19 13:39 [PATCH v2] Input: xen-kbdfront - allow better run-time configuration Oleksandr Andrushchenko
@ 2018-04-23  8:02 ` Oleksandr Andrushchenko
  2018-04-23  8:23   ` Juergen Gross
  2018-04-23 18:40 ` [Xen-devel] " Jason Andryuk
  1 sibling, 1 reply; 7+ messages in thread
From: Oleksandr Andrushchenko @ 2018-04-23  8:02 UTC (permalink / raw)
  To: xen-devel, linux-input, linux-kernel, dmitry.torokhov, jgross,
	lyan, boris.ostrovsky
  Cc: andrii_chepurnyi, Oleksandr Andrushchenko

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) {

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] Input: xen-kbdfront - allow better run-time configuration
  2018-04-23  8:02 ` Oleksandr Andrushchenko
@ 2018-04-23  8:23   ` Juergen Gross
  2018-04-23  8:29     ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 7+ messages in thread
From: Juergen Gross @ 2018-04-23  8:23 UTC (permalink / raw)
  To: Oleksandr Andrushchenko, xen-devel, linux-input, linux-kernel,
	dmitry.torokhov, lyan, boris.ostrovsky
  Cc: andrii_chepurnyi, Oleksandr Andrushchenko

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] Input: xen-kbdfront - allow better run-time configuration
  2018-04-23  8:23   ` Juergen Gross
@ 2018-04-23  8:29     ` Oleksandr Andrushchenko
  2018-04-23 18:55       ` Dmitry Torokhov
  0 siblings, 1 reply; 7+ messages in thread
From: Oleksandr Andrushchenko @ 2018-04-23  8:29 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, linux-input, linux-kernel,
	dmitry.torokhov, lyan, boris.ostrovsky
  Cc: andrii_chepurnyi, Oleksandr Andrushchenko

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Xen-devel] [PATCH v2] Input: xen-kbdfront - allow better run-time configuration
  2018-04-19 13:39 [PATCH v2] Input: xen-kbdfront - allow better run-time configuration Oleksandr Andrushchenko
  2018-04-23  8:02 ` Oleksandr Andrushchenko
@ 2018-04-23 18:40 ` Jason Andryuk
  1 sibling, 0 replies; 7+ messages in thread
From: Jason Andryuk @ 2018-04-23 18:40 UTC (permalink / raw)
  To: Oleksandr Andrushchenko
  Cc: xen-devel, linux-input, open list, Dmitry Torokhov,
	Juergen Gross, lyan, Boris Ostrovsky, andrii_chepurnyi,
	Oleksandr Andrushchenko

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] Input: xen-kbdfront - allow better run-time configuration
  2018-04-23  8:29     ` Oleksandr Andrushchenko
@ 2018-04-23 18:55       ` Dmitry Torokhov
  2018-04-24  6:05         ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 7+ messages in thread
From: Dmitry Torokhov @ 2018-04-23 18:55 UTC (permalink / raw)
  To: Oleksandr Andrushchenko
  Cc: Juergen Gross, xen-devel, linux-input, linux-kernel, lyan,
	boris.ostrovsky, andrii_chepurnyi, Oleksandr Andrushchenko

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.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] Input: xen-kbdfront - allow better run-time configuration
  2018-04-23 18:55       ` Dmitry Torokhov
@ 2018-04-24  6:05         ` Oleksandr Andrushchenko
  0 siblings, 0 replies; 7+ messages in thread
From: Oleksandr Andrushchenko @ 2018-04-24  6:05 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Juergen Gross, xen-devel, linux-input, linux-kernel, lyan,
	boris.ostrovsky, andrii_chepurnyi, Oleksandr Andrushchenko

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.
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-04-24  6:05 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-19 13:39 [PATCH v2] Input: xen-kbdfront - allow better run-time configuration Oleksandr Andrushchenko
2018-04-23  8:02 ` Oleksandr Andrushchenko
2018-04-23  8:23   ` Juergen Gross
2018-04-23  8:29     ` Oleksandr Andrushchenko
2018-04-23 18:55       ` Dmitry Torokhov
2018-04-24  6:05         ` Oleksandr Andrushchenko
2018-04-23 18:40 ` [Xen-devel] " Jason Andryuk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).