* RE: [PATCH] Add: (1) Detection for newer Elantech touchpads, so that kernel doesn't fall-back to default PS/2 driver. (2) Enable hardware version 4 touchpad right click function.
From: duson @ 2013-12-10 12:18 UTC (permalink / raw)
To: 'Dmitry Torokhov'; +Cc: linux-kernel, linux-input
In-Reply-To: <20131209063213.GA30195@core.coreip.homeip.net>
Hi Dmitry
> -----Original Message-----
> From: linux-input-owner@vger.kernel.org
[mailto:linux-input-owner@vger.kernel.org]
> On Behalf Of Dmitry Torokhov
> Sent: Monday, December 09, 2013 2:32 PM
> To: Duson Lin
> Cc: linux-kernel@vger.kernel.org; linux-input@vger.kernel.org
> Subject: Re: [PATCH] Add: (1) Detection for newer Elantech touchpads, so
that
> kernel doesn't fall-back to default PS/2 driver. (2) Enable hardware
version 4
> touchpad right click function.
>
> Hi Duson.
>
>
> On Mon, Dec 09, 2013 at 10:59:50AM +0800, Duson Lin wrote:
> > Modify:
> > (1) crc_enabled only support for v3 and v4 touchpad, so initialize
crc_enabled as
> false first and
> > check this hardware flag when hw_version as 3 or 4.
>
> It looks to me there are several fixes rolled up together in this patch:
>
> 1. Support for the new hardware signatures (8 and 10, although I already
> applied patch for 8)
> 2. Fix to handle CRC check
> 3. Changes to report BTN_RIGHT.
>
> Could you please split them up please?
OK, I will split them up.
>
> Also, I am not sure if we can simply start reporting BTN_RIGHT as
> present, even on devices that don't actually have the right button, as
> this will interfere with userspace providing emulation for multiple
> buttons.
>
> Is it possible to determine if a given model had right button or not?
It is hard to determine if a given model had right button, but depend on
per-discuss in Hans de Goede's mailing list, it is possible to determine if
given model need right click function.
Thanks.
Duosn
>
> >
> > Signed-off-by: Duson Lin <dusonlin@emc.com.tw>
> > ---
> > drivers/input/mouse/elantech.c | 30 ++++++++++++++----------------
> > drivers/input/mouse/elantech.h | 2 +-
> > 2 files changed, 15 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/input/mouse/elantech.c
b/drivers/input/mouse/elantech.c
> > index 8551dca..b3627cf 100644
> > --- a/drivers/input/mouse/elantech.c
> > +++ b/drivers/input/mouse/elantech.c
> > @@ -1,5 +1,5 @@
> > /*
> > - * Elantech Touchpad driver (v6)
> > + * Elantech Touchpad driver (v7)
> > *
> > * Copyright (C) 2007-2009 Arjan Opmeer <arjan@opmeer.net>
> > *
> > @@ -486,6 +486,7 @@ static void elantech_input_sync_v4(struct psmouse
> *psmouse)
> > unsigned char *packet = psmouse->packet;
> >
> > input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
> > + input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
> > input_mt_report_pointer_emulation(dev, true);
> > input_sync(dev);
> > }
> > @@ -1047,9 +1048,7 @@ static int elantech_set_input_params(struct
psmouse
> *psmouse)
> > */
> > psmouse_warn(psmouse, "couldn't query resolution
data.\n");
> > }
> > - /* v4 is clickpad, with only one button. */
> > __set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
> > - __clear_bit(BTN_RIGHT, dev->keybit);
> > __set_bit(BTN_TOOL_QUADTAP, dev->keybit);
> > /* For X to recognize me as touchpad. */
> > input_set_abs_params(dev, ABS_X, x_min, x_max, 0, 0);
> > @@ -1186,19 +1185,12 @@ static struct attribute_group
elantech_attr_group = {
> >
> > static bool elantech_is_signature_valid(const unsigned char *param)
> > {
> > - static const unsigned char rates[] = { 200, 100, 80, 60, 40, 20, 10
};
> > - int i;
> > -
> > if (param[0] == 0)
> > return false;
> >
> > if (param[1] == 0)
> > return true;
> >
> > - for (i = 0; i < ARRAY_SIZE(rates); i++)
> > - if (param[2] == rates[i])
> > - return false;
> > -
> > return true;
> > }
> >
> > @@ -1298,6 +1290,14 @@ static int elantech_set_properties(struct
> elantech_data *etd)
> > {
> > /* This represents the version of IC body. */
> > int ver = (etd->fw_version & 0x0f0000) >> 16;
> > + /*
> > + * The signatures of v3 and v4 packets change depending on the
> > + * value of this hardware flag. But the v1 and v2 have not crc
> > + * check mechanism and the same hardware flag are also defined
> > + * as other function. So crc_enabled must be initialized as false
> > + * first and checking by different hw_version.
> > + */
> > + etd->crc_enabled = false;
> >
> > /* Early version of Elan touchpads doesn't obey the rule. */
> > if (etd->fw_version < 0x020030 || etd->fw_version == 0x020600)
> > @@ -1309,10 +1309,14 @@ static int elantech_set_properties(struct
> elantech_data *etd)
> > etd->hw_version = 2;
> > break;
> > case 5:
> > + etd->crc_enabled = ((etd->fw_version & 0x4000) ==
0x4000);
> > etd->hw_version = 3;
> > break;
> > case 6:
> > case 7:
> > + case 8:
> > + case 10:
> > + etd->crc_enabled = ((etd->fw_version & 0x4000) ==
0x4000);
> > etd->hw_version = 4;
> > break;
> > default:
> > @@ -1343,12 +1347,6 @@ static int elantech_set_properties(struct
> elantech_data *etd)
> > etd->reports_pressure = true;
> > }
> >
> > - /*
> > - * The signatures of v3 and v4 packets change depending on the
> > - * value of this hardware flag.
> > - */
> > - etd->crc_enabled = ((etd->fw_version & 0x4000) == 0x4000);
> > -
> > return 0;
> > }
> >
> > diff --git a/drivers/input/mouse/elantech.h
b/drivers/input/mouse/elantech.h
> > index 036a04a..c963ac8 100644
> > --- a/drivers/input/mouse/elantech.h
> > +++ b/drivers/input/mouse/elantech.h
> > @@ -1,5 +1,5 @@
> > /*
> > - * Elantech Touchpad driver (v6)
> > + * Elantech Touchpad driver (v7)
> > *
> > * Copyright (C) 2007-2009 Arjan Opmeer <arjan@opmeer.net>
> > *
> > --
> > 1.7.10.4
> >
>
> --
> Dmitry
> --
> 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
* [PATCH V2] Input: define KEY_WWAN for Wireless WAN
From: Rafał Miłecki @ 2013-12-10 10:29 UTC (permalink / raw)
To: linux-input, Jiri Kosina; +Cc: Hauke Mehrtens, Rafał Miłecki
In-Reply-To: <1385664173-18879-1-git-send-email-zajec5@gmail.com>
Some devices with support for mobile networks may have buttons for
enabling/disabling such connection. An example can be Linksys router
54G3G.
We already have KEY_BLUETOOTH, KEY_WLAN and KEY_UWB so it makes sense
to add KEY_WWAN as well.
As we already have KEY_WIMAX, use it's value for KEY_WWAN and make it an
alias.
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
V2: Re-use 246 value for WWAN and make WIMAX an aliast to WWAN
---
include/uapi/linux/input.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index a372627..7c69941 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -464,7 +464,8 @@ struct input_keymap_entry {
#define KEY_BRIGHTNESS_ZERO 244 /* brightness off, use ambient */
#define KEY_DISPLAY_OFF 245 /* display device to off state */
-#define KEY_WIMAX 246
+#define KEY_WWAN 246 /* Wireless WAN (LTE, UMTS, GSM, etc.) */
+#define KEY_WIMAX KEY_WWAN
#define KEY_RFKILL 247 /* Key that controls all radios */
#define KEY_MICMUTE 248 /* Mute / unmute the microphone */
--
1.7.10.4
--
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 related
* the touchpad won't work after closing and opening the lid on some laptops
From: Hui Wang @ 2013-12-10 10:28 UTC (permalink / raw)
To: dtor, dmitry.torokhov, linux-input, Anthony Wong, YK
The problem:
On some lenovo laptops (this problem has been reproduced on 3 lenovo
laptop models), we let the machine boot into the desktop and login to
the desktop, the touchpad workes very well. But After we close the
lid then after a while open the lid, the touchpad won't work anymore.
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1089105
The root cause:
Through investigation, i found on some lenovo laptop models, when we
close the lid, the i8042/keyboard will generate a touchpad toggle key
event (generating a keycode same as the one generated by pressing
touchpad toggle functionkey/hotkey), while opening the lid, the
i8042/keyboard also generate a touchpad toggle key event.
If users close the lid under the gnome environment, the touchpad toggle
key event will be captured by the gnome-settings-daemon, so the touchpad
is disabled by the daemon, when users open the lid, the system will go to
the login window (gnome-screensaver), so the touchpad toggle key event
generated by opening lid doesn't pass to gnome-settings-daemon, so the
touchpad doesn't restore to the original status.
The keyboard driver is:
drivers/input/keyboard/atkbd.c
How to identify a machine whether has this problem or not:
An easy way to identify whether the machine has this problem is:
booting to gnome environment, pressing ALT+CTRL+F1 to enter text console,
executing showkey, closing lid, waiting several seconds, opening the lid,
if following logs are produced, your machine will have this problem.
keycode 191 press
keycode 191 release
keycode 191 press
keycode 191 release
My question:
Most of lenovo laptop models don't generate touchpad toggle event when
closing or opening the lid, so far this problem only happens on 3 laptop
models. I don't know who let the i8042/keyboard generate a touchpad toggle
key event when closing or opening the lid on those machines, do we have
a software/firmware method to let the i8042/keyboard don't generate this
event when closing or opening the lid?
Thanks,
Hui.
^ permalink raw reply
* Re: [PATCH 1/2] elantech: Properly differentiate between clickpads and normal touchpads
From: Peter Hutterer @ 2013-12-10 9:58 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Hans de Goede, linux-input, Peter Hutterer, Duson Lin,
Dmitry Torokhov
In-Reply-To: <CAN+gG=FgR9cBwJ_GFrU3RAmwma0XWzt8Wdi-HM2zEOOmdPNS+A@mail.gmail.com>
On Mon, Dec 09, 2013 at 02:21:19PM -0500, Benjamin Tissoires wrote:
> On Mon, Dec 9, 2013 at 2:14 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> > Hi,
> >
> >
> > On 12/09/2013 07:02 PM, Benjamin Tissoires wrote:
> >>
> >> Hi Hans,
> >>
> >> adding in CC Duson, who seems to be working on the same driver
> >> currently, and Dmitry, the input maintainer.
> >>
> >> On Mon, Dec 9, 2013 at 9:32 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> >>>
> >>> The current assumption in the elantech driver that hw version 3 touchpads
> >>> are
> >>> never clickpads and hw version 4 touchpads are always clickpads is wrong.
> >>>
> >>> There are several bug reports for this, ie:
> >>> https://bugzilla.redhat.com/show_bug.cgi?id=1030802
> >>>
> >>> http://superuser.com/questions/619582/right-elantech-touchpad-button-not-working-in-linux
> >>>
> >>> I've spend a couple of hours wading through various bugzillas,
> >>> launchpads and forum posts to create a list of fw-versions and
> >>> capabilities
> >>> for different laptop models to find a good method to differentiate
> >>> between
> >>> clickpads and versions with separate hardware buttons.
> >>>
> >>> Which shows that a device being a clickpad is reliable indicated by bit
> >>> 12
> >>> being set in the fw_version. I've included the gathered list inside the
> >>> driver,
> >>> so that we've this info at hand if we need to revisit this later.
> >>
> >>
> >> Duson, can you confirm this? It's not that I don't trust Hans, but if
> >> we could have the hardware maker validating this part, this would be
> >> great.
> >>
> >>>
> >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>> ---
> >>> drivers/input/mouse/elantech.c | 43
> >>> +++++++++++++++++++++++++++++++++++++++---
> >>> 1 file changed, 40 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/input/mouse/elantech.c
> >>> b/drivers/input/mouse/elantech.c
> >>> index 8551dca..f7baa32 100644
> >>> --- a/drivers/input/mouse/elantech.c
> >>> +++ b/drivers/input/mouse/elantech.c
> >>> @@ -486,6 +486,7 @@ static void elantech_input_sync_v4(struct psmouse
> >>> *psmouse)
> >>> unsigned char *packet = psmouse->packet;
> >>>
> >>> input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
> >>> + input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
> >>> input_mt_report_pointer_emulation(dev, true);
> >>> input_sync(dev);
> >>> }
> >>> @@ -984,6 +985,42 @@ static int elantech_get_resolution_v4(struct psmouse
> >>> *psmouse,
> >>> }
> >>>
> >>> /*
> >>> + * Advertise INPUT_PROP_BUTTONPAD for clickpads. The testing of bit 12
> >>> in
> >>> + * fw_version for this is based on the following fw_version & caps
> >>> table:
> >>> + *
> >>> + * Laptop-model: fw_version: caps: buttons:
> >>> + * Acer S3 0x461f00 10, 13, 0e clickpad
> >>> + * Acer S7-392 0x581f01 50, 17, 0d clickpad
> >>> + * Acer V5-131 0x461f02 01, 16, 0c clickpad
> >>> + * Acer V5-551 0x461f00 ? clickpad
> >>> + * Asus K53SV 0x450f01 78, 15, 0c 2 hw buttons
> >>> + * Asus G46VW 0x460f02 00, 18, 0c 2 hw buttons
> >>> + * Asus G750JX 0x360f00 00, 16, 0c 2 hw buttons
> >>> + * Asus UX31 0x361f00 20, 15, 0e clickpad
> >>> + * Asus UX32VD 0x361f02 00, 15, 0e clickpad
> >>> + * Avatar AVIU-145A2 0x361f00 ? clickpad
> >>> + * Gigabyte U2442 0x450f01 58, 17, 0c 2 hw buttons
> >>> + * Lenovo L430 0x350f02 b9, 15, 0c 2 hw buttons
> >>> (*)
> >>> + * Samsung NF210 0x150b00 78, 14, 0a 2 hw buttons
> >>> + * Samsung NP770Z5E 0x575f01 10, 15, 0f clickpad
> >>> + * Samsung NP700Z5B 0x361f06 21, 15, 0f clickpad
> >>> + * Samsung NP900X3E-A02 0x575f03 ? clickpad
> >>> + * Samsung NP-QX410 0x851b00 19, 14, 0c clickpad
> >>> + * Samsung RC512 0x450f00 08, 15, 0c 2 hw buttons
> >>> + * Samsung RF710 0x450f00 ? 2 hw buttons
> >>> + * System76 Pangolin 0x250f01 ? 2 hw buttons
> >>> + * (*) + 3 trackpoint buttons
> >>> + */
> >>> +static void elantech_set_buttonpad_prop(struct psmouse *psmouse)
> >>> +{
> >>> + struct input_dev *dev = psmouse->dev;
> >>> + struct elantech_data *etd = psmouse->private;
> >>> +
> >>> + if (etd->fw_version & 0x001000)
> >>> + __set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
> >>
> >>
> >> Small question here: if the touchpad is a clickpad, should'nt we also
> >> remove the BTN_RIGHT bit too?
> >
> >
> > We could, but I don't see much value in that, and it would also require
> > if statements in the sync methods to ensure that we don't tree to send
> > a button event for a button we don't advertise.
>
> We don't need this test in the sync method. The test is already
> implemented in input_event. So now, it's just a matter of taste
> regarding upper layers. Peter, any thoughts on this?
sorry, a bit late to the party. as Dmitry said, please don't advertise
buttons that don't exist, just makes it harder to guess on top of what we're
already guessing anyway. So removing the BTN_RIGHT bit is the right thing to
do here.
Cheers,
Peter
>
> Anyway, other than that:
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
> Cheers,
> Benjamin
>
> >
> > Regards,
> >
> > Hans
> --
> 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
* [PATCH v2] elantech: Properly differentiate between clickpads and normal touchpads
From: Hans de Goede @ 2013-12-10 9:31 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Peter Hutterer, Hans de Goede
The current assumption in the elantech driver that hw version 3 touchpads are
never clickpads and hw version 4 touchpads are always clickpads is wrong.
There are several bug reports for this, ie:
https://bugzilla.redhat.com/show_bug.cgi?id=1030802
http://superuser.com/questions/619582/right-elantech-touchpad-button-not-working-in-linux
I've spend a couple of hours wading through various bugzillas,
launchpads and forum posts to create a list of fw-versions and capabilities
for different laptop models to find a good method to differentiate between
clickpads and versions with separate hardware buttons.
Which shows that a device being a clickpad is reliable indicated by bit 12
being set in the fw_version. I've included the gathered list inside the driver,
so that we've this info at hand if we need to revisit this later.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/input/mouse/elantech.c | 45 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 42 insertions(+), 3 deletions(-)
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 597e9b8..ef1cf52 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -486,6 +486,7 @@ static void elantech_input_sync_v4(struct psmouse *psmouse)
unsigned char *packet = psmouse->packet;
input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
+ input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
input_mt_report_pointer_emulation(dev, true);
input_sync(dev);
}
@@ -984,6 +985,44 @@ static int elantech_get_resolution_v4(struct psmouse *psmouse,
}
/*
+ * Advertise INPUT_PROP_BUTTONPAD for clickpads. The testing of bit 12 in
+ * fw_version for this is based on the following fw_version & caps table:
+ *
+ * Laptop-model: fw_version: caps: buttons:
+ * Acer S3 0x461f00 10, 13, 0e clickpad
+ * Acer S7-392 0x581f01 50, 17, 0d clickpad
+ * Acer V5-131 0x461f02 01, 16, 0c clickpad
+ * Acer V5-551 0x461f00 ? clickpad
+ * Asus K53SV 0x450f01 78, 15, 0c 2 hw buttons
+ * Asus G46VW 0x460f02 00, 18, 0c 2 hw buttons
+ * Asus G750JX 0x360f00 00, 16, 0c 2 hw buttons
+ * Asus UX31 0x361f00 20, 15, 0e clickpad
+ * Asus UX32VD 0x361f02 00, 15, 0e clickpad
+ * Avatar AVIU-145A2 0x361f00 ? clickpad
+ * Gigabyte U2442 0x450f01 58, 17, 0c 2 hw buttons
+ * Lenovo L430 0x350f02 b9, 15, 0c 2 hw buttons (*)
+ * Samsung NF210 0x150b00 78, 14, 0a 2 hw buttons
+ * Samsung NP770Z5E 0x575f01 10, 15, 0f clickpad
+ * Samsung NP700Z5B 0x361f06 21, 15, 0f clickpad
+ * Samsung NP900X3E-A02 0x575f03 ? clickpad
+ * Samsung NP-QX410 0x851b00 19, 14, 0c clickpad
+ * Samsung RC512 0x450f00 08, 15, 0c 2 hw buttons
+ * Samsung RF710 0x450f00 ? 2 hw buttons
+ * System76 Pangolin 0x250f01 ? 2 hw buttons
+ * (*) + 3 trackpoint buttons
+ */
+static void elantech_set_buttonpad_prop(struct psmouse *psmouse)
+{
+ struct input_dev *dev = psmouse->dev;
+ struct elantech_data *etd = psmouse->private;
+
+ if (etd->fw_version & 0x001000) {
+ __set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
+ __clear_bit(BTN_RIGHT, dev->keybit);
+ }
+}
+
+/*
* Set the appropriate event bits for the input subsystem
*/
static int elantech_set_input_params(struct psmouse *psmouse)
@@ -1026,6 +1065,8 @@ static int elantech_set_input_params(struct psmouse *psmouse)
__set_bit(INPUT_PROP_SEMI_MT, dev->propbit);
/* fall through */
case 3:
+ if (etd->hw_version == 3)
+ elantech_set_buttonpad_prop(psmouse);
input_set_abs_params(dev, ABS_X, x_min, x_max, 0, 0);
input_set_abs_params(dev, ABS_Y, y_min, y_max, 0, 0);
if (etd->reports_pressure) {
@@ -1047,9 +1088,7 @@ static int elantech_set_input_params(struct psmouse *psmouse)
*/
psmouse_warn(psmouse, "couldn't query resolution data.\n");
}
- /* v4 is clickpad, with only one button. */
- __set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
- __clear_bit(BTN_RIGHT, dev->keybit);
+ elantech_set_buttonpad_prop(psmouse);
__set_bit(BTN_TOOL_QUADTAP, dev->keybit);
/* For X to recognize me as touchpad. */
input_set_abs_params(dev, ABS_X, x_min, x_max, 0, 0);
--
1.8.4.2
^ permalink raw reply related
* RE: [PATCH 1/2] elantech: Properly differentiate between clickpads and normal touchpads
From: duson @ 2013-12-10 9:29 UTC (permalink / raw)
To: 'Hans de Goede'
Cc: 'linux-input', 'Peter Hutterer',
'黃世鵬', 'Dmitry Torokhov',
'Benjamin Tissoires'
In-Reply-To: <52A6D4B0.20206@redhat.com>
Hi Hans:
> -----Original Message-----
> From: linux-input-owner@vger.kernel.org
[mailto:linux-input-owner@vger.kernel.org]
> On Behalf Of Hans de Goede
> Sent: Tuesday, December 10, 2013 4:46 PM
> To: duson; 'Dmitry Torokhov'; 'Benjamin Tissoires'
> Cc: 'linux-input'; 'Peter Hutterer'; '黃世鵬'
> Subject: Re: [PATCH 1/2] elantech: Properly differentiate between
clickpads and
> normal touchpads
>
> Hi Duson,
>
> On 12/10/2013 09:35 AM, duson wrote:
>
> <snip>
>
> >> We should not advertise events that device does not generate. This
> >> should help userspace to decide if emulation is needed or not. Input
> >> core will drop events that are not set up as valid for device, so if we
> >> clear BTN_RIGHT there it should all work.
> >
> > Actually, our touchpad for PS2 protocol implements the left and right
click
> > function, even thought, it is a click-pad. And the flag for left/right
click
> > information is recorded in the first byte of the packet (when doing sync
> > method).
> > Byte0 Bit 0 --> for left click flag
> > Byte0 Bit1 --> for right click flag
> > When user presses the left-bottom area of the click-pad, only the left
click
> > flag will be set to "1". On the other hand, pressing the right-bottom
area
> > of the click-pad, only the right click flag will be set to "1".
> > So, I think this is the cause of the BTN_RIGHT need to be set.
>
> Are you sure? All info we have so far indicates that the right button
needs
> to be emulated in software in the clickpad case, ie a while back this
> commit was added:
>
>
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drive
rs/input/mous
> e/elantech.c?id=e3dde4fba94e0ba5e1fd79ea9e5389eea1f0cfec
>
> To fix the right button not working, this also removes the checking
> for bit 1 / reporting of BTN_RIGHT events, so if the right click area
> would actually send right clicks I would expect the above commit to
> have broken right clicking for tons of users, and we have no bug reports
> about this (my patch fixes the right button being broken in the non
> clickpad case).
>
> Note that the Linux driver uses the touchpad in absolute mode, not in
> relative / ps/2 mouse emulation mode. Could it be that the handling of the
> right button emulation for clickpads is only done in the firmware in
> relative mode ?
>
> Or maybe it is done in firmware for hardware v3 clickpads, but not for
> v4 clickpads ?
>
Sorry, this is my mistake. I had confirmed our firmware engineer again.
The correct answer is "When using click-pad in absolute mode only the left
click function work." So, both the left/right click function is enabled when
using normal smart-pad (non-clickpad) or clickpad in relative mode.
Thanks,
Duson
--
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 1/2] elantech: Properly differentiate between clickpads and normal touchpads
From: Hans de Goede @ 2013-12-10 8:45 UTC (permalink / raw)
To: duson, 'Dmitry Torokhov', 'Benjamin Tissoires'
Cc: 'linux-input', 'Peter Hutterer',
'黃世鵬'
In-Reply-To: <CF3F8F5F02EE401CBA8F5EFBCC62074D@elan.corp>
Hi Duson,
On 12/10/2013 09:35 AM, duson wrote:
<snip>
>> We should not advertise events that device does not generate. This
>> should help userspace to decide if emulation is needed or not. Input
>> core will drop events that are not set up as valid for device, so if we
>> clear BTN_RIGHT there it should all work.
>
> Actually, our touchpad for PS2 protocol implements the left and right click
> function, even thought, it is a click-pad. And the flag for left/right click
> information is recorded in the first byte of the packet (when doing sync
> method).
> Byte0 Bit 0 --> for left click flag
> Byte0 Bit1 --> for right click flag
> When user presses the left-bottom area of the click-pad, only the left click
> flag will be set to "1". On the other hand, pressing the right-bottom area
> of the click-pad, only the right click flag will be set to "1".
> So, I think this is the cause of the BTN_RIGHT need to be set.
Are you sure? All info we have so far indicates that the right button needs
to be emulated in software in the clickpad case, ie a while back this
commit was added:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/input/mouse/elantech.c?id=e3dde4fba94e0ba5e1fd79ea9e5389eea1f0cfec
To fix the right button not working, this also removes the checking
for bit 1 / reporting of BTN_RIGHT events, so if the right click area
would actually send right clicks I would expect the above commit to
have broken right clicking for tons of users, and we have no bug reports
about this (my patch fixes the right button being broken in the non
clickpad case).
Note that the Linux driver uses the touchpad in absolute mode, not in
relative / ps/2 mouse emulation mode. Could it be that the handling of the
right button emulation for clickpads is only done in the firmware in
relative mode ?
Or maybe it is done in firmware for hardware v3 clickpads, but not for
v4 clickpads ?
Thanks & Regards,
Hans
^ permalink raw reply
* Re: [PATCH 1/2] elantech: Properly differentiate between clickpads and normal touchpads
From: Hans de Goede @ 2013-12-10 8:37 UTC (permalink / raw)
To: Dmitry Torokhov, Benjamin Tissoires
Cc: linux-input, Peter Hutterer, Duson Lin
In-Reply-To: <20131210061256.GA12524@core.coreip.homeip.net>
Hi,
On 12/10/2013 07:12 AM, Dmitry Torokhov wrote:
<snip>
>>>> Small question here: if the touchpad is a clickpad, should'nt we also
>>>> remove the BTN_RIGHT bit too?
>>>
>>>
>>> We could, but I don't see much value in that, and it would also require
>>> if statements in the sync methods to ensure that we don't tree to send
>>> a button event for a button we don't advertise.
>>
>> We don't need this test in the sync method. The test is already
>> implemented in input_event. So now, it's just a matter of taste
>> regarding upper layers. Peter, any thoughts on this?
>
> We should not advertise events that device does not generate. This
> should help userspace to decide if emulation is needed or not. Input
> core will drop events that are not set up as valid for device, so if we
> clear BTN_RIGHT there it should all work.
Ok, v2 adding the clearing of the BTN_RIGHT bit is on its way.
Regards,
Hans
^ permalink raw reply
* RE: [PATCH 1/2] elantech: Properly differentiate between clickpads and normal touchpads
From: duson @ 2013-12-10 8:35 UTC (permalink / raw)
To: 'Dmitry Torokhov', 'Benjamin Tissoires'
Cc: 'Hans de Goede', 'linux-input',
'Peter Hutterer', '黃世鵬'
In-Reply-To: <20131210061256.GA12524@core.coreip.homeip.net>
HI Dmitry and Benjamin:
> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: Tuesday, December 10, 2013 2:13 PM
> To: Benjamin Tissoires
> Cc: Hans de Goede; linux-input; Peter Hutterer; Duson Lin
> Subject: Re: [PATCH 1/2] elantech: Properly differentiate between
clickpads and
> normal touchpads
>
> On Mon, Dec 09, 2013 at 02:21:19PM -0500, Benjamin Tissoires wrote:
> > On Mon, Dec 9, 2013 at 2:14 PM, Hans de Goede <hdegoede@redhat.com>
> wrote:
> > > Hi,
> > >
> > >
> > > On 12/09/2013 07:02 PM, Benjamin Tissoires wrote:
> > >>
> > >> Hi Hans,
> > >>
> > >> adding in CC Duson, who seems to be working on the same driver
> > >> currently, and Dmitry, the input maintainer.
> > >>
> > >> On Mon, Dec 9, 2013 at 9:32 AM, Hans de Goede <hdegoede@redhat.com>
> wrote:
> > >>>
> > >>> The current assumption in the elantech driver that hw version 3
touchpads
> > >>> are
> > >>> never clickpads and hw version 4 touchpads are always clickpads is
wrong.
> > >>>
> > >>> There are several bug reports for this, ie:
> > >>> https://bugzilla.redhat.com/show_bug.cgi?id=1030802
> > >>>
> > >>>
>
http://superuser.com/questions/619582/right-elantech-touchpad-button-not-wor
king-
> in-linux
> > >>>
> > >>> I've spend a couple of hours wading through various bugzillas,
> > >>> launchpads and forum posts to create a list of fw-versions and
> > >>> capabilities
> > >>> for different laptop models to find a good method to differentiate
> > >>> between
> > >>> clickpads and versions with separate hardware buttons.
> > >>>
> > >>> Which shows that a device being a clickpad is reliable indicated by
bit
> > >>> 12
> > >>> being set in the fw_version. I've included the gathered list inside
the
> > >>> driver,
> > >>> so that we've this info at hand if we need to revisit this later.
> > >>
> > >>
> > >> Duson, can you confirm this? It's not that I don't trust Hans, but if
> > >> we could have the hardware maker validating this part, this would be
> > >> great.
> > >>
> > >>>
> > >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > >>> ---
> > >>> drivers/input/mouse/elantech.c | 43
> > >>> +++++++++++++++++++++++++++++++++++++++---
> > >>> 1 file changed, 40 insertions(+), 3 deletions(-)
> > >>>
> > >>> diff --git a/drivers/input/mouse/elantech.c
> > >>> b/drivers/input/mouse/elantech.c
> > >>> index 8551dca..f7baa32 100644
> > >>> --- a/drivers/input/mouse/elantech.c
> > >>> +++ b/drivers/input/mouse/elantech.c
> > >>> @@ -486,6 +486,7 @@ static void elantech_input_sync_v4(struct
psmouse
> > >>> *psmouse)
> > >>> unsigned char *packet = psmouse->packet;
> > >>>
> > >>> input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
> > >>> + input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
> > >>> input_mt_report_pointer_emulation(dev, true);
> > >>> input_sync(dev);
> > >>> }
> > >>> @@ -984,6 +985,42 @@ static int elantech_get_resolution_v4(struct
> psmouse
> > >>> *psmouse,
> > >>> }
> > >>>
> > >>> /*
> > >>> + * Advertise INPUT_PROP_BUTTONPAD for clickpads. The testing of bit
12
> > >>> in
> > >>> + * fw_version for this is based on the following fw_version & caps
> > >>> table:
> > >>> + *
> > >>> + * Laptop-model: fw_version: caps: buttons:
> > >>> + * Acer S3 0x461f00 10, 13, 0e clickpad
> > >>> + * Acer S7-392 0x581f01 50, 17, 0d clickpad
> > >>> + * Acer V5-131 0x461f02 01, 16, 0c clickpad
> > >>> + * Acer V5-551 0x461f00 ? clickpad
> > >>> + * Asus K53SV 0x450f01 78, 15, 0c 2 hw
> buttons
> > >>> + * Asus G46VW 0x460f02 00, 18, 0c 2 hw
> buttons
> > >>> + * Asus G750JX 0x360f00 00, 16, 0c 2 hw
> buttons
> > >>> + * Asus UX31 0x361f00 20, 15, 0e clickpad
> > >>> + * Asus UX32VD 0x361f02 00, 15, 0e clickpad
> > >>> + * Avatar AVIU-145A2 0x361f00 ? clickpad
> > >>> + * Gigabyte U2442 0x450f01 58, 17, 0c 2 hw
> buttons
> > >>> + * Lenovo L430 0x350f02 b9, 15, 0c 2 hw
> buttons
> > >>> (*)
> > >>> + * Samsung NF210 0x150b00 78, 14, 0a 2 hw
> buttons
> > >>> + * Samsung NP770Z5E 0x575f01 10, 15, 0f clickpad
> > >>> + * Samsung NP700Z5B 0x361f06 21, 15, 0f clickpad
> > >>> + * Samsung NP900X3E-A02 0x575f03 ?
> clickpad
> > >>> + * Samsung NP-QX410 0x851b00 19, 14, 0c clickpad
> > >>> + * Samsung RC512 0x450f00 08, 15, 0c 2 hw
> buttons
> > >>> + * Samsung RF710 0x450f00 ? 2 hw
> buttons
> > >>> + * System76 Pangolin 0x250f01 ? 2 hw
> buttons
> > >>> + * (*) + 3 trackpoint buttons
> > >>> + */
> > >>> +static void elantech_set_buttonpad_prop(struct psmouse *psmouse)
> > >>> +{
> > >>> + struct input_dev *dev = psmouse->dev;
> > >>> + struct elantech_data *etd = psmouse->private;
> > >>> +
> > >>> + if (etd->fw_version & 0x001000)
> > >>> + __set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
> > >>
> > >>
> > >> Small question here: if the touchpad is a clickpad, should'nt we also
> > >> remove the BTN_RIGHT bit too?
> > >
> > >
> > > We could, but I don't see much value in that, and it would also
require
> > > if statements in the sync methods to ensure that we don't tree to send
> > > a button event for a button we don't advertise.
> >
> > We don't need this test in the sync method. The test is already
> > implemented in input_event. So now, it's just a matter of taste
> > regarding upper layers. Peter, any thoughts on this?
>
> We should not advertise events that device does not generate. This
> should help userspace to decide if emulation is needed or not. Input
> core will drop events that are not set up as valid for device, so if we
> clear BTN_RIGHT there it should all work.
Actually, our touchpad for PS2 protocol implements the left and right click
function, even thought, it is a click-pad. And the flag for left/right click
information is recorded in the first byte of the packet (when doing sync
method).
Byte0 Bit 0 --> for left click flag
Byte0 Bit1 --> for right click flag
When user presses the left-bottom area of the click-pad, only the left click
flag will be set to "1". On the other hand, pressing the right-bottom area
of the click-pad, only the right click flag will be set to "1".
So, I think this is the cause of the BTN_RIGHT need to be set.
Thanks,
Duson
^ permalink raw reply
* Re: [PATCH V2] input synaptics-rmi4: Reorder declarations in rmi_bus.c
From: Dmitry Torokhov @ 2013-12-10 6:46 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Christopher Heiny, Linux Input, Andrew Duggan, Vincent Huang,
Vivian Ly, Daniel Rosenberg, Jean Delvare, Joerie de Gram,
Linus Walleij
In-Reply-To: <20131210063914.GD12524@core.coreip.homeip.net>
On Mon, Dec 09, 2013 at 10:39:14PM -0800, Dmitry Torokhov wrote:
> On Mon, Dec 09, 2013 at 03:14:26PM -0500, Benjamin Tissoires wrote:
> > Hi Chris,
> >
> > On 05/12/13 19:29, Christopher Heiny wrote:
> > > This patch implements changes to the synaptics-rmi4 branch of
> > > Dmitry's input tree. The base for the patch is commit
> > > 8ca01dc61a42b6f7bcba052a8c084000f7057a34.
> > >
> > > This patch primarily reorders the various declarations in rmi_bus.c in order to
> > > group related elements together, along with some typo fixes. The code is still
> > > horribly broken, but this change should make the following fixes easier to
> > > review.
> > >
> > > Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > Cc: Jean Delvare <khali@linux-fr.org>
> > > Cc: Linus Walleij <linus.walleij@stericsson.com>
> > > Cc: Joerie de Gram <j.de.gram@gmail.com>
> > > Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > >
> > > ---
> >
> > FWIW, I made a review of the patch.
> > The patches does not only reorder the functions, but also fix some few
> > things I will detail later (plus fixes of whitespace/comments issues).
> > It also changes the exported functions as GPL.
> >
> > Dmitry, given the current state of the driver (which does not work at
> > all if I understood correctly), maybe you can pick this one in its
> > current state.
>
> Applied, thank you.
Well, I had to pull up rmi_debugfs_root declaration to avoid:
CC [M] drivers/input/rmi4/rmi_driver.o
drivers/input/rmi4/rmi_bus.c: In function ‘rmi_physical_setup_debugfs’:
drivers/input/rmi4/rmi_bus.c:51:10: error: ‘rmi_debugfs_root’ undeclared
(first use in this function)
rmi_debugfs_root);
Guys, a bit better compile coverage would be appreciated.
Thanks.
--
Dmitry
--
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] input synaptics-rmi4: Reorder declarations in rmi_bus.c
From: Dmitry Torokhov @ 2013-12-10 6:39 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Christopher Heiny, Linux Input, Andrew Duggan, Vincent Huang,
Vivian Ly, Daniel Rosenberg, Jean Delvare, Joerie de Gram,
Linus Walleij
In-Reply-To: <52A624A2.5000405@redhat.com>
On Mon, Dec 09, 2013 at 03:14:26PM -0500, Benjamin Tissoires wrote:
> Hi Chris,
>
> On 05/12/13 19:29, Christopher Heiny wrote:
> > This patch implements changes to the synaptics-rmi4 branch of
> > Dmitry's input tree. The base for the patch is commit
> > 8ca01dc61a42b6f7bcba052a8c084000f7057a34.
> >
> > This patch primarily reorders the various declarations in rmi_bus.c in order to
> > group related elements together, along with some typo fixes. The code is still
> > horribly broken, but this change should make the following fixes easier to
> > review.
> >
> > Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: Jean Delvare <khali@linux-fr.org>
> > Cc: Linus Walleij <linus.walleij@stericsson.com>
> > Cc: Joerie de Gram <j.de.gram@gmail.com>
> > Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> >
> > ---
>
> FWIW, I made a review of the patch.
> The patches does not only reorder the functions, but also fix some few
> things I will detail later (plus fixes of whitespace/comments issues).
> It also changes the exported functions as GPL.
>
> Dmitry, given the current state of the driver (which does not work at
> all if I understood correctly), maybe you can pick this one in its
> current state.
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input synaptics-rmi4: elliminate multiple sensor support from rmi_f11.c
From: Dmitry Torokhov @ 2013-12-10 6:34 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Andrew Duggan, Linux Input, Christopher Heiny, Vincent Huang,
Vivian Ly, Daniel Rosenberg, Jean Delvare, Joerie de Gram,
Linus Walleij
In-Reply-To: <52A6301B.1090507@redhat.com>
On Mon, Dec 09, 2013 at 04:03:23PM -0500, Benjamin Tissoires wrote:
> On 05/12/13 20:59, Andrew Duggan wrote:
> > This patch implements changes to the synaptics-rmi4 branch of
> > Dmitry's input tree. The base for the patch is commit
> > 8ca01dc61a42b6f7bcba052a8c084000f7057a34.
>
> FWIW, these three lines above could (should) go after the very first
> "---" before the stats. This way, you will tell the list which base is
> the patch based, but this will not go into Linus' tree (which would be
> irrelevant).
>
> >
> > This patch elliminates support for multiple sensors in rmi_f11. This feature
> > has been removed from the RMI4 spec and no devices have every used multiple
> > F11 sensors on a single device.
> >
> > Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
> > ---
>
> I did not saw anything but what the commit message claims. So:
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] drivers/input/misc/adxl34x: Fix bug in definition of ADXL346_2D_ORIENT
From: Dmitry Torokhov @ 2013-12-10 6:14 UTC (permalink / raw)
To: michael.hennerich; +Cc: linux-input
In-Reply-To: <1386588910-4604-1-git-send-email-michael.hennerich@analog.com>
On Mon, Dec 09, 2013 at 12:35:10PM +0100, michael.hennerich@analog.com wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
>
> Coverity report pointet out by Dmitry
>
> Reported-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Applied, thank you.
> ---
> drivers/input/misc/adxl34x.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/misc/adxl34x.c b/drivers/input/misc/adxl34x.c
> index 0735de3..1cb1da2 100644
> --- a/drivers/input/misc/adxl34x.c
> +++ b/drivers/input/misc/adxl34x.c
> @@ -158,7 +158,7 @@
>
> /* ORIENT ADXL346 only */
> #define ADXL346_2D_VALID (1 << 6)
> -#define ADXL346_2D_ORIENT(x) (((x) & 0x3) >> 4)
> +#define ADXL346_2D_ORIENT(x) (((x) & 0x30) >> 4)
> #define ADXL346_3D_VALID (1 << 3)
> #define ADXL346_3D_ORIENT(x) ((x) & 0x7)
> #define ADXL346_2D_PORTRAIT_POS 0 /* +X */
> --
> 1.7.9.5
>
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/2] elantech: Properly differentiate between clickpads and normal touchpads
From: Dmitry Torokhov @ 2013-12-10 6:12 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Hans de Goede, linux-input, Peter Hutterer, Duson Lin
In-Reply-To: <CAN+gG=FgR9cBwJ_GFrU3RAmwma0XWzt8Wdi-HM2zEOOmdPNS+A@mail.gmail.com>
On Mon, Dec 09, 2013 at 02:21:19PM -0500, Benjamin Tissoires wrote:
> On Mon, Dec 9, 2013 at 2:14 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> > Hi,
> >
> >
> > On 12/09/2013 07:02 PM, Benjamin Tissoires wrote:
> >>
> >> Hi Hans,
> >>
> >> adding in CC Duson, who seems to be working on the same driver
> >> currently, and Dmitry, the input maintainer.
> >>
> >> On Mon, Dec 9, 2013 at 9:32 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> >>>
> >>> The current assumption in the elantech driver that hw version 3 touchpads
> >>> are
> >>> never clickpads and hw version 4 touchpads are always clickpads is wrong.
> >>>
> >>> There are several bug reports for this, ie:
> >>> https://bugzilla.redhat.com/show_bug.cgi?id=1030802
> >>>
> >>> http://superuser.com/questions/619582/right-elantech-touchpad-button-not-working-in-linux
> >>>
> >>> I've spend a couple of hours wading through various bugzillas,
> >>> launchpads and forum posts to create a list of fw-versions and
> >>> capabilities
> >>> for different laptop models to find a good method to differentiate
> >>> between
> >>> clickpads and versions with separate hardware buttons.
> >>>
> >>> Which shows that a device being a clickpad is reliable indicated by bit
> >>> 12
> >>> being set in the fw_version. I've included the gathered list inside the
> >>> driver,
> >>> so that we've this info at hand if we need to revisit this later.
> >>
> >>
> >> Duson, can you confirm this? It's not that I don't trust Hans, but if
> >> we could have the hardware maker validating this part, this would be
> >> great.
> >>
> >>>
> >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>> ---
> >>> drivers/input/mouse/elantech.c | 43
> >>> +++++++++++++++++++++++++++++++++++++++---
> >>> 1 file changed, 40 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/input/mouse/elantech.c
> >>> b/drivers/input/mouse/elantech.c
> >>> index 8551dca..f7baa32 100644
> >>> --- a/drivers/input/mouse/elantech.c
> >>> +++ b/drivers/input/mouse/elantech.c
> >>> @@ -486,6 +486,7 @@ static void elantech_input_sync_v4(struct psmouse
> >>> *psmouse)
> >>> unsigned char *packet = psmouse->packet;
> >>>
> >>> input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
> >>> + input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
> >>> input_mt_report_pointer_emulation(dev, true);
> >>> input_sync(dev);
> >>> }
> >>> @@ -984,6 +985,42 @@ static int elantech_get_resolution_v4(struct psmouse
> >>> *psmouse,
> >>> }
> >>>
> >>> /*
> >>> + * Advertise INPUT_PROP_BUTTONPAD for clickpads. The testing of bit 12
> >>> in
> >>> + * fw_version for this is based on the following fw_version & caps
> >>> table:
> >>> + *
> >>> + * Laptop-model: fw_version: caps: buttons:
> >>> + * Acer S3 0x461f00 10, 13, 0e clickpad
> >>> + * Acer S7-392 0x581f01 50, 17, 0d clickpad
> >>> + * Acer V5-131 0x461f02 01, 16, 0c clickpad
> >>> + * Acer V5-551 0x461f00 ? clickpad
> >>> + * Asus K53SV 0x450f01 78, 15, 0c 2 hw buttons
> >>> + * Asus G46VW 0x460f02 00, 18, 0c 2 hw buttons
> >>> + * Asus G750JX 0x360f00 00, 16, 0c 2 hw buttons
> >>> + * Asus UX31 0x361f00 20, 15, 0e clickpad
> >>> + * Asus UX32VD 0x361f02 00, 15, 0e clickpad
> >>> + * Avatar AVIU-145A2 0x361f00 ? clickpad
> >>> + * Gigabyte U2442 0x450f01 58, 17, 0c 2 hw buttons
> >>> + * Lenovo L430 0x350f02 b9, 15, 0c 2 hw buttons
> >>> (*)
> >>> + * Samsung NF210 0x150b00 78, 14, 0a 2 hw buttons
> >>> + * Samsung NP770Z5E 0x575f01 10, 15, 0f clickpad
> >>> + * Samsung NP700Z5B 0x361f06 21, 15, 0f clickpad
> >>> + * Samsung NP900X3E-A02 0x575f03 ? clickpad
> >>> + * Samsung NP-QX410 0x851b00 19, 14, 0c clickpad
> >>> + * Samsung RC512 0x450f00 08, 15, 0c 2 hw buttons
> >>> + * Samsung RF710 0x450f00 ? 2 hw buttons
> >>> + * System76 Pangolin 0x250f01 ? 2 hw buttons
> >>> + * (*) + 3 trackpoint buttons
> >>> + */
> >>> +static void elantech_set_buttonpad_prop(struct psmouse *psmouse)
> >>> +{
> >>> + struct input_dev *dev = psmouse->dev;
> >>> + struct elantech_data *etd = psmouse->private;
> >>> +
> >>> + if (etd->fw_version & 0x001000)
> >>> + __set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
> >>
> >>
> >> Small question here: if the touchpad is a clickpad, should'nt we also
> >> remove the BTN_RIGHT bit too?
> >
> >
> > We could, but I don't see much value in that, and it would also require
> > if statements in the sync methods to ensure that we don't tree to send
> > a button event for a button we don't advertise.
>
> We don't need this test in the sync method. The test is already
> implemented in input_event. So now, it's just a matter of taste
> regarding upper layers. Peter, any thoughts on this?
We should not advertise events that device does not generate. This
should help userspace to decide if emulation is needed or not. Input
core will drop events that are not set up as valid for device, so if we
clear BTN_RIGHT there it should all work.
Thanks.
--
Dmitry
^ permalink raw reply
* RE: [PATCH 1/2] elantech: Properly differentiate between clickpads and normal touchpads
From: duson @ 2013-12-10 5:15 UTC (permalink / raw)
To: 'Benjamin Tissoires'
Cc: 'linux-input', 'Peter Hutterer',
'Dmitry Torokhov', 'Hans de Goede'
In-Reply-To: <CAN+gG=Eh72eBS67db_KNsUzXRNsOUXB8YkzuuHoJca22xRAkdQ@mail.gmail.com>
Hi Benjamin:
I am sorry for the late reply.
Yes. I can confirm this.
Thanks,
Duson
-----Original Message-----
From: Benjamin Tissoires [mailto:benjamin.tissoires@gmail.com]
Sent: Tuesday, December 10, 2013 2:03 AM
To: Hans de Goede
Cc: linux-input; Peter Hutterer; Duson Lin; Dmitry Torokhov
Subject: Re: [PATCH 1/2] elantech: Properly differentiate between clickpads
and normal touchpads
Hi Hans,
adding in CC Duson, who seems to be working on the same driver
currently, and Dmitry, the input maintainer.
On Mon, Dec 9, 2013 at 9:32 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> The current assumption in the elantech driver that hw version 3 touchpads
are
> never clickpads and hw version 4 touchpads are always clickpads is wrong.
>
> There are several bug reports for this, ie:
> https://bugzilla.redhat.com/show_bug.cgi?id=1030802
>
http://superuser.com/questions/619582/right-elantech-touchpad-button-not-wor
king-in-linux
>
> I've spend a couple of hours wading through various bugzillas,
> launchpads and forum posts to create a list of fw-versions and
capabilities
> for different laptop models to find a good method to differentiate between
> clickpads and versions with separate hardware buttons.
>
> Which shows that a device being a clickpad is reliable indicated by bit 12
> being set in the fw_version. I've included the gathered list inside the
driver,
> so that we've this info at hand if we need to revisit this later.
Duson, can you confirm this? It's not that I don't trust Hans, but if
we could have the hardware maker validating this part, this would be
great.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/input/mouse/elantech.c | 43
+++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/mouse/elantech.c
b/drivers/input/mouse/elantech.c
> index 8551dca..f7baa32 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -486,6 +486,7 @@ static void elantech_input_sync_v4(struct psmouse
*psmouse)
> unsigned char *packet = psmouse->packet;
>
> input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
> + input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
> input_mt_report_pointer_emulation(dev, true);
> input_sync(dev);
> }
> @@ -984,6 +985,42 @@ static int elantech_get_resolution_v4(struct psmouse
*psmouse,
> }
>
> /*
> + * Advertise INPUT_PROP_BUTTONPAD for clickpads. The testing of bit 12 in
> + * fw_version for this is based on the following fw_version & caps table:
> + *
> + * Laptop-model: fw_version: caps: buttons:
> + * Acer S3 0x461f00 10, 13, 0e clickpad
> + * Acer S7-392 0x581f01 50, 17, 0d clickpad
> + * Acer V5-131 0x461f02 01, 16, 0c clickpad
> + * Acer V5-551 0x461f00 ? clickpad
> + * Asus K53SV 0x450f01 78, 15, 0c 2 hw buttons
> + * Asus G46VW 0x460f02 00, 18, 0c 2 hw buttons
> + * Asus G750JX 0x360f00 00, 16, 0c 2 hw buttons
> + * Asus UX31 0x361f00 20, 15, 0e clickpad
> + * Asus UX32VD 0x361f02 00, 15, 0e clickpad
> + * Avatar AVIU-145A2 0x361f00 ? clickpad
> + * Gigabyte U2442 0x450f01 58, 17, 0c 2 hw buttons
> + * Lenovo L430 0x350f02 b9, 15, 0c 2 hw buttons
(*)
> + * Samsung NF210 0x150b00 78, 14, 0a 2 hw buttons
> + * Samsung NP770Z5E 0x575f01 10, 15, 0f clickpad
> + * Samsung NP700Z5B 0x361f06 21, 15, 0f clickpad
> + * Samsung NP900X3E-A02 0x575f03 ? clickpad
> + * Samsung NP-QX410 0x851b00 19, 14, 0c clickpad
> + * Samsung RC512 0x450f00 08, 15, 0c 2 hw buttons
> + * Samsung RF710 0x450f00 ? 2 hw buttons
> + * System76 Pangolin 0x250f01 ? 2 hw buttons
> + * (*) + 3 trackpoint buttons
> + */
> +static void elantech_set_buttonpad_prop(struct psmouse *psmouse)
> +{
> + struct input_dev *dev = psmouse->dev;
> + struct elantech_data *etd = psmouse->private;
> +
> + if (etd->fw_version & 0x001000)
> + __set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
Small question here: if the touchpad is a clickpad, should'nt we also
remove the BTN_RIGHT bit too?
Cheers,
Benjamin
> +}
> +
> +/*
> * Set the appropriate event bits for the input subsystem
> */
> static int elantech_set_input_params(struct psmouse *psmouse)
> @@ -1026,6 +1063,8 @@ static int elantech_set_input_params(struct psmouse
*psmouse)
> __set_bit(INPUT_PROP_SEMI_MT, dev->propbit);
> /* fall through */
> case 3:
> + if (etd->hw_version == 3)
> + elantech_set_buttonpad_prop(psmouse);
> input_set_abs_params(dev, ABS_X, x_min, x_max, 0, 0);
> input_set_abs_params(dev, ABS_Y, y_min, y_max, 0, 0);
> if (etd->reports_pressure) {
> @@ -1047,9 +1086,7 @@ static int elantech_set_input_params(struct psmouse
*psmouse)
> */
> psmouse_warn(psmouse, "couldn't query resolution
data.\n");
> }
> - /* v4 is clickpad, with only one button. */
> - __set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
> - __clear_bit(BTN_RIGHT, dev->keybit);
> + elantech_set_buttonpad_prop(psmouse);
> __set_bit(BTN_TOOL_QUADTAP, dev->keybit);
> /* For X to recognize me as touchpad. */
> input_set_abs_params(dev, ABS_X, x_min, x_max, 0, 0);
> --
> 1.8.4.2
>
> --
> 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: Xpad Driver Replacement
From: Zachary Lund @ 2013-12-10 3:27 UTC (permalink / raw)
To: linux-input
In-Reply-To: <52A40F19.3020109@computerquip.com>
On 12/08/13 00:18, Zachary Lund wrote:
> Secondly, the Xbox 360 controllers claim to be HID compliant... this
> is not an HID driver. That's because the report descriptor is missing
> and I, unfortunately, do not know what to do about that. Some drivers
> like XBCD and the driver found at tattiebogle.net both provide their
> own report descriptor and work from there. While I'd like to do the
> same eventually, it will take me longer than a week to do that as I'd
> have to educate myself on HID and figure out what to do about the
> missing descriptors.
I've run into an instant road block. The controller claims
bInterfaceClass to be 0xFF (Vendor-specific) so usbhid won't probe it. I
didn't think it would be so difficult to work around that but I've spent
the better part of today trying to figure out just that. usbhid is a
usb_driver that has only one requirement to be probed: bInterfaceClass
be 0x03. Unfortunately, the device fails this requirement.
Does anyone know of a way around this mechanism? Or perhaps I should
take a different approach?
^ permalink raw reply
* [MailServer Notification]Attachment Blocking Notification
From: ExchangeNTTrendMicro @ 2013-12-10 2:50 UTC (permalink / raw)
To: dh.herrmann; +Cc: linux-input, ospite
The des_bt.bin has been blocked,
and Replace has been taken on 12/9/2013 6:50:47 PM.
Message details:
Server:WACOM-NT10
Sender: simon@mungewell.org;
Recipient:dh.herrmann@gmail.com;linux-input@vger.kernel.org;ospite@studenti.unina.it;
Subject:Re: Sony DualShock4 - basic functions work, but looking to improve support
Attachment name:des_bt.bin
^ permalink raw reply
* Re: Sony DualShock4 - basic functions work, but looking to improve support
From: simon @ 2013-12-10 2:50 UTC (permalink / raw)
To: David Herrmann; +Cc: Linux Input, Antonio Ospite
In-Reply-To: <CANq1E4Q=4DEDFibVtifMVE-2wq7Bt0KOPRRE5nFy9Hu-tgy_Cg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 569 bytes --]
> The report descriptor actually describes a lot more report-IDs than
> just "0x01" which you describe in hidraw.txt. Are you sure the
> report-descriptors are the same for USB and BT?
The report descriptor when connected via BT is different, or at least what
I found at '/sys/bus/hid/devices/0005\:054C\:05C4.0006/report_descriptor'.
Unfortunately I wasn't able to decode it.
--
simon@womble:~/dualshock4$ ~/hidrd-0.2.0/src/hidrd-convert des_bt.bin -o code
Failed to read input item at offset 356:
invalid item encountered
--
Thanks for the other suggestions.
Simon
[-- Attachment #2: des_bt.bin --]
[-- Type: application/octet-stream, Size: 357 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] Documentation: dt: Document TSC2005 DT binding
From: Tony Lindgren @ 2013-12-10 0:06 UTC (permalink / raw)
To: Dmitry Torokhov, Dmitry Torokhov, linux-input, Rob Herring,
Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
Rob Landley, Grant Likely, devicetree, linux-omap, linux-kernel
In-Reply-To: <20131209182409.GA7626@earth.universe>
* Sebastian Reichel <sre@debian.org> [131209 10:25]:
> On Mon, Dec 09, 2013 at 09:46:38AM -0800, Tony Lindgren wrote:
> > > +Optional properties:
> > > + - ti,fuzz-x : integer, X noise value of the touchscreen
> > > + (defaults to 4)
> > > + - ti,fuzz-y : integer, Y noise value of the touchscreen
> > > + (defaults to 8)
> > > + - ti,fuzz-pressure : integer, pressure noise value of the touchscreen
> > > + (defaults to 2)
> > > + - ti,max-x : integer, maximum reported x value
> > > + (defaults to 4096)
> > > + - ti,max-y : integer, maximum reported y value
> > > + (defaults to 4096)
> > > + - ti,max-pressure : integer, maximum reported pressure
> > > + (defaults to 4096)
> > > + - ti,x-plate-resistance : integer, resistance of the touchscreen's X plates
> > > + in ohm (defaults to 280)
> > > + - ti,esd-recovery-timeout-ms : integer, if the touchscreen does not respond after
> > > + the configured time (in milli seconds), the driver
> > > + will reset it. This is disabled by default.
> >
> > Instead of adding these optional ti,* properties you can set them in the
> > driver directly in the of_match table based on the compatible flag. Then
> > you can pass compatible flag like ti,tsc2005-nokia-n900, or the name of
> > the LCD panel. Most likely these depend on the LCD panel selected.
>
> I could certainly do this, but it would move the board specific data
> from the boardcode into the driver. That looks contra-productive to
> me. Is there a good reason to do it this way?
You can leave out the custom properties that way for something that probably
should be grouped by the touchpanel type connected as the values are the
same.
So for example just a few compatible flags like ti,tsc2005-panel-abc and
ti,tsc2005-panel-xyz we could potentially cover all the configurations
we're aware of without any need for custom properties. And this is way
easier to support in the long run assuming we don't end up with tons of
compatible flags. Of course if we end up with a new compatible flag for
each configuration, then it makes sense to set up the custom properties,
but I doubt that's the case here.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH] input synaptics-rmi4: elliminate multiple sensor support from rmi_f11.c
From: Benjamin Tissoires @ 2013-12-09 21:03 UTC (permalink / raw)
To: Andrew Duggan, Dmitry Torokhov
Cc: Linux Input, Christopher Heiny, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij
In-Reply-To: <1386295167-866-1-git-send-email-aduggan@synaptics.com>
On 05/12/13 20:59, Andrew Duggan wrote:
> This patch implements changes to the synaptics-rmi4 branch of
> Dmitry's input tree. The base for the patch is commit
> 8ca01dc61a42b6f7bcba052a8c084000f7057a34.
FWIW, these three lines above could (should) go after the very first
"---" before the stats. This way, you will tell the list which base is
the patch based, but this will not go into Linus' tree (which would be
irrelevant).
>
> This patch elliminates support for multiple sensors in rmi_f11. This feature
> has been removed from the RMI4 spec and no devices have every used multiple
> F11 sensors on a single device.
>
> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
> ---
I did not saw anything but what the commit message claims. So:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH V2] input synaptics-rmi4: Reorder declarations in rmi_bus.c
From: Benjamin Tissoires @ 2013-12-09 20:14 UTC (permalink / raw)
To: Christopher Heiny, Dmitry Torokhov
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij
In-Reply-To: <1386289756-12429-1-git-send-email-cheiny@synaptics.com>
Hi Chris,
On 05/12/13 19:29, Christopher Heiny wrote:
> This patch implements changes to the synaptics-rmi4 branch of
> Dmitry's input tree. The base for the patch is commit
> 8ca01dc61a42b6f7bcba052a8c084000f7057a34.
>
> This patch primarily reorders the various declarations in rmi_bus.c in order to
> group related elements together, along with some typo fixes. The code is still
> horribly broken, but this change should make the following fixes easier to
> review.
>
> Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Jean Delvare <khali@linux-fr.org>
> Cc: Linus Walleij <linus.walleij@stericsson.com>
> Cc: Joerie de Gram <j.de.gram@gmail.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
> ---
FWIW, I made a review of the patch.
The patches does not only reorder the functions, but also fix some few
things I will detail later (plus fixes of whitespace/comments issues).
It also changes the exported functions as GPL.
Dmitry, given the current state of the driver (which does not work at
all if I understood correctly), maybe you can pick this one in its
current state.
> drivers/input/rmi4/rmi_bus.c | 189 +++++++++++++++++++++----------------------
> 1 file changed, 90 insertions(+), 99 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
> index 88f60ca..d9c450b 100644
> --- a/drivers/input/rmi4/rmi_bus.c
> +++ b/drivers/input/rmi4/rmi_bus.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2011, 2012 Synaptics Incorporated
> + * Copyright (c) 2011-2013 Synaptics Incorporated
> * Copyright (c) 2011 Unixphere
> *
> * This program is free software; you can redistribute it and/or modify it
> @@ -19,77 +19,20 @@
> #include "rmi_bus.h"
> #include "rmi_driver.h"
>
> -static int rmi_function_match(struct device *dev, struct device_driver *drv)
> -{
> - struct rmi_function_handler *handler = to_rmi_function_handler(drv);
> - struct rmi_function *fn = to_rmi_function(dev);
> -
> - return fn->fd.function_number == handler->func;
> -}
> -
> -static int rmi_bus_match(struct device *dev, struct device_driver *drv)
> -{
> - bool physical = rmi_is_physical_device(dev);
> -
> - /* First see if types are not compatible */
> - if (physical != rmi_is_physical_driver(drv))
> - return 0;
> -
> - return physical || rmi_function_match(dev, drv);
> -}
> -
> -struct bus_type rmi_bus_type = {
> - .match = rmi_bus_match,
> - .name = "rmi",
> -};
> -
> -#ifdef CONFIG_RMI4_DEBUG
> -
> -static struct dentry *rmi_debugfs_root;
> -
> -static void rmi_bus_setup_debugfs(void)
> -{
> - rmi_debugfs_root = debugfs_create_dir(rmi_bus_type.name, NULL);
> - if (!rmi_debugfs_root)
> - pr_err("%s: Failed to create debugfs root\n",
> - __func__);
> -}
> -
> -static void rmi_bus_teardown_debugfs(void)
> -{
> - if (rmi_debugfs_root)
> - debugfs_remove_recursive(rmi_debugfs_root);
> -}
> -
> -#else
> -
> -static void rmi_bus_setup_debugfs(void)
> -{
> -}
> -
> -static void rmi_bus_teardown_debugfs(void)
> -{
> -}
> -
> -#endif
> -
> -
> /*
> * RMI Physical devices
> *
> * Physical RMI device consists of several functions serving particular
> - * purpose. For example F11 is a 2D touch sensor while F10 is a generic
> + * purpose. For example F11 is a 2D touch sensor while F01 is a generic
> * function present in every RMI device.
> */
>
> static void rmi_release_device(struct device *dev)
> {
> struct rmi_device *rmi_dev = to_rmi_device(dev);
> -
> kfree(rmi_dev);
> }
>
> -/* Device type for physical RMI devices */
> struct device_type rmi_device_type = {
> .name = "rmi_sensor",
> .release = rmi_release_device,
> @@ -118,7 +61,7 @@ static void rmi_physical_teardown_debugfs(struct rmi_device *rmi_dev)
>
> #else
>
> -static void rmi_physocal_setup_debugfs(struct rmi_device *rmi_dev)
> +static void rmi_physical_setup_debugfs(struct rmi_device *rmi_dev)
This typo makes me wonder how nobody saw this before. This will not
compile without CONFIG_RMI4_DEBUG... :(
> {
> }
>
> @@ -128,7 +71,6 @@ static void rmi_physical_teardown_debugfs(struct rmi_device *rmi_dev)
>
> #endif
>
> -
> /**
> * rmi_register_physical_device - register a physical device connection on the RMI
> * bus. Physical drivers provide communication from the devices on the bus to
> @@ -174,7 +116,7 @@ int rmi_register_physical_device(struct rmi_phys_device *phys)
>
> return 0;
> }
> -EXPORT_SYMBOL(rmi_register_physical_device);
> +EXPORT_SYMBOL_GPL(rmi_register_physical_device);
>
> /**
> * rmi_unregister_physical_device - unregister a physical device connection
> @@ -191,18 +133,14 @@ void rmi_unregister_physical_device(struct rmi_phys_device *phys)
> EXPORT_SYMBOL(rmi_unregister_physical_device);
>
>
> -/*
> - * RMI Function devices and their handlers
> - */
> +/* Function specific stuff */
>
> static void rmi_release_function(struct device *dev)
> {
> struct rmi_function *fn = to_rmi_function(dev);
> -
> kfree(fn);
> }
>
> -/* Device type for RMI Function devices */
> struct device_type rmi_function_type = {
> .name = "rmi_function",
> .release = rmi_release_function,
> @@ -244,35 +182,12 @@ static void rmi_function_teardown_debugfs(struct rmi_function *fn)
>
> #endif
>
> -int rmi_register_function(struct rmi_function *fn)
> +static int rmi_function_match(struct device *dev, struct device_driver *drv)
> {
> - struct rmi_device *rmi_dev = fn->rmi_dev;
> - int error;
> -
> - dev_set_name(&fn->dev, "%s.fn%02x",
> - dev_name(&rmi_dev->dev), fn->fd.function_number);
> -
> - fn->dev.parent = &rmi_dev->dev;
> - fn->dev.type = &rmi_function_type;
> - fn->dev.bus = &rmi_bus_type;
> -
> - error = device_register(&fn->dev);
> - if (error) {
> - dev_err(&rmi_dev->dev,
> - "Failed device_register function device %s\n",
> - dev_name(&fn->dev));
> - }
> -
> - dev_dbg(&rmi_dev->dev, "Registered F%02X.\n", fn->fd.function_number);
> -
> - rmi_function_setup_debugfs(fn);
> - return 0;
> -}
> + struct rmi_function_handler *handler = to_rmi_function_handler(drv);
> + struct rmi_function *fn = to_rmi_function(dev);
>
> -void rmi_unregister_function(struct rmi_function *fn)
> -{
> - rmi_function_teardown_debugfs(fn);
> - device_unregister(&fn->dev);
> + return fn->fd.function_number == handler->func;
> }
>
> static int rmi_function_probe(struct device *dev)
> @@ -302,6 +217,38 @@ static int rmi_function_remove(struct device *dev)
> return 0;
> }
>
> +int rmi_register_function(struct rmi_function *fn)
> +{
> + struct rmi_device *rmi_dev = fn->rmi_dev;
> + int error;
> +
> + dev_set_name(&fn->dev, "%s.fn%02x",
> + dev_name(&rmi_dev->dev), fn->fd.function_number);
> +
> + fn->dev.parent = &rmi_dev->dev;
> + fn->dev.type = &rmi_function_type;
> + fn->dev.bus = &rmi_bus_type;
> +
> + error = device_register(&fn->dev);
> + if (error) {
> + dev_err(&rmi_dev->dev,
> + "Failed device_register function device %s\n",
> + dev_name(&fn->dev));
> + return error;
this return statement was not in the previous version of rmi_bus.c, but
it looks obviously correct.
So to sum up:
Reviewed-by: benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
> + }
> +
> + dev_dbg(&rmi_dev->dev, "Registered F%02X.\n", fn->fd.function_number);
> +
> + rmi_function_setup_debugfs(fn);
> + return 0;
> +}
> +
> +void rmi_unregister_function(struct rmi_function *fn)
> +{
> + rmi_function_teardown_debugfs(fn);
> + device_unregister(&fn->dev);
> +}
> +
> /**
> * rmi_register_function_handler - register a handler for an RMI function
> * @handler: RMI handler that should be registered.
> @@ -334,7 +281,7 @@ int __rmi_register_function_handler(struct rmi_function_handler *handler,
>
> return 0;
> }
> -EXPORT_SYMBOL(__rmi_register_function_handler);
> +EXPORT_SYMBOL_GPL(__rmi_register_function_handler);
>
> /**
> * rmi_unregister_function_handler - unregister given RMI function handler
> @@ -347,11 +294,55 @@ void rmi_unregister_function_handler(struct rmi_function_handler *handler)
> {
> driver_unregister(&handler->driver);
> }
> -EXPORT_SYMBOL(rmi_unregister_function_handler);
> +EXPORT_SYMBOL_GPL(rmi_unregister_function_handler);
>
> -/*
> - * Bus registration and tear-down
> - */
> +/* Bus specific stuff */
> +
> +static int rmi_bus_match(struct device *dev, struct device_driver *drv)
> +{
> + bool physical = rmi_is_physical_device(dev);
> +
> + /* First see if types are not compatible */
> + if (physical != rmi_is_physical_driver(drv))
> + return 0;
> +
> + return physical || rmi_function_match(dev, drv);
> +}
> +
> +struct bus_type rmi_bus_type = {
> + .match = rmi_bus_match,
> + .name = "rmi",
> +};
> +
> +#ifdef CONFIG_RMI4_DEBUG
> +
> +static struct dentry *rmi_debugfs_root;
> +
> +static void rmi_bus_setup_debugfs(void)
> +{
> + rmi_debugfs_root = debugfs_create_dir(rmi_bus_type.name, NULL);
> + if (!rmi_debugfs_root)
> + pr_err("%s: Failed to create debugfs root\n",
> + __func__);
> +}
> +
> +static void rmi_bus_teardown_debugfs(void)
> +{
> + if (rmi_debugfs_root)
> + debugfs_remove_recursive(rmi_debugfs_root);
> +}
> +
> +#else
> +
> +static void rmi_bus_setup_debugfs(void)
> +{
> +}
> +
> +static void rmi_bus_teardown_debugfs(void)
> +{
> +}
> +
> +#endif
>
> static int __init rmi_bus_init(void)
> {
>
^ permalink raw reply
* Re: [PATCH 1/2] elantech: Properly differentiate between clickpads and normal touchpads
From: Benjamin Tissoires @ 2013-12-09 19:21 UTC (permalink / raw)
To: Hans de Goede; +Cc: linux-input, Peter Hutterer, Duson Lin, Dmitry Torokhov
In-Reply-To: <52A6169C.1020904@redhat.com>
On Mon, Dec 9, 2013 at 2:14 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 12/09/2013 07:02 PM, Benjamin Tissoires wrote:
>>
>> Hi Hans,
>>
>> adding in CC Duson, who seems to be working on the same driver
>> currently, and Dmitry, the input maintainer.
>>
>> On Mon, Dec 9, 2013 at 9:32 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> The current assumption in the elantech driver that hw version 3 touchpads
>>> are
>>> never clickpads and hw version 4 touchpads are always clickpads is wrong.
>>>
>>> There are several bug reports for this, ie:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1030802
>>>
>>> http://superuser.com/questions/619582/right-elantech-touchpad-button-not-working-in-linux
>>>
>>> I've spend a couple of hours wading through various bugzillas,
>>> launchpads and forum posts to create a list of fw-versions and
>>> capabilities
>>> for different laptop models to find a good method to differentiate
>>> between
>>> clickpads and versions with separate hardware buttons.
>>>
>>> Which shows that a device being a clickpad is reliable indicated by bit
>>> 12
>>> being set in the fw_version. I've included the gathered list inside the
>>> driver,
>>> so that we've this info at hand if we need to revisit this later.
>>
>>
>> Duson, can you confirm this? It's not that I don't trust Hans, but if
>> we could have the hardware maker validating this part, this would be
>> great.
>>
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>> drivers/input/mouse/elantech.c | 43
>>> +++++++++++++++++++++++++++++++++++++++---
>>> 1 file changed, 40 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/input/mouse/elantech.c
>>> b/drivers/input/mouse/elantech.c
>>> index 8551dca..f7baa32 100644
>>> --- a/drivers/input/mouse/elantech.c
>>> +++ b/drivers/input/mouse/elantech.c
>>> @@ -486,6 +486,7 @@ static void elantech_input_sync_v4(struct psmouse
>>> *psmouse)
>>> unsigned char *packet = psmouse->packet;
>>>
>>> input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
>>> + input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
>>> input_mt_report_pointer_emulation(dev, true);
>>> input_sync(dev);
>>> }
>>> @@ -984,6 +985,42 @@ static int elantech_get_resolution_v4(struct psmouse
>>> *psmouse,
>>> }
>>>
>>> /*
>>> + * Advertise INPUT_PROP_BUTTONPAD for clickpads. The testing of bit 12
>>> in
>>> + * fw_version for this is based on the following fw_version & caps
>>> table:
>>> + *
>>> + * Laptop-model: fw_version: caps: buttons:
>>> + * Acer S3 0x461f00 10, 13, 0e clickpad
>>> + * Acer S7-392 0x581f01 50, 17, 0d clickpad
>>> + * Acer V5-131 0x461f02 01, 16, 0c clickpad
>>> + * Acer V5-551 0x461f00 ? clickpad
>>> + * Asus K53SV 0x450f01 78, 15, 0c 2 hw buttons
>>> + * Asus G46VW 0x460f02 00, 18, 0c 2 hw buttons
>>> + * Asus G750JX 0x360f00 00, 16, 0c 2 hw buttons
>>> + * Asus UX31 0x361f00 20, 15, 0e clickpad
>>> + * Asus UX32VD 0x361f02 00, 15, 0e clickpad
>>> + * Avatar AVIU-145A2 0x361f00 ? clickpad
>>> + * Gigabyte U2442 0x450f01 58, 17, 0c 2 hw buttons
>>> + * Lenovo L430 0x350f02 b9, 15, 0c 2 hw buttons
>>> (*)
>>> + * Samsung NF210 0x150b00 78, 14, 0a 2 hw buttons
>>> + * Samsung NP770Z5E 0x575f01 10, 15, 0f clickpad
>>> + * Samsung NP700Z5B 0x361f06 21, 15, 0f clickpad
>>> + * Samsung NP900X3E-A02 0x575f03 ? clickpad
>>> + * Samsung NP-QX410 0x851b00 19, 14, 0c clickpad
>>> + * Samsung RC512 0x450f00 08, 15, 0c 2 hw buttons
>>> + * Samsung RF710 0x450f00 ? 2 hw buttons
>>> + * System76 Pangolin 0x250f01 ? 2 hw buttons
>>> + * (*) + 3 trackpoint buttons
>>> + */
>>> +static void elantech_set_buttonpad_prop(struct psmouse *psmouse)
>>> +{
>>> + struct input_dev *dev = psmouse->dev;
>>> + struct elantech_data *etd = psmouse->private;
>>> +
>>> + if (etd->fw_version & 0x001000)
>>> + __set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
>>
>>
>> Small question here: if the touchpad is a clickpad, should'nt we also
>> remove the BTN_RIGHT bit too?
>
>
> We could, but I don't see much value in that, and it would also require
> if statements in the sync methods to ensure that we don't tree to send
> a button event for a button we don't advertise.
We don't need this test in the sync method. The test is already
implemented in input_event. So now, it's just a matter of taste
regarding upper layers. Peter, any thoughts on this?
Anyway, other than that:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
>
> Regards,
>
> Hans
^ permalink raw reply
* Re: [PATCH 1/2] elantech: Properly differentiate between clickpads and normal touchpads
From: Hans de Goede @ 2013-12-09 19:14 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: linux-input, Peter Hutterer, Duson Lin, Dmitry Torokhov
In-Reply-To: <CAN+gG=Eh72eBS67db_KNsUzXRNsOUXB8YkzuuHoJca22xRAkdQ@mail.gmail.com>
Hi,
On 12/09/2013 07:02 PM, Benjamin Tissoires wrote:
> Hi Hans,
>
> adding in CC Duson, who seems to be working on the same driver
> currently, and Dmitry, the input maintainer.
>
> On Mon, Dec 9, 2013 at 9:32 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> The current assumption in the elantech driver that hw version 3 touchpads are
>> never clickpads and hw version 4 touchpads are always clickpads is wrong.
>>
>> There are several bug reports for this, ie:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1030802
>> http://superuser.com/questions/619582/right-elantech-touchpad-button-not-working-in-linux
>>
>> I've spend a couple of hours wading through various bugzillas,
>> launchpads and forum posts to create a list of fw-versions and capabilities
>> for different laptop models to find a good method to differentiate between
>> clickpads and versions with separate hardware buttons.
>>
>> Which shows that a device being a clickpad is reliable indicated by bit 12
>> being set in the fw_version. I've included the gathered list inside the driver,
>> so that we've this info at hand if we need to revisit this later.
>
> Duson, can you confirm this? It's not that I don't trust Hans, but if
> we could have the hardware maker validating this part, this would be
> great.
>
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> drivers/input/mouse/elantech.c | 43 +++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 40 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
>> index 8551dca..f7baa32 100644
>> --- a/drivers/input/mouse/elantech.c
>> +++ b/drivers/input/mouse/elantech.c
>> @@ -486,6 +486,7 @@ static void elantech_input_sync_v4(struct psmouse *psmouse)
>> unsigned char *packet = psmouse->packet;
>>
>> input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
>> + input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
>> input_mt_report_pointer_emulation(dev, true);
>> input_sync(dev);
>> }
>> @@ -984,6 +985,42 @@ static int elantech_get_resolution_v4(struct psmouse *psmouse,
>> }
>>
>> /*
>> + * Advertise INPUT_PROP_BUTTONPAD for clickpads. The testing of bit 12 in
>> + * fw_version for this is based on the following fw_version & caps table:
>> + *
>> + * Laptop-model: fw_version: caps: buttons:
>> + * Acer S3 0x461f00 10, 13, 0e clickpad
>> + * Acer S7-392 0x581f01 50, 17, 0d clickpad
>> + * Acer V5-131 0x461f02 01, 16, 0c clickpad
>> + * Acer V5-551 0x461f00 ? clickpad
>> + * Asus K53SV 0x450f01 78, 15, 0c 2 hw buttons
>> + * Asus G46VW 0x460f02 00, 18, 0c 2 hw buttons
>> + * Asus G750JX 0x360f00 00, 16, 0c 2 hw buttons
>> + * Asus UX31 0x361f00 20, 15, 0e clickpad
>> + * Asus UX32VD 0x361f02 00, 15, 0e clickpad
>> + * Avatar AVIU-145A2 0x361f00 ? clickpad
>> + * Gigabyte U2442 0x450f01 58, 17, 0c 2 hw buttons
>> + * Lenovo L430 0x350f02 b9, 15, 0c 2 hw buttons (*)
>> + * Samsung NF210 0x150b00 78, 14, 0a 2 hw buttons
>> + * Samsung NP770Z5E 0x575f01 10, 15, 0f clickpad
>> + * Samsung NP700Z5B 0x361f06 21, 15, 0f clickpad
>> + * Samsung NP900X3E-A02 0x575f03 ? clickpad
>> + * Samsung NP-QX410 0x851b00 19, 14, 0c clickpad
>> + * Samsung RC512 0x450f00 08, 15, 0c 2 hw buttons
>> + * Samsung RF710 0x450f00 ? 2 hw buttons
>> + * System76 Pangolin 0x250f01 ? 2 hw buttons
>> + * (*) + 3 trackpoint buttons
>> + */
>> +static void elantech_set_buttonpad_prop(struct psmouse *psmouse)
>> +{
>> + struct input_dev *dev = psmouse->dev;
>> + struct elantech_data *etd = psmouse->private;
>> +
>> + if (etd->fw_version & 0x001000)
>> + __set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
>
> Small question here: if the touchpad is a clickpad, should'nt we also
> remove the BTN_RIGHT bit too?
We could, but I don't see much value in that, and it would also require
if statements in the sync methods to ensure that we don't tree to send
a button event for a button we don't advertise.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH V2] input synaptics-rmi4: Fix ms calculation in poll interval
From: Benjamin Tissoires @ 2013-12-09 18:53 UTC (permalink / raw)
To: Christopher Heiny, Dmitry Torokhov
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij
In-Reply-To: <1386288825-11837-1-git-send-email-cheiny@synaptics.com>
On 05/12/13 19:13, Christopher Heiny wrote:
> [Previous patch re-used an old template for the subject line. Sorry about that]
>
> This patch implements changes to the synaptics-rmi4 branch of
> Dmitry's input tree. The base for the patch is commit
> 8ca01dc61a42b6f7bcba052a8c084000f7057a34.
>
> Fix computation of poll interval milliseconds.
>
> Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Jean Delvare <khali@linux-fr.org>
> Cc: Linus Walleij <linus.walleij@stericsson.com>
> Cc: Joerie de Gram <j.de.gram@gmail.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
> ---
> drivers/input/rmi4/rmi_driver.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index 9af297e..dffbfa0 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -934,7 +934,7 @@ static int rmi_driver_probe(struct device *dev)
> } else
> data->poll_interval = ktime_set(0,
> (pdata->poll_interval_ms ? pdata->poll_interval_ms :
> - DEFAULT_POLL_INTERVAL_MS) * 1000);
> + DEFAULT_POLL_INTERVAL_MS) * 1000 * 1000);
>
> if (data->f01_container->dev.driver) {
> /* Driver already bound, so enable ATTN now. */
>
^ permalink raw reply
* Re: [PATCH 2/2] Documentation: dt: Document TSC2005 DT binding
From: Sebastian Reichel @ 2013-12-09 18:24 UTC (permalink / raw)
To: Tony Lindgren
Cc: Dmitry Torokhov, Dmitry Torokhov, linux-input, Rob Herring,
Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
Rob Landley, Grant Likely, devicetree, linux-omap, linux-kernel
In-Reply-To: <20131209174638.GC12527@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 1501 bytes --]
On Mon, Dec 09, 2013 at 09:46:38AM -0800, Tony Lindgren wrote:
> > +Optional properties:
> > + - ti,fuzz-x : integer, X noise value of the touchscreen
> > + (defaults to 4)
> > + - ti,fuzz-y : integer, Y noise value of the touchscreen
> > + (defaults to 8)
> > + - ti,fuzz-pressure : integer, pressure noise value of the touchscreen
> > + (defaults to 2)
> > + - ti,max-x : integer, maximum reported x value
> > + (defaults to 4096)
> > + - ti,max-y : integer, maximum reported y value
> > + (defaults to 4096)
> > + - ti,max-pressure : integer, maximum reported pressure
> > + (defaults to 4096)
> > + - ti,x-plate-resistance : integer, resistance of the touchscreen's X plates
> > + in ohm (defaults to 280)
> > + - ti,esd-recovery-timeout-ms : integer, if the touchscreen does not respond after
> > + the configured time (in milli seconds), the driver
> > + will reset it. This is disabled by default.
>
> Instead of adding these optional ti,* properties you can set them in the
> driver directly in the of_match table based on the compatible flag. Then
> you can pass compatible flag like ti,tsc2005-nokia-n900, or the name of
> the LCD panel. Most likely these depend on the LCD panel selected.
I could certainly do this, but it would move the board specific data
from the boardcode into the driver. That looks contra-productive to
me. Is there a good reason to do it this way?
-- Sebastian
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
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