linux-kernel - Re: [PATCH 02/12] Input: synaptics

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 5 Jul 2011 12:29:51 +0800
From:	Daniel Kurtz <djkurtz@...omium.org>
To:	Henrik Rydberg <rydberg@...omail.se>
Cc:	dmitry.torokhov@...il.com, chase.douglas@...onical.com,
	rubini@...l.unipv.it, linux-input@...r.kernel.org,
	linux-kernel@...r.kernel.org, derek.foreman@...labora.co.uk,
	daniel.stone@...labora.co.uk, olofj@...omium.org
Subject: Re: [PATCH 02/12] Input: synaptics - do not invert y if 0

Hi Henrik
Thanks for the review.

On Tue, Jul 5, 2011 at 5:08 AM, Henrik Rydberg <rydberg@...omail.se> wrote:
>
> Hi Daniel,
>
> > Synaptics touchpads report increasing y from bottom to top.
> > This is inverted from normal userspace "top of screen is 0" coordinates.
> > Thus, the kernel driver reports inverted y coordinates to userspace.
> >
> > In some cases, however, y = 0 is sent by the touchpad.
> > In these cases, the kernel driver should not invert, and just report 0.
> >
> > This patch also refactors the inversion into a macro, and moves it
> > into packet processing instead of during position reporting.
>
> The patch seems to invert the current output?

By 'current' do you mean referenced from the previous implementation?
Or referenced from the raw input.
It does indeed invert the raw input.
This is the same as the previous implementation did.
The difference is that it does not also invert the special 'y=0' into
an arbitrarily large value.
Is this your concern?
-Dan

>
> >
> > Signed-off-by: Daniel Kurtz <djkurtz@...omium.org>
> > ---
> >  drivers/input/mouse/synaptics.c |   16 +++++++++-------
> >  1 files changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> > index e06e045..f6d0c04 100644
> > --- a/drivers/input/mouse/synaptics.c
> > +++ b/drivers/input/mouse/synaptics.c
> > @@ -44,6 +44,8 @@
> >  #define YMIN_NOMINAL 1408
> >  #define YMAX_NOMINAL 4448
> >
> > +#define INVERT_Y(y) (((y) != 0) ? (YMAX_NOMINAL + YMIN_NOMINAL - (y)) : 0)
> > +
> >
> >  /*****************************************************************************
> >   *   Stuff we need even when we do not want native Synaptics support
> > @@ -409,9 +411,9 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
> >               hw->x = (((buf[3] & 0x10) << 8) |
> >                        ((buf[1] & 0x0f) << 8) |
> >                        buf[4]);
> > -             hw->y = (((buf[3] & 0x20) << 7) |
> > +             hw->y = INVERT_Y((((buf[3] & 0x20) << 7) |
> >                        ((buf[1] & 0xf0) << 4) |
> > -                      buf[5]);
> > +                      buf[5]));
> >
> >               hw->z = buf[2];
> >               hw->w = (((buf[0] & 0x30) >> 2) |
> > @@ -421,7 +423,8 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
> >               if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c) && hw->w == 2) {
> >                       /* Gesture packet: (x, y, z) at half resolution */
> >                       priv->mt.x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
> > -                     priv->mt.y = (((buf[4] & 0xf0) << 4) | buf[2]) << 1;
> > +                     priv->mt.y = INVERT_Y((((buf[4] & 0xf0) << 4)
> > +                                           | buf[2]) << 1);
> >                       priv->mt.z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 1;
> >                       return 1;
> >               }
> > @@ -473,7 +476,7 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
> >               }
> >       } else {
> >               hw->x = (((buf[1] & 0x1f) << 8) | buf[2]);
> > -             hw->y = (((buf[4] & 0x1f) << 8) | buf[5]);
> > +             hw->y = INVERT_Y(((buf[4] & 0x1f) << 8) | buf[5]);
> >
> >               hw->z = (((buf[0] & 0x30) << 2) | (buf[3] & 0x3F));
> >               hw->w = (((buf[1] & 0x80) >> 4) | ((buf[0] & 0x04) >> 1));
> > @@ -491,8 +494,7 @@ static void set_slot(struct input_dev *dev, int slot, bool active, int x, int y)
> >       input_mt_report_slot_state(dev, MT_TOOL_FINGER, active);
> >       if (active) {
> >               input_report_abs(dev, ABS_MT_POSITION_X, x);
> > -             input_report_abs(dev, ABS_MT_POSITION_Y,
> > -                              YMAX_NOMINAL + YMIN_NOMINAL - y);
> > +             input_report_abs(dev, ABS_MT_POSITION_Y, y);
> >       }
> >  }
> >
> > @@ -584,7 +586,7 @@ static void synaptics_process_packet(struct psmouse *psmouse)
> >
> >       if (num_fingers > 0) {
> >               input_report_abs(dev, ABS_X, hw.x);
> > -             input_report_abs(dev, ABS_Y, YMAX_NOMINAL + YMIN_NOMINAL - hw.y);
> > +             input_report_abs(dev, ABS_Y, hw.y);
> >       }
> >       input_report_abs(dev, ABS_PRESSURE, hw.z);
> >
> > --
> > 1.7.3.1
> >
>
> Thanks,
> Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/