* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute
From: Dmitry Torokhov @ 2014-03-28 16:04 UTC (permalink / raw)
To: Matthew Garrett
Cc: Hans de Goede, Benjamin Tissoires, Peter Hutterer,
platform-driver-x86, linux-input
In-Reply-To: <20140328090053.GA10597@srcf.ucam.org>
On Fri, Mar 28, 2014 at 09:00:53AM +0000, Matthew Garrett wrote:
> On Fri, Mar 28, 2014 at 01:52:07AM -0700, Dmitry Torokhov wrote:
> >
> > Are we even certain that they will be consistent in use of these special
> > PNP ID's? Maybe you should really do DMI match...
>
> The Windows drivers bind on PNP IDs, not DMI.
OK, fair enough.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: synaptics add manual min/max quirk
From: Dmitry Torokhov @ 2014-03-28 16:09 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Christopher Heiny, Andrew Duggan, linux-input, linux-kernel,
Peter Hutterer, Stephen Chandler Paul, Hans de Goede
In-Reply-To: <20140328082949.GJ22093@core.coreip.homeip.net>
On Fri, Mar 28, 2014 at 01:29:50AM -0700, Dmitry Torokhov wrote:
> On Fri, Mar 07, 2014 at 10:49:24AM -0500, Benjamin Tissoires wrote:
> > The new Lenovo Haswell series (-40's) contains a new Synaptics touchpad.
> > However, these new Synaptics devices report bad axis ranges.
> > Under Windows, it is not a problem because the Windows driver uses RMI4
> > over SMBus to talk to the device. Under Linux, we are using the PS/2
> > fallback interface and it occurs the reported ranges are wrong.
> >
> > Of course, it would be too easy to have only one range for the whole
> > series, each touchpad seems to be calibrated in a different way.
> >
> > We can not use SMBus to get the actual range because I suspect the firmware
> > will switch into the SMBus mode and stop talking through PS/2 (this is the
> > case for hybrid HID over I2C / PS/2 Synaptics touchpads).
> >
> > So as a temporary solution (until RMI4 land into upstream), start a new
> > list of quirks with the min/max manually set.
> >
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > CC: stable@vger.kernel.org
>
>
> Applied, thank you.
That was giving compiler errors when configured without synaptics
support so I had fiddle with the patch a bit. I'll take full
responsibility for any breakage ;)
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/3] Input: synaptics-rmi4 - add capabilities for touchpads
From: Dmitry Torokhov @ 2014-03-28 16:15 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Christopher Heiny, Linux Input, Andrew Duggan, Vincent Huang,
Vivian Ly, Daniel Rosenberg, Linus Walleij, David Herrmann,
Jiri Kosina
In-Reply-To: <5329A9CE.1080504@redhat.com>
On Wed, Mar 19, 2014 at 10:29:34AM -0400, Benjamin Tissoires wrote:
> Hi Chris,
>
> On 03/18/2014 09:03 PM, Christopher Heiny wrote:
> >When the device is a touchpad additional capabilities need to
> >be set and reported.
> >
>
> We have a problem here. While this patch would have been fine in the
> pre-v3.8 era, it is not true anymore.
> However, the current branch where synaptics-rmi4 is attached is v3.4.
>
> So if you use the right API (the current one), it will not compile :(
>
> Dmitry, would it be possible to update the branch to at least v3.8
> to get the new input-mt API? (if the Synaptics guys are ok).
If we are getting ready to pull it into mainline (and I think we are
for F01 and F11 support) then I think I should simply uprev to the
latest released kernel.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCHv6 2/5] Input: edt-ft5x06: Add DT support
From: Dmitry Torokhov @ 2014-03-28 16:26 UTC (permalink / raw)
To: Lothar Waßmann
Cc: Fugang Duan, Grant Likely, Henrik Rydberg, Ian Campbell,
Jingoo Han, Kumar Gala, Mark Rutland, Pawel Moll, Rob Herring,
Rob Landley, Sachin Kamat, devicetree, linux-doc, linux-input,
linux-kernel, Simon Budig, Daniel Wagener
In-Reply-To: <1395673870-29435-3-git-send-email-LW@KARO-electronics.de>
Hi Lothar,
On Mon, Mar 24, 2014 at 04:11:07PM +0100, Lothar Waßmann wrote:
> +#else
> +static inline int edt_ft5x06_i2c_ts_probe_dt(struct device *dev,
> + struct edt_ft5x06_i2c_ts_data *tsdata)
This is wrong (not-existant) type for tsdata, I fixed it up.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCHv6 0/5] Input: edt-ft5x06: Add DT support
From: Dmitry Torokhov @ 2014-03-28 16:31 UTC (permalink / raw)
To: Lothar Waßmann
Cc: Fugang Duan, Grant Likely, Henrik Rydberg, Ian Campbell,
Jingoo Han, Kumar Gala, Mark Rutland, Pawel Moll, Rob Herring,
Rob Landley, Sachin Kamat, devicetree, linux-doc, linux-input,
linux-kernel, Simon Budig, Daniel Wagener
In-Reply-To: <1395673870-29435-1-git-send-email-LW@KARO-electronics.de>
On Mon, Mar 24, 2014 at 04:11:05PM +0100, Lothar Waßmann wrote:
> Changes wrt. v1:
> addressed the comments from Jingoo Han and Mark Rutland
> - added another patch to convert the driver to use devm_* functions
> - removed sysfs reference from bindings documentation
> - changed '_' to '-' in property name
> - added 'edt,' prefix to properties names
> - added sanity check for parameters read from DT
> - cleaned up the gpio handling code
>
> Changes wrt. v2:
> - fixed the devm_* messup reported by Dmitry Torokhov
> - added unit for report-rate property to the binding doc
> - added separate patch to fix the reset delays
>
> Changes wrt: v3:
> - removed patches that have already been applied in the mean time
> - ignore touchdown events, since those may report bad coordinates
> - added support for a new firmware version
>
> Changes wrt: v4:
> - removed some empty lines in the cleanup patch
> - addressed comments by Mark Rutland concerning the binding doc
> - use of_property_read_u32() instead of of_property_get()
> - dropped the 'report_rate' property
> - addressed comments by Fugang Duan
> - added Daniel Wagener and myself to the Copyright header in the
> source file
> - use msleep() rather than mdelay() for the reset/wake pin timing
>
> Changes wrt: v5:
> - added missing '&' to parameter of of_property_read_u32()
>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 1/2] input: Add new driver for ARM CLPS711X keypad
From: Dmitry Torokhov @ 2014-03-28 16:42 UTC (permalink / raw)
To: Alexander Shiyan; +Cc: linux-input
In-Reply-To: <1395165739-17378-1-git-send-email-shc_work@mail.ru>
Hi Alexander,
On Tue, Mar 18, 2014 at 10:02:19PM +0400, Alexander Shiyan wrote:
> + err = input_register_polled_device(poll_dev);
> + if (!err) {
> + /* Report initial state */
> + clps711x_keypad_poll(poll_dev);
> +
> + return 0;
> + }
The polled device core will report initial state when input device is
opened (as long as polling is enabled) so I removed this cunk and
applied the rest.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH RESEND] input: dts: Add commonly used event types
From: Dmitry Torokhov @ 2014-03-28 16:43 UTC (permalink / raw)
To: Alexander Shiyan
Cc: linux-input, devicetree, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Grant Likely
In-Reply-To: <1395165888-17434-1-git-send-email-shc_work@mail.ru>
On Tue, Mar 18, 2014 at 10:04:47PM +0400, Alexander Shiyan wrote:
> Patch adds commonly used event types (EV_KEY and EV_SW) to
> devicetree bindings header for input subsystem.
>
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
> include/dt-bindings/input/input.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/include/dt-bindings/input/input.h b/include/dt-bindings/input/input.h
> index 042e7b3..3a141d92 100644
> --- a/include/dt-bindings/input/input.h
> +++ b/include/dt-bindings/input/input.h
> @@ -9,6 +9,9 @@
> #ifndef _DT_BINDINGS_INPUT_INPUT_H
> #define _DT_BINDINGS_INPUT_INPUT_H
>
> +#define EV_KEY 0x01
> +#define EV_SW 0x05
> +
> #define KEY_RESERVED 0
> #define KEY_ESC 1
> #define KEY_1 2
> --
> 1.8.3.2
>
This I believe should go through device tree.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 2/2] input: clps711x-keypad: dts: Add bindings documentation
From: Dmitry Torokhov @ 2014-03-28 16:43 UTC (permalink / raw)
To: Alexander Shiyan
Cc: linux-input, devicetree, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Grant Likely
In-Reply-To: <1395165888-17434-2-git-send-email-shc_work@mail.ru>
On Tue, Mar 18, 2014 at 10:04:48PM +0400, Alexander Shiyan wrote:
> This patch adds the devicetree documentation for the Cirrus Logic
> CLPS711X keypad.
>
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
I picked this one up and folded into the driver patch.
> ---
> .../devicetree/bindings/input/clps711x-keypad.txt | 27 ++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/clps711x-keypad.txt
>
> diff --git a/Documentation/devicetree/bindings/input/clps711x-keypad.txt b/Documentation/devicetree/bindings/input/clps711x-keypad.txt
> new file mode 100644
> index 0000000..93fa2c1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/clps711x-keypad.txt
> @@ -0,0 +1,27 @@
> +* Cirrus Logic CLPS711X matrix keypad device tree bindings
> +
> +Required Properties:
> +- compatible: Shall contain "cirrus,clps711x-keypad".
> +- row-gpios: List of GPIOs used as row lines.
> +- poll-interval: Poll interval time in milliseconds.
> +- linux,keymap: The definition can be found at
> + bindings/input/matrix-keymap.txt.
> +
> +Optional Properties:
> +- autorepeat: Enable autorepeat feature.
> +
> +Example:
> + keypad {
> + compatible = "cirrus,ep7312-keypad", "cirrus,clps711x-keypad";
> + autorepeat;
> + poll-interval = <120>;
> + row-gpios = <&porta 0 0>,
> + <&porta 1 0>;
> +
> + linux,keymap = <
> + MATRIX_KEY(0, 0, KEY_UP)
> + MATRIX_KEY(0, 1, KEY_DOWN)
> + MATRIX_KEY(1, 0, KEY_LEFT)
> + MATRIX_KEY(1, 1, KEY_RIGHT)
> + >;
> + };
> --
> 1.8.3.2
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 1/2] input: Add new driver for ARM CLPS711X keypad
From: Alexander Shiyan @ 2014-03-28 16:46 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
In-Reply-To: <20140328164228.GA7582@core.coreip.homeip.net>
Fri, 28 Mar 2014 09:42:28 -0700 от Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> Hi Alexander,
>
> On Tue, Mar 18, 2014 at 10:02:19PM +0400, Alexander Shiyan wrote:
> > + err = input_register_polled_device(poll_dev);
> > + if (!err) {
> > + /* Report initial state */
> > + clps711x_keypad_poll(poll_dev);
> > +
> > + return 0;
> > + }
>
> The polled device core will report initial state when input device is
> opened (as long as polling is enabled) so I removed this cunk and
> applied the rest.
Today I stumbled with race condition with this, so your solution is OK.
Thanks.
---
^ permalink raw reply
* Re: [PATCH 1/3] Input: synaptics-rmi4 - add capabilities for touchpads
From: Christopher Heiny @ 2014-03-28 18:24 UTC (permalink / raw)
To: Dmitry Torokhov, Benjamin Tissoires
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Linus Walleij, David Herrmann, Jiri Kosina
In-Reply-To: <20140328161505.GC22658@core.coreip.homeip.net>
On 03/28/2014 09:15 AM, Dmitry Torokhov wrote:
> On Wed, Mar 19, 2014 at 10:29:34AM -0400, Benjamin Tissoires wrote:
>> >Hi Chris,
>> >
>> >On 03/18/2014 09:03 PM, Christopher Heiny wrote:
>>> > >When the device is a touchpad additional capabilities need to
>>> > >be set and reported.
>>> > >
>> >
>> >We have a problem here. While this patch would have been fine in the
>> >pre-v3.8 era, it is not true anymore.
>> >However, the current branch where synaptics-rmi4 is attached is v3.4.
>> >
>> >So if you use the right API (the current one), it will not compile :(
>> >
>> >Dmitry, would it be possible to update the branch to at least v3.8
>> >to get the new input-mt API? (if the Synaptics guys are ok).
>
> If we are getting ready to pull it into mainline (and I think we are
> for F01 and F11 support) then I think I should simply uprev to the
> latest released kernel.
That works for us.
^ permalink raw reply
* [git pull] Input updates for 3.14-rc8
From: Dmitry Torokhov @ 2014-03-28 19:36 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-input
[-- Attachment #1: Type: text/plain, Size: 1132 bytes --]
Hi Linus,
Please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
or
master.kernel.org:/pub/scm/linux/kernel/git/dtor/input.git for-linus
to receive updates for the input subsystem. You will get updates to
Synaptics touchpad to better cope with devices in Lenovo laptops, and a
couple more fixes.
Changelog:
---------
Anthony Olech (1):
Input: da9052_onkey - use correct register bit for key status
Benjamin Tissoires (1):
Input: synaptics - add manual min/max quirk
Hans de Goede (2):
Input: cypress_ps2 - don't report as a button pads
Input: synaptics - add manual min/max quirk for ThinkPad X240
Jean-Francois Dagenais (1):
Input: adp5588-keys - get value from data out when dir is out
Diffstat:
--------
drivers/input/keyboard/adp5588-keys.c | 12 +++++++-
drivers/input/misc/da9052_onkey.c | 29 +++++++++---------
drivers/input/mouse/cypress_ps2.c | 1 -
drivers/input/mouse/synaptics.c | 55 +++++++++++++++++++++++++++++++++++
4 files changed, 82 insertions(+), 15 deletions(-)
--
Dmitry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] Input: synaptics add manual min/max quirk
From: Benjamin Tissoires @ 2014-03-28 20:25 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Benjamin Tissoires, Christopher Heiny, Andrew Duggan, linux-input,
linux-kernel@vger.kernel.org, Peter Hutterer,
Stephen Chandler Paul, Hans de Goede
In-Reply-To: <20140328160924.GB22658@core.coreip.homeip.net>
On Fri, Mar 28, 2014 at 12:09 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Fri, Mar 28, 2014 at 01:29:50AM -0700, Dmitry Torokhov wrote:
>> On Fri, Mar 07, 2014 at 10:49:24AM -0500, Benjamin Tissoires wrote:
>> > The new Lenovo Haswell series (-40's) contains a new Synaptics touchpad.
>> > However, these new Synaptics devices report bad axis ranges.
>> > Under Windows, it is not a problem because the Windows driver uses RMI4
>> > over SMBus to talk to the device. Under Linux, we are using the PS/2
>> > fallback interface and it occurs the reported ranges are wrong.
>> >
>> > Of course, it would be too easy to have only one range for the whole
>> > series, each touchpad seems to be calibrated in a different way.
>> >
>> > We can not use SMBus to get the actual range because I suspect the firmware
>> > will switch into the SMBus mode and stop talking through PS/2 (this is the
>> > case for hybrid HID over I2C / PS/2 Synaptics touchpads).
>> >
>> > So as a temporary solution (until RMI4 land into upstream), start a new
>> > list of quirks with the min/max manually set.
>> >
>> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> > CC: stable@vger.kernel.org
>>
>>
>> Applied, thank you.
>
> That was giving compiler errors when configured without synaptics
> support so I had fiddle with the patch a bit. I'll take full
> responsibility for any breakage ;)
>
Thanks for fixing it. Tested and approved by myself, so no breakage to
report here :)
BTW, I have to send out also the min/max for the new X1 Carbon and the
Yoga (not the 2)... I am wondering how to make sure the kernel will
discriminate the Yoga from the Yoga 2 now :/
I'll come back as soon as I can.
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH v2] input: misc: Add driver for Intel Bay Trail GPIO buttons
From: Dmitry Torokhov @ 2014-03-28 20:47 UTC (permalink / raw)
To: Zhu, Lejun; +Cc: One Thousand Gnomes, linux-kernel, linux-input
In-Reply-To: <533467C3.4030503@linux.intel.com>
Hi Lejun,
On Fri, Mar 28, 2014 at 02:02:43AM +0800, Zhu, Lejun wrote:
> +
> +static struct soc_button_info soc_button_tbl[] = {
> + {"power", 0, KEY_POWER, 0, 1, -1},
> + {"home", 1, KEY_HOME, 0, 1, -1},
> + {"volume_up", 2, KEY_VOLUMEUP, 1, 0, -1},
> + {"volume_down", 3, KEY_VOLUMEDOWN, 1, 0, -1},
> + {"rotation_lock", 4, KEY_RO, 0, 0, -1},
KEY_RO is not for rotating but rather for switching one of Japanese
layouts. I think we shoudl allow using switchws and use SW_ROTATE_LOCK
for that.
Also I do not quite like so many static structures, what do you think
about the version below?
Thanks!
--
Dmitry
Input: misc - Add driver for Intel Bay Trail GPIO buttons
From: Lejun Zhu <lejun.zhu@linux.intel.com>
This patch adds support for the GPIO buttons on some Intel Bay Trail
tablets originally running Windows 8. The ACPI description of these
buttons follows "Windows ACPI Design Guide for SoC Platforms".
Signed-off-by: Lejun Zhu <lejun.zhu@linux.intel.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/Kconfig | 10 ++
drivers/input/misc/Makefile | 1
drivers/input/misc/soc_button_array.c | 216 +++++++++++++++++++++++++++++++++
3 files changed, 227 insertions(+)
create mode 100644 drivers/input/misc/soc_button_array.c
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 7674f05..f772981 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -666,4 +666,14 @@ config INPUT_IDEAPAD_SLIDEBAR
To compile this driver as a module, choose M here: the
module will be called ideapad_slidebar.
+config INPUT_SOC_BUTTON_ARRAY
+ tristate "Windows-compatible SoC Button Array"
+ depends on KEYBOARD_GPIO
+ help
+ Say Y here if you have a SoC-based tablet that originally
+ runs Windows 8.
+
+ To compile this driver as a module, choose M here: the
+ module will be called soc_button_array.
+
endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index cda71fc..4955ad3 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_INPUT_RETU_PWRBUTTON) += retu-pwrbutton.o
obj-$(CONFIG_INPUT_GPIO_ROTARY_ENCODER) += rotary_encoder.o
obj-$(CONFIG_INPUT_SGI_BTNS) += sgi_btns.o
obj-$(CONFIG_INPUT_SIRFSOC_ONKEY) += sirfsoc-onkey.o
+obj-$(CONFIG_INPUT_SOC_BUTTON_ARRAY) += soc_button_array.o
obj-$(CONFIG_INPUT_SPARCSPKR) += sparcspkr.o
obj-$(CONFIG_INPUT_TWL4030_PWRBUTTON) += twl4030-pwrbutton.o
obj-$(CONFIG_INPUT_TWL4030_VIBRA) += twl4030-vibra.o
diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
new file mode 100644
index 0000000..b78fa3d
--- /dev/null
+++ b/drivers/input/misc/soc_button_array.c
@@ -0,0 +1,216 @@
+/*
+ * Supports for the button array on SoC tablets originally running Windows 8.
+ *
+ * (C) Copyright 2014 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#include <linux/module.h>
+#include <linux/input.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/acpi.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio_keys.h>
+#include <linux/input.h>
+#include <linux/platform_device.h>
+#include <linux/pnp.h>
+
+/*
+ * Definition of buttons on the tablet. The ACPI index of each button
+ * is defined in section 2.8.7.2 of "Windows ACPI Design Guide for SoC
+ * Platforms"
+ */
+#define MAX_NBUTTONS 5
+
+struct soc_button_info {
+ const char *name;
+ int acpi_index;
+ unsigned int event_type;
+ unsigned int event_code;
+ bool autorepeat;
+ bool wakeup;
+};
+
+/*
+ * Some of the buttons like volume up/down are auto repeat, while others
+ * are not. To support both, we register two platform devices, and put
+ * buttons into them based on whether the key should be auto repeat.
+ */
+#define BUTTON_TYPES 2
+
+struct soc_button_data {
+ struct platform_device *children[BUTTON_TYPES];
+};
+
+/*
+ * Get the Nth GPIO number from the ACPI object.
+ */
+static int soc_button_lookup_gpio(struct device *dev, int acpi_index)
+{
+ struct gpio_desc *desc;
+ int gpio;
+
+ desc = gpiod_get_index(dev, KBUILD_MODNAME, acpi_index);
+ if (IS_ERR(desc))
+ return PTR_ERR(desc);
+
+ gpio = desc_to_gpio(desc);
+
+ gpiod_put(desc);
+
+ return gpio;
+}
+
+static struct platform_device *
+soc_button_device_create(struct pnp_dev *pdev,
+ const struct soc_button_info *button_info,
+ bool autorepeat)
+{
+ const struct soc_button_info *info;
+ struct platform_device *pd;
+ struct gpio_keys_button *gpio_keys;
+ struct gpio_keys_platform_data *gpio_keys_pdata;
+ int n_buttons = 0;
+ int gpio;
+ int error;
+
+ gpio_keys_pdata = devm_kzalloc(&pdev->dev,
+ sizeof(*gpio_keys_pdata) +
+ sizeof(*gpio_keys) * MAX_NBUTTONS,
+ GFP_KERNEL);
+ gpio_keys = (void *)(gpio_keys_pdata++);
+
+ for (info = button_info; info->name; info++) {
+ if (info->autorepeat != autorepeat)
+ continue;
+
+ gpio = soc_button_lookup_gpio(&pdev->dev, info->acpi_index);
+ continue;
+
+ gpio_keys[n_buttons].type = info->event_type;
+ gpio_keys[n_buttons].code = info->event_code;
+ gpio_keys[n_buttons].gpio = gpio;
+ gpio_keys[n_buttons].active_low = 1;
+ gpio_keys[n_buttons].desc = info->name;
+ gpio_keys[n_buttons].wakeup = info->wakeup;
+ n_buttons++;
+ }
+
+ if (n_buttons == 0) {
+ error = -ENODEV;
+ goto err_free_mem;
+ }
+
+ gpio_keys_pdata->buttons = gpio_keys;
+ gpio_keys_pdata->nbuttons = n_buttons;
+ gpio_keys_pdata->rep = autorepeat;
+
+ pd = platform_device_alloc("gpio-keys", PLATFORM_DEVID_AUTO);
+ if (!pd) {
+ error = -ENOMEM;
+ goto err_free_mem;
+ }
+
+ error = platform_device_add_data(pd, gpio_keys_pdata,
+ sizeof(*gpio_keys_pdata));
+ if (error)
+ goto err_free_pdev;
+
+ error = platform_device_add(pd);
+ if (error)
+ goto err_free_pdev;
+
+ return pd;
+
+err_free_pdev:
+ platform_device_put(pd);
+err_free_mem:
+ devm_kfree(&pdev->dev, gpio_keys_pdata);
+ return ERR_PTR(error);
+}
+
+static void soc_button_remove(struct pnp_dev *pdev)
+{
+ struct soc_button_data *priv = pnp_get_drvdata(pdev);
+ int i;
+
+ for (i = 0; i < BUTTON_TYPES; i++)
+ if (priv->children[i])
+ platform_device_unregister(priv->children[i]);
+}
+
+static int soc_button_pnp_probe(struct pnp_dev *pdev,
+ const struct pnp_device_id *id)
+{
+ const struct soc_button_info *button_info = (void *)id->driver_data;
+ struct soc_button_data *priv;
+ struct platform_device *pd;
+ int i;
+ int error;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ pnp_set_drvdata(pdev, priv);
+
+ for (i = 0; i < BUTTON_TYPES; i++) {
+ pd = soc_button_device_create(pdev, button_info, i == 0);
+ if (IS_ERR(pd)) {
+ error = PTR_ERR(pd);
+ if (error != -ENODEV) {
+ soc_button_remove(pdev);
+ return error;
+ }
+ }
+
+ priv->children[i] = pd;
+ }
+
+ if (!priv->children[0] && !priv->children[1])
+ return -ENODEV;
+
+ return 0;
+}
+
+static struct soc_button_info soc_button_INTCFD9[] = {
+ { "power", 0, EV_KEY, KEY_POWER, false, true },
+ { "home", 1, EV_KEY, KEY_HOME, false, true },
+ { "volume_up", 2, EV_KEY, KEY_VOLUMEUP, true, false },
+ { "volume_down", 3, EV_KEY, KEY_VOLUMEDOWN, true, false },
+ { "rotation_lock", 4, EV_SW, SW_ROTATE_LOCK, false, false },
+ { }
+};
+
+static const struct pnp_device_id soc_button_pnp_match[] = {
+ { .id = "INTCFD9", .driver_data = (long)soc_button_INTCFD9 },
+ { .id = "" }
+};
+MODULE_DEVICE_TABLE(pnp, soc_button_pnp_match);
+
+static struct pnp_driver soc_button_pnp_driver = {
+ .name = KBUILD_MODNAME,
+ .id_table = soc_button_pnp_match,
+ .probe = soc_button_pnp_probe,
+ .remove = soc_button_remove,
+};
+
+static int __init soc_button_init(void)
+{
+ return pnp_register_driver(&soc_button_pnp_driver);
+}
+
+static void __exit soc_button_exit(void)
+{
+ pnp_unregister_driver(&soc_button_pnp_driver);
+}
+
+module_init(soc_button_init);
+module_exit(soc_button_exit);
+
+MODULE_LICENSE("GPL");
^ permalink raw reply related
* Re: [RFC PATCH] HID: Stop hiding options with !EXPERT
From: Jiri Kosina @ 2014-03-28 22:13 UTC (permalink / raw)
To: Jean Delvare; +Cc: linux-input
In-Reply-To: <20140321094636.66fc62f2@endymion.delvare>
On Fri, 21 Mar 2014, Jean Delvare wrote:
> Many HID driver options are hidden unless EXPERT is set. While I
> understand the idea of simplifying the kernel configuration for most
> users, in practice I believe it adds more confusion than it helps.
Hi Jean,
actually this question has been raised by several people in the past
already. Copy/pasting the latest reply I have sent on this topic
===
mostly this is because we don't want to bother users with asking for every
single quirky device/vendor, as there are unfortunately a lot of them.
Usually compiling everything in doesn't waste runtime footprint (the
modules don't get loaded unless needed), and they don't cost too much disk
space either.
And if you really want to disable them, CONFIG_EMBEDDED allows you to do
that.
Please see last paragraph on
https://lkml.org/lkml/2008/10/14/266
===
i.e. it was explicitly requested by Linus.
That's the history.
Now, admittedly, it was quite some time ago, and HID world has matured a
lot since then, so the 'quirks' turned out into proper drivers. So maybe
there indeed might be a good time to try it again.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* RE: [PATCH] Add VID/PID for HID-type Multi-Touch Module of AFO CO., LTD.
From: Jiri Kosina @ 2014-03-28 23:17 UTC (permalink / raw)
To: YongHwan Ki; +Cc: rydberg, linux-input, linux-kernel
In-Reply-To: <003a01cf488c$ab9c4400$02d4cc00$@afoi.co.kr>
On Wed, 26 Mar 2014, YongHwan Ki wrote:
> Sorry, I woud like to add the AFO defines in the Linux Kernel.
> No afo defines exists in the current kernel tree.
> I correctly changed the log for adding the afo defines.
>
> Kernel Version : linux-3.14.rc7
> Signed-off-by: Yonghwan Ki <kyhw@afoi.co.kr>
>
> diff -uprN -X Documentation/dontdiff ./drivers/hid/hid-core.c ../linux-3.14-rc7m/drivers/hid/hid-core.c
> --- ./drivers/hid/hid-core.c 2014-03-17 10:51:24.000000000 +0900
> +++ ../linux-3.14-rc7m/drivers/hid/hid-core.c 2014-03-21 17:41:51.846939444 +0900
> @@ -1881,6 +1881,8 @@ static const struct hid_device_id hid_ha
> { HID_USB_DEVICE(USB_VENDOR_ID_ZEROPLUS, 0x0005) },
> { HID_USB_DEVICE(USB_VENDOR_ID_ZEROPLUS, 0x0030) },
> { HID_USB_DEVICE(USB_VENDOR_ID_ZYDACRON, USB_DEVICE_ID_ZYDACRON_REMOTE_CONTROL) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_AFO, USB_DEVICE_ID_AFO_TCM) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_AFO, USB_DEVICE_ID_AFO_THM) },
>
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_PRESENTER_8K_BT) },
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_WIIMOTE) },
> diff -uprN -X Documentation/dontdiff ./drivers/hid/hid-ids.h ../linux-3.14-rc7m/drivers/hid/hid-ids.h
> --- ./drivers/hid/hid-ids.h 2014-03-17 10:51:24.000000000 +0900
> +++ ../linux-3.14-rc7m/drivers/hid/hid-ids.h 2014-03-21 17:30:34.478959555 +0900
> @@ -960,5 +960,9 @@
> #define USB_VENDOR_ID_PRIMAX 0x0461
> #define USB_DEVICE_ID_PRIMAX_KEYBOARD 0x4e05
>
> +#define USB_VENDOR_ID_AFO 0x2576
> +#define USB_DEVICE_ID_AFO_TCM 0x0003
> +#define USB_DEVICE_ID_AFO_BL 0x0005
> +#define USB_DEVICE_ID_AFO_THM 0x0011
>
> #endif
> diff -uprN -X Documentation/dontdiff ./drivers/hid/hid-multitouch.c ../linux-3.14-rc7m/drivers/hid/hid-multitouch.c
> --- ./drivers/hid/hid-multitouch.c 2014-03-17 10:51:24.000000000 +0900
> +++ ../linux-3.14-rc7m/drivers/hid/hid-multitouch.c 2014-03-21 17:45:25.946933088 +0900
> @@ -1395,6 +1395,15 @@ static const struct hid_device_id mt_dev
> { .driver_data = MT_CLS_WIN_8,
> HID_DEVICE(HID_BUS_ANY, HID_GROUP_MULTITOUCH_WIN_8,
> HID_ANY_ID, HID_ANY_ID) },
> +
> + /* AFO MultiTouch device */
> + { .driver_data = MT_CLS_SERIAL,
> + HID_USB_DEVICE(USB_VENDOR_ID_AFO,
> + USB_DEVICE_ID_AFO_TCM) },
> + { .driver_data = MT_CLS_SERIAL,
> + HID_USB_DEVICE(USB_VENDOR_ID_AFO,
> + USB_DEVICE_ID_AFO_THM) },
> +
> { }
> };
> MODULE_DEVICE_TABLE(hid, mt_devices);
> Binary files ./kernel/x509_certificate_list and ../linux-3.14-rc7m/kernel/x509_certificate_list differ
> diff -uprN -X Documentation/dontdiff ./lib/oid_registry_data.c ../linux-3.14-rc7m/lib/oid_registry_data.c
> --- ./lib/oid_registry_data.c 2014-03-21 18:45:37.598825862 +0900
> +++ ../linux-3.14-rc7m/lib/oid_registry_data.c 2014-03-24 21:43:59.826812763 +0900
> @@ -1,5 +1,5 @@
> /*
> - * Automatically generated by /usr/src/linux-3.14-rc7/lib/build_OID_registry. Do not edit
> + * Automatically generated by /usr/src/linux-3.14-rc7m/lib/build_OID_registry. Do not edit
Please remove this hunk, it has nothing to do with the patchset.
> */
>
> static const unsigned short oid_index[OID__NR + 1] = {
> diff -uprN -X Documentation/dontdiff ./signing_key.priv ../linux-3.14-rc7m/signing_key.priv
> --- ./signing_key.priv 2014-03-21 17:24:11.874970914 +0900
> +++ ../linux-3.14-rc7m/signing_key.priv 2014-03-24 20:22:01.946958769 +0900
> @@ -1,52 +1,52 @@
> -----BEGIN PRIVATE KEY-----
Again, I don't think you want to be sending this out :)
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [PATCH v2 1/1] Input: don't modify the id of ioctl-provided ff effect on upload failure
From: Elias Vanderstuyft @ 2014-03-29 12:16 UTC (permalink / raw)
To: dmitry.torokhov
Cc: Elias Vanderstuyft, Anssi Hannula, Michal Malý, linux-input,
linux-kernel
If a new (id == -1) ff effect was uploaded from userspace,
ff-core.c::input_ff_upload() will have assigned
a positive number to the new effect id.
Currently, evdev.c::evdev_do_ioctl() will save this new id to userspace,
regardless of whether the upload succeeded or not.
On upload failure, this can be confusing because the dev->ff->effects[] array
will not contain an element at the index of that new effect id.
This patch fixes this by leaving the id unchanged after upload fails.
Note: Unfortunately applications should still expect changed effect id for
quite some time.
This has been discussed on:
http://www.mail-archive.com/linux-input@vger.kernel.org/msg08513.html
("ff-core effect id handling in case of a failed effect upload")
Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Elias Vanderstuyft <elias.vds@gmail.com>
Cc: Anssi Hannula <anssi.hannula@iki.fi>
Cc: Michal Malý <madcatxster@devoid-pointer.net>
Cc: linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
v2:
Only added one line to the commit message to say
what this patch actually does,
instead of only stating the reason why it's submitted.
drivers/input/evdev.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index a06e125..ce953d8 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -954,11 +954,13 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
return -EFAULT;
error = input_ff_upload(dev, &effect, file);
+ if (error)
+ return error;
if (put_user(effect.id, &(((struct ff_effect __user *)p)->id)))
return -EFAULT;
- return error;
+ return 0;
}
/* Multi-number variable-length handlers */
--
1.8.3.1
^ permalink raw reply related
* Re: [RFC PATCH] HID: Stop hiding options with !EXPERT
From: Jean Delvare @ 2014-03-29 14:06 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input
In-Reply-To: <alpine.LNX.2.00.1403281511120.16202@pobox.suse.cz>
Hi Jiri,
On Fri, 28 Mar 2014 15:13:39 -0700 (PDT), Jiri Kosina wrote:
> On Fri, 21 Mar 2014, Jean Delvare wrote:
>
> > Many HID driver options are hidden unless EXPERT is set. While I
> > understand the idea of simplifying the kernel configuration for most
> > users, in practice I believe it adds more confusion than it helps.
>
> actually this question has been raised by several people in the past
> already. Copy/pasting the latest reply I have sent on this topic
>
> ===
> mostly this is because we don't want to bother users with asking for every
> single quirky device/vendor, as there are unfortunately a lot of them.
> Usually compiling everything in doesn't waste runtime footprint (the
> modules don't get loaded unless needed), and they don't cost too much disk
> space either.
The problem is that the hidden options may end up built-in, not
modular, in which case they are actually wasting runtime footprint, and
polluting /sys/bus/hid/drivers.
> And if you really want to disable them, CONFIG_EMBEDDED allows you to do
> that.
True, but CONFIG_EMBEDDED / CONFIG_EXPERT are tree-wide settings.
Enabling them un-hides a lot more options than just these HID drivers.
Not everyone wants that.
Other subsystems have solved the problem with specific options, which
give the user more flexibility over which areas he/she wants to
fine-tune. See for example CONFIG_I2C_HELPER_AUTO and
CONFIG_MEDIA_SUBDRV_AUTOSELECT. That might be an alternative if you
really think that these drivers entries should be hidden by default (I
don't.)
> Please see last paragraph on
>
> https://lkml.org/lkml/2008/10/14/266
>
> ===
>
> i.e. it was explicitly requested by Linus.
>
> That's the history.
Well, I see that Linus was mostly unhappy because the of the new
warnings introduced by the change. Plus...
> Now, admittedly, it was quite some time ago, and HID world has matured a
> lot since then, so the 'quirks' turned out into proper drivers. So maybe
> there indeed might be a good time to try it again.
... these were boolean quirks, and IIRC everything ended up being built
in the same module. So the discussion was relevant back then.
Now we have separate drivers, controlled by tristate options. This is a
completely different situation.
Also, many drivers were added since then. I doubt that hiding 11
drivers, while about 50 are always presented to the user, really helps
easing the configuration, if that was the actual goal.
In fact, looking at the git history, I can see that more drivers used
to depend on CONFIG_EMBEDDED / CONFIG_EXPERT, and these dependencies
were removed over time:
commit 4b186f72033611c2b526c7341534e71ee4afd222
Author: Jiri Kosina <jkosina@suse.cz>
Date: Wed Dec 23 13:12:32 2009 +0100
HID: make 3M PCT touchscreen driver standalone config option
commit 92688c0c3c1c9e2daf705d307e8fda1b5a180d26
Author: Jiri Kosina <jkosina@suse.cz>
Date: Mon Jan 4 12:04:59 2010 +0100
HID: make Stantum driver standalone config option
commit 2dbf209d7a7ab94266b936bd2da6a4026c279992
Author: Jiri Kosina <jkosina@suse.cz>
Date: Wed Feb 3 16:11:12 2010 +0100
HID: make full-fledged hid-bus drivers properly selectable
commit 23d386d85a9144612c4a13733aa1ca6e5a21f4a2
Author: Jiri Kosina <jkosina@suse.cz>
Date: Mon Mar 22 16:33:15 2010 +0100
HID: fixup Kconfig entry for Roccat Kone
commit 95736de984dec5b80ea9d6640d4d55ca8ff98db4
Author: Jiri Kosina <jkosina@suse.cz>
Date: Wed May 12 15:27:00 2010 +0200
HID: make Prodikeys driver standalone config option
commit 73d5e8f77e88a4d3a154dfdbb4ed2cf461b7bf21
Author: Jiri Kosina <jkosina@suse.cz>
Date: Fri May 21 13:15:17 2010 +0200
HID: fix up 'EMBEDDED' mess in Kconfig
commit f36ee074d5d563a832fbfc378207739db3a0a205
Author: Jiri Kosina <jkosina@suse.cz>
Date: Mon Aug 9 19:56:01 2010 +0200
HID: uclogic: fix up Kconfig entry
commit dfe9a31211c0a3a0252af6c87935d7ac718aadf9
Author: Jiri Kosina <jkosina@suse.cz>
Date: Mon Oct 17 17:04:58 2011 +0200
HID: primax: remove spurious dependency
commit 22ca20b250f5c9672a53b34f032f43dd2c4a4aaf
Author: Nikolai Kondrashov <spbnick@gmail.com>
Date: Tue Feb 28 13:01:46 2012 +0200
HID: kye: Add support for 3 tablets
And I may have missed some. So all I am really asking here, is that we
finish a cleanup that already started over 4 years ago.
If this is really only a question of which devices are more likely to
be used by a large number of users, then that's what defaults are for.
Thanks,
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply
* Re: Re: [PATCH v3 04/10] input: misc: Add driver for AXP20x Power Enable Key
From: Carlo Caione @ 2014-03-29 15:36 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <20140328073803.GA22093-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>
On Fri, Mar 28, 2014 at 12:38:03AM -0700, Dmitry Torokhov wrote:
> Hi Carlo,
Hi Dmitry,
>
> On Thu, Mar 27, 2014 at 10:29:18PM +0100, Carlo Caione wrote:
[...]
> > +#include <linux/errno.h>
> > +#include <linux/irq.h>
> > +#include <linux/init.h>
> > +#include <linux/input.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/kernel.h>
> > +#include <linux/mfd/axp20x.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/slab.h>
> > +
> > +#define AXP20X_PEK_STARTUP_MASK (0xc0)
> > +#define AXP20X_PEK_SHUTDOWN_MASK (0x03)
> > +
> > +static const char const *startup_time[] = { "128ms", "3s" , "1s", "2s" };
> > +static const char const *shutdown_time[] = { "4s", "6s" , "8s", "10s" };
>
> Why not have everything expressed in milliseconds and have sysfs
> attribute apply the closest one possible?
Yep, I agree that could be nice. Fix in v3.
> By the way, do you want to plumb these through device tree as well?
Hum, not sure about that. AFAIK device tree is used for hardware
description whereas these are configuration parameters. Moreover you
would have these parameters saved through reboots.
> > +
> > +struct axp20x_pek {
> > + struct axp20x_dev *axp20x;
> > + struct input_dev *input;
> > + int irq_dbr;
> > + int irq_dbf;
> > +};
> > +
> > +struct axp20x_pek_ext_attr {
> > + const char const **str;
> > + unsigned int mask;
> > +};
> > +
> > +static struct axp20x_pek_ext_attr axp20x_pek_startup_ext_attr = {
> > + .str = startup_time,
> > + .mask = AXP20X_PEK_STARTUP_MASK,
> > +};
> > +
> > +static struct axp20x_pek_ext_attr axp20x_pek_shutdown_ext_attr = {
> > + .str = shutdown_time,
> > + .mask = AXP20X_PEK_SHUTDOWN_MASK,
> > +};
> > +
> > +static struct axp20x_pek_ext_attr *get_axp_ext_attr(struct device_attribute *attr)
> > +{
> > + return container_of(attr, struct dev_ext_attribute, attr)->var;
> > +}
> > +
> > +static ssize_t axp20x_show_ext_attr(struct device *dev, struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
> > + struct axp20x_pek_ext_attr *axp20x_ea = get_axp_ext_attr(attr);
> > + unsigned int val;
> > + int ret, i;
> > + int cnt = 0;
> > +
> > + ret = regmap_read(axp20x_pek->axp20x->regmap, AXP20X_PEK_KEY, &val);
> > + if (ret != 0)
> > + return ret;
> > +
> > + val &= axp20x_ea->mask;
> > + val >>= ffs(axp20x_ea->mask) - 1;
> > +
> > + for (i = 0; i < 4; i++) {
> > + if (val == i)
> > + cnt += sprintf(buf + cnt, "[%s] ", axp20x_ea->str[i]);
> > + else
> > + cnt += sprintf(buf + cnt, "%s ", axp20x_ea->str[i]);
>
> Please just return the current value; why do we need pretty-printing?
It was done to have the possibility to look at the accepted values. With
the modification you suggested before this is not necessary anymore.
I'll change it.
> > + }
> > +
> > + cnt += sprintf(buf + cnt, "\n");
> > +
> > + return cnt;
> > +}
> > +
> > +static ssize_t axp20x_store_ext_attr(struct device *dev, struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
> > + struct axp20x_pek_ext_attr *axp20x_ea = get_axp_ext_attr(attr);
> > + char val_str[20];
> > + int ret, i;
> > + size_t len;
> > +
> > + val_str[sizeof(val_str) - 1] = '\0';
> > + strncpy(val_str, buf, sizeof(val_str) - 1);
> > + len = strlen(val_str);
> > +
> > + if (len && val_str[len - 1] == '\n')
> > + val_str[len - 1] = '\0';
> > +
> > + for (i = 0; i < 4; i++) {
> > + if (!strcmp(val_str, axp20x_ea->str[i])) {
> > +
> > + i <<= ffs(axp20x_ea->mask) - 1;
> > + ret = regmap_update_bits(axp20x_pek->axp20x->regmap,
> > + AXP20X_PEK_KEY,
> > + axp20x_ea->mask, i);
> > + if (ret != 0)
> > + return -EINVAL;
> > + return count;
> > + }
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +
> > +static struct dev_ext_attribute axp20x_dev_attr_startup = {
> > + .attr = __ATTR(startup, 0644, axp20x_show_ext_attr, axp20x_store_ext_attr),
> > + .var = &axp20x_pek_startup_ext_attr
> > +};
> > +
> > +static struct dev_ext_attribute axp20x_dev_attr_shutdown = {
> > + __ATTR(shutdown, 0644, axp20x_show_ext_attr, axp20x_store_ext_attr),
> > + &axp20x_pek_shutdown_ext_attr
> > +};
> > +
> > +static struct attribute *dev_attrs[] = {
> > + &axp20x_dev_attr_startup.attr.attr,
> > + &axp20x_dev_attr_shutdown.attr.attr,
> > + NULL,
> > +};
> > +
> > +static struct attribute_group dev_attr_group = {
> > + .attrs = dev_attrs,
> > +};
> > +
> > +static const struct attribute_group *dev_attr_groups[] = {
> > + &dev_attr_group,
> > + NULL,
> > +};
> > +
> > +static irqreturn_t axp20x_pek_irq(int irq, void *pwr)
> > +{
> > + struct input_dev *idev = pwr;
> > + struct axp20x_pek *axp20x_pek = input_get_drvdata(idev);
> > +
> > + if (irq == axp20x_pek->irq_dbr)
> > + input_report_key(idev, KEY_POWER, true);
> > + else if (irq == axp20x_pek->irq_dbf)
> > + input_report_key(idev, KEY_POWER, false);
> > +
> > + input_sync(idev);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int axp20x_pek_probe(struct platform_device *pdev)
> > +{
> > + struct axp20x_pek *axp20x_pek;
> > + struct axp20x_dev *axp20x;
> > + struct input_dev *idev;
> > + int error;
> > +
> > + axp20x_pek = devm_kzalloc(&pdev->dev, sizeof(struct axp20x_pek),
> > + GFP_KERNEL);
> > + if (!axp20x_pek)
> > + return -ENOMEM;
> > +
> > + axp20x_pek->axp20x = dev_get_drvdata(pdev->dev.parent);
> > + axp20x = axp20x_pek->axp20x;
> > +
> > + axp20x_pek->irq_dbr = platform_get_irq_byname(pdev, "PEK_DBR");
> > + if (axp20x_pek->irq_dbr < 0) {
> > + dev_err(&pdev->dev, "No IRQ for PEK_DBR, error=%d\n",
> > + axp20x_pek->irq_dbr);
> > + return axp20x_pek->irq_dbr;
> > + }
> > + axp20x_pek->irq_dbr = regmap_irq_get_virq(axp20x->regmap_irqc,
> > + axp20x_pek->irq_dbr);
> > +
> > + axp20x_pek->irq_dbf = platform_get_irq_byname(pdev, "PEK_DBF");
> > + if (axp20x_pek->irq_dbf < 0) {
> > + dev_err(&pdev->dev, "No IRQ for PEK_DBF, error=%d\n",
> > + axp20x_pek->irq_dbf);
> > + return axp20x_pek->irq_dbf;
> > + }
> > + axp20x_pek->irq_dbf = regmap_irq_get_virq(axp20x->regmap_irqc,
> > + axp20x_pek->irq_dbf);
> > +
> > + axp20x_pek->input = devm_input_allocate_device(&pdev->dev);
> > + if (!axp20x_pek->input)
> > + return -ENOMEM;
> > +
> > + idev = axp20x_pek->input;
> > +
> > + idev->name = "axp20x-pek";
> > + idev->phys = "m1kbd/input2";
> > + idev->dev.parent = &pdev->dev;
> > +
> > + input_set_capability(idev, EV_KEY, KEY_POWER);
> > +
> > + input_set_drvdata(idev, axp20x_pek);
> > +
> > + error = devm_request_threaded_irq(&pdev->dev, axp20x_pek->irq_dbr,
> > + NULL, axp20x_pek_irq, 0,
> > + "axp20x-pek-dbr", idev);
>
> Why does it have to be threaded IRQ?
Because this is already handled as a nested irq from a irq thread. The
parent threaded irq is installed by regmap_add_irq_chip() in the MFD
driver core.
> > + if (error) {
> > + dev_err(axp20x->dev, "Failed to request dbr IRQ#%d: %d\n",
> > + axp20x_pek->irq_dbr, error);
> > +
> > + return error;
> > + }
> > +
> > + error = devm_request_threaded_irq(&pdev->dev, axp20x_pek->irq_dbf,
> > + NULL, axp20x_pek_irq, 0,
> > + "axp20x-pek-dbf", idev);
> > + if (error) {
> > + dev_err(axp20x->dev, "Failed to request dbf IRQ#%d: %d\n",
> > + axp20x_pek->irq_dbf, error);
> > + return error;
> > + }
> > +
> > + idev->dev.groups = dev_attr_groups;
>
> These are not generic input attributes so they should belong to the platform
> device, not input device.
Ok. Can I ask why they cannot be considered an input attributes?
> > + error = input_register_device(idev);
> > + if (error) {
> > + dev_err(axp20x->dev, "Can't register input device: %d\n", error);
> > + return error;
> > + }
> > +
> > + platform_set_drvdata(pdev, axp20x_pek);
> > +
> > + return 0;
> > +}
> > +
> > +static int axp20x_pek_remove(struct platform_device *pdev)
> > +{
> > + struct axp20x_pek *axp20x_pek = platform_get_drvdata(pdev);
> > +
> > + input_unregister_device(axp20x_pek->input);
>
> No need to call input_unregister_device() for managed input devices.
Right. Fix in v3.
> > +
> > + return 0;
> > +}
> > +
> > +static struct platform_driver axp20x_pek_driver = {
> > + .probe = axp20x_pek_probe,
> > + .remove = axp20x_pek_remove,
> > + .driver = {
> > + .name = "axp20x-pek",
> > + .owner = THIS_MODULE,
> > + },
> > +};
> > +module_platform_driver(axp20x_pek_driver);
> > +
> > +MODULE_DESCRIPTION("axp20x Power Button");
> > +MODULE_AUTHOR("Carlo Caione <carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org>");
> > +MODULE_LICENSE("GPL");
> > --
> > 1.8.3.2
> >
>
> Thanks.
Thank you,
--
Carlo Caione
^ permalink raw reply
* Re: Re: [PATCH v3 01/10] mfd: AXP20x: Add mfd driver for AXP20x PMIC
From: Carlo Caione @ 2014-03-29 15:53 UTC (permalink / raw)
To: Lee Jones
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <20140328092154.GF17779@lee--X1>
On Fri, Mar 28, 2014 at 09:21:54AM +0000, Lee Jones wrote:
> > This patch introduces the preliminary support for PMICs X-Powers AXP202
> > and AXP209. The AXP209 and AXP202 are the PMUs (Power Management Unit)
> > used by A10, A13 and A20 SoCs and developed by X-Powers, a sister company
> > of Allwinner.
> >
> > The core enables support for two subsystems:
> > - PEK (Power Enable Key)
> > - Regulators
> >
> > Signed-off-by: Carlo Caione <carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org>
> > ---
> > drivers/mfd/Kconfig | 12 +++
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/axp20x.c | 240 +++++++++++++++++++++++++++++++++++++++++++++
> > include/linux/mfd/axp20x.h | 180 ++++++++++++++++++++++++++++++++++
> > 4 files changed, 433 insertions(+)
> > create mode 100644 drivers/mfd/axp20x.c
> > create mode 100644 include/linux/mfd/axp20x.h
> >
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index 49bb445..24ba61a 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -59,6 +59,18 @@ config MFD_AAT2870_CORE
> > additional drivers must be enabled in order to use the
> > functionality of the device.
> >
> > +config MFD_AXP20X
> > + bool "X-Powers AXP20X"
> > + select MFD_CORE
> > + select REGMAP_I2C
> > + select REGMAP_IRQ
> > + depends on I2C=y
> > + help
> > + If you say Y here you get support for the AXP20X.
> > + This driver provides common support for accessing the device,
> > + additional drivers must be enabled in order to use the
> > + functionality of the device.
>
> Please tell us what this device is and what sub-devices are available?
Ok
> [...]
>
> > diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> > new file mode 100644
> > index 0000000..f77a663
> > --- /dev/null
> > +++ b/drivers/mfd/axp20x.c
> > @@ -0,0 +1,240 @@
> > +/*
> > + * axp20x.c - MFD core driver for the X-Powers AXP202 and AXP209
>
> ... which consist of ...
Yeah, I should improve my code documentation :)
> [...]
>
> > +static struct resource axp20x_pek_resources[] = {
> > + {
> > + .name = "PEK_DBR",
> > + .start = AXP20X_IRQ_PEK_RIS_EDGE,
> > + .end = AXP20X_IRQ_PEK_RIS_EDGE,
> > + .flags = IORESOURCE_IRQ,
> > + }, {
> > + .name = "PEK_DBF",
> > + .start = AXP20X_IRQ_PEK_FAL_EDGE,
> > + .end = AXP20X_IRQ_PEK_FAL_EDGE,
> > + .flags = IORESOURCE_IRQ,
> > + },
> > +};
>
> Have you considered doing this in the Device Tree? It's a lot less
> code/overhead.
Ok, I'll change it in v4.
> > +static const struct i2c_device_id axp20x_i2c_id[] = {
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);
>
> We really should consider changing the I2C subsystem!
>
> Can you add a comment here describing why we have to add this
> seemingly pointless empty struct please?
Sure.
> > +static struct mfd_cell axp20x_cells[] = {
> > + {
> > + .name = "axp20x-pek",
> > + .num_resources = ARRAY_SIZE(axp20x_pek_resources),
> > + .resources = axp20x_pek_resources,
> > + }, {
> > + .name = "axp20x-regulator",
> > + },
> > +};
>
> Do these drivers don't look inside the DTB at all?
Only the regulators driver.
> > + of_id = of_match_device(axp20x_of_match, &i2c->dev);
> > + if (!of_id) {
> > + dev_err(&i2c->dev, "Unable to setup AXP20X data\n");
> > + return -ENODEV;
> > + }
> > + axp20x->variant = (int) of_id->data;
>
> 'variant' needs to be a (unsigned?) long or it will break on 64bit
> architectures.
Ok, I really need to start thinking to 64bit also.
Thank you,
--
Carlo Caione
^ permalink raw reply
* Re: [PATCH v3 07/10] ARM: sunxi: dt: Add x-powers-axp209.dtsi file
From: Carlo Caione @ 2014-03-29 16:14 UTC (permalink / raw)
To: Mark Brown
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20140328133438.GA19846-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
On Fri, Mar 28, 2014 at 01:34:38PM +0000, Mark Brown wrote:
> On Thu, Mar 27, 2014 at 10:29:21PM +0100, Carlo Caione wrote:
> > This dtsi describes the axp209 PMIC, and is to be included from inside
> > the i2c controller node to which the axp209 is connected.
>
> > arch/arm/boot/dts/x-powers-axp209.dtsi | 54 ++++++++++++++++++++++++++++++++++
>
> Something is wrong here. Either the changelog is inaccurate or this is
> broken, either way this should be cleaned up because this looks like
> very bad practice. If this is a common include file for boards derived
> from some reference design then the changelog is misleading and the DT
> should be clearer about the board tie ins. Otherwise the .dtsi is
> defining what should be board specific parameters.
>
> The fact that the DT names everything after the regulator on the PMIC
> and not after the supply names on the board is especially suspicious and
> glancing at a couple of the regulators it looks like the constraints
> here are the maximum ranges the PMIC supports rather than anything to do
> with any board.
>
> Please take a step back and think about what the regulator constraints
> are intended to do - they're about matching the regulator capabilities
> to the board since it is rare for boards to be able to do everything the
> regulator can support. Duplicating the basic device capabilities into
> the DT would be a pointless waste of time.
>
> > + axp_dcdc2: dcdc2 {
> > + regulator-min-microvolt = <700000>;
> > + regulator-max-microvolt = <2275000>;
> > + regulator-always-on;
> > + };
>
> What is this configuration actually for - what guarantee do we have that
> the above is safe for a given board using this regulator?
>
> In general I'd expect to see anything specifying variability for a
> regulator to also include the relevant consumer node.
Ok, you are completely right. These values in the DTSI are wrongly the
maximum and minimum ranges for the PMIC. Even worst I didn't specify the
board specific parameters for the regulators.
Thank you for reviewing and noticing it,
--
Carlo Caione
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH v3 06/10] regulator: AXP20x: Add support for regulators subsystem
From: Carlo Caione @ 2014-03-29 17:52 UTC (permalink / raw)
To: Mark Brown
Cc: linux-arm-kernel, linux-sunxi, maxime.ripard, hdegoede, emilio,
wens, sameo, dmitry.torokhov, linux-input, linux-doc, lgirdwood
In-Reply-To: <20140328133934.GQ30768@sirena.org.uk>
On Fri, Mar 28, 2014 at 01:39:34PM +0000, Mark Brown wrote:
> On Thu, Mar 27, 2014 at 10:29:20PM +0100, Carlo Caione wrote:
>
> > +static int axp20x_set_suspend_voltage(struct regulator_dev *rdev, int uV)
> > +{
> > + int sel = regulator_map_voltage_iterate(rdev, uV, uV);
> > +
> > + if (sel < 0)
> > + return sel;
> > +
> > + return regulator_set_voltage_sel_regmap(rdev, sel);
> > +}
>
> This is fairly obviously broken - it's overwriting the normal runtime
> value, this will disrupt the running system if we want the value we use
> on suspend is different to the value we want at runtime.
Ok, silly question: isn't it exactly what we want? Set the voltage for
the regulator when the system is suspended?
> Think about it - if this was a sane thing to do the core would just do
> it without needing driver specific code, we already know how to set the
> voltage for the device.
I thought it was because some regulators can have specific regs for
managing the suspend mode.
BTW, but then what is the difference between my code and (i.e.) the same
routine in da9055-regulator.c?
http://lxr.linux.no/linux+v3.13.5/drivers/regulator/da9055-regulator.c#L276
Thanks,
--
Carlo Caione
^ permalink raw reply
* Re: Re: [PATCH v3 04/10] input: misc: Add driver for AXP20x Power Enable Key
From: Dmitry Torokhov @ 2014-03-29 19:05 UTC (permalink / raw)
To: Carlo Caione
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <20140329153624.GA3952-bi+AKbBUZKZeIdyRz4JgOMwOAu8XWILU@public.gmane.org>
On Sat, Mar 29, 2014 at 04:36:24PM +0100, Carlo Caione wrote:
> On Fri, Mar 28, 2014 at 12:38:03AM -0700, Dmitry Torokhov wrote:
> > Hi Carlo,
>
> Hi Dmitry,
>
> >
> > On Thu, Mar 27, 2014 at 10:29:18PM +0100, Carlo Caione wrote:
[...]
> > > +
> > > + error = devm_request_threaded_irq(&pdev->dev, axp20x_pek->irq_dbr,
> > > + NULL, axp20x_pek_irq, 0,
> > > + "axp20x-pek-dbr", idev);
> >
> > Why does it have to be threaded IRQ?
>
> Because this is already handled as a nested irq from a irq thread. The
> parent threaded irq is installed by regmap_add_irq_chip() in the MFD
> driver core.
Then you probably want to use devm_request_any_context_irq() - the
driver does not necessarily care if IRQ is threaded or not, t can work
either way.
>
> > > + if (error) {
> > > + dev_err(axp20x->dev, "Failed to request dbr IRQ#%d: %d\n",
> > > + axp20x_pek->irq_dbr, error);
> > > +
> > > + return error;
> > > + }
> > > +
> > > + error = devm_request_threaded_irq(&pdev->dev, axp20x_pek->irq_dbf,
> > > + NULL, axp20x_pek_irq, 0,
> > > + "axp20x-pek-dbf", idev);
> > > + if (error) {
> > > + dev_err(axp20x->dev, "Failed to request dbf IRQ#%d: %d\n",
> > > + axp20x_pek->irq_dbf, error);
> > > + return error;
> > > + }
> > > +
> > > + idev->dev.groups = dev_attr_groups;
> >
> > These are not generic input attributes so they should belong to the platform
> > device, not input device.
>
> Ok. Can I ask why they cannot be considered an input attributes?
Input layer is an abstraction; I am trying to keep all attributes
generic and applicable to all input devices. If you look at other input
drivers you will see that attributes that control hardware behavior are
attached to the devices representing the hardware itself. For example
attributes to control report rate of PS/2 mouse belong to serio port
that represents the physical mouse device; typematic rate of AT
keyboards also tied to serio port, etc, etc.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 1/1] Input: don't modify the id of ioctl-provided ff effect on upload failure
From: Dmitry Torokhov @ 2014-03-29 19:12 UTC (permalink / raw)
To: Elias Vanderstuyft
Cc: Anssi Hannula, Michal Malý, linux-input, linux-kernel
In-Reply-To: <1396095396-23551-1-git-send-email-elias.vds@gmail.com>
On Sat, Mar 29, 2014 at 01:16:36PM +0100, Elias Vanderstuyft wrote:
> If a new (id == -1) ff effect was uploaded from userspace,
> ff-core.c::input_ff_upload() will have assigned
> a positive number to the new effect id.
> Currently, evdev.c::evdev_do_ioctl() will save this new id to userspace,
> regardless of whether the upload succeeded or not.
>
> On upload failure, this can be confusing because the dev->ff->effects[] array
> will not contain an element at the index of that new effect id.
>
> This patch fixes this by leaving the id unchanged after upload fails.
>
> Note: Unfortunately applications should still expect changed effect id for
> quite some time.
>
> This has been discussed on:
> http://www.mail-archive.com/linux-input@vger.kernel.org/msg08513.html
> ("ff-core effect id handling in case of a failed effect upload")
>
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Elias Vanderstuyft <elias.vds@gmail.com>
> Cc: Anssi Hannula <anssi.hannula@iki.fi>
> Cc: Michal Malý <madcatxster@devoid-pointer.net>
> Cc: linux-input@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
Applied, thank you, and sorry for the delay.
> ---
> v2:
> Only added one line to the commit message to say
> what this patch actually does,
> instead of only stating the reason why it's submitted.
>
> drivers/input/evdev.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index a06e125..ce953d8 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -954,11 +954,13 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> return -EFAULT;
>
> error = input_ff_upload(dev, &effect, file);
> + if (error)
> + return error;
>
> if (put_user(effect.id, &(((struct ff_effect __user *)p)->id)))
> return -EFAULT;
>
> - return error;
> + return 0;
> }
>
> /* Multi-number variable-length handlers */
> --
> 1.8.3.1
>
--
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 v5 2/3] input: appletouch: implement sensor data smoothing
From: Clinton Sprain @ 2014-03-29 21:47 UTC (permalink / raw)
To: Dmitry Torokhov, Clinton Sprain; +Cc: linux-input, Henrik Rydberg
In-Reply-To: <20140327182606.GA8902@core.coreip.homeip.net>
input: appletouch: implement sensor data smoothing
Use smoothed version of sensor array data to calculate movement and add weight
to prior values when calculating average. This gives more granular and more
predictable movement.
Changes since v4:
- Scale is now defined as ATP_SCALE
- Smoothing scratch buffers now a part of dev structure
- Now passing dev into atp_calculate_abs
- Now leaving atp_calculate_abs early if no fingers are detected
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 110 +++++++++++++++++++++++++++-----------
1 file changed, 80 insertions(+), 30 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 2745832..8e01363 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -161,6 +161,12 @@ MODULE_DEVICE_TABLE(usb, atp_table);
#define ATP_XSENSORS 26
#define ATP_YSENSORS 16
+/*
+ * The largest possible bank of sensors with additional buffer of 4 extra values
+ * on either side, for an array of smoothed sensor values.
+ */
+#define ATP_SMOOTHSIZE 34
+
/* maximum pressure this driver will report */
#define ATP_PRESSURE 300
@@ -168,7 +174,13 @@ MODULE_DEVICE_TABLE(usb, atp_table);
* Threshold for the touchpad sensors. Any change less than ATP_THRESHOLD is
* ignored.
*/
-#define ATP_THRESHOLD 5
+#define ATP_THRESHOLD 5
+
+/*
+ * How far we'll bitshift our sensor values before averaging them. Mitigates
+ * rounding errors.
+ */
+#define ATP_SCALE 12
/* Geyser initialization constants */
#define ATP_GEYSER_MODE_READ_REQUEST_ID 1
@@ -211,6 +223,8 @@ struct atp {
signed char xy_cur[ATP_XSENSORS + ATP_YSENSORS];
signed char xy_old[ATP_XSENSORS + ATP_YSENSORS];
int xy_acc[ATP_XSENSORS + ATP_YSENSORS];
+ int smooth[ATP_SMOOTHSIZE];
+ int smooth_tmp[ATP_SMOOTHSIZE];
int idlecount; /* number of empty packets */
struct work_struct work;
};
@@ -329,10 +343,15 @@ static void atp_reinit(struct work_struct *work)
retval);
}
-static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
- int *z, int *fingers)
+static int atp_calculate_abs(struct atp *dev, int offset, int nb_sensors,
+ int fact, int *z, int *fingers)
{
- int i;
+ int i, k;
+
+ /* Use offset to point xy_sensors at the first value in dev->xy_acc */
+ /* for whichever dimension we're looking at this particular go-round. */
+ int *xy_sensors = dev->xy_acc + offset;
+
/* values to calculate mean */
int pcum = 0, psum = 0;
int is_increasing = 0;
@@ -344,9 +363,6 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
if (is_increasing)
is_increasing = 0;
- continue;
- }
-
/*
* Makes the finger detection more versatile. For example,
* two fingers with no gap will be detected. Also, my
@@ -361,27 +377,60 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
*
* - Jason Parekh <jasonparekh@gmail.com>
*/
- if (i < 1 ||
+
+ } else if (i < 1 ||
(!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
(*fingers)++;
is_increasing = 1;
} else if (i > 0 && (xy_sensors[i - 1] - xy_sensors[i] > threshold)) {
is_increasing = 0;
}
+ }
- /*
- * Subtracts threshold so a high sensor that just passes the
- * threshold won't skew the calculated absolute coordinate.
- * Fixes an issue where slowly moving the mouse would
- * occasionally jump a number of pixels (slowly moving the
- * finger makes this issue most apparent.)
- */
- pcum += (xy_sensors[i] - threshold) * i;
- psum += (xy_sensors[i] - threshold);
+ if (*fingers < 1) /* No need to continue if no fingers are found. */
+ return 0;
+
+ /*
+ * Use a smoothed version of sensor data for movement calculations, to
+ * combat noise without needing to rely so heavily on a threshold.
+ * This improves tracking.
+ *
+ * The smoothed array is bigger than the original so that the smoothing
+ * doesn't result in edge values being truncated.
+ */
+
+ memset(dev->smooth, 0, sizeof(dev->smooth));
+ memset(dev->smooth_tmp, 0, sizeof(dev->smooth_tmp));
+
+ /* Pull base values, scaled up to help avoid truncation errors. */
+
+ for (i = 0; i < nb_sensors; i++)
+ dev->smooth[i + 4] = xy_sensors[i] << ATP_SCALE;
+
+ for (k = 0; k < 4; k++) {
+ /* Handle edge. */
+ dev->smooth_tmp[0] = (dev->smooth[0] + dev->smooth[1]) >> 1;
+
+ /* Average values with neighbors. */
+ for (i = 1; i < nb_sensors + 7; i++)
+ dev->smooth_tmp[i] = (dev->smooth[i - 1] + dev->smooth[i] * 2 + dev->smooth[i + 1]) >> 2;
+
+ /* Handle other edge. */
+ dev->smooth_tmp[nb_sensors + 7] = (dev->smooth[nb_sensors + 7] + dev->smooth[nb_sensors + 6]) >> 1;
+
+ for (i = 0; i < nb_sensors + 8; i++)
+ dev->smooth[i] = dev->smooth_tmp[i];
+ }
+
+ for (i = 0; i < nb_sensors + 8; i++) {
+ if ((dev->smooth[i] >> ATP_SCALE) > 0) { /* Skip values if */
+ pcum += (dev->smooth[i]) * i; /* they're small enough */
+ psum += (dev->smooth[i]); /* to be truncated to 0 */
+ } /* by scale. Mostly noise. */
}
if (psum > 0) {
- *z = psum;
+ *z = psum >> ATP_SCALE; /* Scale down pressure output. */
return pcum * fact / psum;
}
@@ -551,16 +600,16 @@ static void atp_complete_geyser_1_2(struct urb *urb)
dbg_dump("accumulator", dev->xy_acc);
- x = atp_calculate_abs(dev->xy_acc, ATP_XSENSORS,
- dev->info->xfact, &x_z, &x_f);
- y = atp_calculate_abs(dev->xy_acc + ATP_XSENSORS, ATP_YSENSORS,
- dev->info->yfact, &y_z, &y_f);
+ x = atp_calculate_abs(dev, 0, ATP_XSENSORS, dev->info->xfact,
+ &x_z, &x_f);
+ y = atp_calculate_abs(dev, ATP_XSENSORS, ATP_YSENSORS, dev->info->yfact,
+ &y_z, &y_f);
key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
if (x && y) {
if (dev->x_old != -1) {
- x = (dev->x_old * 3 + x) >> 2;
- y = (dev->y_old * 3 + y) >> 2;
+ x = (dev->x_old * 7 + x) >> 3;
+ y = (dev->y_old * 7 + y) >> 3;
dev->x_old = x;
dev->y_old = y;
@@ -663,16 +712,17 @@ static void atp_complete_geyser_3_4(struct urb *urb)
dbg_dump("accumulator", dev->xy_acc);
- x = atp_calculate_abs(dev->xy_acc, ATP_XSENSORS,
- dev->info->xfact, &x_z, &x_f);
- y = atp_calculate_abs(dev->xy_acc + ATP_XSENSORS, ATP_YSENSORS,
- dev->info->yfact, &y_z, &y_f);
+ x = atp_calculate_abs(dev, 0, ATP_XSENSORS, dev->info->xfact,
+ &x_z, &x_f);
+ y = atp_calculate_abs(dev, ATP_XSENSORS, ATP_YSENSORS, dev->info->yfact,
+ &y_z, &y_f);
+
key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
if (x && y) {
if (dev->x_old != -1) {
- x = (dev->x_old * 3 + x) >> 2;
- y = (dev->y_old * 3 + y) >> 2;
+ x = (dev->x_old * 7 + x) >> 3;
+ y = (dev->y_old * 7 + y) >> 3;
dev->x_old = x;
dev->y_old = y;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v5 3/3] input: appletouch: fix jumps when additional fingers are detected
From: Clinton Sprain @ 2014-03-29 21:48 UTC (permalink / raw)
To: Dmitry Torokhov, Clinton Sprain; +Cc: linux-input, Henrik Rydberg
In-Reply-To: <20140327182606.GA8902@core.coreip.homeip.net>
input: appletouch: fix jumps when additional fingers are detected
Addresses issues related to when a second finger enters or leaves the field,
causing the cursor to jump or the page to scroll unexpectedly; now, we
discard any movement change that happens at the exact moment we detect a
change in the number of fingers touching the trackpad. This doesn't
completely resolve the issue but does greatly mitigate it.
(No changes since v4)
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 8e01363..39c7ba2 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -218,6 +218,7 @@ struct atp {
bool valid; /* are the samples valid? */
bool size_detect_done;
bool overflow_warned;
+ int fingers_old; /* last reported finger count */
int x_old; /* last reported x/y, */
int y_old; /* used for smoothing */
signed char xy_cur[ATP_XSENSORS + ATP_YSENSORS];
@@ -523,7 +524,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
{
int x, y, x_z, y_z, x_f, y_f;
int retval, i, j;
- int key;
+ int key, fingers;
struct atp *dev = urb->context;
int status = atp_status_check(urb);
@@ -606,7 +607,9 @@ static void atp_complete_geyser_1_2(struct urb *urb)
&y_z, &y_f);
key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
- if (x && y) {
+ fingers = max(x_f, y_f);
+
+ if (x && y && (fingers == dev->fingers_old)) {
if (dev->x_old != -1) {
x = (dev->x_old * 7 + x) >> 3;
y = (dev->y_old * 7 + y) >> 3;
@@ -623,7 +626,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
input_report_abs(dev->input, ABS_Y, y);
input_report_abs(dev->input, ABS_PRESSURE,
min(ATP_PRESSURE, x_z + y_z));
- atp_report_fingers(dev->input, max(x_f, y_f));
+ atp_report_fingers(dev->input, fingers);
}
dev->x_old = x;
dev->y_old = y;
@@ -631,6 +634,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
} else if (!x && !y) {
dev->x_old = dev->y_old = -1;
+ dev->fingers_old = 0;
input_report_key(dev->input, BTN_TOUCH, 0);
input_report_abs(dev->input, ABS_PRESSURE, 0);
atp_report_fingers(dev->input, 0);
@@ -639,6 +643,10 @@ static void atp_complete_geyser_1_2(struct urb *urb)
memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
}
+ if (fingers != dev->fingers_old)
+ dev->x_old = dev->y_old = -1;
+ dev->fingers_old = fingers;
+
input_report_key(dev->input, BTN_LEFT, key);
input_sync(dev->input);
@@ -656,7 +664,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
{
int x, y, x_z, y_z, x_f, y_f;
int retval, i, j;
- int key;
+ int key, fingers;
struct atp *dev = urb->context;
int status = atp_status_check(urb);
@@ -719,7 +727,9 @@ static void atp_complete_geyser_3_4(struct urb *urb)
key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
- if (x && y) {
+ fingers = max(x_f, y_f);
+
+ if (x && y && (fingers == dev->fingers_old)) {
if (dev->x_old != -1) {
x = (dev->x_old * 7 + x) >> 3;
y = (dev->y_old * 7 + y) >> 3;
@@ -736,7 +746,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
input_report_abs(dev->input, ABS_Y, y);
input_report_abs(dev->input, ABS_PRESSURE,
min(ATP_PRESSURE, x_z + y_z));
- atp_report_fingers(dev->input, max(x_f, y_f));
+ atp_report_fingers(dev->input, fingers);
}
dev->x_old = x;
dev->y_old = y;
@@ -744,6 +754,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
} else if (!x && !y) {
dev->x_old = dev->y_old = -1;
+ dev->fingers_old = 0;
input_report_key(dev->input, BTN_TOUCH, 0);
input_report_abs(dev->input, ABS_PRESSURE, 0);
atp_report_fingers(dev->input, 0);
@@ -752,6 +763,10 @@ static void atp_complete_geyser_3_4(struct urb *urb)
memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
}
+ if (fingers != dev->fingers_old)
+ dev->x_old = dev->y_old = -1;
+ dev->fingers_old = fingers;
+
input_report_key(dev->input, BTN_LEFT, key);
input_sync(dev->input);
--
1.7.9.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox