* 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 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: 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
* [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: 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
* 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: [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] 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] 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 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 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 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 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: 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: 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
* [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: 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
* 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
* [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
* 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 v1 1/2] Input: gpio_keys - use dev instead of pdev in gpio_keys_setup_key()
From: Andy Shevchenko @ 2013-12-10 14:03 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input; +Cc: Andy Shevchenko
The platform device is not used in gpio_keys_setup_key(). This patch
substitutes it by struct device.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/input/keyboard/gpio_keys.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 2db1324..209d4c6 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -424,13 +424,12 @@ out:
return IRQ_HANDLED;
}
-static int gpio_keys_setup_key(struct platform_device *pdev,
+static int gpio_keys_setup_key(struct device *dev,
struct input_dev *input,
struct gpio_button_data *bdata,
const struct gpio_keys_button *button)
{
const char *desc = button->desc ? button->desc : "gpio_keys";
- struct device *dev = &pdev->dev;
irq_handler_t isr;
unsigned long irqflags;
int irq, error;
@@ -719,7 +718,7 @@ static int gpio_keys_probe(struct platform_device *pdev)
input->name = pdata->name ? : pdev->name;
input->phys = "gpio-keys/input0";
- input->dev.parent = &pdev->dev;
+ input->dev.parent = dev;
input->open = gpio_keys_open;
input->close = gpio_keys_close;
@@ -736,7 +735,7 @@ static int gpio_keys_probe(struct platform_device *pdev)
const struct gpio_keys_button *button = &pdata->buttons[i];
struct gpio_button_data *bdata = &ddata->data[i];
- error = gpio_keys_setup_key(pdev, input, bdata, button);
+ error = gpio_keys_setup_key(dev, input, bdata, button);
if (error)
goto fail2;
--
1.8.4.4
^ permalink raw reply related
* [PATCH v1 2/2] Input: gpio_keys - convert to use devm_*
From: Andy Shevchenko @ 2013-12-10 14:03 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input; +Cc: Andy Shevchenko
In-Reply-To: <1386684237-31013-1-git-send-email-andriy.shevchenko@linux.intel.com>
This makes the error handling much more simpler than open-coding everything and
in addition makes the probe function smaller an tidier.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/input/keyboard/gpio_keys.c | 75 +++++++++++++-------------------------
1 file changed, 25 insertions(+), 50 deletions(-)
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 209d4c6..8791d94 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -47,7 +47,7 @@ struct gpio_keys_drvdata {
const struct gpio_keys_platform_data *pdata;
struct input_dev *input;
struct mutex disable_lock;
- struct gpio_button_data data[0];
+ struct gpio_button_data *data;
};
/*
@@ -577,25 +577,22 @@ gpio_keys_get_devtree_pdata(struct device *dev)
int i;
node = dev->of_node;
- if (!node) {
- error = -ENODEV;
- goto err_out;
- }
+ if (!node)
+ return ERR_PTR(-ENODEV);
nbuttons = of_get_child_count(node);
- if (nbuttons == 0) {
- error = -ENODEV;
- goto err_out;
- }
+ if (nbuttons == 0)
+ return ERR_PTR(-ENODEV);
- pdata = kzalloc(sizeof(*pdata) + nbuttons * (sizeof *button),
- GFP_KERNEL);
- if (!pdata) {
- error = -ENOMEM;
- goto err_out;
- }
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return ERR_PTR(-ENOMEM);
+
+ pdata->buttons = devm_kcalloc(dev, nbuttons, sizeof (*button),
+ GFP_KERNEL);
+ if (!pdata->buttons)
+ return ERR_PTR(-ENOMEM);
- pdata->buttons = (struct gpio_keys_button *)(pdata + 1);
pdata->nbuttons = nbuttons;
pdata->rep = !!of_get_property(node, "autorepeat", NULL);
@@ -618,7 +615,7 @@ gpio_keys_get_devtree_pdata(struct device *dev)
dev_err(dev,
"Failed to get gpio flags, error: %d\n",
error);
- goto err_free_pdata;
+ return ERR_PTR(error);
}
button = &pdata->buttons[i++];
@@ -629,8 +626,7 @@ gpio_keys_get_devtree_pdata(struct device *dev)
if (of_property_read_u32(pp, "linux,code", &button->code)) {
dev_err(dev, "Button without keycode: 0x%x\n",
button->gpio);
- error = -EINVAL;
- goto err_free_pdata;
+ return ERR_PTR(-EINVAL);
}
button->desc = of_get_property(pp, "label", NULL);
@@ -645,17 +641,10 @@ gpio_keys_get_devtree_pdata(struct device *dev)
button->debounce_interval = 5;
}
- if (pdata->nbuttons == 0) {
- error = -EINVAL;
- goto err_free_pdata;
- }
+ if (pdata->nbuttons == 0)
+ return ERR_PTR(-EINVAL);
return pdata;
-
-err_free_pdata:
- kfree(pdata);
-err_out:
- return ERR_PTR(error);
}
static struct of_device_id gpio_keys_of_match[] = {
@@ -699,16 +688,18 @@ static int gpio_keys_probe(struct platform_device *pdev)
return PTR_ERR(pdata);
}
- ddata = kzalloc(sizeof(struct gpio_keys_drvdata) +
- pdata->nbuttons * sizeof(struct gpio_button_data),
- GFP_KERNEL);
- input = input_allocate_device();
+ ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
+ input = devm_input_allocate_device(dev);
if (!ddata || !input) {
dev_err(dev, "failed to allocate state\n");
- error = -ENOMEM;
- goto fail1;
+ return -ENOMEM;
}
+ ddata->data = devm_kcalloc(dev, pdata->nbuttons, sizeof(*ddata->data),
+ GFP_KERNEL);
+ if (!ddata->data)
+ return -ENOMEM;
+
ddata->pdata = pdata;
ddata->input = input;
mutex_init(&ddata->disable_lock);
@@ -767,20 +758,12 @@ static int gpio_keys_probe(struct platform_device *pdev)
while (--i >= 0)
gpio_remove_key(&ddata->data[i]);
- fail1:
- input_free_device(input);
- kfree(ddata);
- /* If we have no platform data, we allocated pdata dynamically. */
- if (!dev_get_platdata(&pdev->dev))
- kfree(pdata);
-
return error;
}
static int gpio_keys_remove(struct platform_device *pdev)
{
struct gpio_keys_drvdata *ddata = platform_get_drvdata(pdev);
- struct input_dev *input = ddata->input;
int i;
sysfs_remove_group(&pdev->dev.kobj, &gpio_keys_attr_group);
@@ -790,14 +773,6 @@ static int gpio_keys_remove(struct platform_device *pdev)
for (i = 0; i < ddata->pdata->nbuttons; i++)
gpio_remove_key(&ddata->data[i]);
- input_unregister_device(input);
-
- /* If we have no platform data, we allocated pdata dynamically. */
- if (!dev_get_platdata(&pdev->dev))
- kfree(ddata->pdata);
-
- kfree(ddata);
-
return 0;
}
--
1.8.4.4
^ permalink raw reply related
* Re: [PATCH V2] input synaptics-rmi4: Reorder declarations in rmi_bus.c
From: Benjamin Tissoires @ 2013-12-10 14:58 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Benjamin Tissoires, Christopher Heiny, Linux Input, Andrew Duggan,
Vincent Huang, Vivian Ly, Daniel Rosenberg, Jean Delvare,
Joerie de Gram, Linus Walleij
In-Reply-To: <20131210064624.GE12524@core.coreip.homeip.net>
On Tue, Dec 10, 2013 at 1:46 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> 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.
oops, sorry for not having spot this one. My mistake.
Thanks for fixing it.
Cheers,
Benjamin
--
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] Input: zforce: fix possible driver hang during suspend
From: Heiko Stübner @ 2013-12-10 15:50 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Henrik Rydberg, linux-input
handle_level_irq masks the interrupt before handling it, and only
unmasks it after the handler is finished. So when a touch event
happens after threads are suspended, but before the system is fully asleep
the irq handler tries to wakeup the thread which will only happen on the
next resume, resulting in the wakeup event never being sent and the driver
not being able to wake the system from sleep due to the masked irq.
Therefore move the wakeup_event to a small non-threaded handler.
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
drivers/input/touchscreen/zforce_ts.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
index 75762d6..aa127ba 100644
--- a/drivers/input/touchscreen/zforce_ts.c
+++ b/drivers/input/touchscreen/zforce_ts.c
@@ -455,7 +455,18 @@ static void zforce_complete(struct zforce_ts *ts, int cmd, int result)
}
}
-static irqreturn_t zforce_interrupt(int irq, void *dev_id)
+static irqreturn_t zforce_irq(int irq, void *dev_id)
+{
+ struct zforce_ts *ts = dev_id;
+ struct i2c_client *client = ts->client;
+
+ if (ts->suspended && device_may_wakeup(&client->dev))
+ pm_wakeup_event(&client->dev, 500);
+
+ return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t zforce_irq_thread(int irq, void *dev_id)
{
struct zforce_ts *ts = dev_id;
struct i2c_client *client = ts->client;
@@ -465,12 +476,10 @@ static irqreturn_t zforce_interrupt(int irq, void *dev_id)
u8 *payload;
/*
- * When suspended, emit a wakeup signal if necessary and return.
+ * When still suspended, return.
* Due to the level-interrupt we will get re-triggered later.
*/
if (ts->suspended) {
- if (device_may_wakeup(&client->dev))
- pm_wakeup_event(&client->dev, 500);
msleep(20);
return IRQ_HANDLED;
}
@@ -763,8 +772,8 @@ static int zforce_probe(struct i2c_client *client,
* Therefore we can trigger the interrupt anytime it is low and do
* not need to limit it to the interrupt edge.
*/
- ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
- zforce_interrupt,
+ ret = devm_request_threaded_irq(&client->dev, client->irq,
+ zforce_irq, zforce_irq_thread,
IRQF_TRIGGER_LOW | IRQF_ONESHOT,
input_dev->name, ts);
if (ret) {
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH V2] input synaptics-rmi4: Reorder declarations in rmi_bus.c
From: Christopher Heiny @ 2013-12-10 17:41 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Benjamin Tissoires, Linux Input, Andrew Duggan, Vincent Huang,
Vivian Ly, Daniel Rosenberg, Jean Delvare, Joerie de Gram,
Linus Walleij
In-Reply-To: <20131210064624.GE12524@core.coreip.homeip.net>
On 12/09/2013 10:46 PM, Dmitry Torokhov wrote:
> 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.
Um, well, OK. As mentioned in a previous patch, the code is still
horribly broken, although it's getting better. I should have included
that previous patch's disclaimer with this one.
There's a bunch of debugfs problems in the branch as it stands, such as
failure to compile in certain configurations, kernel panics in others,
problematic initialization sequence, and so on. I was planning to fix
all that in a separate patch following this one, rather than putting
changes in piece meal, or piggybacking the changes onto another patch.
Chris
--
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
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