* Re: [PATCH 0/6] input: cyapa: integrated with gen5 trackpad supported in one driver.
From: Dmitry Torokhov @ 2014-04-23 5:59 UTC (permalink / raw)
To: Dudley Du
Cc: Benson Leung, Daniel Kurtz, David Solda,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <77BC725C9062764F874D79F51E1F1A8F40C153FF@S04-MBX01-01.s04.local>
Hi Dudley,
On Tue, Apr 22, 2014 at 08:39:43AM +0000, Dudley Du wrote:
> Hi Dmitry,
>
> Could you help review the patches of input: cyapa for re-architecture
> and supporting new trackpad devices, I'm really looking forward your
> responses.
It is on my TODO list, sorry it takes too long...
Thanks.
--
Dmitry
^ permalink raw reply
* [git pull] Input updates for 3.15-rc2
From: Dmitry Torokhov @ 2014-04-23 6:06 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-input
[-- Attachment #1: Type: text/plain, Size: 2462 bytes --]
Hi Linus,
Please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
or
master.kernel.org:/pub/scm/linux/kernel/git/dtor/input.git for-linus
to receive updates for the input subsystem.
The main change is that we now publish "firmware ID" for the serio
devices to help userspace figure out the kind of touchpads it is dealing
with: i8042 will export PS/2 port's PNP IDs as firmware IDs.
You will also get more quirks for Synaptics touchpads in various Lenovo
laptops, a change to elantech driver to recognize even more models, and
fixups to wacom and couple other drivers.
Changelog:
---------
Adam Thomson (1):
Input: da9055_onkey - remove use of regmap_irq_get_virq()
Alexander Stein (1):
Input: ads7846 - fix device usage within attribute show
Hans de Goede (5):
Input: serio - add firmware_id sysfs attribute
Input: i8042 - add firmware_id support
Input: Add INPUT_PROP_TOPBUTTONPAD device property
Input: synaptics - report INPUT_PROP_TOPBUTTONPAD property
Input: synaptics - add min/max quirk for ThinkPad T431s, L440, L540, S1 Yoga and X1
Jason Gerecke (4):
Input: wacom - use full 32-bit HID Usage value in switch statement
Input: wacom - override 'pressure_max' with value from HID_USAGE_PRESSURE
Input: wacom - references to 'wacom->data' should use 'unsigned char*'
Input: wacom - handle 1024 pressure levels in wacom_tpc_pen
Jordan Rife (1):
Input: elantech - add support for newer elantech touchpads
Lejun Zhu (1):
Input: soc_button_array - fix a crash during rmmod
Ping Cheng (1):
Input: wacom - missed the last bit of expresskey for DTU-1031
Diffstat:
--------
drivers/input/misc/da9055_onkey.c | 1 -
drivers/input/misc/soc_button_array.c | 1 +
drivers/input/mouse/elantech.c | 1 +
drivers/input/mouse/synaptics.c | 97 +++++++++++++-
drivers/input/serio/i8042-x86ia64io.h | 15 +++
drivers/input/serio/i8042.c | 6 +
drivers/input/serio/serio.c | 14 ++
drivers/input/tablet/wacom_sys.c | 246 ++++++++++++++++------------------
drivers/input/tablet/wacom_wac.c | 29 ++--
drivers/input/touchscreen/ads7846.c | 2 +-
include/linux/serio.h | 1 +
include/uapi/linux/input.h | 1 +
12 files changed, 263 insertions(+), 151 deletions(-)
--
Dmitry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] evdev: flush ABS_* events during EVIOCGABS
From: Dmitry Torokhov @ 2014-04-23 6:07 UTC (permalink / raw)
To: Peter Hutterer
Cc: David Herrmann, open list:HID CORE LAYER, Benjamin Tissoires
In-Reply-To: <20140423055528.GA10740@yabbi.redhat.com>
On Wed, Apr 23, 2014 at 03:55:28PM +1000, Peter Hutterer wrote:
> On Tue, Apr 22, 2014 at 10:46:47PM -0700, Dmitry Torokhov wrote:
> > On Wed, Apr 23, 2014 at 03:38:49PM +1000, Peter Hutterer wrote:
> > > On Wed, Apr 23, 2014 at 10:21:03AM +1000, Peter Hutterer wrote:
> > > > On Tue, Apr 22, 2014 at 08:21:54AM +0200, David Herrmann wrote:
> > > > > Hi Peter
> > > > >
> > > > > On Tue, Apr 22, 2014 at 6:15 AM, Peter Hutterer
> > > > > <peter.hutterer@who-t.net> wrote:
> > > > > > How are you planning to handle the slot-based events? We'd either need to
> > > > > > add something similar (but more complex) to evdev_handle_mt_request or rely
> > > > > > on the caller to call the whole EV_ABS range and ditch anything ABS_MT_.
> > > > > > I'd prefer the former, the latter is yet more behaviour that's easy to get
> > > > > > wrong.
> > > > >
> > > > > This is all racy..
> > > > >
> > > > > We _really_ need an ioctl to receive _all_ ABS information atomically.
> > > > > I mean, there's no way we can know the user's state from the kernel.
> > > > > Even if the user resyncs via EVIOCGMTSLOTS, we can never flush the
> > > > > whole ABS queue. Problem is, the user has to call the ioctl for _each_
> > > > > available MT code and events might get queued in between. So yeah,
> > > > > this patch doesn't help much..
> > > > >
> > > > > I have no better idea than adding a new EVIOCGABS call that retrieves
> > > > > ABS values for all slots atomically (and for all other axes..). No
> > > > > idea how to properly fix the old ioctls.
> > > >
> > > > bonus points for making that ioctl fetch the state of the last SYN_DROPPED
> > > > and leave the events since in the client buffer. That way we can smooth over
> > > > SYN_DROPPED and lose less information.
> > >
> > > to expand on this, something like the below would work from userspace:
> > >
> > > 1. userspace opens fd, EVIOCGBIT for everything
> > > 2. userspace calls EVIOCGABSATOMIC
> > > 3. kernel empties the event queue, flags the client as capable
> > > 4. kernel copies current device state into client-specific struct
> > > 5. kernel replies with that device state to the ioctl
> > > 6. client reads events
> > > ..
> > > 7. kernel sees a SYN_DROPPED for this client. Takes a snapshot of the device
> > > for the client, empties the buffer, leaves SYN_DROPPED in the buffer
> > > (current behaviour)
> > > 8. client reads SYN_DROPPED, calls EVIOCGABSATOMIC
> > > 9. kernel replies with the snapshot state, leaves the event buffer otherwise
> > > unmodified
> > > 10. client reads all events after SYN_DROPPED, gets a smooth continuation
> > > 11. goto 6
> > >
> > > if the buffer overflows multiple times, repeat 7 so that the snapshot state
> > > is always the last SYN_DROPPED state. well, technically the state should be
> > > the state of the device at the first SYN_REPORT after the last SYN_DROPPED,
> > > since the current API says that interrupted event is incomplete.
> > >
> > > there are two oddities here:
> > > 1. the first ioctl will have to flush the buffer to guarantee consistent state,
> > > though you could even avoid that by taking a snapshot of the device on
> > > open(). though that comes with a disadvantage, you don't know if the client
> > > supports the new approach so you're wasting effort and memory here.
> > > 2. I'm not quite sure how to handle multiple repeated calls short of
> > > updating the client-specific snapshot with every event as it is read
> > > successfully.
> > >
> > > any comments?
> >
> > Do we really need to optimize the case when we are dropping events?
>
> It happens frequently, to the point where on some laptops you're pretty much
> guaranteed to get SYN_DROPPED events on resume and sometimes even during
> normal multi-finger user.
>
> I don't have any measurements on how many events are dropped on average.
> Could be one or two, could be several buffer sizes, I honestly don't know.
I think we need to figure this out. The idea is that dropping events
should be an exception, not a rule.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input: add support for ALPS v7 protocol device
From: Elaine Chen @ 2014-04-23 6:47 UTC (permalink / raw)
To: Peter Hutterer, Dmitry Torokhov
Cc: Kevin Cernekee, david turvene, linux-input, Justin Clift,
Qiting Chen, Hans de Goede
In-Reply-To: <20140422065503.GA27970@yabbi.redhat.com>
Hi, Peter, Dmitry,
I see. As the xorg dosn't fully support Resting Finger now, will this
patch be applied as a temp solution?
Or this patch will be skipped. And we resubmit a new patch without
kernel support of Resting Finger later?
Please tell me. Thank you.
Thanks.
2014-04-22 14:55 GMT+08:00 Peter Hutterer <peter.hutterer@who-t.net>:
> On Tue, Apr 22, 2014 at 02:11:18PM +0800, Elaine Chen wrote:
>> Hi Peter,
>>
>> Thank you! I know there's a flag to register our device as clickpad to
>> X system. But it seems the clickpad attribution only
>> deal with soft button. It won't handle "Resting Finger" function,
>> isn't it? (Resting Finger function: place one or more fingers still in
>> soft button zone, these fingers
>> won't affact other finger's cursoring and gestures.) If the X system
>> doesn't support this function, our driver has to implement it in
>> kernel. This is why we didn't use
>> the clickpad flag. If I'm wrong, please tell me.
>
> we've added a few patches recently that amongst other things ignore finger
> events in the softbutton area, so that should fix that issue and stop
> messing up pointer movement. I'm the first to admit that the xorg driver
> hasn't been up to scratch in many regards, but much of that was for lack of
> developer time. Hans has recently improved the behaviour quite a bit. For
> the future, it's probably easier (or better) to fix up the xorg driver to
> get rid of the specific problems you're seeing than hacking up the kernel
> driver.
>
> Cheers,
> Peter
>
>> 2014-04-22 13:42 GMT+08:00 Peter Hutterer <peter.hutterer@who-t.net>:
>> > On Mon, Apr 21, 2014 at 10:26:04PM -0700, Dmitry Torokhov wrote:
>> >> Hi Qiting,
>> >>
>> >> On Wed, Mar 19, 2014 at 04:55:53PM +0800, Qiting Chen wrote:
>> >> > Here is a patch of supporting ALPS v7 protocol device.
>> >> > ALPS v7 protocol device is a clickpad that is currently used on
>> >> > Lenovo S430/S435/S530, Lenovo Z410/Z510, HP Odie, HP Revolve 810 G1,
>> >> > as well as other machines with ALPS Touchpad of following infomation:
>> >> > Device ID = 0x73, 0x03, 0x0a
>> >> > Firmware ID = 0x88, 0xb*, 0x**
>> >> >
>> >> > A v7 protocol support patch is first relesed 2 months ago:
>> >> > http://www.spinics.net/lists/linux-input/msg29084.html
>> >> > After that some feedbacks were received from end user. Now this patch fixed the bugs
>> >> > reported by them:
>> >> > 1) Fix cursor jump when doing a right click drag
>> >> > 2) Fix cursor jitter when button clicking
>> >>
>> >> My biggest question is whether the soft buttons should be processed in
>> >> kernel driver or in userspace.
>> >>
>> >> Peter, don't Synaptics clickpads need similar functionality? I thought X
>> >> driver already handles soft button areas... Am I mistaken?
>> >
>> > yeah, we do handle those software button areas in the xorg synaptics driver
>> > (and libinput very soon, for that matter). I'd prefer this to be handled in
>> > userspace so we can get some sort of unified behaviour. There is one
>> > touchpad series that handles it in firmware though (see Hans' recent patch
>> > for the Cypress touchpads) but on the whole it's better to leave it to
>> > userspace.
>> >
>> > Cheers,
>> > Peter
>> >
>> >> > Signed-off-by: Qiting Chen <qiting.chen@cn.alps.com>
>> >> > ---
>> >> > drivers/input/mouse/alps.c | 560 ++++++++++++++++++++++++++++++++++++++++++---
>> >> > drivers/input/mouse/alps.h | 132 +++++++++--
>> >> > 2 files changed, 641 insertions(+), 51 deletions(-)
>> >> >
>> >> > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
>> >> > index fb15c64..383281f 100644
>> >> > --- a/drivers/input/mouse/alps.c
>> >> > +++ b/drivers/input/mouse/alps.c
>> >> > @@ -32,6 +32,13 @@
>> >> > #define ALPS_REG_BASE_RUSHMORE 0xc2c0
>> >> > #define ALPS_REG_BASE_PINNACLE 0x0000
>> >> >
>> >> > +#define LEFT_BUTTON_BIT 0x01
>> >> > +#define RIGHT_BUTTON_BIT 0x02
>> >> > +
>> >> > +#define V7_LARGE_MOVEMENT 130
>> >> > +#define V7_DEAD_ZONE_OFFSET_X 72
>> >> > +#define V7_DEAD_ZONE_OFFSET_Y 72
>> >> > +
>> >> > static const struct alps_nibble_commands alps_v3_nibble_commands[] = {
>> >> > { PSMOUSE_CMD_SETPOLL, 0x00 }, /* 0 */
>> >> > { PSMOUSE_CMD_RESET_DIS, 0x00 }, /* 1 */
>> >> > @@ -99,6 +106,7 @@ static const struct alps_nibble_commands alps_v6_nibble_commands[] = {
>> >> > #define ALPS_FOUR_BUTTONS 0x40 /* 4 direction button present */
>> >> > #define ALPS_PS2_INTERLEAVED 0x80 /* 3-byte PS/2 packet interleaved with
>> >> > 6-byte ALPS packet */
>> >> > +#define ALPS_BTNLESS 0x100 /* ALPS ClickPad flag */
>> >> >
>> >> > static const struct alps_model_info alps_model_data[] = {
>> >> > { { 0x32, 0x02, 0x14 }, 0x00, ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT }, /* Toshiba Salellite Pro M10 */
>> >> > @@ -140,6 +148,20 @@ static void alps_set_abs_params_mt(struct alps_data *priv,
>> >> > * isn't valid per PS/2 spec.
>> >> > */
>> >> >
>> >> > +static unsigned int alps_pt_distance(struct alps_abs_data *pt0,
>> >> > + struct alps_abs_data *pt1)
>> >> > +{
>> >> > + int vect_x, vect_y;
>> >> > +
>> >> > + if (!pt0 || !pt1)
>> >> > + return 0;
>> >>
>> >> In which case can either of this pointers being NULL?
>> >>
>> >> > +
>> >> > + vect_x = pt0->x - pt1->x;
>> >> > + vect_y = pt0->y - pt1->y;
>> >> > +
>> >> > + return int_sqrt(vect_x * vect_x + vect_y * vect_y);
>> >> > +}
>> >> > +
>> >> > /* Packet formats are described in Documentation/input/alps.txt */
>> >> >
>> >> > static bool alps_is_valid_first_byte(struct alps_data *priv,
>> >> > @@ -320,8 +342,8 @@ static void alps_process_bitmap_dolphin(struct alps_data *priv,
>> >> > end_bit = y_msb - 1;
>> >> > box_middle_y = (priv->y_max * (start_bit + end_bit)) /
>> >> > (2 * (priv->y_bits - 1));
>> >> > - *x1 = fields->x;
>> >> > - *y1 = fields->y;
>> >> > + *x1 = fields->pt.x;
>> >> > + *y1 = fields->pt.y;
>> >> > *x2 = 2 * box_middle_x - *x1;
>> >> > *y2 = 2 * box_middle_y - *y1;
>> >> > }
>> >> > @@ -461,6 +483,38 @@ static void alps_report_semi_mt_data(struct input_dev *dev, int num_fingers,
>> >> > alps_set_slot(dev, 1, num_fingers == 2, x2, y2);
>> >> > }
>> >> >
>> >> > +static void alps_report_coord_and_btn(struct psmouse *psmouse,
>> >> > + struct alps_fields *f)
>> >> > +{
>> >> > + struct input_dev *dev;
>> >> > +
>> >> > + if (!psmouse || !f)
>> >> > + return;
>> >>
>> >> Can either of these 2 pointers ever be NULL?
>> >>
>> >> > +
>> >> > + dev = psmouse->dev;
>> >> > +
>> >> > + if (f->fingers) {
>> >> > + input_report_key(dev, BTN_TOUCH, 1);
>> >> > + alps_report_semi_mt_data(dev, f->fingers,
>> >> > + f->pt_img[0].x, f->pt_img[0].y,
>> >> > + f->pt_img[1].x, f->pt_img[1].y);
>> >> > + input_mt_report_finger_count(dev, f->fingers);
>> >> > +
>> >> > + input_report_abs(dev, ABS_X, f->pt_img[0].x);
>> >> > + input_report_abs(dev, ABS_Y, f->pt_img[0].y);
>> >> > + input_report_abs(dev, ABS_PRESSURE, f->pt_img[0].z);
>> >> > + } else {
>> >> > + input_report_key(dev, BTN_TOUCH, 0);
>> >> > + input_mt_report_finger_count(dev, 0);
>> >> > + input_report_abs(dev, ABS_PRESSURE, 0);
>> >> > + }
>> >> > +
>> >> > + input_report_key(dev, BTN_LEFT, f->btn.left);
>> >> > + input_report_key(dev, BTN_RIGHT, f->btn.right);
>> >> > +
>> >> > + input_sync(dev);
>> >> > +}
>> >> > +
>> >> > static void alps_process_trackstick_packet_v3(struct psmouse *psmouse)
>> >> > {
>> >> > struct alps_data *priv = psmouse->private;
>> >> > @@ -523,13 +577,13 @@ static void alps_process_trackstick_packet_v3(struct psmouse *psmouse)
>> >> >
>> >> > static void alps_decode_buttons_v3(struct alps_fields *f, unsigned char *p)
>> >> > {
>> >> > - f->left = !!(p[3] & 0x01);
>> >> > - f->right = !!(p[3] & 0x02);
>> >> > - f->middle = !!(p[3] & 0x04);
>> >> > + f->btn.left = !!(p[3] & 0x01);
>> >> > + f->btn.right = !!(p[3] & 0x02);
>> >> > + f->btn.middle = !!(p[3] & 0x04);
>> >> >
>> >> > - f->ts_left = !!(p[3] & 0x10);
>> >> > - f->ts_right = !!(p[3] & 0x20);
>> >> > - f->ts_middle = !!(p[3] & 0x40);
>> >> > + f->btn.ts_left = !!(p[3] & 0x10);
>> >> > + f->btn.ts_right = !!(p[3] & 0x20);
>> >> > + f->btn.ts_middle = !!(p[3] & 0x40);
>> >> > }
>> >> >
>> >> > static void alps_decode_pinnacle(struct alps_fields *f, unsigned char *p,
>> >> > @@ -546,10 +600,10 @@ static void alps_decode_pinnacle(struct alps_fields *f, unsigned char *p,
>> >> > ((p[2] & 0x7f) << 1) |
>> >> > (p[4] & 0x01);
>> >> >
>> >> > - f->x = ((p[1] & 0x7f) << 4) | ((p[4] & 0x30) >> 2) |
>> >> > + f->pt.x = ((p[1] & 0x7f) << 4) | ((p[4] & 0x30) >> 2) |
>> >> > ((p[0] & 0x30) >> 4);
>> >> > - f->y = ((p[2] & 0x7f) << 4) | (p[4] & 0x0f);
>> >> > - f->z = p[5] & 0x7f;
>> >> > + f->pt.y = ((p[2] & 0x7f) << 4) | (p[4] & 0x0f);
>> >> > + f->pt.z = p[5] & 0x7f;
>> >> >
>> >> > alps_decode_buttons_v3(f, p);
>> >> > }
>> >> > @@ -573,9 +627,9 @@ static void alps_decode_dolphin(struct alps_fields *f, unsigned char *p,
>> >> > f->is_mp = !!(p[0] & 0x20);
>> >> >
>> >> > if (!f->is_mp) {
>> >> > - f->x = ((p[1] & 0x7f) | ((p[4] & 0x0f) << 7));
>> >> > - f->y = ((p[2] & 0x7f) | ((p[4] & 0xf0) << 3));
>> >> > - f->z = (p[0] & 4) ? 0 : p[5] & 0x7f;
>> >> > + f->pt.x = ((p[1] & 0x7f) | ((p[4] & 0x0f) << 7));
>> >> > + f->pt.y = ((p[2] & 0x7f) | ((p[4] & 0xf0) << 3));
>> >> > + f->pt.z = (p[0] & 4) ? 0 : p[5] & 0x7f;
>> >> > alps_decode_buttons_v3(f, p);
>> >> > } else {
>> >> > f->fingers = ((p[0] & 0x6) >> 1 |
>> >> > @@ -687,7 +741,7 @@ static void alps_process_touchpad_packet_v3_v5(struct psmouse *psmouse)
>> >> > * with x, y, and z all zero, so these seem to be flukes.
>> >> > * Ignore them.
>> >> > */
>> >> > - if (f.x && f.y && !f.z)
>> >> > + if (f.pt.x && f.pt.y && !f.pt.z)
>> >> > return;
>> >> >
>> >> > /*
>> >> > @@ -695,12 +749,12 @@ static void alps_process_touchpad_packet_v3_v5(struct psmouse *psmouse)
>> >> > * to rely on ST data.
>> >> > */
>> >> > if (!fingers) {
>> >> > - x1 = f.x;
>> >> > - y1 = f.y;
>> >> > - fingers = f.z > 0 ? 1 : 0;
>> >> > + x1 = f.pt.x;
>> >> > + y1 = f.pt.y;
>> >> > + fingers = f.pt.z > 0 ? 1 : 0;
>> >> > }
>> >> >
>> >> > - if (f.z >= 64)
>> >> > + if (f.pt.z >= 64)
>> >> > input_report_key(dev, BTN_TOUCH, 1);
>> >> > else
>> >> > input_report_key(dev, BTN_TOUCH, 0);
>> >> > @@ -709,22 +763,22 @@ static void alps_process_touchpad_packet_v3_v5(struct psmouse *psmouse)
>> >> >
>> >> > input_mt_report_finger_count(dev, fingers);
>> >> >
>> >> > - input_report_key(dev, BTN_LEFT, f.left);
>> >> > - input_report_key(dev, BTN_RIGHT, f.right);
>> >> > - input_report_key(dev, BTN_MIDDLE, f.middle);
>> >> > + input_report_key(dev, BTN_LEFT, f.btn.left);
>> >> > + input_report_key(dev, BTN_RIGHT, f.btn.right);
>> >> > + input_report_key(dev, BTN_MIDDLE, f.btn.middle);
>> >> >
>> >> > - if (f.z > 0) {
>> >> > - input_report_abs(dev, ABS_X, f.x);
>> >> > - input_report_abs(dev, ABS_Y, f.y);
>> >> > + if (f.pt.z > 0) {
>> >> > + input_report_abs(dev, ABS_X, f.pt.x);
>> >> > + input_report_abs(dev, ABS_Y, f.pt.y);
>> >> > }
>> >> > - input_report_abs(dev, ABS_PRESSURE, f.z);
>> >> > + input_report_abs(dev, ABS_PRESSURE, f.pt.z);
>> >> >
>> >> > input_sync(dev);
>> >> >
>> >> > if (!(priv->quirks & ALPS_QUIRK_TRACKSTICK_BUTTONS)) {
>> >> > - input_report_key(dev2, BTN_LEFT, f.ts_left);
>> >> > - input_report_key(dev2, BTN_RIGHT, f.ts_right);
>> >> > - input_report_key(dev2, BTN_MIDDLE, f.ts_middle);
>> >> > + input_report_key(dev2, BTN_LEFT, f.btn.ts_left);
>> >> > + input_report_key(dev2, BTN_RIGHT, f.btn.ts_right);
>> >> > + input_report_key(dev2, BTN_MIDDLE, f.btn.ts_middle);
>> >> > input_sync(dev2);
>> >> > }
>> >> > }
>> >> > @@ -916,6 +970,364 @@ static void alps_process_packet_v4(struct psmouse *psmouse)
>> >> > input_sync(dev);
>> >> > }
>> >> >
>> >> > +static bool alps_is_valid_package_v7(struct psmouse *psmouse)
>> >> > +{
>> >> > + if ((psmouse->pktcnt == 3) && ((psmouse->packet[2] & 0x40) != 0x40))
>> >> > + return false;
>> >> > + if ((psmouse->pktcnt == 4) && ((psmouse->packet[3] & 0x48) != 0x48))
>> >> > + return false;
>> >> > + if ((psmouse->pktcnt == 6) && ((psmouse->packet[5] & 0x40) != 0x0))
>> >> > + return false;
>> >>
>> >> Maybe do:
>> >>
>> >> switch (psmouse->pktcnt) {
>> >> case 3:
>> >> if ((psmouse->packet[2] & 0x40) != 0x40)
>> >> return false;
>> >> break;
>> >> ...
>> >> }
>> >>
>> >> > + return true;
>> >> > +}
>> >> > +
>> >> > +static int alps_drop_unsupported_packet_v7(struct psmouse *psmouse)
>> >> > +{
>> >> > + struct alps_data *priv = psmouse->private;
>> >> > + int drop = 1;
>> >>
>> >> bool drop = true;
>> >>
>> >> > +
>> >> > + if (priv->r.v7.pkt_id == V7_PACKET_ID_NEW ||
>> >> > + priv->r.v7.pkt_id == V7_PACKET_ID_TWO ||
>> >> > + priv->r.v7.pkt_id == V7_PACKET_ID_MULTI ||
>> >> > + priv->r.v7.pkt_id == V7_PACKET_ID_IDLE)
>> >> > + drop = 0;
>> >>
>> >> if (... ||
>> >> ...) {
>> >> drop = false;
>> >> }
>> >> > +
>> >> > + return drop;
>> >> > +}
>> >> > +
>> >> > +static unsigned char alps_get_packet_id_v7(char *byte)
>> >> > +{
>> >> > + unsigned char packet_id;
>> >> > +
>> >> > + if (byte[4] & 0x40)
>> >> > + packet_id = V7_PACKET_ID_TWO;
>> >> > + else if (byte[4] & 0x01)
>> >> > + packet_id = V7_PACKET_ID_MULTI;
>> >> > + else if ((byte[0] & 0x10) && !(byte[4] & 0x43))
>> >> > + packet_id = V7_PACKET_ID_NEW;
>> >> > + else
>> >> > + packet_id = V7_PACKET_ID_IDLE;
>> >> > +
>> >> > + return packet_id;
>> >> > +}
>> >> > +
>> >> > +static void alps_get_finger_coordinate_v7(struct alps_abs_data *pt,
>> >> > + unsigned char *pkt,
>> >> > + unsigned char pkt_id)
>> >> > +{
>> >> > + if ((pkt_id == V7_PACKET_ID_TWO) ||
>> >> > + (pkt_id == V7_PACKET_ID_MULTI) ||
>> >> > + (pkt_id == V7_PACKET_ID_NEW)) {
>> >> > + pt[0].x = ((pkt[2] & 0x80) << 4);
>> >> > + pt[0].x |= ((pkt[2] & 0x3F) << 5);
>> >> > + pt[0].x |= ((pkt[3] & 0x30) >> 1);
>> >> > + pt[0].x |= (pkt[3] & 0x07);
>> >> > + pt[0].y = (pkt[1] << 3) | (pkt[0] & 0x07);
>> >> > +
>> >> > + pt[1].x = ((pkt[3] & 0x80) << 4);
>> >> > + pt[1].x |= ((pkt[4] & 0x80) << 3);
>> >> > + pt[1].x |= ((pkt[4] & 0x3F) << 4);
>> >> > + pt[1].y = ((pkt[5] & 0x80) << 3);
>> >> > + pt[1].y |= ((pkt[5] & 0x3F) << 4);
>> >> > +
>> >> > + if (pkt_id == V7_PACKET_ID_TWO) {
>> >> > + pt[1].x &= ~0x000F;
>> >> > + pt[1].y |= 0x000F;
>> >> > + } else if (pkt_id == V7_PACKET_ID_MULTI) {
>> >> > + pt[1].x &= ~0x003F;
>> >> > + pt[1].y &= ~0x0020;
>> >> > + pt[1].y |= ((pkt[4] & 0x02) << 4);
>> >> > + pt[1].y |= 0x001F;
>> >> > + } else if (pkt_id == V7_PACKET_ID_NEW) {
>> >> > + pt[1].x &= ~0x003F;
>> >> > + pt[1].x |= (pkt[0] & 0x20);
>> >> > + pt[1].y |= 0x000F;
>> >> > + }
>> >> > +
>> >> > + pt[0].y = 0x7FF - pt[0].y;
>> >> > + pt[1].y = 0x7FF - pt[1].y;
>> >> > +
>> >> > + pt[0].z = (pt[0].x && pt[0].y) ? 62 : 0;
>> >> > + pt[1].z = (pt[1].x && pt[1].y) ? 62 : 0;
>> >> > + }
>> >> > +}
>> >> > +
>> >> > +static void alps_decode_packet_v7(struct alps_fields *f,
>> >> > + unsigned char *p,
>> >> > + struct psmouse *psmouse)
>> >> > +{
>> >> > + struct alps_data *priv = psmouse->private;
>> >> > + static struct v7_raw prev_r;
>> >> > +
>> >> > + priv->r.v7.pkt_id = alps_get_packet_id_v7(p);
>> >> > +
>> >> > + alps_get_finger_coordinate_v7(f->pt_img, p, priv->r.v7.pkt_id);
>> >> > +
>> >> > + priv->r.v7.rest_left = 0;
>> >> > + priv->r.v7.rest_right = 0;
>> >> > + priv->r.v7.additional_fingers = 0;
>> >> > + priv->phy_btn = 0;
>> >> > +
>> >> > + if (priv->r.v7.pkt_id == V7_PACKET_ID_TWO ||
>> >> > + priv->r.v7.pkt_id == V7_PACKET_ID_MULTI) {
>> >> > + priv->r.v7.rest_left = (p[0] & 0x10) >> 4;
>> >> > + priv->r.v7.rest_right = (p[0] & 0x20) >> 5;
>> >> > + }
>> >> > +
>> >> > + if (priv->r.v7.pkt_id == V7_PACKET_ID_MULTI)
>> >> > + priv->r.v7.additional_fingers = p[5] & 0x03;
>> >> > +
>> >> > + priv->phy_btn = (p[0] & 0x80) >> 7;
>> >> > +
>> >> > + if (priv->r.v7.pkt_id == V7_PACKET_ID_TWO) {
>> >> > + if (f->pt_img[0].z != 0 && f->pt_img[1].z != 0)
>> >> > + priv->r.v7.raw_fn = 2;
>> >> > + else
>> >> > + priv->r.v7.raw_fn = 1;
>> >> > + } else if (priv->r.v7.pkt_id == V7_PACKET_ID_MULTI)
>> >> > + priv->r.v7.raw_fn = 3 + priv->r.v7.additional_fingers;
>> >> > + else if (priv->r.v7.pkt_id == V7_PACKET_ID_IDLE)
>> >> > + priv->r.v7.raw_fn = 0;
>> >> > + else if (priv->r.v7.pkt_id == V7_PACKET_ID_NEW)
>> >> > + priv->r.v7.raw_fn = prev_r.raw_fn;
>> >> > +
>> >> > + /* It is a trick to bypass firmware bug of older version
>> >> > + that 'New' Packet is missed when finger number changed.
>> >> > + We fake a 'New' Packet in such cases.*/
>> >>
>> >> Multi-line comments should be formatted as follows:
>> >>
>> >> /*
>> >> * This is a multi
>> >> * line comment.
>> >> */
>> >>
>> >>
>> >> > + if (priv->r.v7.pkt_id == V7_PACKET_ID_TWO ||
>> >> > + priv->r.v7.pkt_id == V7_PACKET_ID_MULTI ||
>> >> > + priv->r.v7.pkt_id == V7_PACKET_ID_IDLE) {
>> >> > + if (priv->r.v7.raw_fn != prev_r.raw_fn)
>> >> > + priv->r.v7.pkt_id = V7_PACKET_ID_NEW;
>> >> > + }
>> >> > +
>> >> > + memcpy(&prev_r, &priv->r.v7, sizeof(struct v7_raw));
>> >> > +}
>> >> > +
>> >> > +static void alps_set_each_pt_attr_v7(struct psmouse *psmouse,
>> >> > + struct alps_abs_data *pt,
>> >> > + struct alps_bl_pt_attr *pt_attr)
>> >> > +{
>> >> > + struct alps_data *priv = psmouse->private;
>> >> > + unsigned int dist;
>> >> > +
>> >> > + if (!pt_attr->is_init_pt_got && pt->z != 0) {
>> >> > + pt_attr->is_init_pt_got = 1;
>> >> > + pt_attr->is_counted = 0;
>> >> > + memcpy(&pt_attr->init_pt, pt, sizeof(pt_attr->init_pt));
>> >> > + }
>> >> > +
>> >> > + if (pt->z != 0) {
>> >> > + if (pt->y < priv->resting_zone_y_min) {
>> >> > + /* A finger is recognized as a non-resting finger
>> >> > + if it's position is outside the resting finger zone.*/
>> >> > + pt_attr->zone = ZONE_NORMAL;
>> >> > + pt_attr->is_counted = 1;
>> >> > + } else {
>> >> > + /* A finger is recognized as a resting finger if it's
>> >> > + position is inside the resting finger zone and there's
>> >> > + no large movement from it's touch down position.*/
>> >> > + pt_attr->zone = ZONE_RESTING;
>> >> > +
>> >> > + if (pt->x > priv->x_max / 2)
>> >> > + pt_attr->zone |= ZONE_RIGHT_BTN;
>> >> > + else
>> >> > + pt_attr->zone |= ZONE_LEFT_BTN;
>> >> > +
>> >> > + /* A resting finger will turn to be a non-resting
>> >> > + finger if it has made large movement from it's touch
>> >> > + down position. A non-resting finger will never turn
>> >> > + to a resting finger before it leaves the touchpad
>> >> > + surface.*/
>> >> > + if (pt_attr->is_init_pt_got) {
>> >> > + dist = alps_pt_distance(pt, &pt_attr->init_pt);
>> >> > +
>> >> > + if (dist > V7_LARGE_MOVEMENT)
>> >> > + pt_attr->is_counted = 1;
>> >> > + }
>> >> > + }
>> >> > + }
>> >> > +}
>> >> > +
>> >> > +static void alps_set_pt_attr_v7(struct psmouse *psmouse,
>> >> > + struct alps_fields *f)
>> >> > +{
>> >> > + struct alps_data *priv = psmouse->private;
>> >> > + int i;
>> >> > +
>> >> > + switch (priv->r.v7.pkt_id) {
>> >> > + case V7_PACKET_ID_TWO:
>> >> > + case V7_PACKET_ID_MULTI:
>> >> > + for (i = 0; i < V7_IMG_PT_NUM; i++) {
>> >> > + alps_set_each_pt_attr_v7(psmouse,
>> >> > + &f->pt_img[i],
>> >> > + &priv->pt_attr[i]);
>> >> > + }
>> >> > + break;
>> >> > + default:
>> >> > + /*All finger attributes are cleared when packet ID is
>> >> > + 'IDLE', 'New'or other unknown IDs. An 'IDLE' packet
>> >> > + indicates that there's no finger and no button activity.
>> >> > + A 'NEW' packet indicates the finger position in packet
>> >> > + is not continues from previous packet. Such as the
>> >> > + condition there's finger placed or lifted. In these cases,
>> >> > + finger attributes will be reset.*/
>> >> > + memset(priv->pt_attr, 0, sizeof(priv->pt_attr[0]) * 2);
>> >> > + break;
>> >> > + }
>> >> > +}
>> >> > +
>> >> > +static void alps_cal_output_finger_num_v7(struct psmouse *psmouse,
>> >> > + struct alps_fields *f)
>> >> > +{
>> >> > + struct alps_data *priv = psmouse->private;
>> >> > + unsigned int fn = 0;
>> >> > + int i;
>> >> > +
>> >> > + switch (priv->r.v7.pkt_id) {
>> >> > + case V7_PACKET_ID_IDLE:
>> >> > + case V7_PACKET_ID_NEW:
>> >> > + /*No finger is reported when packet ID is 'IDLE' or 'New'.
>> >> > + An 'IDLE' packet indicates that there's no finger on touchpad.
>> >> > + A 'NEW' packet indicates there's finger placed or lifted.
>> >> > + Finger position of 'New' packet is not continues from the
>> >> > + previous packet.*/
>> >> > + fn = 0;
>> >> > + break;
>> >> > + case V7_PACKET_ID_TWO:
>> >> > + if (f->pt_img[0].z == 0) {
>> >> > + /*The first finger slot is zero when a non-resting
>> >> > + finger lifted and remaining only one resting finger
>> >> > + on touchpad. Hardware report the remaining resting
>> >> > + finger in second slot. This resting is ignored*/
>> >> > + fn = 0;
>> >> > + } else if (f->pt_img[1].z == 0) {
>> >> > + /* The second finger slot is zero if there's
>> >> > + only one finger*/
>> >> > + fn = 1;
>> >> > + } else {
>> >> > + /*All non-resting fingers will be counted to report*/
>> >> > + fn = 0;
>> >> > + for (i = 0; i < V7_IMG_PT_NUM; i++) {
>> >> > + if (priv->pt_attr[i].is_counted)
>> >> > + fn++;
>> >> > + }
>> >> > +
>> >> > + /*In the case that both fingers are
>> >> > + resting fingers, report the first one*/
>> >> > + if (!priv->pt_attr[0].is_counted &&
>> >> > + !priv->pt_attr[1].is_counted) {
>> >> > + fn = 1;
>> >> > + }
>> >> > + }
>> >> > + break;
>> >> > + case V7_PACKET_ID_MULTI:
>> >> > + /*A packet ID 'MULTI' indicats that at least 3 non-resting
>> >> > + finger exist.*/
>> >> > + fn = 3 + priv->r.v7.additional_fingers;
>> >> > + break;
>> >> > + }
>> >> > +
>> >> > + f->fingers = fn;
>> >> > +}
>> >> > +
>> >> > +static void alps_button_dead_zone_filter(struct psmouse *psmouse,
>> >> > + struct alps_fields *f,
>> >> > + struct alps_fields *prev_f)
>> >> > +{
>> >> > + struct alps_data *priv = psmouse->private;
>> >> > + int dx, dy;
>> >> > +
>> >> > + if (priv->prev_phy_btn == 0 && priv->phy_btn != 0) {
>> >> > + memcpy(&priv->pt_attr[0].init_dead_pt,
>> >> > + &f->pt_img[0],
>> >> > + sizeof(struct alps_abs_data));
>> >> > + }
>> >> > +
>> >> > + if (priv->pt_attr[0].init_dead_pt.x != 0 &&
>> >> > + priv->pt_attr[0].init_dead_pt.x != 0) {
>> >> > + dx = f->pt_img[0].x - priv->pt_attr[0].init_dead_pt.x;
>> >> > + dy = f->pt_img[0].y - priv->pt_attr[0].init_dead_pt.y;
>> >> > + if ((abs(dx) > V7_DEAD_ZONE_OFFSET_X) ||
>> >> > + (abs(dy) > V7_DEAD_ZONE_OFFSET_Y)) {
>> >> > + memset(&priv->pt_attr[0].init_dead_pt, 0,
>> >> > + sizeof(struct alps_abs_data));
>> >> > + priv->btn_delay_cnt = 0;
>> >> > + } else {
>> >> > + memcpy(&f->pt_img[0],
>> >> > + &prev_f->pt_img[0],
>> >> > + sizeof(struct alps_abs_data));
>> >> > + if (priv->prev_phy_btn == 0 && priv->phy_btn != 0)
>> >> > + priv->btn_delay_cnt = 2;
>> >> > + }
>> >> > + }
>> >> > +
>> >> > + if (priv->btn_delay_cnt > 0) {
>> >> > + f->btn.left = 0;
>> >> > + f->btn.right = 0;
>> >> > + priv->btn_delay_cnt--;
>> >> > + }
>> >> > +}
>> >> > +
>> >> > +static void alps_assign_buttons_v7(struct psmouse *psmouse,
>> >> > + struct alps_fields *f,
>> >> > + struct alps_fields *prev_f)
>> >> > +{
>> >> > + struct alps_data *priv = psmouse->private;
>> >> > +
>> >> > + if (priv->phy_btn) {
>> >> > + if (!priv->prev_phy_btn) {
>> >> > + /* Report a right click as long as there's finger on
>> >> > + right button zone. Othrewise, report a left click.*/
>> >> > + if (priv->r.v7.rest_right ||
>> >> > + priv->pt_attr[0].zone & ZONE_RIGHT_BTN ||
>> >> > + priv->pt_attr[1].zone & ZONE_RIGHT_BTN) {
>> >> > + f->btn.right = 1;
>> >> > + priv->pressed_btn_bits |= RIGHT_BUTTON_BIT;
>> >> > + } else {
>> >> > + f->btn.left = 1;
>> >> > + priv->pressed_btn_bits |= LEFT_BUTTON_BIT;
>> >> > + }
>> >> > + } else {
>> >> > + if (priv->pressed_btn_bits & RIGHT_BUTTON_BIT)
>> >> > + f->btn.right = 1;
>> >> > + if (priv->pressed_btn_bits & LEFT_BUTTON_BIT)
>> >> > + f->btn.left = 1;
>> >> > + }
>> >> > + } else {
>> >> > + priv->pressed_btn_bits = 0;
>> >> > + f->btn.right = 0;
>> >> > + f->btn.left = 0;
>> >> > + }
>> >> > +
>> >> > + alps_button_dead_zone_filter(psmouse, f, prev_f);
>> >> > +
>> >> > + priv->prev_phy_btn = priv->phy_btn;
>> >> > +}
>> >> > +
>> >> > +static void alps_process_packet_v7(struct psmouse *psmouse)
>> >> > +{
>> >> > + struct alps_data *priv = psmouse->private;
>> >> > + struct alps_fields f = {0};
>> >> > + static struct alps_fields prev_f;
>> >> > + unsigned char *packet = psmouse->packet;
>> >> > +
>> >> > + priv->decode_fields(&f, packet, psmouse);
>> >> > +
>> >> > + if (alps_drop_unsupported_packet_v7(psmouse))
>> >> > + return;
>> >> > +
>> >> > + alps_set_pt_attr_v7(psmouse, &f);
>> >> > +
>> >> > + alps_cal_output_finger_num_v7(psmouse, &f);
>> >> > +
>> >> > + alps_assign_buttons_v7(psmouse, &f, &prev_f);
>> >> > +
>> >> > + alps_report_coord_and_btn(psmouse, &f);
>> >> > +
>> >> > + memcpy(&prev_f, &f, sizeof(struct alps_fields));
>> >> > +}
>> >> > +
>> >> > static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
>> >> > unsigned char packet[],
>> >> > bool report_buttons)
>> >> > @@ -1080,6 +1492,14 @@ static psmouse_ret_t alps_process_byte(struct psmouse *psmouse)
>> >> > return PSMOUSE_BAD_DATA;
>> >> > }
>> >> >
>> >> > + if ((priv->proto_version == ALPS_PROTO_V7 &&
>> >> > + !alps_is_valid_package_v7(psmouse))) {
>> >> > + psmouse_dbg(psmouse, "refusing packet[%i] = %x\n",
>> >> > + psmouse->pktcnt - 1,
>> >> > + psmouse->packet[psmouse->pktcnt - 1]);
>> >> > + return PSMOUSE_BAD_DATA;
>> >> > + }
>> >> > +
>> >> > if (psmouse->pktcnt == psmouse->pktsize) {
>> >> > priv->process_packet(psmouse);
>> >> > return PSMOUSE_FULL_PACKET;
>> >> > @@ -1192,6 +1612,22 @@ static int alps_rpt_cmd(struct psmouse *psmouse, int init_command,
>> >> > return 0;
>> >> > }
>> >> >
>> >> > +static int alps_check_valid_firmware_id(unsigned char id[])
>> >> > +{
>> >> > + int valid = 1;
>> >>
>> >> bool valid = true;
>> >>
>> >> > +
>> >> > + if (id[0] == 0x73)
>> >> > + valid = 1;
>> >> > + else if (id[0] == 0x88) {
>> >> > + if ((id[1] == 0x07) ||
>> >> > + (id[1] == 0x08) ||
>> >> > + ((id[1] & 0xf0) == 0xB0))
>> >> > + valid = 1;
>> >> > + }
>> >> > +
>> >> > + return valid;
>> >> > +}
>> >> > +
>> >> > static int alps_enter_command_mode(struct psmouse *psmouse)
>> >> > {
>> >> > unsigned char param[4];
>> >> > @@ -1201,8 +1637,7 @@ static int alps_enter_command_mode(struct psmouse *psmouse)
>> >> > return -1;
>> >> > }
>> >> >
>> >> > - if ((param[0] != 0x88 || (param[1] != 0x07 && param[1] != 0x08)) &&
>> >> > - param[0] != 0x73) {
>> >> > + if (!alps_check_valid_firmware_id(param)) {
>> >> > psmouse_dbg(psmouse,
>> >> > "unknown response while entering command mode\n");
>> >> > return -1;
>> >> > @@ -1704,6 +2139,36 @@ error:
>> >> > return ret;
>> >> > }
>> >> >
>> >> > +static int alps_hw_init_v7(struct psmouse *psmouse)
>> >> > +{
>> >> > + struct ps2dev *ps2dev = &psmouse->ps2dev;
>> >> > + int reg_val, ret = -1;
>> >> > +
>> >> > + if (alps_enter_command_mode(psmouse))
>> >> > + goto error;
>> >> > +
>> >> > + reg_val = alps_command_mode_read_reg(psmouse, 0xc2d9);
>> >> > + if (reg_val == -1)
>> >> > + goto error;
>> >> > +
>> >> > + if (alps_command_mode_write_reg(psmouse, 0xc2c9, 0x64))
>> >> > + goto error;
>> >> > +
>> >> > + reg_val = alps_command_mode_read_reg(psmouse, 0xc2c4);
>> >> > + if (reg_val == -1)
>> >> > + goto error;
>> >> > +
>> >> > + if (__alps_command_mode_write_reg(psmouse, reg_val | 0x02))
>> >> > + goto error;
>> >> > +
>> >> > + alps_exit_command_mode(psmouse);
>> >> > + return ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE);
>> >> > +
>> >> > +error:
>> >> > + alps_exit_command_mode(psmouse);
>> >> > + return ret;
>> >> > +}
>> >> > +
>> >> > /* Must be in command mode when calling this function */
>> >> > static int alps_absolute_mode_v4(struct psmouse *psmouse)
>> >> > {
>> >> > @@ -1875,6 +2340,7 @@ static void alps_set_defaults(struct alps_data *priv)
>> >> > priv->set_abs_params = alps_set_abs_params_st;
>> >> > priv->x_max = 1023;
>> >> > priv->y_max = 767;
>> >> > + priv->slot_number = 1;
>> >> > break;
>> >> > case ALPS_PROTO_V3:
>> >> > priv->hw_init = alps_hw_init_v3;
>> >> > @@ -1883,6 +2349,7 @@ static void alps_set_defaults(struct alps_data *priv)
>> >> > priv->decode_fields = alps_decode_pinnacle;
>> >> > priv->nibble_commands = alps_v3_nibble_commands;
>> >> > priv->addr_command = PSMOUSE_CMD_RESET_WRAP;
>> >> > + priv->slot_number = 2;
>> >> > break;
>> >> > case ALPS_PROTO_V4:
>> >> > priv->hw_init = alps_hw_init_v4;
>> >> > @@ -1890,6 +2357,7 @@ static void alps_set_defaults(struct alps_data *priv)
>> >> > priv->set_abs_params = alps_set_abs_params_mt;
>> >> > priv->nibble_commands = alps_v4_nibble_commands;
>> >> > priv->addr_command = PSMOUSE_CMD_DISABLE;
>> >> > + priv->slot_number = 2;
>> >> > break;
>> >> > case ALPS_PROTO_V5:
>> >> > priv->hw_init = alps_hw_init_dolphin_v1;
>> >> > @@ -1905,6 +2373,7 @@ static void alps_set_defaults(struct alps_data *priv)
>> >> > priv->y_max = 660;
>> >> > priv->x_bits = 23;
>> >> > priv->y_bits = 12;
>> >> > + priv->slot_number = 2;
>> >> > break;
>> >> > case ALPS_PROTO_V6:
>> >> > priv->hw_init = alps_hw_init_v6;
>> >> > @@ -1913,6 +2382,28 @@ static void alps_set_defaults(struct alps_data *priv)
>> >> > priv->nibble_commands = alps_v6_nibble_commands;
>> >> > priv->x_max = 2047;
>> >> > priv->y_max = 1535;
>> >> > + priv->slot_number = 2;
>> >> > + break;
>> >> > + case ALPS_PROTO_V7:
>> >> > + priv->hw_init = alps_hw_init_v7;
>> >> > + priv->process_packet = alps_process_packet_v7;
>> >> > + priv->decode_fields = alps_decode_packet_v7;
>> >> > + priv->set_abs_params = alps_set_abs_params_mt;
>> >> > + priv->nibble_commands = alps_v3_nibble_commands;
>> >> > + priv->addr_command = PSMOUSE_CMD_RESET_WRAP;
>> >> > + priv->x_max = 0xfff;
>> >> > + priv->y_max = 0x7ff;
>> >> > + priv->resting_zone_y_min = 0x654;
>> >> > + priv->byte0 = 0x48;
>> >> > + priv->mask0 = 0x48;
>> >> > + priv->flags = 0;
>> >> > + priv->slot_number = 2;
>> >> > +
>> >> > + priv->phy_btn = 0;
>> >> > + priv->prev_phy_btn = 0;
>> >> > + priv->btn_delay_cnt = 0;
>> >> > + priv->pressed_btn_bits = 0;
>> >> > + memset(priv->pt_attr, 0, sizeof(priv->pt_attr[0]) * 2);
>> >> > break;
>> >> > }
>> >> > }
>> >> > @@ -1982,6 +2473,11 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
>> >> > return -EIO;
>> >> > else
>> >> > return 0;
>> >> > + } else if (ec[0] == 0x88 && (ec[1] & 0xf0) == 0xB0) {
>> >> > + priv->proto_version = ALPS_PROTO_V7;
>> >> > + alps_set_defaults(priv);
>> >> > +
>> >> > + return 0;
>> >> > } else if (ec[0] == 0x88 && ec[1] == 0x08) {
>> >> > priv->proto_version = ALPS_PROTO_V3;
>> >> > alps_set_defaults(priv);
>> >> > @@ -2045,7 +2541,7 @@ static void alps_set_abs_params_mt(struct alps_data *priv,
>> >> > struct input_dev *dev1)
>> >> > {
>> >> > set_bit(INPUT_PROP_SEMI_MT, dev1->propbit);
>> >> > - input_mt_init_slots(dev1, 2, 0);
>> >> > + input_mt_init_slots(dev1, priv->slot_number, 0);
>> >> > input_set_abs_params(dev1, ABS_MT_POSITION_X, 0, priv->x_max, 0, 0);
>> >> > input_set_abs_params(dev1, ABS_MT_POSITION_Y, 0, priv->y_max, 0, 0);
>> >> >
>> >> > diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
>> >> > index 03f88b6..dedbd27 100644
>> >> > --- a/drivers/input/mouse/alps.h
>> >> > +++ b/drivers/input/mouse/alps.h
>> >> > @@ -18,11 +18,36 @@
>> >> > #define ALPS_PROTO_V4 4
>> >> > #define ALPS_PROTO_V5 5
>> >> > #define ALPS_PROTO_V6 6
>> >> > +#define ALPS_PROTO_V7 7
>> >> > +
>> >> > +#define MAX_IMG_PT_NUM 5
>> >> > +#define V7_IMG_PT_NUM 2
>> >> > +
>> >> > +#define ZONE_NORMAL 0x01
>> >> > +#define ZONE_RESTING 0x02
>> >> > +#define ZONE_LEFT_BTN 0x04
>> >> > +#define ZONE_RIGHT_BTN 0x08
>> >> >
>> >> > #define DOLPHIN_COUNT_PER_ELECTRODE 64
>> >> > #define DOLPHIN_PROFILE_XOFFSET 8 /* x-electrode offset */
>> >> > #define DOLPHIN_PROFILE_YOFFSET 1 /* y-electrode offset */
>> >> >
>> >> > +/*
>> >> > + * enum V7_PACKET_ID - defines the packet type for V7
>> >> > + * V7_PACKET_ID_IDLE: There's no finger and no button activity.
>> >> > + * V7_PACKET_ID_TWO: There's one or two non-resting fingers on touchpad
>> >> > + * or there's button activities.
>> >> > + * V7_PACKET_ID_MULTI: There are at least three non-resting fingers.
>> >> > + * V7_PACKET_ID_NEW: The finger position in slot is not continues from
>> >> > + * previous packet.
>> >> > +*/
>> >> > +enum V7_PACKET_ID {
>> >> > + V7_PACKET_ID_IDLE,
>> >> > + V7_PACKET_ID_TWO,
>> >> > + V7_PACKET_ID_MULTI,
>> >> > + V7_PACKET_ID_NEW,
>> >> > +};
>> >> > +
>> >> > /**
>> >> > * struct alps_model_info - touchpad ID table
>> >> > * @signature: E7 response string to match.
>> >> > @@ -66,15 +91,7 @@ struct alps_nibble_commands {
>> >> > };
>> >> >
>> >> > /**
>> >> > - * struct alps_fields - decoded version of the report packet
>> >> > - * @x_map: Bitmap of active X positions for MT.
>> >> > - * @y_map: Bitmap of active Y positions for MT.
>> >> > - * @fingers: Number of fingers for MT.
>> >> > - * @x: X position for ST.
>> >> > - * @y: Y position for ST.
>> >> > - * @z: Z position for ST.
>> >> > - * @first_mp: Packet is the first of a multi-packet report.
>> >> > - * @is_mp: Packet is part of a multi-packet report.
>> >> > + * struct alps_btn - decoded version of the button status
>> >> > * @left: Left touchpad button is active.
>> >> > * @right: Right touchpad button is active.
>> >> > * @middle: Middle touchpad button is active.
>> >> > @@ -82,16 +99,7 @@ struct alps_nibble_commands {
>> >> > * @ts_right: Right trackstick button is active.
>> >> > * @ts_middle: Middle trackstick button is active.
>> >> > */
>> >> > -struct alps_fields {
>> >> > - unsigned int x_map;
>> >> > - unsigned int y_map;
>> >> > - unsigned int fingers;
>> >> > - unsigned int x;
>> >> > - unsigned int y;
>> >> > - unsigned int z;
>> >> > - unsigned int first_mp:1;
>> >> > - unsigned int is_mp:1;
>> >> > -
>> >> > +struct alps_btn {
>> >> > unsigned int left:1;
>> >> > unsigned int right:1;
>> >> > unsigned int middle:1;
>> >> > @@ -102,6 +110,73 @@ struct alps_fields {
>> >> > };
>> >> >
>> >> > /**
>> >> > + * struct alps_btn - decoded version of the X Y Z postion for ST.
>> >> > + * @x: X position for ST.
>> >> > + * @y: Y position for ST.
>> >> > + * @z: Z position for ST.
>> >> > + */
>> >> > +struct alps_abs_data {
>> >> > + unsigned int x;
>> >> > + unsigned int y;
>> >> > + unsigned int z;
>> >> > +};
>> >> > +
>> >> > +/**
>> >> > + * struct alps_fields - decoded version of the report packet
>> >> > + * @fingers: Number of fingers for MT.
>> >> > + * @pt: X Y Z postion for ST.
>> >> > + * @pt: X Y Z postion for image MT.
>> >> > + * @x_map: Bitmap of active X positions for MT.
>> >> > + * @y_map: Bitmap of active Y positions for MT.
>> >> > + * @first_mp: Packet is the first of a multi-packet report.
>> >> > + * @is_mp: Packet is part of a multi-packet report.
>> >> > + * @btn: Button activity status
>> >> > + */
>> >> > +struct alps_fields {
>> >> > + unsigned int fingers;
>> >> > + struct alps_abs_data pt;
>> >> > + struct alps_abs_data pt_img[MAX_IMG_PT_NUM];
>> >> > + unsigned int x_map;
>> >> > + unsigned int y_map;
>> >> > + unsigned int first_mp:1;
>> >> > + unsigned int is_mp:1;
>> >> > + struct alps_btn btn;
>> >>
>> >> Splitting button data and abs data into separate structures might make
>> >> sense, but please split off these changes into a separate patch so that
>> >> they are not obscuring changes necessary for v7 support.
>> >>
>> >> > +};
>> >> > +
>> >> > +/**
>> >> > + * struct v7_raw - data decoded from raw packet for V7.
>> >> > + * @pkt_id: An id that specifies the type of packet.
>> >> > + * @additional_fingers: Number of additional finger that is neighter included
>> >> > + * in pt slot nor reflected in rest_left and rest_right flag of data packet.
>> >> > + * @rest_left: There are fingers on left resting zone.
>> >> > + * @rest_right: There are fingers on right resting zone.
>> >> > + * @raw_fn: The number of finger on touchpad.
>> >> > + */
>> >> > +struct v7_raw {
>> >> > + unsigned char pkt_id;
>> >> > + unsigned int additional_fingers;
>> >> > + unsigned char rest_left;
>> >> > + unsigned char rest_right;
>> >> > + unsigned char raw_fn;
>> >> > +};
>> >> > +
>> >> > +/**
>> >> > + * struct alps_bl_pt_attr - generic attributes of touch points for buttonless device
>> >> > + * @zone: The part of touchpad that the touch point locates
>> >> > + * @is_counted: The touch point is not a resting finger.
>> >> > + * @is_init_pt_got: The touch down point is got.
>> >> > + * @init_pt: The X Y Z position of the touch down point.
>> >> > + * @init_dead_pt: The touch down point of a finger used by dead zone process.
>> >> > + */
>> >> > +struct alps_bl_pt_attr {
>> >> > + unsigned char zone;
>> >> > + unsigned char is_counted;
>> >> > + unsigned char is_init_pt_got;
>> >> > + struct alps_abs_data init_pt;
>> >> > + struct alps_abs_data init_dead_pt;
>> >> > +};
>> >> > +
>> >> > +/**
>> >> > * struct alps_data - private data structure for the ALPS driver
>> >> > * @dev2: "Relative" device used to report trackstick or mouse activity.
>> >> > * @phys: Physical path for the relative device.
>> >> > @@ -116,8 +191,10 @@ struct alps_fields {
>> >> > * @flags: Additional device capabilities (passthrough port, trackstick, etc.).
>> >> > * @x_max: Largest possible X position value.
>> >> > * @y_max: Largest possible Y position value.
>> >> > + * @resting_zone_y_min: Smallest Y postion value of the bottom resting zone.
>> >> > * @x_bits: Number of X bits in the MT bitmap.
>> >> > * @y_bits: Number of Y bits in the MT bitmap.
>> >> > + * @img_fingers: Number of image fingers.
>> >> > * @hw_init: Protocol-specific hardware init function.
>> >> > * @process_packet: Protocol-specific function to process a report packet.
>> >> > * @decode_fields: Protocol-specific function to read packet bitfields.
>> >> > @@ -132,6 +209,11 @@ struct alps_fields {
>> >> > * @fingers: Number of fingers from last MT report.
>> >> > * @quirks: Bitmap of ALPS_QUIRK_*.
>> >> > * @timer: Timer for flushing out the final report packet in the stream.
>> >> > + * @v7: Data decoded from raw packet for V7
>> >> > + * @phy_btn: Physical button is active.
>> >> > + * @prev_phy_btn: Physical button of previous packet is active.
>> >> > + * @pressed_btn_bits: Pressed positon of button zone
>> >> > + * @pt_attr: Generic attributes of touch points for buttonless device.
>> >> > */
>> >> > struct alps_data {
>> >> > struct input_dev *dev2;
>> >> > @@ -145,8 +227,10 @@ struct alps_data {
>> >> > unsigned char flags;
>> >> > int x_max;
>> >> > int y_max;
>> >> > + int resting_zone_y_min;
>> >> > int x_bits;
>> >> > int y_bits;
>> >> > + unsigned char slot_number;
>> >> >
>> >> > int (*hw_init)(struct psmouse *psmouse);
>> >> > void (*process_packet)(struct psmouse *psmouse);
>> >> > @@ -161,6 +245,16 @@ struct alps_data {
>> >> > int fingers;
>> >> > u8 quirks;
>> >> > struct timer_list timer;
>> >> > +
>> >> > + /* these are used for buttonless touchpad*/
>> >> > + union {
>> >> > + struct v7_raw v7;
>> >> > + } r;
>> >>
>> >> Why do you need a union here? A single-field union does not seem to be
>> >> terribly useful.
>> >>
>> >>
>> >> > + unsigned char phy_btn;
>> >> > + unsigned char prev_phy_btn;
>> >> > + unsigned char btn_delay_cnt;
>> >> > + unsigned char pressed_btn_bits;
>> >> > + struct alps_bl_pt_attr pt_attr[MAX_IMG_PT_NUM];
>> >> > };
>> >> >
>> >> > #define ALPS_QUIRK_TRACKSTICK_BUTTONS 1 /* trakcstick buttons in trackstick packet */
>> >> > --
>> >> > 1.8.3.2
>> >> >
>> >>
>> >> Thanks.
>> >>
>> >> --
>> >> Dmitry
^ permalink raw reply
* Re: [PATCH] evdev: flush ABS_* events during EVIOCGABS
From: Peter Hutterer @ 2014-04-23 7:09 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: David Herrmann, open list:HID CORE LAYER, Benjamin Tissoires
In-Reply-To: <20140423060747.GE24854@core.coreip.homeip.net>
On Tue, Apr 22, 2014 at 11:07:47PM -0700, Dmitry Torokhov wrote:
> On Wed, Apr 23, 2014 at 03:55:28PM +1000, Peter Hutterer wrote:
> > On Tue, Apr 22, 2014 at 10:46:47PM -0700, Dmitry Torokhov wrote:
> > > On Wed, Apr 23, 2014 at 03:38:49PM +1000, Peter Hutterer wrote:
> > > > On Wed, Apr 23, 2014 at 10:21:03AM +1000, Peter Hutterer wrote:
> > > > > On Tue, Apr 22, 2014 at 08:21:54AM +0200, David Herrmann wrote:
..
> > > >
> > > > any comments?
> > >
> > > Do we really need to optimize the case when we are dropping events?
> >
> > It happens frequently, to the point where on some laptops you're pretty much
> > guaranteed to get SYN_DROPPED events on resume and sometimes even during
> > normal multi-finger user.
> >
> > I don't have any measurements on how many events are dropped on average.
> > Could be one or two, could be several buffer sizes, I honestly don't know.
>
> I think we need to figure this out. The idea is that dropping events
> should be an exception, not a rule.
increase the buffer size for the devices I guess, with some heuristics
maybe. you could dynamically grow the buffer in the kernel, if the buffer
gets full or close to full, grow it.
but really, this is a moving target, eventually you will get the
SYN_DROPPED. if a client sleeps for a second or more, the events from a
second ago are likely useless anyway, so the need for an atomic sync exists
regardless. fwiw, I can live with a atomic sync that clears the client
buffer provided I get the correct state. any optimisation on top of that can
be done afterwards.
Cheers,
Peter
^ permalink raw reply
* Re: HID vendor access from user space
From: Nestor Lopez Casado @ 2014-04-23 9:14 UTC (permalink / raw)
To: Jiri Kosina
Cc: David Herrmann, Dmitry Torokhov, open list:HID CORE LAYER,
Olivier Gay, Benjamin Tissoires, Mathieu Meisser
In-Reply-To: <alpine.LNX.2.00.1404161649540.15539@pobox.suse.cz>
On Wed, Apr 16, 2014 at 4:53 PM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Tue, 15 Apr 2014, Nestor Lopez Casado wrote:
>
>> > Frankly, I'd really like to avoid any parsing (deciding about collections)
>> > whatsoever around hidraw, as that was the whole point of creating it in
>> > parallel to hiddev was simply 'no parsing, raw access'.
>>
>> We should avoid naming the new nodes "hidrawX" they could be
>> "hidvendorX" for instance, or it could also be "hidcolX" where col
>> stands for collection. We will be moving the "raw" one level down.
>
> I am still having trouble seeing how this would work together with current
> hidraw.
>
> In your proposal, there will be both hidrawX and hidcolX existing in
> parallel, right?
Yes.
> Who will be deciding what goes to hidcolX instead of
> hidrawX?
The additional code that we will put in place.
Having said that, hidrawX will still get the same reports as before,
nothing will change for those nodes.
> One of the pain points would be -- /dev/hidrawX will have to stay
> anyway for compatibility reasons, so we can't just divide it to multiple
> /dev/hidcolX all of a sudden and never look back.
That's true.
A device with one mouse collection and one vendor collection would
give something like this:
/dev/input/eventX (no change) this comes via hid-input
/dev/mouse (no change)
/dev/hidrawX (no change) (all reports still show up here)
/dev/hidcolX (reports from corresponding colX will show up here)
So only top level vendor collections which are not parsed (claimed) by
any hid-xx module today would appear under hidcolX.
Unless you have a major blocker, as David said, I think the best would
be to come up with some code and post a proposal patch for
consideration.
>
> --
> Jiri Kosina
> SUSE Labs
Cheers,
-nestor
^ permalink raw reply
* Re: HID vendor access from user space
From: David Herrmann @ 2014-04-23 9:27 UTC (permalink / raw)
To: Nestor Lopez Casado
Cc: Jiri Kosina, Dmitry Torokhov, open list:HID CORE LAYER,
Olivier Gay, Benjamin Tissoires, Mathieu Meisser
In-Reply-To: <CAE7qMro1UVGW-T5CiYv3+vE=_diar9SE9+r=WYrwOfk82PyweA@mail.gmail.com>
Hi
On Wed, Apr 23, 2014 at 11:14 AM, Nestor Lopez Casado
<nlopezcasad@logitech.com> wrote:
> A device with one mouse collection and one vendor collection would
> give something like this:
> /dev/input/eventX (no change) this comes via hid-input
> /dev/mouse (no change)
> /dev/hidrawX (no change) (all reports still show up here)
> /dev/hidcolX (reports from corresponding colX will show up here)
>
> So only top level vendor collections which are not parsed (claimed) by
> any hid-xx module today would appear under hidcolX.
I still think you should do that in user-space. That is, whatever
daemon opens the input-device should provide this abstraction (like
via XInput2 etc.). Otherwise, someone else pops up and tells us a
specific non-vendor extension is also unprivileged and wants them as
separate interface. There is no clear split how fine-grained the
access-control should be. Yes, you might have a specific example, but
we don't want to make that a generic interface.
Thanks
David
^ permalink raw reply
* Re: [PATCH] HID: add missing hid usages
From: Olivier Gay @ 2014-04-23 9:37 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jiri Kosina, linux-input, Nestor Lopez Casado, Mathieu Meisser
In-Reply-To: <20140423000457.GA22183@core.coreip.homeip.net>
On Wed, Apr 23, 2014 at 2:04 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Mon, Apr 07, 2014 at 09:49:52AM +0200, Jiri Kosina wrote:
>> > +#define KEY_BRIGHTNESS 0x250 /* Display Brightness */
>
> What is it supposed to do exactly? Show OSD for brightness?
No, it's a linear control usage to (HUTTR41) "Sets brightness to a
value between logical min and max.". I'm changing the comment to /*
Set Brightness */.
>> > +#define KEY_BRIGHTNESS_TOGGLE 0x251 /* Backlight Toggle */
>
> We already have KEY_DISPLAYTOGGLE, isn't this the same?
I agree, they are the same and we should alias it to
KEY_DISPLAYTOGGLE. I'm sending a v2 patch.
Best regards,
Olivier
^ permalink raw reply
* [PATCH v2] HID: add missing hid usages
From: Olivier Gay @ 2014-04-23 9:38 UTC (permalink / raw)
To: linux-input
Cc: Dmitry Torokhov, Jiri Kosina, Nestor Lopez Casado, Olivier Gay,
Mathieu Meisser
Add some missing hid usages from consumer page, add
display brightness control usages from approved hid usage
table request HUTTR41:
http://www.usb.org/developers/hidpage/HUTRR41.pdf
and add voice command usage from approved request HUTTR45:
http://www.usb.org/developers/hidpage/Voice_Command_Usage.pdf
Signed-off-by: Olivier Gay <ogay@logitech.com>
Signed-off-by: Mathieu Meisser <mmeisser@logitech.com>
---
drivers/hid/hid-debug.c | 14 ++++++++++++++
drivers/hid/hid-input.c | 15 +++++++++++++++
include/uapi/linux/input.h | 16 ++++++++++++++++
3 files changed, 45 insertions(+)
diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index 53b771d..742d78b 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -855,6 +855,20 @@ static const char *keys[KEY_MAX + 1] = {
[KEY_KBDILLUMDOWN] = "KbdIlluminationDown",
[KEY_KBDILLUMUP] = "KbdIlluminationUp",
[KEY_SWITCHVIDEOMODE] = "SwitchVideoMode",
+ [KEY_BUTTONCONFIG] = "ButtonConfig",
+ [KEY_TASKMANAGER] = "TaskManager",
+ [KEY_JOURNAL] = "Journal",
+ [KEY_CONTROLPANEL] = "ControlPanel",
+ [KEY_APPSELECT] = "AppSelect",
+ [KEY_SCREENSAVER] = "ScreenSaver",
+ [KEY_VOICECOMMAND] = "VoiceCommand",
+ [KEY_BRIGHTNESS_INC] = "BrightnessInc",
+ [KEY_BRIGHTNESS_DEC] = "BrightnessDec",
+ [KEY_BRIGHTNESS] = "Brightness",
+ [KEY_BRIGHTNESS_TOGGLE] = "BrightnessToggle",
+ [KEY_BRIGHTNESS_MIN] = "BrightnessMin",
+ [KEY_BRIGHTNESS_MAX] = "BrightnessMax",
+ [KEY_BRIGHTNESS_AUTO] = "BrightnessAuto",
};
static const char *relatives[REL_MAX + 1] = {
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index e7e8b19..f78b18d 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -721,6 +721,14 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
case 0x06c: map_key_clear(KEY_YELLOW); break;
case 0x06d: map_key_clear(KEY_ZOOM); break;
+ case 0x06f: map_key_clear(KEY_BRIGHTNESS_INC); break;
+ case 0x070: map_key_clear(KEY_BRIGHTNESS_DEC); break;
+ case 0x071: map_key_clear(KEY_BRIGHTNESS); break;
+ case 0x072: map_key_clear(KEY_BRIGHTNESS_TOGGLE); break;
+ case 0x073: map_key_clear(KEY_BRIGHTNESS_MIN); break;
+ case 0x074: map_key_clear(KEY_BRIGHTNESS_MAX); break;
+ case 0x075: map_key_clear(KEY_BRIGHTNESS_AUTO); break;
+
case 0x082: map_key_clear(KEY_VIDEO_NEXT); break;
case 0x083: map_key_clear(KEY_LAST); break;
case 0x084: map_key_clear(KEY_ENTER); break;
@@ -761,6 +769,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
case 0x0bf: map_key_clear(KEY_SLOW); break;
case 0x0cd: map_key_clear(KEY_PLAYPAUSE); break;
+ case 0x0cf: map_key_clear(KEY_VOICECOMMAND); break;
case 0x0e0: map_abs_clear(ABS_VOLUME); break;
case 0x0e2: map_key_clear(KEY_MUTE); break;
case 0x0e5: map_key_clear(KEY_BASSBOOST); break;
@@ -768,6 +777,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
case 0x0ea: map_key_clear(KEY_VOLUMEDOWN); break;
case 0x0f5: map_key_clear(KEY_SLOW); break;
+ case 0x181: map_key_clear(KEY_BUTTONCONFIG); break;
case 0x182: map_key_clear(KEY_BOOKMARKS); break;
case 0x183: map_key_clear(KEY_CONFIG); break;
case 0x184: map_key_clear(KEY_WORDPROCESSOR); break;
@@ -781,6 +791,8 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
case 0x18c: map_key_clear(KEY_VOICEMAIL); break;
case 0x18d: map_key_clear(KEY_ADDRESSBOOK); break;
case 0x18e: map_key_clear(KEY_CALENDAR); break;
+ case 0x18f: map_key_clear(KEY_TASKMANAGER); break;
+ case 0x190: map_key_clear(KEY_JOURNAL); break;
case 0x191: map_key_clear(KEY_FINANCE); break;
case 0x192: map_key_clear(KEY_CALC); break;
case 0x193: map_key_clear(KEY_PLAYER); break;
@@ -789,12 +801,15 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
case 0x199: map_key_clear(KEY_CHAT); break;
case 0x19c: map_key_clear(KEY_LOGOFF); break;
case 0x19e: map_key_clear(KEY_COFFEE); break;
+ case 0x19f: map_key_clear(KEY_CONTROLPANEL); break;
+ case 0x1a2: map_key_clear(KEY_APPSELECT); break;
case 0x1a3: map_key_clear(KEY_NEXT); break;
case 0x1a4: map_key_clear(KEY_PREVIOUS); break;
case 0x1a6: map_key_clear(KEY_HELP); break;
case 0x1a7: map_key_clear(KEY_DOCUMENTS); break;
case 0x1ab: map_key_clear(KEY_SPELLCHECK); break;
case 0x1ae: map_key_clear(KEY_KEYBOARD); break;
+ case 0x1b1: map_key_clear(KEY_SCREENSAVER); break;
case 0x1b4: map_key_clear(KEY_FILE); break;
case 0x1b6: map_key_clear(KEY_IMAGES); break;
case 0x1b7: map_key_clear(KEY_AUDIO); break;
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index bd24470..389be5d 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -722,6 +722,22 @@ struct input_keymap_entry {
#define KEY_ALS_TOGGLE 0x230 /* Ambient light sensor */
+#define KEY_BUTTONCONFIG 0x240 /* AL Button Configuration */
+#define KEY_TASKMANAGER 0x241 /* AL Task/Project Manager */
+#define KEY_JOURNAL 0x242 /* AL Log/Journal/Timecard */
+#define KEY_CONTROLPANEL 0x243 /* AL Control Panel */
+#define KEY_APPSELECT 0x244 /* AL Select Task/Application */
+#define KEY_SCREENSAVER 0x245 /* AL Screen Saver */
+#define KEY_VOICECOMMAND 0x246 /* Listening Voice Command */
+
+#define KEY_BRIGHTNESS_INC KEY_BRIGHTNESSUP
+#define KEY_BRIGHTNESS_DEC KEY_BRIGHTNESSDOWN
+#define KEY_BRIGHTNESS 0x250 /* Set Brightness */
+#define KEY_BRIGHTNESS_TOGGLE KEY_DISPLAYTOGGLE
+#define KEY_BRIGHTNESS_MIN 0x251 /* Set Brightness to Minimum */
+#define KEY_BRIGHTNESS_MAX 0x252 /* Set Brightness to Maximum */
+#define KEY_BRIGHTNESS_AUTO 0x253 /* Set Auto Brightness */
+
#define BTN_TRIGGER_HAPPY 0x2c0
#define BTN_TRIGGER_HAPPY1 0x2c0
#define BTN_TRIGGER_HAPPY2 0x2c1
--
1.9.0
^ permalink raw reply related
* Re: [PATCH] Input: ads7877: Remove bitrotted comment
From: Mark Brown @ 2014-04-23 10:16 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Michael Hennerich, linux-input, linaro-kernel
In-Reply-To: <20140423003928.GB22547@core.coreip.homeip.net>
[-- Attachment #1: Type: text/plain, Size: 791 bytes --]
On Tue, Apr 22, 2014 at 05:39:28PM -0700, Dmitry Torokhov wrote:
> On Tue, Apr 22, 2014 at 10:18:21PM +0100, Mark Brown wrote:
> > Remove the bitrotted comment, though in actual fact the use case mentioned
> > is a great use for spi_async() since it would cut down on latency handling
> > the interrupt by saving us a context switch before we start SPI.
> > This was previously implemented, it was removed in commit b534422b2d11
> > (Input: ad7877 - switch to using threaded IRQ) for code complexity reasons.
> > It may be better to revert that commit instead.
> Hmm, maybe.. although I think original would cause device 'stuck' if
> call to spi_async() fails, so probably not a straight revert...
Probably best just to apply this, then - someone can always reimplement
if they need to.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v2 01/24] input: Add ff-memless-next module
From: Oliver Neukum @ 2014-04-23 12:12 UTC (permalink / raw)
To: Michal Malý
Cc: linux-input, linux-kernel, dmitry.torokhov, jkosina, elias.vds,
anssi.hannula, simon
In-Reply-To: <1398175209-9565-2-git-send-email-madcatxster@devoid-pointer.net>
On Tue, 2014-04-22 at 15:59 +0200, Michal Malý wrote:
> +/* Some devices might have a limit on how many uncombinable effects
> + * can be played at once */
> +static int mlnx_upload_conditional(struct mlnx_device *mlnxdev,
> + const struct ff_effect *effect)
> +{
> + struct mlnx_effect_command ecmd = {
> + .cmd = MLNX_UPLOAD_UNCOMB,
> + .u.uncomb.id = effect->id,
> + .u.uncomb.effect = effect
> + };
> + return mlnxdev->control_effect(mlnxdev->dev, mlnxdev->private,
> &ecmd);
> +}
> +
This mean you are building the structure on the stack
1. Are you sure nobody retains a reference?
2. That is needlessly inefficient
Regards
Oliver
--
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
^ permalink raw reply
* Re: [PATCH v2 11/24] input: Port hid-holtekff to ff-memless-next
From: Oliver Neukum @ 2014-04-23 12:17 UTC (permalink / raw)
To: Michal Malý
Cc: linux-input, linux-kernel, dmitry.torokhov, jkosina, elias.vds,
anssi.hannula, simon
In-Reply-To: <1398175209-9565-12-git-send-email-madcatxster@devoid-pointer.net>
On Tue, 2014-04-22 at 15:59 +0200, Michal Malý wrote:
> static int holtekff_play(struct input_dev *dev, void *data,
> - struct ff_effect *effect)
> + const struct mlnx_effect_command *command)
> {
> struct hid_device *hid = input_get_drvdata(dev);
> struct holtekff_device *holtekff = data;
> + const struct mlnx_rumble_force *rumble_force =
> &command->u.rumble_force;
> int left, right;
> /* effect type 1, length 65535 msec */
> u8 buf[HOLTEKFF_MSG_LENGTH] =
> { 0x01, 0x01, 0xff, 0xff, 0x10, 0xe0, 0x00 };
On the kernel stack.
>
> - left = effect->u.rumble.strong_magnitude;
> - right = effect->u.rumble.weak_magnitude;
> - dbg_hid("called with 0x%04x 0x%04x\n", left, right);
> + switch (command->cmd) {
> + case MLNX_START_RUMBLE:
> + left = rumble_force->strong;
> + right = rumble_force->weak;
> + dbg_hid("called with 0x%04x 0x%04x\n", left, right);
>
> - if (!left && !right) {
> - holtekff_send(holtekff, hid, stop_all6);
> - return 0;
> - }
> + if (!left && !right) {
> + holtekff_send(holtekff, hid, stop_all6);
> + return 0;
> + }
>
> - if (left)
> - buf[1] |= 0x80;
> - if (right)
> - buf[1] |= 0x40;
> + if (left)
> + buf[1] |= 0x80;
> + if (right)
> + buf[1] |= 0x40;
>
> - /* The device takes a single magnitude, so we just sum them
> up. */
> - buf[6] = min(0xf, (left >> 12) + (right >> 12));
> + /* The device takes a single magnitude, so we just sum
> them up. */
> + buf[6] = min(0xf, (left >> 12) + (right >> 12));
>
> - holtekff_send(holtekff, hid, buf);
> - holtekff_send(holtekff, hid, start_effect_1);
> + holtekff_send(holtekff, hid, buf);
> + holtekff_send(holtekff, hid, start_effect_1);
> + return 0;
> + case MLNX_STOP_RUMBLE:
> + holtekff_send(holtekff, hid, stop_all6);
> + return 0;
> + default:
> + return -EINVAL;
> + }
>
> return 0;
> }
This looks very much like doing DMA on the kernel stack.
That is very strictly forbidden. The bug is also in the current
code, but would you care to fix it up?
Regards
Oliver
^ permalink raw reply
* Re: [PATCH v2 01/24] input: Add ff-memless-next module
From: Michal Malý @ 2014-04-23 12:30 UTC (permalink / raw)
To: Oliver Neukum
Cc: linux-input, linux-kernel, dmitry.torokhov, jkosina, elias.vds,
anssi.hannula, simon
In-Reply-To: <1398255179.32091.1.camel@linux-fkkt.site>
On Wednesday 23 of April 2014 14:12:59 Oliver Neukum wrote:
> On Tue, 2014-04-22 at 15:59 +0200, Michal Malý wrote:
> > +/* Some devices might have a limit on how many uncombinable effects
> > + * can be played at once */
> > +static int mlnx_upload_conditional(struct mlnx_device *mlnxdev,
> > + const struct ff_effect *effect)
> > +{
> > + struct mlnx_effect_command ecmd = {
> > + .cmd = MLNX_UPLOAD_UNCOMB,
> > + .u.uncomb.id = effect->id,
> > + .u.uncomb.effect = effect
> > + };
> > + return mlnxdev->control_effect(mlnxdev->dev, mlnxdev->private,
> > &ecmd);
> > +}
> > +
>
> This mean you are building the structure on the stack
>
> 1. Are you sure nobody retains a reference?
Yes. The command is a one-shot thing so it makes no sense to hold a persistent
reference to it. Should the HW-specific driver need to keep any data from the
command - if the uses a workqueue to submit data to the device for instance -
it should keep its own copy of the data. The idea is to keep MLNX and HW-
specific driver as separated as possible to prevent any race conditions.
> 2. That is needlessly inefficient
Are you suggesting I drop the 'consts' and keep the memory preallocated?
Thanks for the feedback,
Michal
^ permalink raw reply
* Re: [PATCH v2 11/24] input: Port hid-holtekff to ff-memless-next
From: Michal Malý @ 2014-04-23 12:31 UTC (permalink / raw)
To: Oliver Neukum
Cc: linux-input, linux-kernel, dmitry.torokhov, jkosina, elias.vds,
anssi.hannula, simon
In-Reply-To: <1398255470.32091.3.camel@linux-fkkt.site>
On Wednesday 23 of April 2014 14:17:50 Oliver Neukum wrote:
> On Tue, 2014-04-22 at 15:59 +0200, Michal Malý wrote:
> > static int holtekff_play(struct input_dev *dev, void *data,
> >
> > - struct ff_effect *effect)
> > + const struct mlnx_effect_command *command)
> >
> > {
> >
> > struct hid_device *hid = input_get_drvdata(dev);
> > struct holtekff_device *holtekff = data;
> >
> > + const struct mlnx_rumble_force *rumble_force =
> > &command->u.rumble_force;
> >
> > int left, right;
> > /* effect type 1, length 65535 msec */
> > u8 buf[HOLTEKFF_MSG_LENGTH] =
> >
> > { 0x01, 0x01, 0xff, 0xff, 0x10, 0xe0, 0x00 };
>
> On the kernel stack.
>
> > - left = effect->u.rumble.strong_magnitude;
> > - right = effect->u.rumble.weak_magnitude;
> > - dbg_hid("called with 0x%04x 0x%04x\n", left, right);
> > + switch (command->cmd) {
> > + case MLNX_START_RUMBLE:
> > + left = rumble_force->strong;
> > + right = rumble_force->weak;
> > + dbg_hid("called with 0x%04x 0x%04x\n", left, right);
> >
> > - if (!left && !right) {
> > - holtekff_send(holtekff, hid, stop_all6);
> > - return 0;
> > - }
> > + if (!left && !right) {
> > + holtekff_send(holtekff, hid, stop_all6);
> > + return 0;
> > + }
> >
> > - if (left)
> > - buf[1] |= 0x80;
> > - if (right)
> > - buf[1] |= 0x40;
> > + if (left)
> > + buf[1] |= 0x80;
> > + if (right)
> > + buf[1] |= 0x40;
> >
> > - /* The device takes a single magnitude, so we just sum them
> > up. */
> > - buf[6] = min(0xf, (left >> 12) + (right >> 12));
> > + /* The device takes a single magnitude, so we just sum
> > them up. */
> > + buf[6] = min(0xf, (left >> 12) + (right >> 12));
> >
> > - holtekff_send(holtekff, hid, buf);
> > - holtekff_send(holtekff, hid, start_effect_1);
> > + holtekff_send(holtekff, hid, buf);
> > + holtekff_send(holtekff, hid, start_effect_1);
> > + return 0;
> > + case MLNX_STOP_RUMBLE:
> > + holtekff_send(holtekff, hid, stop_all6);
> > + return 0;
> > + default:
> > + return -EINVAL;
> > + }
> >
> > return 0;
> >
> > }
>
> This looks very much like doing DMA on the kernel stack.
> That is very strictly forbidden. The bug is also in the current
> code, but would you care to fix it up?
Okay, I'll look into it.
Michal
^ permalink raw reply
* Re: [PATCH v2 01/24] input: Add ff-memless-next module
From: Oliver Neukum @ 2014-04-23 13:31 UTC (permalink / raw)
To: Michal Malý
Cc: linux-input, linux-kernel, dmitry.torokhov, jkosina, elias.vds,
anssi.hannula, simon
In-Reply-To: <12060887.UbTEf5RVBB@sigyn>
On Wed, 2014-04-23 at 14:30 +0200, Michal Malý wrote:
> > 2. That is needlessly inefficient
> Are you suggesting I drop the 'consts' and keep the memory preallocated?
Yes.
Regards
Oliver
^ permalink raw reply
* Re: [PATCH v2 09/24] input: Port hid-dr to ff-memless-next
From: Oliver Neukum @ 2014-04-23 13:41 UTC (permalink / raw)
To: Michal Malý
Cc: linux-input, linux-kernel, dmitry.torokhov, jkosina, elias.vds,
anssi.hannula, simon
In-Reply-To: <1398175209-9565-10-git-send-email-madcatxster@devoid-pointer.net>
On Tue, 2014-04-22 at 15:59 +0200, Michal Malý wrote:
> static int drff_play(struct input_dev *dev, void *data,
> - struct ff_effect *effect)
> + const struct mlnx_effect_command *command)
> {
> struct hid_device *hid = input_get_drvdata(dev);
> struct drff_device *drff = data;
> + const struct mlnx_rumble_force *rumble_force =
> &command->u.rumble_force;
> int strong, weak;
>
> - strong = effect->u.rumble.strong_magnitude;
> - weak = effect->u.rumble.weak_magnitude;
> + strong = rumble_force->strong;
> + weak = rumble_force->weak;
>
> dbg_hid("called with 0x%04x 0x%04x", strong, weak);
>
> - if (strong || weak) {
> - strong = strong * 0xff / 0xffff;
> - weak = weak * 0xff / 0xffff;
> -
> - /* While reverse engineering this device, I found that
> when
> - this value is set, it causes the strong rumble to
> function
> - at a near maximum speed, so we'll bypass it. */
> - if (weak == 0x0a)
> - weak = 0x0b;
> -
> - drff->report->field[0]->value[0] = 0x51;
> - drff->report->field[0]->value[1] = 0x00;
> - drff->report->field[0]->value[2] = weak;
> - drff->report->field[0]->value[4] = strong;
> - hid_hw_request(hid, drff->report, HID_REQ_SET_REPORT);
> -
> - drff->report->field[0]->value[0] = 0xfa;
> - drff->report->field[0]->value[1] = 0xfe;
> - } else {
> + switch (command->cmd) {
> + case MLNX_START_RUMBLE:
> + if (strong || weak) {
> + strong = strong * 0xff / 0xffff;
> + weak = weak * 0xff / 0xffff;
> +
> + /* While reverse engineering this device, I
> found that when
> + this value is set, it causes the strong rumble
> to function
> + at a near maximum speed, so we'll bypass it.
> */
> + if (weak == 0x0a)
> + weak = 0x0b;
> +
> + drff->report->field[0]->value[0] = 0x51;
> + drff->report->field[0]->value[1] = 0x00;
> + drff->report->field[0]->value[2] = weak;
> + drff->report->field[0]->value[4] = strong;
This looks like an endianness bug.
> + hid_hw_request(hid, drff->report,
> HID_REQ_SET_REPORT);
Regards
Oliver
^ permalink raw reply
* [PATCH] HID: rmi: do not handle touchscreens through hid-rmi
From: Benjamin Tissoires @ 2014-04-23 14:31 UTC (permalink / raw)
To: Jiri Kosina, Andrew Duggan, Christopher Heiny, linux-input,
linux-kernel
Currently, hid-rmi drives every Synaptics product, but the touchscreens
on the Windows tablets should be handled through hid-multitouch.
Instead of providing a long list of PIDs, rely on the scan_report
capability to detect which should go to hid-multitouch, and which
should not go to hid-rmi.
We introduce a generic HID_GROUP_HAVE_SPECIAL_DRIVER which can be reused
amoung other drivers if they want to have a catch rule but still
have multitouch devices handled through hid-multitouch.
related bug:
https://bugzilla.kernel.org/show_bug.cgi?id=74241
https://bugzilla.redhat.com/show_bug.cgi?id=1089583
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
Hi Jiri,
well, this is the patch we are going to carry in Fedora as we are already
carrying hid-rmi.
Still, I am not 100% happy with it, especially the HID_GROUP_HAVE_SPECIAL_DRIVER
thing.
I would also say we should re-think the way the scanning is called in
hid_add_device because I am starting using more and more the scanning capability
to decide if the device should be handled by generic, a special driver or simply
ignored.
Anyway, this could be decided later. If you prefer me to use a hid-rmi specific
group like HID_GROUP_RMI instead of HID_GROUP_HAVE_SPECIAL_DRIVER, I can also
update the patch.
Cheers,
Benjamin
drivers/hid/hid-core.c | 10 ++++++++--
drivers/hid/hid-rmi.c | 6 ++++--
include/linux/hid.h | 13 +++++++++++++
3 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index f05255d..46b27f3 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -776,6 +776,14 @@ static int hid_scan_report(struct hid_device *hid)
(hid->group == HID_GROUP_MULTITOUCH))
hid->group = HID_GROUP_MULTITOUCH_WIN_8;
+ /*
+ * Vendor specific handlings
+ */
+ if ((hid->vendor == USB_VENDOR_ID_SYNAPTICS) &&
+ (hid->group == HID_GROUP_GENERIC))
+ /* hid-rmi should take care of them, not hid-generic */
+ hid->group = HID_GROUP_HAVE_SPECIAL_DRIVER;
+
vfree(parser);
return 0;
}
@@ -1882,8 +1890,6 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGP_MOUSE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SUNPLUS, USB_DEVICE_ID_SUNPLUS_WDESKTOP) },
- { HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, HID_ANY_ID) },
- { HID_I2C_DEVICE(USB_VENDOR_ID_SYNAPTICS, HID_ANY_ID) },
{ HID_USB_DEVICE(USB_VENDOR_ID_THINGM, USB_DEVICE_ID_BLINK1) },
{ HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb300) },
{ HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb304) },
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index 7da9509..3980955 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -861,8 +861,10 @@ static void rmi_remove(struct hid_device *hdev)
}
static const struct hid_device_id rmi_id[] = {
- { HID_I2C_DEVICE(USB_VENDOR_ID_SYNAPTICS, HID_ANY_ID) },
- { HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, HID_ANY_ID) },
+ { HID_DEVICE(BUS_I2C, HID_GROUP_HAVE_SPECIAL_DRIVER,
+ USB_VENDOR_ID_SYNAPTICS, HID_ANY_ID) },
+ { HID_DEVICE(BUS_USB, HID_GROUP_HAVE_SPECIAL_DRIVER,
+ USB_VENDOR_ID_SYNAPTICS, HID_ANY_ID) },
{ }
};
MODULE_DEVICE_TABLE(hid, rmi_id);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 54f855b..888acfd 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -304,6 +304,19 @@ struct hid_item {
#define HID_GROUP_MULTITOUCH 0x0002
#define HID_GROUP_SENSOR_HUB 0x0003
#define HID_GROUP_MULTITOUCH_WIN_8 0x0004
+#define HID_GROUP_HAVE_SPECIAL_DRIVER 0xffff
+/*
+ * HID_GROUP_HAVE_SPECIAL_DRIVER should be used when the device needs to be
+ * scanned in case it is handled by either hid-multitouch, hid-generic,
+ * hid-sensor-hub or any other generic hid driver.
+ *
+ * Devices declared in hid_have_special_driver[] in hid-core.c can use
+ * HID_GROUP_ANY instead because there will be not overlap between their
+ * specific driver and a generic one.
+ *
+ * Note: HID_GROUP_ANY is declared in linux/mod_devicetable.h
+ * and has a value of 0x0000
+ */
/*
* This is the global environment of the parser. This information is
--
1.9.0
^ permalink raw reply related
* [PATCH] HID: multitouch: add support of EliteGroup 05D8 panels
From: Benjamin Tissoires @ 2014-04-23 14:42 UTC (permalink / raw)
To: Tomas Sokorai, Jiri Kosina, linux-input, linux-kernel
From: Tomas Sokorai <tsokorai@gmail.com>
They need to have the class SERIAL.
Note: it is a Win 7 panel, not Win 8 certified.
Signed-off-by: Tomas Sokorai <tsokorai@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-ids.h | 3 +++
drivers/hid/hid-multitouch.c | 5 +++++
2 files changed, 8 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index bd22126..f1712b5 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -301,6 +301,9 @@
#define USB_VENDOR_ID_DREAM_CHEEKY 0x1d34
+#define USB_VENDOR_ID_ELITEGROUP 0x03fc
+#define USB_DEVICE_ID_ELITEGROUP_05D8 0x05d8
+
#define USB_VENDOR_ID_ELO 0x04E7
#define USB_DEVICE_ID_ELO_TS2515 0x0022
#define USB_DEVICE_ID_ELO_TS2700 0x0020
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 35278e4..51e25b9 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1155,6 +1155,11 @@ static const struct hid_device_id mt_devices[] = {
MT_USB_DEVICE(USB_VENDOR_ID_DWAV,
USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_A001) },
+ /* Elitegroup panel */
+ { .driver_data = MT_CLS_SERIAL,
+ MT_USB_DEVICE(USB_VENDOR_ID_ELITEGROUP,
+ USB_DEVICE_ID_ELITEGROUP_05D8) },
+
/* Flatfrog Panels */
{ .driver_data = MT_CLS_FLATFROG,
MT_USB_DEVICE(USB_VENDOR_ID_FLATFROG,
--
1.9.0
^ permalink raw reply related
* Re: HID: hid-logitech - missing HID_OUTPUT_REPORT 0
From: simon @ 2014-04-23 14:50 UTC (permalink / raw)
To: Kees Cook
Cc: Benjamin Tissoires, Simon Wood, linux-input, Jiri Kosina,
Elias Vanderstuyft
In-Reply-To: <CAGXu5jKVCme=a8EVg1BOrB+5enpjT5aABy3QB=div-t230+NSA@mail.gmail.com>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 9e8064205bc7..5205ebb76cd2 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -840,6 +840,15 @@ struct hid_report *hid_validate_values(struct
> hid_device *h
> * drivers go to access report values.
> */
> report = hid->report_enum[type].report_id_hash[id];
> + if (!report && id == 0) {
> + /*
> + * Requesting id 0 means we should fall back to the first
> + * report in the list.
> + */
> + report = list_entry(
> + hid->report_enum[type].report_list.next,
> + struct hid_report, list);
> + }
> if (!report) {
> hid_err(hid, "missing %s %u\n", hid_report_names[type],
> id);
> return NULL;
>
> Alternatively, should hid_register_report add a default to the hash
> instead, so no change to hid_validate_values is needed?
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 9e8064205bc7..5d07124457ba 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -80,6 +80,8 @@ struct hid_report *hid_register_report(struct hid_device
> *devi
> report->size = 0;
> report->device = device;
> report_enum->report_id_hash[id] = report;
> + if (!report_enum->report_id_hash[0])
> + report_enum->report_id_hash[0] = report;
>
> list_add_tail(&report->list, &report_enum->report_list);
>
>
>
> -Kees
I tried this patch last night and saw Kernel Panics on controller
insertion and/or removal....
And just noticed that you have another more recent one, so I'll try that
tonight. My bad :-(
Simon.
^ permalink raw reply
* Re: [PATCH 3/9] drivers/hid/hid-lg4ff.c: avoid world-writable sysfs files.
From: simon @ 2014-04-23 15:06 UTC (permalink / raw)
To: Rusty Russell; +Cc: "Michal Malý", linux-kernel, HID CORE LAYER
In-Reply-To: <87r44ofw1x.fsf@rustcorp.com.au>
> simon@mungewell.org writes:
>>> In line with practice for module parameters, we're adding a build-time
>>> check that sysfs files aren't world-writable.
>>
>> So this is the equivalent of 'chmod 774 ...' rather than 'chmod
>> 777...'?
>
> Yep. Though not sure why it was 777 rather than 666...
>
>> Yep I'm OK with that, however what it the recommended way to make sure
>> that the end user is able to send changes to this /sys portal? I asked
>> the
>> same question before regarding the led class /sys interface, but never
>> got
>> any suggestions.
>>
>> Signed-off-by: Simon Wood <simon@mungewell.org>
>
> If you need that, we'll need to make an exception. That's one purpose
> of spamming everyone with these changs...
What's the right way of doing it?... I don't need to be 'special'. ;-)
The '/sys/.../range' control allows the user to limit the rotation of the
gaming wheel from a maximum of 900' down to match the 'car' they
sim-driving. Probably not many people use it, but it probably should be
assigned properly.
With gaming controllers the /dev/input/event* seem to get set
appropriately, but I'm not sure how this happens.
The same /should/ also happen for all the LED class controls, I don't want
to have to 'sudo' just to set a LED on/off. This is currently a problem
for (at least) hid-lg, hid-sony and hid-steelseries.
Simon
^ permalink raw reply
* Re: [PATCH v2 09/24] input: Port hid-dr to ff-memless-next
From: simon @ 2014-04-23 15:47 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: madcatxster, oneukum, linux-kernel, jkosina, elias.vds,
anssi.hannula, simon, linux-input
In-Reply-To: <20140423154201.GA10531@core.coreip.homeip.net>
> On Wed, Apr 23, 2014 at 03:14:44PM +0000, madcatxster@devoid-pointer.net
> wrote:
>> This is another case where even the old code was flawed, right? Should
>> I try to stuff the fixes into these patches or would a few extra
>> patches addressing these problems be an easier to review solution? I
>> can append such patches to the MLNX patchset.
>
> Changes addressing pre-existing problem should go into separate patches
> (preferably applicable first).
>
As a by-stander who would like to see MLNX move forward, should it be
heldback by pre-existing problems in drivers that the MLNX dev(s) don't
have hardware to test against...?
Simon.
^ permalink raw reply
* Re: [PATCH v2 09/24] input: Port hid-dr to ff-memless-next
From: Michal Malý @ 2014-04-23 15:57 UTC (permalink / raw)
To: simon
Cc: Dmitry Torokhov, oneukum, linux-kernel, jkosina, elias.vds,
anssi.hannula, linux-input
In-Reply-To: <0792885b3347c5935a0af7a7a3f3bc70.squirrel@mungewell.org>
On Wednesday 23 of April 2014 11:47:05 simon@mungewell.org wrote:
> > On Wed, Apr 23, 2014 at 03:14:44PM +0000, madcatxster@devoid-pointer.net
> >
> > wrote:
> >> This is another case where even the old code was flawed, right? Should
> >> I try to stuff the fixes into these patches or would a few extra
> >> patches addressing these problems be an easier to review solution? I
> >> can append such patches to the MLNX patchset.
> >
> > Changes addressing pre-existing problem should go into separate patches
> > (preferably applicable first).
>
> As a by-stander who would like to see MLNX move forward, should it be
> heldback by pre-existing problems in drivers that the MLNX dev(s) don't
> have hardware to test against...?
>
> Simon.
Either approach is fine be me - I can rebase the MLNX patchset against the fixes
and submit it again. I suppose that this is a good opportunity to fix a bunch
old issues that would pass unnoticed otherwise. I would however appreciate as
much comments regarding MLNX itself before I begin cleaning the ancient dust.
Thanks for your input,
Michal
^ permalink raw reply
* Re: [PATCH v2 01/24] input: Add ff-memless-next module
From: Dmitry Torokhov @ 2014-04-23 15:59 UTC (permalink / raw)
To: Oliver Neukum
Cc: Michal Malý, linux-input, linux-kernel, jkosina, elias.vds,
anssi.hannula, simon
In-Reply-To: <1398255179.32091.1.camel@linux-fkkt.site>
On Wed, Apr 23, 2014 at 02:12:59PM +0200, Oliver Neukum wrote:
> On Tue, 2014-04-22 at 15:59 +0200, Michal Malý wrote:
> > +/* Some devices might have a limit on how many uncombinable effects
> > + * can be played at once */
> > +static int mlnx_upload_conditional(struct mlnx_device *mlnxdev,
> > + const struct ff_effect *effect)
> > +{
> > + struct mlnx_effect_command ecmd = {
> > + .cmd = MLNX_UPLOAD_UNCOMB,
> > + .u.uncomb.id = effect->id,
> > + .u.uncomb.effect = effect
> > + };
> > + return mlnxdev->control_effect(mlnxdev->dev, mlnxdev->private,
> > &ecmd);
> > +}
> > +
>
> This mean you are building the structure on the stack
>
> 1. Are you sure nobody retains a reference?
> 2. That is needlessly inefficient
Why is it inefficient?
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 11/24] input: Port hid-holtekff to ff-memless-next
From: Dmitry Torokhov @ 2014-04-23 16:02 UTC (permalink / raw)
To: Oliver Neukum
Cc: Michal Malý, linux-input, linux-kernel, jkosina, elias.vds,
anssi.hannula, simon
In-Reply-To: <1398255470.32091.3.camel@linux-fkkt.site>
On Wed, Apr 23, 2014 at 02:17:50PM +0200, Oliver Neukum wrote:
> On Tue, 2014-04-22 at 15:59 +0200, Michal Malý wrote:
> > static int holtekff_play(struct input_dev *dev, void *data,
> > - struct ff_effect *effect)
> > + const struct mlnx_effect_command *command)
> > {
> > struct hid_device *hid = input_get_drvdata(dev);
> > struct holtekff_device *holtekff = data;
> > + const struct mlnx_rumble_force *rumble_force =
> > &command->u.rumble_force;
> > int left, right;
> > /* effect type 1, length 65535 msec */
> > u8 buf[HOLTEKFF_MSG_LENGTH] =
> > { 0x01, 0x01, 0xff, 0xff, 0x10, 0xe0, 0x00 };
>
> On the kernel stack.
>
> >
> > - left = effect->u.rumble.strong_magnitude;
> > - right = effect->u.rumble.weak_magnitude;
> > - dbg_hid("called with 0x%04x 0x%04x\n", left, right);
> > + switch (command->cmd) {
> > + case MLNX_START_RUMBLE:
> > + left = rumble_force->strong;
> > + right = rumble_force->weak;
> > + dbg_hid("called with 0x%04x 0x%04x\n", left, right);
> >
> > - if (!left && !right) {
> > - holtekff_send(holtekff, hid, stop_all6);
> > - return 0;
> > - }
> > + if (!left && !right) {
> > + holtekff_send(holtekff, hid, stop_all6);
> > + return 0;
> > + }
> >
> > - if (left)
> > - buf[1] |= 0x80;
> > - if (right)
> > - buf[1] |= 0x40;
> > + if (left)
> > + buf[1] |= 0x80;
> > + if (right)
> > + buf[1] |= 0x40;
> >
> > - /* The device takes a single magnitude, so we just sum them
> > up. */
> > - buf[6] = min(0xf, (left >> 12) + (right >> 12));
> > + /* The device takes a single magnitude, so we just sum
> > them up. */
> > + buf[6] = min(0xf, (left >> 12) + (right >> 12));
> >
> > - holtekff_send(holtekff, hid, buf);
> > - holtekff_send(holtekff, hid, start_effect_1);
> > + holtekff_send(holtekff, hid, buf);
> > + holtekff_send(holtekff, hid, start_effect_1);
> > + return 0;
> > + case MLNX_STOP_RUMBLE:
> > + holtekff_send(holtekff, hid, stop_all6);
> > + return 0;
> > + default:
> > + return -EINVAL;
> > + }
> >
> > return 0;
> > }
>
> This looks very much like doing DMA on the kernel stack.
It isn't:
static void holtekff_send(struct holtekff_device *holtekff,
struct hid_device *hid,
const u8 data[HOLTEKFF_MSG_LENGTH])
{
int i;
for (i = 0; i < HOLTEKFF_MSG_LENGTH; i++) {
holtekff->field->value[i] = data[i];
}
dbg_hid("sending %7ph\n", data);
hid_hw_request(hid, holtekff->field->report, HID_REQ_SET_REPORT);
}
And also hid layer copies data a bunch of times over into DMA-safe
buffers.
> That is very strictly forbidden. The bug is also in the current
> code, but would you care to fix it up?
>
> Regards
> Oliver
>
>
--
Dmitry
^ permalink raw reply
* [PATCH v3] synaptics: Add min/max quirk for ThinkPad Edge E431
From: Hans de Goede @ 2014-04-23 16:24 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Benjamin Tissoires, Peter Hutterer, linux-input, Hans de Goede,
stable
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/input/mouse/synaptics.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 7c9f509..93cc8fd 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -1566,6 +1566,14 @@ static const struct dmi_system_id min_max_dmi_table[] __initconst = {
.driver_data = (int []){1232, 5710, 1156, 4696},
},
{
+ /* Lenovo ThinkPad Edge E431 */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad Edge E431"),
+ },
+ .driver_data = (int []){1024, 5022, 2508, 4832},
+ },
+ {
/* Lenovo ThinkPad T431s */
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
--
1.9.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox