Re: [Xen-devel] [PATCH v3 2/2] Input: xen-kbdfront - allow better run-time configuration
On Thu, May 17, 2018 at 04:09:04PM +0300, Oleksandr Andrushchenko wrote: > On 05/17/2018 04:08 PM, Boris Ostrovsky wrote: > > On 05/17/2018 01:31 AM, Oleksandr Andrushchenko wrote: > > > I will go with the change you suggested and > > > I'll send v4 tomorrow then. > > > > > > Please make sure your changes to kbdif.h are in Xen first. I believe you > > submitted a patch there but I don't see it in the staging tree yet. > Sure, I already have Release ack for the one which is not > in Xen tree yet, hope Konrad can apply it today, > so tomorrow Xen and Linux are both ok (or by the time It is checked in (on the Xen tree). > I send v4). > > -boris > Thank you, > Oleksandr ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 2/2] Input: xen-kbdfront - allow better run-time configuration
On 05/17/2018 04:08 PM, Boris Ostrovsky wrote: On 05/17/2018 01:31 AM, Oleksandr Andrushchenko wrote: I will go with the change you suggested and I'll send v4 tomorrow then. Please make sure your changes to kbdif.h are in Xen first. I believe you submitted a patch there but I don't see it in the staging tree yet. Sure, I already have Release ack for the one which is not in Xen tree yet, hope Konrad can apply it today, so tomorrow Xen and Linux are both ok (or by the time I send v4). -boris Thank you, Oleksandr ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 2/2] Input: xen-kbdfront - allow better run-time configuration
On 05/17/2018 01:31 AM, Oleksandr Andrushchenko wrote: > I will go with the change you suggested and > I'll send v4 tomorrow then. Please make sure your changes to kbdif.h are in Xen first. I believe you submitted a patch there but I don't see it in the staging tree yet. -boris ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 2/2] Input: xen-kbdfront - allow better run-time configuration
On 05/17/2018 12:08 AM, Dmitry Torokhov wrote:
On Wed, May 16, 2018 at 08:47:30PM +0300, Oleksandr Andrushchenko wrote:
On 05/16/2018 08:15 PM, Dmitry Torokhov wrote:
Hi Oleksandr,
On Mon, May 14, 2018 at 05:40:29PM +0300, Oleksandr Andrushchenko wrote:
@@ -211,93 +220,114 @@ 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;
- }
- }
+ /*
+* The below are reverse logic, e.g. if the feature is set, then
+* do not expose the corresponding virtual device.
+*/
+ with_kbd = !xenbus_read_unsigned(dev->nodename,
+XENKBD_FIELD_FEAT_DSBL_KEYBRD, 0);
- touch = xenbus_read_unsigned(dev->nodename,
-XENKBD_FIELD_FEAT_MTOUCH, 0);
- if (touch) {
+ with_ptr = !xenbus_read_unsigned(dev->nodename,
+XENKBD_FIELD_FEAT_DSBL_POINTER, 0);
+
+ /* Direct logic: if set, then create multi-touch device. */
+ with_mtouch = xenbus_read_unsigned(dev->nodename,
+ XENKBD_FIELD_FEAT_MTOUCH, 0);
+ if (with_mtouch) {
ret = xenbus_write(XBT_NIL, dev->nodename,
XENKBD_FIELD_REQ_MTOUCH, "1");
if (ret) {
pr_warn("xenkbd: can't request multi-touch");
- touch = 0;
+ with_mtouch = 0;
}
}
Does it make sense to still end up calling xenkbd_connect_backend() when
all interfaces (keyboard, pointer, and multitouch) are disabled? Should
we do:
if (!(with_kbd || || with_ptr || with_mtouch))
return -ENXIO;
?
It does make sense. Then we probably need to move all xenbus_read_unsigned
calls to the very beginning of the .probe, so no memory allocations are made
which will be useless if we return -ENXIO, e.g. something like
static int xenkbd_probe(struct xenbus_device *dev,
const struct xenbus_device_id *id)
{
int ret, i;
bool with_mtouch, with_kbd, with_ptr;
struct xenkbd_info *info;
struct input_dev *kbd, *ptr, *mtouch;
if (!(with_kbd | with_ptr | with_mtouch))
return -ENXIO;
Does the above looks ok?
Yes. Another option is to keep the check where I suggested and do
if (...) {
ret = -ENXIO;
goto error;
}
Whichever you prefer is fine with me.
I will go with the change you suggested and
I'll send v4 tomorrow then.
Thanks.
Thank you,
Oleksandr
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 2/2] Input: xen-kbdfront - allow better run-time configuration
On Wed, May 16, 2018 at 08:47:30PM +0300, Oleksandr Andrushchenko wrote:
> On 05/16/2018 08:15 PM, Dmitry Torokhov wrote:
> > Hi Oleksandr,
> >
> > On Mon, May 14, 2018 at 05:40:29PM +0300, Oleksandr Andrushchenko wrote:
> > > @@ -211,93 +220,114 @@ 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;
> > > - }
> > > - }
> > > + /*
> > > + * The below are reverse logic, e.g. if the feature is set, then
> > > + * do not expose the corresponding virtual device.
> > > + */
> > > + with_kbd = !xenbus_read_unsigned(dev->nodename,
> > > + XENKBD_FIELD_FEAT_DSBL_KEYBRD, 0);
> > > - touch = xenbus_read_unsigned(dev->nodename,
> > > - XENKBD_FIELD_FEAT_MTOUCH, 0);
> > > - if (touch) {
> > > + with_ptr = !xenbus_read_unsigned(dev->nodename,
> > > + XENKBD_FIELD_FEAT_DSBL_POINTER, 0);
> > > +
> > > + /* Direct logic: if set, then create multi-touch device. */
> > > + with_mtouch = xenbus_read_unsigned(dev->nodename,
> > > +XENKBD_FIELD_FEAT_MTOUCH, 0);
> > > + if (with_mtouch) {
> > > ret = xenbus_write(XBT_NIL, dev->nodename,
> > > XENKBD_FIELD_REQ_MTOUCH, "1");
> > > if (ret) {
> > > pr_warn("xenkbd: can't request multi-touch");
> > > - touch = 0;
> > > + with_mtouch = 0;
> > > }
> > > }
> > Does it make sense to still end up calling xenkbd_connect_backend() when
> > all interfaces (keyboard, pointer, and multitouch) are disabled? Should
> > we do:
> >
> > if (!(with_kbd || || with_ptr || with_mtouch))
> > return -ENXIO;
> >
> > ?
> It does make sense. Then we probably need to move all xenbus_read_unsigned
> calls to the very beginning of the .probe, so no memory allocations are made
> which will be useless if we return -ENXIO, e.g. something like
>
> static int xenkbd_probe(struct xenbus_device *dev,
> const struct xenbus_device_id *id)
> {
> int ret, i;
> bool with_mtouch, with_kbd, with_ptr;
> struct xenkbd_info *info;
> struct input_dev *kbd, *ptr, *mtouch;
>
>
>
> if (!(with_kbd | with_ptr | with_mtouch))
> return -ENXIO;
>
> Does the above looks ok?
Yes. Another option is to keep the check where I suggested and do
if (...) {
ret = -ENXIO;
goto error;
}
Whichever you prefer is fine with me.
Thanks.
--
Dmitry
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 2/2] Input: xen-kbdfront - allow better run-time configuration
On 05/16/2018 08:15 PM, Dmitry Torokhov wrote:
Hi Oleksandr,
On Mon, May 14, 2018 at 05:40:29PM +0300, Oleksandr Andrushchenko wrote:
@@ -211,93 +220,114 @@ 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;
- }
- }
+ /*
+* The below are reverse logic, e.g. if the feature is set, then
+* do not expose the corresponding virtual device.
+*/
+ with_kbd = !xenbus_read_unsigned(dev->nodename,
+XENKBD_FIELD_FEAT_DSBL_KEYBRD, 0);
- touch = xenbus_read_unsigned(dev->nodename,
-XENKBD_FIELD_FEAT_MTOUCH, 0);
- if (touch) {
+ with_ptr = !xenbus_read_unsigned(dev->nodename,
+XENKBD_FIELD_FEAT_DSBL_POINTER, 0);
+
+ /* Direct logic: if set, then create multi-touch device. */
+ with_mtouch = xenbus_read_unsigned(dev->nodename,
+ XENKBD_FIELD_FEAT_MTOUCH, 0);
+ if (with_mtouch) {
ret = xenbus_write(XBT_NIL, dev->nodename,
XENKBD_FIELD_REQ_MTOUCH, "1");
if (ret) {
pr_warn("xenkbd: can't request multi-touch");
- touch = 0;
+ with_mtouch = 0;
}
}
Does it make sense to still end up calling xenkbd_connect_backend() when
all interfaces (keyboard, pointer, and multitouch) are disabled? Should
we do:
if (!(with_kbd || || with_ptr || with_mtouch))
return -ENXIO;
?
It does make sense. Then we probably need to move all xenbus_read_unsigned
calls to the very beginning of the .probe, so no memory allocations are made
which will be useless if we return -ENXIO, e.g. something like
static int xenkbd_probe(struct xenbus_device *dev,
const struct xenbus_device_id *id)
{
int ret, i;
bool with_mtouch, with_kbd, with_ptr;
struct xenkbd_info *info;
struct input_dev *kbd, *ptr, *mtouch;
if (!(with_kbd | with_ptr | with_mtouch))
return -ENXIO;
Does the above looks ok?
Thanks.
Thank you,
Oleksandr
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 2/2] Input: xen-kbdfront - allow better run-time configuration
Hi Oleksandr,
On Mon, May 14, 2018 at 05:40:29PM +0300, Oleksandr Andrushchenko wrote:
> @@ -211,93 +220,114 @@ 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;
> - }
> - }
> + /*
> + * The below are reverse logic, e.g. if the feature is set, then
> + * do not expose the corresponding virtual device.
> + */
> + with_kbd = !xenbus_read_unsigned(dev->nodename,
> + XENKBD_FIELD_FEAT_DSBL_KEYBRD, 0);
>
> - touch = xenbus_read_unsigned(dev->nodename,
> - XENKBD_FIELD_FEAT_MTOUCH, 0);
> - if (touch) {
> + with_ptr = !xenbus_read_unsigned(dev->nodename,
> + XENKBD_FIELD_FEAT_DSBL_POINTER, 0);
> +
> + /* Direct logic: if set, then create multi-touch device. */
> + with_mtouch = xenbus_read_unsigned(dev->nodename,
> +XENKBD_FIELD_FEAT_MTOUCH, 0);
> + if (with_mtouch) {
> ret = xenbus_write(XBT_NIL, dev->nodename,
> XENKBD_FIELD_REQ_MTOUCH, "1");
> if (ret) {
> pr_warn("xenkbd: can't request multi-touch");
> - touch = 0;
> + with_mtouch = 0;
> }
> }
Does it make sense to still end up calling xenkbd_connect_backend() when
all interfaces (keyboard, pointer, and multitouch) are disabled? Should
we do:
if (!(with_kbd || || with_ptr || with_mtouch))
return -ENXIO;
?
Thanks.
--
Dmitry
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
