* Re: [PATCH 01/15] Input: synaptics-rmi4 - fix checkpatch.pl, sparse and GCC warnings
From: Christopher Heiny @ 2014-02-06 1:36 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Courtney Cavin, linux-input
In-Reply-To: <20140206010938.GA27770@core.coreip.homeip.net>
On 02/05/2014 05:09 PM, Dmitry Torokhov wrote:
> On Tue, Feb 04, 2014 at 03:08:12PM -0800, Christopher Heiny wrote:
>> >On 01/23/2014 04:00 PM, Courtney Cavin wrote:
>>> > >Cc: Christopher Heiny<cheiny@synaptics.com>
>>> > >Cc: Dmitry Torokhov<dmitry.torokhov@gmail.com>
>>> > >Signed-off-by: Courtney Cavin<courtney.cavin@sonymobile.com>
>>> > >---
>>> > > drivers/input/rmi4/rmi_bus.c | 4 ++--
>>> > > drivers/input/rmi4/rmi_bus.h | 2 +-
>>> > > drivers/input/rmi4/rmi_driver.c | 17 ++++++++++++-----
>>> > > drivers/input/rmi4/rmi_f11.c | 4 +++-
>>> > > 4 files changed, 18 insertions(+), 9 deletions(-)
>>> > >
>>> > >diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
>>> > >index 96a76e7..8a939f3 100644
>>> > >--- a/drivers/input/rmi4/rmi_bus.c
>>> > >+++ b/drivers/input/rmi4/rmi_bus.c
>>> > >@@ -37,7 +37,7 @@ static void rmi_release_device(struct device *dev)
>>> > > kfree(rmi_dev);
>>> > > }
>>> > >
>>> > >-struct device_type rmi_device_type = {
>>> > >+static struct device_type rmi_device_type = {
>>> > > .name = "rmi_sensor",
>>> > > .release = rmi_release_device,
>>> > > };
>> >
>> >This struct is used by diagnostic modules to identify sensor
>> >devices, so it cannot be static.
>
> Then we need to declare it somewhere or provide an accessor function.
Currently it's in a header not included in the patches. We'll move it
to rmi_bus.h.
^ permalink raw reply
* [PATCH] input synaptics-rmi4: rmi_driver.c tidying
From: Christopher Heiny @ 2014-02-06 2:15 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Linux Input, Christopher Heiny, Andrew Duggan, Vincent Huang,
Vivian Ly, Daniel Rosenberg, Jean Delvare, Joerie de Gram,
Linus Walleij, Benjamin Tissoires, David Herrmann, Jiri Kosina
Remove pdt_mutex, since it's no longer needed.
Remove obsolete comment in rmi_drive_irq_save(). Fix
some operator spacing (or lack thereof) in rmi_drive_irq_save().
Remove XXX comment about F01 ordering. New structure doesn't
require F01 to be first.
Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Linux Walleij <linus.walleij@linaro.org>
Cc: David Herrmann <dh.herrmann@gmail.com>
Cc: Jiri Kosina <jkosina@suse.cz>
---
drivers/input/rmi4/rmi_driver.c | 24 ++----------------------
drivers/input/rmi4/rmi_driver.h | 1 -
2 files changed, 2 insertions(+), 23 deletions(-)
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 96bb47d..2943b2c4 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -347,21 +347,15 @@ static int rmi_driver_irq_save(struct rmi_device *rmi_dev,
if (!data->irq_stored) {
/* Save current enabled interrupts */
retval = rmi_read_block(rmi_dev,
- data->f01_container->fd.control_base_addr+1,
+ data->f01_container->fd.control_base_addr + 1,
data->irq_mask_store, data->num_of_irq_regs);
if (retval < 0) {
dev_err(dev, "%s: Failed to read enabled interrupts!",
__func__);
goto error_unlock;
}
- /*
- * Disable every interrupt except for function 54
- * TODO:Will also want to not disable function 1-like functions.
- * No need to take care of this now, since there's no good way
- * to identify them.
- */
retval = rmi_write_block(rmi_dev,
- data->f01_container->fd.control_base_addr+1,
+ data->f01_container->fd.control_base_addr + 1,
new_ints, data->num_of_irq_regs);
if (retval < 0) {
dev_err(dev, "%s: Failed to change enabled interrupts!",
@@ -562,24 +556,15 @@ static int rmi_scan_pdt(struct rmi_device *rmi_dev, void *ctx,
void *ctx,
const struct pdt_entry *entry))
{
- struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
int page;
int retval = RMI_SCAN_DONE;
- /*
- * TODO: With F01 and reflash as part of the core now, is this
- * lock still required?
- */
- mutex_lock(&data->pdt_mutex);
-
for (page = 0; page <= RMI4_MAX_PAGE; page++) {
retval = rmi_scan_pdt_page(rmi_dev, page, ctx, callback);
if (retval != RMI_SCAN_CONTINUE)
break;
}
- mutex_unlock(&data->pdt_mutex);
-
return retval < 0 ? retval : 0;
}
@@ -829,7 +814,6 @@ static int rmi_driver_probe(struct device *dev)
INIT_LIST_HEAD(&data->function_list);
data->rmi_dev = rmi_dev;
dev_set_drvdata(&rmi_dev->dev, data);
- mutex_init(&data->pdt_mutex);
/*
* Right before a warm boot, the sensor might be in some unusual state,
@@ -897,10 +881,6 @@ static int rmi_driver_probe(struct device *dev)
data->current_irq_mask = irq_memory + size * 2;
data->irq_mask_store = irq_memory + size * 3;
- /*
- * XXX need to make sure we create F01 first...
- * XXX or do we? It might not be required in the updated structure.
- */
irq_count = 0;
dev_dbg(dev, "Creating functions.");
retval = rmi_scan_pdt(rmi_dev, &irq_count, rmi_create_function);
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index d071ff5..a22a4e6 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -56,7 +56,6 @@ struct rmi_driver_data {
struct work_struct poll_work;
ktime_t poll_interval;
- struct mutex pdt_mutex;
u8 pdt_props;
u8 bsr;
^ permalink raw reply related
* Re: [PATCH 0/7] Input: xpad: Fix wireless controller connection and leds
From: Zachary Lund @ 2014-02-06 3:11 UTC (permalink / raw)
To: linux-input
In-Reply-To: <1391173414-6199-1-git-send-email-gregkh@linuxfoundation.org>
Greg Kroah-Hartman <gregkh <at> linuxfoundation.org> writes:
>
> Here's a series of patches from Pierre-Loup and I that rework the xpad
> driver to fix the issue where when a wireless xpad controller is plugged
> in, 4 joystick devices are created, no matter how many are really
> attached to the system. Pierre-Loup's patches fix this by dynamically
> creating the devices only when they are found by the wireless
> controller.
>
> Bonus is that the LEDs now work correctly with this series too, when
> mixing wireless and wired, we can properly identify which device is
> which on the X-LED. We also now name the LED device based on the
> joystick id, not on an atomic number that keeps incrementing, never
> decrementing when removed from the system.
>
> Note, patch 4/7 is a crazy hack to get the minor number of the joystick
> id that we have registered with the system. Pierre-Loup came up with
> this, and I really can't figure out any other way to do it, the joydev
> layer doesn't even know this information, otherwise I would have added
> it to that layer so the xpad driver could call it. Am I missing
> something here that we could do instead?
>
> The first patch in the series fixes a really annoying bug when plugging
> in one of these controllers to a USB 3 system. The URB type is
> incorrect so the xhci driver complains about it, rightly so. I think
> the original code was incorrect, but left it alone incase there is
> really a crazy device with a bulk endpoint instead of interrupt.
>
> Also, even with this series applied, the xpad driver needs a bunch of
> work. The LED device seems pointless as it doesn't actually work, and
> given that the number/name of the LED device was impossible to actually
> map to the proper xpad device, there's no way userspace code could ever
> actually use it. I think it should be removed entirely. There's also
> some other USB things that should be cleaned up, the bulk urb doesn't
> need to be always running (with lots of disconnect/connect cycles you
> can get a warning about it running when trying to submit it again), and
> the urb callbacks seem a bit strange. I'll work on those issues next...
>
> This series was based on a larger patch from Pierre-Loup that I broke up
> into pieces, and added some of my own where needed to resolve other
> issues.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo <at> vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
Since it may help, I'll add a link to my driver here. I have no intention of
trying to mainstream this driver and have used it personally for slightly
better support than what's provided now. I've recently done even more work
on it for the fun of it (not sure if I'm cut out for kernel hacking...).
Use at your own risk:
https://github.com/computerquip/xpad360wr
I find that the main problem with any of this is that I don't know how to
query features e.g. whether or not a device actually has FF capabilities.
It's currently just assumed to have FF capabilities. I suspect its in the
announce packet for wireless devices. For wired, it's just matched up
against a table of devices to determine what features it has.
^ permalink raw reply
* Re: [PATCH] input synaptics-rmi4: rmi_driver.c tidying
From: Dmitry Torokhov @ 2014-02-06 6:07 UTC (permalink / raw)
To: Christopher Heiny
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
Benjamin Tissoires, David Herrmann, Jiri Kosina
In-Reply-To: <1391652949-21083-1-git-send-email-cheiny@synaptics.com>
On Wed, Feb 05, 2014 at 06:15:49PM -0800, Christopher Heiny wrote:
> Remove pdt_mutex, since it's no longer needed.
>
> Remove obsolete comment in rmi_drive_irq_save(). Fix
> some operator spacing (or lack thereof) in rmi_drive_irq_save().
>
> Remove XXX comment about F01 ordering. New structure doesn't
> require F01 to be first.
>
> Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
Applied, thank you.
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: Linux Walleij <linus.walleij@linaro.org>
> Cc: David Herrmann <dh.herrmann@gmail.com>
> Cc: Jiri Kosina <jkosina@suse.cz>
>
> ---
>
> drivers/input/rmi4/rmi_driver.c | 24 ++----------------------
> drivers/input/rmi4/rmi_driver.h | 1 -
> 2 files changed, 2 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index 96bb47d..2943b2c4 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -347,21 +347,15 @@ static int rmi_driver_irq_save(struct rmi_device *rmi_dev,
> if (!data->irq_stored) {
> /* Save current enabled interrupts */
> retval = rmi_read_block(rmi_dev,
> - data->f01_container->fd.control_base_addr+1,
> + data->f01_container->fd.control_base_addr + 1,
> data->irq_mask_store, data->num_of_irq_regs);
> if (retval < 0) {
> dev_err(dev, "%s: Failed to read enabled interrupts!",
> __func__);
> goto error_unlock;
> }
> - /*
> - * Disable every interrupt except for function 54
> - * TODO:Will also want to not disable function 1-like functions.
> - * No need to take care of this now, since there's no good way
> - * to identify them.
> - */
> retval = rmi_write_block(rmi_dev,
> - data->f01_container->fd.control_base_addr+1,
> + data->f01_container->fd.control_base_addr + 1,
> new_ints, data->num_of_irq_regs);
> if (retval < 0) {
> dev_err(dev, "%s: Failed to change enabled interrupts!",
> @@ -562,24 +556,15 @@ static int rmi_scan_pdt(struct rmi_device *rmi_dev, void *ctx,
> void *ctx,
> const struct pdt_entry *entry))
> {
> - struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
> int page;
> int retval = RMI_SCAN_DONE;
>
> - /*
> - * TODO: With F01 and reflash as part of the core now, is this
> - * lock still required?
> - */
> - mutex_lock(&data->pdt_mutex);
> -
> for (page = 0; page <= RMI4_MAX_PAGE; page++) {
> retval = rmi_scan_pdt_page(rmi_dev, page, ctx, callback);
> if (retval != RMI_SCAN_CONTINUE)
> break;
> }
>
> - mutex_unlock(&data->pdt_mutex);
> -
> return retval < 0 ? retval : 0;
> }
>
> @@ -829,7 +814,6 @@ static int rmi_driver_probe(struct device *dev)
> INIT_LIST_HEAD(&data->function_list);
> data->rmi_dev = rmi_dev;
> dev_set_drvdata(&rmi_dev->dev, data);
> - mutex_init(&data->pdt_mutex);
>
> /*
> * Right before a warm boot, the sensor might be in some unusual state,
> @@ -897,10 +881,6 @@ static int rmi_driver_probe(struct device *dev)
> data->current_irq_mask = irq_memory + size * 2;
> data->irq_mask_store = irq_memory + size * 3;
>
> - /*
> - * XXX need to make sure we create F01 first...
> - * XXX or do we? It might not be required in the updated structure.
> - */
> irq_count = 0;
> dev_dbg(dev, "Creating functions.");
> retval = rmi_scan_pdt(rmi_dev, &irq_count, rmi_create_function);
> diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
> index d071ff5..a22a4e6 100644
> --- a/drivers/input/rmi4/rmi_driver.h
> +++ b/drivers/input/rmi4/rmi_driver.h
> @@ -56,7 +56,6 @@ struct rmi_driver_data {
> struct work_struct poll_work;
> ktime_t poll_interval;
>
> - struct mutex pdt_mutex;
> u8 pdt_props;
> u8 bsr;
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: hid-sony Report actual brightness value when reading LED
From: Jiri Kosina @ 2014-02-06 8:46 UTC (permalink / raw)
To: Simon Wood; +Cc: linux-input, linux-kernel
In-Reply-To: <1391628858-2916-1-git-send-email-simon@mungewell.org>
On Wed, 5 Feb 2014, Simon Wood wrote:
> The Dualshock4 controller contains a RGB LED, which is enabled via
> the '/sys/class/leds' interface. At present the driver only returns
> whether each of the RGB LEDs is lit (ie not off), but no indication
> of it's brightness.
>
> This patch fixes the reading of the current brightnes so that it
> returns the value (rather than just off=0, on=LED_FULL).
>
> Tested on the DS4 and SixAxis (for compatibility).
>
> Signed-off-by: Simon Wood <simon@mungewell.org>
Applied, thanks Simon.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* keyboard problems on Dell Latitude E6230
From: Mikael Pettersson @ 2014-02-06 9:13 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel
I'm having problems with the built-in keyboard on Dell Latitude E6230
laptops and Linux 3.12/3.13 kernels:
- sometimes the keyboard just keeps sending the same key code, as if
the key was held down permanently; sometimes that can be cured by
pressing ^C or something, but often only a reboot fixes it
- sometimes the keyboard just stops sending key codes; only a reboot
fixes it
This issue has plagued me since Nov last year when I got my first E6230.
Last week I got a replacement machine, but it too has the same problems.
Since I haven't seen this problem with any other laptop under Linux for
the last 15 years or so, I have to conclude that there's some HW issue
with these machines that the Linux kernel doesn't handle.
Any ideas?
^ permalink raw reply
* [PATCH] HID: Add Apple wireless keyboard 2011 JIS model support.
From: Huei-Horng Yo @ 2014-02-06 9:18 UTC (permalink / raw)
To: linux-input; +Cc: Jiri Kosina, Huei-Horng Yo
---
drivers/hid/hid-apple.c | 3 +++
drivers/hid/hid-core.c | 1 +
drivers/hid/hid-ids.h | 1 +
3 files changed, 5 insertions(+)
diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 4975581..f822fd2 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -469,6 +469,9 @@ static const struct hid_device_id apple_devices[] = {
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE,
USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_ANSI),
.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE,
+ USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_JIS),
+ .driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS),
.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ANSI),
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 3bfac3a..bb5c494 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1679,6 +1679,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_2009_JIS) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_ANSI) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_ISO) },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_JIS) },
{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY) },
{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY) },
{ HID_USB_DEVICE(USB_VENDOR_ID_AUREAL, USB_DEVICE_ID_AUREAL_W01RN) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 5a5248f..2c59283 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -135,6 +135,7 @@
#define USB_DEVICE_ID_APPLE_ALU_WIRELESS_2009_JIS 0x023b
#define USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_ANSI 0x0255
#define USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_ISO 0x0256
+#define USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_JIS 0x0257
#define USB_DEVICE_ID_APPLE_WELLSPRING8_ANSI 0x0290
#define USB_DEVICE_ID_APPLE_WELLSPRING8_ISO 0x0291
#define USB_DEVICE_ID_APPLE_WELLSPRING8_JIS 0x0292
--
1.8.5.3
^ permalink raw reply related
* Re: [PATCH] HID: Add Apple wireless keyboard 2011 JIS model support.
From: Jiri Kosina @ 2014-02-06 9:22 UTC (permalink / raw)
To: Huei-Horng Yo; +Cc: linux-input
In-Reply-To: <1391678313-2091-1-git-send-email-hiroshi@ghostsinthelab.org>
On Thu, 6 Feb 2014, Huei-Horng Yo wrote:
> ---
> drivers/hid/hid-apple.c | 3 +++
> drivers/hid/hid-core.c | 1 +
> drivers/hid/hid-ids.h | 1 +
> 3 files changed, 5 insertions(+)
>
Thanks for the patch. Please resend with:
- at least one line of changelog
- your Signed-off-by: line as described in Documentation/SubmittingPatches
so that I could apply it.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 05/15] Input: synaptics-rmi4 - remove gpio handling and polling
From: Linus Walleij @ 2014-02-06 9:28 UTC (permalink / raw)
To: Courtney Cavin
Cc: Christopher Heiny, linux-input@vger.kernel.org,
dmitry.torokhov@gmail.com, Linus WALLEIJ
In-Reply-To: <20140205023113.GE1706@sonymobile.com>
On Wed, Feb 5, 2014 at 3:31 AM, Courtney Cavin
<courtney.cavin@sonymobile.com> wrote:
> On Wed, Feb 05, 2014 at 12:08:35AM +0100, Christopher Heiny wrote:
>> On 01/23/2014 04:00 PM, Courtney Cavin wrote:
>> > Since all the configuration needed for an irq can be provided in other
>> > ways, remove all gpio->irq functionality. This cleans up the code quite
>> > a bit.
>>
>> In certain diagnostic modes, we need to be able to release the IRQ so
>> the GPIO can be monitored from userspace. This patch removes that
>> capability.
>
> Polling a GPIO from userspace is poor design regardless of the use-case, if
> you ask me. It certainly doesn't motivate the extra gpio<->IRQ code.
I think Courtney is right here, if you want this kind of stuff for debugging
it should be debug-add-on-patches, not part of the normal execution
path for the driver.
>> As mentioned in previous patch discussions, the polling functionality is
>> quite useful during new system integration, so we need to retain that.
>> If you have a proposal for where else it could be done, we'd certainly
>> entertain a patch that implements that.
>
> Do you actually have systems that have these hooked up to GPIOs without
> IRQ support?
>
> Regardless, if this is the case, implementing a GPIO polling IRQ chip
> should be possible, if extremely ugly.
OMG that would be quite horrible.
> Linus may have some comments in this area, though. Linus?
A driver may rely on polling but a driver may not rely on polling
done from userspace.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] HID: Add Apple wireless keyboard 2011 JIS model support.
From: Huei-Horng Yo @ 2014-02-06 9:40 UTC (permalink / raw)
To: linux-input; +Cc: Jiri Kosina, Huei-Horng Yo
In-Reply-To: <alpine.LNX.2.00.1402061021170.24838@pobox.suse.cz>
Add Apple wireless keyboard 2011 JIS model (05ac:0257).
Signed-off-by: Huei-Horng Yo <hiroshi@ghostsinthelab.org>
---
drivers/hid/hid-apple.c | 3 +++
drivers/hid/hid-core.c | 1 +
drivers/hid/hid-ids.h | 1 +
3 files changed, 5 insertions(+)
diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 4975581..f822fd2 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -469,6 +469,9 @@ static const struct hid_device_id apple_devices[] = {
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE,
USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_ANSI),
.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE,
+ USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_JIS),
+ .driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS),
.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ANSI),
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 3bfac3a..bb5c494 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1679,6 +1679,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_2009_JIS) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_ANSI) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_ISO) },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_JIS) },
{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY) },
{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY) },
{ HID_USB_DEVICE(USB_VENDOR_ID_AUREAL, USB_DEVICE_ID_AUREAL_W01RN) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 5a5248f..2c59283 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -135,6 +135,7 @@
#define USB_DEVICE_ID_APPLE_ALU_WIRELESS_2009_JIS 0x023b
#define USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_ANSI 0x0255
#define USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_ISO 0x0256
+#define USB_DEVICE_ID_APPLE_ALU_WIRELESS_2011_JIS 0x0257
#define USB_DEVICE_ID_APPLE_WELLSPRING8_ANSI 0x0290
#define USB_DEVICE_ID_APPLE_WELLSPRING8_ISO 0x0291
#define USB_DEVICE_ID_APPLE_WELLSPRING8_JIS 0x0292
--
1.8.5.3
^ permalink raw reply related
* Re: [PATCH] HID: Add Apple wireless keyboard 2011 JIS model support.
From: Jiri Kosina @ 2014-02-06 13:45 UTC (permalink / raw)
To: Huei-Horng Yo; +Cc: linux-input
In-Reply-To: <1391679634-2750-1-git-send-email-hiroshi@ghostsinthelab.org>
On Thu, 6 Feb 2014, Huei-Horng Yo wrote:
> Add Apple wireless keyboard 2011 JIS model (05ac:0257).
>
> Signed-off-by: Huei-Horng Yo <hiroshi@ghostsinthelab.org>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* PROBLEM: [Dell LATITUDE E7440] ALPS touchpad events are not routed into /dev/input/mice and /dev/input/mouseX
From: Vasily Titskiy @ 2014-02-06 17:37 UTC (permalink / raw)
To: linux-input
The issues occurs on every kernel after
1302bac33d9e88cd43e482191a806998f3ed43cc commit. Before the commit,
this touchpad device was detected as PS/2 mouse and "cat
/dev/input/mice" shows data when the touchpad is used.
After the commit, there are no events from the touchpad in
/dev/input/mice or /dev/input/mouseX files anymore.
According to drivers/input/mouse/alps.c source file, all devices with
signatures "ec[0] == 0x88 && ec[1] == 0x08" are treated as "Rushmore
touchpads". This touchpad has E7 signature of "73-03-0A" and EC
signature of "88-08-22", and it looks like that V5 protocol (not
"Rushmore") should be used here. If V5 protocol is forced to use for
this device then everything works fine.
The patch is trivial, but I'm not sure how to properly separate
"Rushmore" and "V5" devices. Are there any datasheets or experts to
help me with it?
Thanks.
--
Vasily Titskiy
^ permalink raw reply
* Re: [PATCH 05/15] Input: synaptics-rmi4 - remove gpio handling and polling
From: Christopher Heiny @ 2014-02-06 20:05 UTC (permalink / raw)
To: Courtney Cavin
Cc: linux-input@vger.kernel.org, dmitry.torokhov@gmail.com,
linus.walleij, linus.walleij
In-Reply-To: <20140205023113.GE1706@sonymobile.com>
On 02/04/2014 06:31 PM, Courtney Cavin wrote:
> On Wed, Feb 05, 2014 at 12:08:35AM +0100, Christopher Heiny wrote:
>> On 01/23/2014 04:00 PM, Courtney Cavin wrote:
>>> Since all the configuration needed for an irq can be provided in other
>>> ways, remove all gpio->irq functionality. This cleans up the code quite
>>> a bit.
>>
>> In certain diagnostic modes, we need to be able to release the IRQ so
>> the GPIO can be monitored from userspace. This patch removes that
>> capability.
>>
>
> Polling a GPIO from userspace is poor design regardless of the use-case, if
> you ask me. It certainly doesn't motivate the extra gpio<->IRQ code.
Fortunately, neither the in-driver polling implementation nor the user
space diagnostic applications are implemented that way.
>>> This also gets rid of polling functionality, as this should be done
>>> elsewhere if absolutely needed.
>>
>> As mentioned in previous patch discussions, the polling functionality is
>> quite useful during new system integration, so we need to retain that.
>> If you have a proposal for where else it could be done, we'd certainly
>> entertain a patch that implements that.
>
> Do you actually have systems that have these hooked up to GPIOs without
> IRQ support?
The right question would be "What are some situations where this polling
functionality would be needed?"
As I mentioned, this is usually needed during new system integration -
that is, when trying to bring up the driver on a new or substantially
modified platform. You might not know where the ATTN GPIO is not known
(either by omission or an error in the specifications) at the time it is
initially brought up, or you might have a prototype board has a layout
error that leaves the ATTN GPIO unconnected. You'd be surprised at how
often these situations occur (twice so far this week, f'rinstance).
Rather than just telling the engineers "Hey, go play MineSweeper for a
few days (or weeks) while someone tracks down the specs (or spins the
PCB)", having in-driver polling support lets you bring up the sensor
right away.
> Regardless, if this is the case, implementing a GPIO polling IRQ chip
> should be possible, if extremely ugly.
>
> Linus may have some comments in this area, though. Linus?
>
>>
>> Thanks,
>> Chris
>>
>>>
>>> Cc: Christopher Heiny <cheiny@synaptics.com>
>>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>> Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com>
>>> ---
>>> drivers/input/rmi4/rmi_driver.c | 143 +++++-----------------------------------
>>> drivers/input/rmi4/rmi_driver.h | 7 --
>>> drivers/input/rmi4/rmi_i2c.c | 24 +------
>>> include/linux/rmi.h | 31 +--------
>>> 4 files changed, 20 insertions(+), 185 deletions(-)
>>>
>>> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
>>> index 5fb582c..780742f 100644
>>> --- a/drivers/input/rmi4/rmi_driver.c
>>> +++ b/drivers/input/rmi4/rmi_driver.c
>>> @@ -20,7 +20,6 @@
>>> #include <linux/delay.h>
>>> #include <linux/device.h>
>>> #include <linux/fs.h>
>>> -#include <linux/gpio.h>
>>> #include <linux/kconfig.h>
>>> #include <linux/list.h>
>>> #include <linux/module.h>
>>> @@ -50,74 +49,17 @@ static irqreturn_t rmi_irq_thread(int irq, void *p)
>>> struct rmi_transport_dev *xport = p;
>>> struct rmi_device *rmi_dev = xport->rmi_dev;
>>> struct rmi_driver *driver = rmi_dev->driver;
>>> - struct rmi_device_platform_data *pdata = xport->dev->platform_data;
>>> struct rmi_driver_data *data;
>>>
>>> data = dev_get_drvdata(&rmi_dev->dev);
>>> -
>>> - if (IRQ_DEBUG(data))
>>> - dev_dbg(xport->dev, "ATTN gpio, value: %d.\n",
>>> - gpio_get_value(pdata->attn_gpio));
>>> -
>>> - if (gpio_get_value(pdata->attn_gpio) == pdata->attn_polarity) {
>>> - data->attn_count++;
>>> - if (driver && driver->irq_handler && rmi_dev)
>>> - driver->irq_handler(rmi_dev, irq);
>>> - }
>>> + if (driver && driver->irq_handler && rmi_dev)
>>> + driver->irq_handler(rmi_dev, irq);
>>>
>>> return IRQ_HANDLED;
>>> }
>>>
>>> static int process_interrupt_requests(struct rmi_device *rmi_dev);
>>>
>>> -static void rmi_poll_work(struct work_struct *work)
>>> -{
>>> - struct rmi_driver_data *data =
>>> - container_of(work, struct rmi_driver_data, poll_work);
>>> - struct rmi_device *rmi_dev = data->rmi_dev;
>>> -
>>> - process_interrupt_requests(rmi_dev);
>>> -}
>>> -
>>> -/*
>>> - * This is the timer function for polling - it simply has to schedule work
>>> - * and restart the timer.
>>> - */
>>> -static enum hrtimer_restart rmi_poll_timer(struct hrtimer *timer)
>>> -{
>>> - struct rmi_driver_data *data =
>>> - container_of(timer, struct rmi_driver_data, poll_timer);
>>> -
>>> - if (!data->enabled)
>>> - return HRTIMER_NORESTART;
>>> - if (!work_pending(&data->poll_work))
>>> - schedule_work(&data->poll_work);
>>> - hrtimer_start(&data->poll_timer, data->poll_interval, HRTIMER_MODE_REL);
>>> - return HRTIMER_NORESTART;
>>> -}
>>> -
>>> -static int enable_polling(struct rmi_device *rmi_dev)
>>> -{
>>> - struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
>>> -
>>> - dev_dbg(&rmi_dev->dev, "Polling enabled.\n");
>>> - INIT_WORK(&data->poll_work, rmi_poll_work);
>>> - hrtimer_init(&data->poll_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>>> - data->poll_timer.function = rmi_poll_timer;
>>> - hrtimer_start(&data->poll_timer, data->poll_interval, HRTIMER_MODE_REL);
>>> -
>>> - return 0;
>>> -}
>>> -
>>> -static void disable_polling(struct rmi_device *rmi_dev)
>>> -{
>>> - struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
>>> -
>>> - dev_dbg(&rmi_dev->dev, "Polling disabled.\n");
>>> - hrtimer_cancel(&data->poll_timer);
>>> - cancel_work_sync(&data->poll_work);
>>> -}
>>> -
>>> static void disable_sensor(struct rmi_device *rmi_dev)
>>> {
>>> struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
>>> @@ -125,9 +67,6 @@ static void disable_sensor(struct rmi_device *rmi_dev)
>>> if (!data->enabled)
>>> return;
>>>
>>> - if (!data->irq)
>>> - disable_polling(rmi_dev);
>>> -
>>> if (rmi_dev->xport->ops->disable_device)
>>> rmi_dev->xport->ops->disable_device(rmi_dev->xport);
>>>
>>> @@ -155,20 +94,14 @@ static int enable_sensor(struct rmi_device *rmi_dev)
>>> }
>>>
>>> xport = rmi_dev->xport;
>>> - if (data->irq) {
>>> - retval = request_threaded_irq(data->irq,
>>> - xport->hard_irq ? xport->hard_irq : NULL,
>>> - xport->irq_thread ?
>>> - xport->irq_thread : rmi_irq_thread,
>>> - data->irq_flags,
>>> - dev_name(&rmi_dev->dev), xport);
>>> - if (retval)
>>> - return retval;
>>> - } else {
>>> - retval = enable_polling(rmi_dev);
>>> - if (retval < 0)
>>> - return retval;
>>> - }
>>> + retval = request_threaded_irq(data->irq,
>>> + xport->hard_irq ? xport->hard_irq : NULL,
>>> + xport->irq_thread ?
>>> + xport->irq_thread : rmi_irq_thread,
>>> + IRQF_ONESHOT,
>>> + dev_name(&rmi_dev->dev), xport);
>>> + if (retval)
>>> + return retval;
>>>
>>> data->enabled = true;
>>>
>>> @@ -751,16 +684,9 @@ static SIMPLE_DEV_PM_OPS(rmi_driver_pm, rmi_driver_suspend, rmi_driver_resume);
>>> static int rmi_driver_remove(struct device *dev)
>>> {
>>> struct rmi_device *rmi_dev = to_rmi_device(dev);
>>> - const struct rmi_device_platform_data *pdata =
>>> - to_rmi_platform_data(rmi_dev);
>>> - const struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
>>> -
>>> disable_sensor(rmi_dev);
>>> rmi_free_function_list(rmi_dev);
>>>
>>> - if (data->gpio_held)
>>> - gpio_free(pdata->attn_gpio);
>>> -
>>> return 0;
>>> }
>>>
>>> @@ -895,51 +821,12 @@ static int rmi_driver_probe(struct device *dev)
>>> mutex_init(&data->suspend_mutex);
>>> }
>>>
>>> - if (gpio_is_valid(pdata->attn_gpio)) {
>>> - static const char GPIO_LABEL[] = "attn";
>>> - unsigned long gpio_flags = GPIOF_DIR_IN;
>>> -
>>> - data->irq = gpio_to_irq(pdata->attn_gpio);
>>> - if (pdata->level_triggered) {
>>> - data->irq_flags = IRQF_ONESHOT |
>>> - ((pdata->attn_polarity == RMI_ATTN_ACTIVE_HIGH)
>>> - ? IRQF_TRIGGER_HIGH : IRQF_TRIGGER_LOW);
>>> - } else {
>>> - data->irq_flags =
>>> - (pdata->attn_polarity == RMI_ATTN_ACTIVE_HIGH)
>>> - ? IRQF_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
>>> - }
>>> + data->irq = pdata->irq;
>>> + if (data->irq < 0) {
>>> + dev_err(dev, "Failed to get attn IRQ.\n");
>>> + retval = data->irq;
>>> + goto err_free_data;
>>>
>>> - if (IS_ENABLED(CONFIG_RMI4_DEV))
>>> - gpio_flags |= GPIOF_EXPORT;
>>> -
>>> - retval = gpio_request_one(pdata->attn_gpio, gpio_flags,
>>> - GPIO_LABEL);
>>> - if (retval) {
>>> - dev_warn(dev, "WARNING: Failed to request ATTN gpio %d, code=%d.\n",
>>> - pdata->attn_gpio, retval);
>>> - retval = 0;
>>> - } else {
>>> - dev_info(dev, "Obtained ATTN gpio %d.\n",
>>> - pdata->attn_gpio);
>>> - data->gpio_held = true;
>>> - if (IS_ENABLED(CONFIG_RMI4_DEV)) {
>>> - retval = gpio_export_link(dev,
>>> - GPIO_LABEL, pdata->attn_gpio);
>>> - if (retval) {
>>> - dev_warn(dev,
>>> - "WARNING: Failed to symlink ATTN gpio!\n");
>>> - retval = 0;
>>> - } else {
>>> - dev_info(dev, "Exported ATTN gpio %d.",
>>> - pdata->attn_gpio);
>>> - }
>>> - }
>>> - }
>>> - } else {
>>> - data->poll_interval = ktime_set(0,
>>> - (pdata->poll_interval_ms ? pdata->poll_interval_ms :
>>> - DEFAULT_POLL_INTERVAL_MS) * 1000 * 1000);
>>> }
>>>
>>> if (data->f01_container->dev.driver) {
>>> diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
>>> index 4f44a54..aef5521 100644
>>> --- a/drivers/input/rmi4/rmi_driver.h
>>> +++ b/drivers/input/rmi4/rmi_driver.h
>>> @@ -39,9 +39,7 @@ struct rmi_driver_data {
>>>
>>> u32 attn_count;
>>> u32 irq_debug; /* Should be bool, but debugfs wants u32 */
>>> - bool gpio_held;
>>> int irq;
>>> - int irq_flags;
>>> int num_of_irq_regs;
>>> int irq_count;
>>> unsigned long *irq_status;
>>> @@ -50,11 +48,6 @@ struct rmi_driver_data {
>>> bool irq_stored;
>>> struct mutex irq_mutex;
>>>
>>> - /* Following are used when polling. */
>>> - struct hrtimer poll_timer;
>>> - struct work_struct poll_work;
>>> - ktime_t poll_interval;
>>> -
>>> struct mutex pdt_mutex;
>>> u8 pdt_props;
>>> u8 bsr;
>>> diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
>>> index 910f05c..aebf974 100644
>>> --- a/drivers/input/rmi4/rmi_i2c.c
>>> +++ b/drivers/input/rmi4/rmi_i2c.c
>>> @@ -196,8 +196,7 @@ static int rmi_i2c_probe(struct i2c_client *client,
>>> return -EINVAL;
>>> }
>>>
>>> - dev_dbg(&client->dev, "Probing %#02x (GPIO %d).\n",
>>> - client->addr, pdata->attn_gpio);
>>> + dev_dbg(&client->dev, "Probing %#02x.\n", client->addr);
>>>
>>> if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
>>> dev_err(&client->dev,
>>> @@ -205,15 +204,6 @@ static int rmi_i2c_probe(struct i2c_client *client,
>>> return -ENODEV;
>>> }
>>>
>>> - if (pdata->gpio_config) {
>>> - retval = pdata->gpio_config(pdata->gpio_data, true);
>>> - if (retval < 0) {
>>> - dev_err(&client->dev, "Failed to configure GPIOs, code: %d.\n",
>>> - retval);
>>> - return retval;
>>> - }
>>> - }
>>> -
>>> rmi_i2c = devm_kzalloc(&client->dev, sizeof(struct rmi_i2c_xport),
>>> GFP_KERNEL);
>>> if (!rmi_i2c)
>>> @@ -240,7 +230,7 @@ static int rmi_i2c_probe(struct i2c_client *client,
>>> if (retval) {
>>> dev_err(&client->dev, "Failed to register transport driver at 0x%.2X.\n",
>>> client->addr);
>>> - goto err_gpio;
>>> + goto err;
>>> }
>>>
>>> i2c_set_clientdata(client, rmi_i2c);
>>> @@ -249,24 +239,16 @@ static int rmi_i2c_probe(struct i2c_client *client,
>>> client->addr);
>>> return 0;
>>>
>>> -err_gpio:
>>> - if (pdata->gpio_config)
>>> - pdata->gpio_config(pdata->gpio_data, false);
>>> -
>>> +err:
>>> return retval;
>>> }
>>>
>>> static int rmi_i2c_remove(struct i2c_client *client)
>>> {
>>> - const struct rmi_device_platform_data *pdata =
>>> - dev_get_platdata(&client->dev);
>>> struct rmi_i2c_xport *rmi_i2c = i2c_get_clientdata(client);
>>>
>>> rmi_unregister_transport_device(&rmi_i2c->xport);
>>>
>>> - if (pdata->gpio_config)
>>> - pdata->gpio_config(pdata->gpio_data, false);
>>> -
>>> return 0;
>>> }
>>>
>>> diff --git a/include/linux/rmi.h b/include/linux/rmi.h
>>> index 65b59b5..326e741 100644
>>> --- a/include/linux/rmi.h
>>> +++ b/include/linux/rmi.h
>>> @@ -23,11 +23,6 @@
>>> #include <linux/wait.h>
>>> #include <linux/debugfs.h>
>>>
>>> -enum rmi_attn_polarity {
>>> - RMI_ATTN_ACTIVE_LOW = 0,
>>> - RMI_ATTN_ACTIVE_HIGH = 1
>>> -};
>>> -
>>> /**
>>> * struct rmi_f11_axis_alignment - target axis alignment
>>> * @swap_axes: set to TRUE if desired to swap x- and y-axis
>>> @@ -194,25 +189,10 @@ struct rmi_device_platform_data_spi {
>>> /**
>>> * struct rmi_device_platform_data - system specific configuration info.
>>> *
>>> + * @irq - attention IRQ
>>> * @firmware_name - if specified will override default firmware name,
>>> * for reflashing.
>>> *
>>> - * @attn_gpio - the index of a GPIO that will be used to provide the ATTN
>>> - * interrupt from the touch sensor.
>>> - * @attn_polarity - indicates whether ATTN is active high or low.
>>> - * @level_triggered - by default, the driver uses edge triggered interrupts.
>>> - * However, this can cause problems with suspend/resume on some platforms. In
>>> - * that case, set this to 1 to use level triggered interrupts.
>>> - * @gpio_config - a routine that will be called when the driver is loaded to
>>> - * perform any platform specific GPIO configuration, and when it is unloaded
>>> - * for GPIO de-configuration. This is typically used to configure the ATTN
>>> - * GPIO and the I2C or SPI pins, if necessary.
>>> - * @gpio_data - platform specific data to be passed to the GPIO configuration
>>> - * function.
>>> - *
>>> - * @poll_interval_ms - the time in milliseconds between reads of the interrupt
>>> - * status register. This is ignored if attn_gpio is non-zero.
>>> - *
>>> * @reset_delay_ms - after issuing a reset command to the touch sensor, the
>>> * driver waits a few milliseconds to give the firmware a chance to
>>> * to re-initialize. You can override the default wait period here.
>>> @@ -245,14 +225,7 @@ struct rmi_device_platform_data_spi {
>>> * functions.
>>> */
>>> struct rmi_device_platform_data {
>>> - int attn_gpio;
>>> - enum rmi_attn_polarity attn_polarity;
>>> - bool level_triggered;
>>> - void *gpio_data;
>>> - int (*gpio_config)(void *gpio_data, bool configure);
>>> -
>>> - int poll_interval_ms;
>>> -
>>> + int irq;
>>> int reset_delay_ms;
>>>
>>> struct rmi_device_platform_data_spi spi_data;
>>>
^ permalink raw reply
* Re: [PATCH 05/15] Input: synaptics-rmi4 - remove gpio handling and polling
From: Christopher Heiny @ 2014-02-06 20:05 UTC (permalink / raw)
To: Linus Walleij, Courtney Cavin
Cc: linux-input@vger.kernel.org, dmitry.torokhov@gmail.com,
Linus WALLEIJ
In-Reply-To: <CACRpkdYtGM283y71yWm-GAHXz9UuvxPRddY3wzzvcn40VfHbUw@mail.gmail.com>
On 02/06/2014 01:28 AM, Linus Walleij wrote:
> On Wed, Feb 5, 2014 at 3:31 AM, Courtney Cavin
> <courtney.cavin@sonymobile.com> wrote:
>> >On Wed, Feb 05, 2014 at 12:08:35AM +0100, Christopher Heiny wrote:
>>> >>On 01/23/2014 04:00 PM, Courtney Cavin wrote:
>>>> >> >Since all the configuration needed for an irq can be provided in other
>>>> >> >ways, remove all gpio->irq functionality. This cleans up the code quite
>>>> >> >a bit.
>>> >>
>>> >>In certain diagnostic modes, we need to be able to release the IRQ so
>>> >>the GPIO can be monitored from userspace. This patch removes that
>>> >>capability.
>> >
>> >Polling a GPIO from userspace is poor design regardless of the use-case, if
>> >you ask me. It certainly doesn't motivate the extra gpio<->IRQ code.
>
> I think Courtney is right here, if you want this kind of stuff for debugging
> it should be debug-add-on-patches, not part of the normal execution
> path for the driver.
I'm OK with replacing the current polling implementation with an
add-on-patch, as long as the support for the debug-add-on-patches
doesn't get torn out of the driver because "hey, I don't see what this
is used for in the core driver code, so it must be unnecessary".
>>> >>As mentioned in previous patch discussions, the polling functionality is
>>> >>quite useful during new system integration, so we need to retain that.
>>> >>If you have a proposal for where else it could be done, we'd certainly
>>> >>entertain a patch that implements that.
>> >
>> >Do you actually have systems that have these hooked up to GPIOs without
>> >IRQ support?
>> >
>> >Regardless, if this is the case, implementing a GPIO polling IRQ chip
>> >should be possible, if extremely ugly.
> OMG that would be quite horrible.
>
>> >Linus may have some comments in this area, though. Linus?
>
> A driver may rely on polling but a driver may not rely on polling
> done from userspace.
Fortunately, the driver doesn't rely on polling from user space.
^ permalink raw reply
* Re: [PATCH v2] Input: add i2c/smbus driver for elan touchpad
From: Benson Leung @ 2014-02-07 0:10 UTC (permalink / raw)
To: Duson Lin
Cc: linux-kernel@vger.kernel.org, linux-input, Dmitry Torokhov,
agnescheng, phoenix
In-Reply-To: <1387338410-5930-1-git-send-email-dusonlin@emc.com.tw>
On Tue, Dec 17, 2013 at 7:46 PM, Duson Lin <dusonlin@emc.com.tw> wrote:
> This driver adds support for elan i2c/smbus touchpad found on some laptops PC
>
> Signed-off-by: Duson Lin <dusonlin@emc.com.tw>
Reviewed-by: Benson Leung <bleung@chromium.org>
Hi Dmitry,
This driver has been posted for a little while now. I've reviewed it,
and it is consistent with the cyapa.c driver that I submitted. Could
you take another look at this?
Thanks,
--
Benson Leung
Software Engineer, Chrom* OS
bleung@chromium.org
^ permalink raw reply
* Re: [PATCH v2] Input: add i2c/smbus driver for elan touchpad
From: Benson Leung @ 2014-02-07 0:21 UTC (permalink / raw)
To: Duson Lin
Cc: linux-kernel@vger.kernel.org, linux-input, Dmitry Torokhov,
agnescheng, phoenix, Jeff.Chuang
In-Reply-To: <CANLzEksP3nT2=GUu8sAA95ikNbyA0KD_s4qVGhno2MHYp7_1YQ@mail.gmail.com>
On Fri, Jan 10, 2014 at 12:30 PM, Benson Leung <bleung@chromium.org> wrote:
> On Mon, Jan 6, 2014 at 7:08 PM, Duson Lin <dusonlin@emc.com.tw> wrote:
>> This driver adds support for elan i2c/smbus touchpad found on some laptops PC
>>
>> Signed-off-by: Duson Lin <dusonlin@emc.com.tw>
>
> Reviewed-by: Benson Leung <bleung@chromium.org>
>
Hi Dmitry, This should be the most up to date version of this driver.
Disregard the one I pinged from Dec 17, 2013 . Could you take another
look at this?
--
Benson Leung
Software Engineer, Chrom* OS
bleung@chromium.org
^ permalink raw reply
* [PATCH v2 0/3] input: appletouch: fixes for jagged/uneven cursor movement
From: Clinton Sprain @ 2014-02-07 0:38 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
In-Reply-To: <52D9DE7A.7040509@gmail.com>
These patches address some usability issues by dialing back the fuzz and threshold parameters, by implementing some smoothing for the sensor data and by discarding movements that directly coincide with a change in the number of fingers detected on the touchpad.
Changes since last time:
1/3: Remove extraneous cleanup - I'll submit a separate patch another day
2/3: Apply smoothing to the sensor data before position calculation, eliminating the need to toss values based on a threshold; simplify formula for averaging with prior values
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
drivers/input/mouse/appletouch.c | 131 ++++++++++++++++++++++++++------------
1 file changed, 92 insertions(+), 39 deletions(-)
^ permalink raw reply
* [PATCH v2 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold
From: Clinton Sprain @ 2014-02-07 0:40 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
In-Reply-To: <52F42AEE.3040505@gmail.com>
Dials back the default fuzz and threshold settings for most devices using this driver, based on user input from forums and bug reports, increasing sensitivity. These two settings can also now both be overridden as module parameters in the event that users have hardware that does not respond well to the new defaults, or there is a desire to change the values for devices that do not have new defaults.
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 48 ++++++++++++++++++++++++--------------
1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 800ca7d..d48181b 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -48,6 +48,8 @@ struct atp_info {
int yfact; /* Y multiplication factor */
int datalen; /* size of USB transfers */
void (*callback)(struct urb *); /* callback function */
+ int fuzz; /* fuzz touchpad generates */
+ int threshold; /* sensitivity threshold */
};
static void atp_complete_geyser_1_2(struct urb *urb);
@@ -61,6 +63,8 @@ static const struct atp_info fountain_info = {
.yfact = 43,
.datalen = 81,
.callback = atp_complete_geyser_1_2,
+ .fuzz = 16,
+ .threshold = 5,
};
static const struct atp_info geyser1_info = {
@@ -71,6 +75,8 @@ static const struct atp_info geyser1_info = {
.yfact = 43,
.datalen = 81,
.callback = atp_complete_geyser_1_2,
+ .fuzz = 16,
+ .threshold = 5,
};
static const struct atp_info geyser2_info = {
@@ -81,6 +87,8 @@ static const struct atp_info geyser2_info = {
.yfact = 43,
.datalen = 64,
.callback = atp_complete_geyser_1_2,
+ .fuzz = 0,
+ .threshold = 3,
};
static const struct atp_info geyser3_info = {
@@ -90,6 +98,8 @@ static const struct atp_info geyser3_info = {
.yfact = 64,
.datalen = 64,
.callback = atp_complete_geyser_3_4,
+ .fuzz = 0,
+ .threshold = 3,
};
static const struct atp_info geyser4_info = {
@@ -99,6 +109,8 @@ static const struct atp_info geyser4_info = {
.yfact = 64,
.datalen = 64,
.callback = atp_complete_geyser_3_4,
+ .fuzz = 0,
+ .threshold = 3,
};
#define ATP_DEVICE(prod, info) \
@@ -155,18 +167,9 @@ MODULE_DEVICE_TABLE(usb, atp_table);
#define ATP_XSENSORS 26
#define ATP_YSENSORS 16
-/* amount of fuzz this touchpad generates */
-#define ATP_FUZZ 16
-
/* maximum pressure this driver will report */
#define ATP_PRESSURE 300
-/*
- * Threshold for the touchpad sensors. Any change less than ATP_THRESHOLD is
- * ignored.
- */
-#define ATP_THRESHOLD 5
-
/* Geyser initialization constants */
#define ATP_GEYSER_MODE_READ_REQUEST_ID 1
#define ATP_GEYSER_MODE_WRITE_REQUEST_ID 9
@@ -235,14 +238,20 @@ MODULE_AUTHOR("Sven Anders");
MODULE_DESCRIPTION("Apple PowerBook and MacBook USB touchpad driver");
MODULE_LICENSE("GPL");
-/*
- * Make the threshold a module parameter
- */
-static int threshold = ATP_THRESHOLD;
+static int threshold = -1;
module_param(threshold, int, 0644);
MODULE_PARM_DESC(threshold, "Discard any change in data from a sensor"
" (the trackpad has many of these sensors)"
- " less than this value.");
+ " less than this value. Devices have defaults;"
+ " this parameter overrides them.");
+static int fuzz = -1;
+
+module_param(fuzz, int, 0644);
+MODULE_PARM_DESC(fuzz, "Fuzz the trackpad generates. The higher"
+ " this is, the less sensitive the trackpad"
+ " will feel, but values too low may generate"
+ " random movement. Devices have defaults;"
+ " this parameter overrides them.");
static int debug;
module_param(debug, int, 0644);
@@ -455,7 +464,7 @@ static void atp_detect_size(struct atp *dev)
input_set_abs_params(dev->input, ABS_X, 0,
(dev->info->xsensors_17 - 1) *
dev->info->xfact - 1,
- ATP_FUZZ, 0);
+ fuzz, 0);
break;
}
}
@@ -808,6 +817,11 @@ static int atp_probe(struct usb_interface *iface,
dev->info = info;
dev->overflow_warned = false;
+ if (fuzz < 0)
+ fuzz = dev->info->fuzz;
+ if (threshold < 0)
+ threshold = dev->info->threshold;
+
dev->urb = usb_alloc_urb(0, GFP_KERNEL);
if (!dev->urb)
goto err_free_devs;
@@ -843,10 +857,10 @@ static int atp_probe(struct usb_interface *iface,
input_set_abs_params(input_dev, ABS_X, 0,
(dev->info->xsensors - 1) * dev->info->xfact - 1,
- ATP_FUZZ, 0);
+ fuzz, 0);
input_set_abs_params(input_dev, ABS_Y, 0,
(dev->info->ysensors - 1) * dev->info->yfact - 1,
- ATP_FUZZ, 0);
+ fuzz, 0);
input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
set_bit(EV_KEY, input_dev->evbit);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 2/3] input: appletouch: use better cursor movement smoothing algorithm
From: Clinton Sprain @ 2014-02-07 0:43 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
In-Reply-To: <52F42AEE.3040505@gmail.com>
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.
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 60 ++++++++++++++++++++++++++------------
1 file changed, 42 insertions(+), 18 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index d48181b..edbdd95 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -338,21 +338,51 @@ static void atp_reinit(struct work_struct *work)
static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
int *z, int *fingers)
{
- int i;
+ int i, k;
+ int smooth[nb_sensors + 2];
+ int smooth_tmp[nb_sensors + 2];
+
/* values to calculate mean */
int pcum = 0, psum = 0;
int is_increasing = 0;
*fingers = 0;
+ /*
+ * Use a smoothed version of sensor data for movement calculations, to
+ * combat noise without needing to toss out values based on a threshold.
+ * This improves tracking. Finger count is calculated separately based
+ * on raw data.
+ */
+
+ for (i = 0; i < nb_sensors; i++) { /* Scale up to minimize */
+ smooth[i] = xy_sensors[i] << 12; /* rounding/truncation. */
+ }
+
+ /* Mitigate some of the data loss from smoothing on the edge sensors. */
+ smooth[-1] = smooth[0] >> 2;
+ smooth[nb_sensors] = smooth[nb_sensors - 1] >> 2;
+
+ for (k = 0; k < 4; k++) {
+ for (i = 0; i < nb_sensors; i++) { /* Blend with neighbors. */
+ smooth_tmp[i] = (smooth[i - 1] + smooth[i] * 2 + smooth[i + 1]) >> 2;
+ }
+
+ for (i = 0; i < nb_sensors; i++) {
+ smooth[i] = smooth_tmp[i];
+ if (k == 3) /* Last go-round, so scale back down. */
+ smooth[i] = smooth[i] >> 12;
+ }
+
+ smooth[-1] = smooth[0] >> 2;
+ smooth[nb_sensors] = smooth[nb_sensors - 1] >> 2;
+ }
+
for (i = 0; i < nb_sensors; i++) {
if (xy_sensors[i] < threshold) {
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
@@ -367,7 +397,8 @@ 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;
@@ -375,15 +406,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
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);
+ pcum += (smooth[i]) * i;
+ psum += (smooth[i]);
}
if (psum > 0) {
@@ -565,8 +589,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
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;
@@ -677,8 +701,8 @@ static void atp_complete_geyser_3_4(struct urb *urb)
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 v2 3/3] input: appletouch: fix jumps when additional fingers are detected
From: Clinton Sprain @ 2014-02-07 0:46 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
In-Reply-To: <52F42AEE.3040505@gmail.com>
Discard cursor movements if they directly coincide with a change in the number of fingers detected. This helps with two issues - a sudden jump of the cursor when a second finger enters or leaves the touchpad, and an unexpected jump in page scrolling under the same scenario. The fix doesn't completely eliminate the problem but does greatly reduce its frequency and severity.
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index edbdd95..370d0e9 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -212,6 +212,7 @@ struct atp {
signed char xy_old[ATP_XSENSORS + ATP_YSENSORS];
int xy_acc[ATP_XSENSORS + ATP_YSENSORS];
int idlecount; /* number of empty packets */
+ int fingers_old; /* last reported finger count */
struct work_struct work;
};
@@ -505,6 +506,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 fingers;
struct atp *dev = urb->context;
int status = atp_status_check(urb);
@@ -587,7 +589,9 @@ static void atp_complete_geyser_1_2(struct urb *urb)
dev->info->yfact, &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;
@@ -604,7 +608,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;
@@ -620,6 +624,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);
@@ -638,6 +646,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 fingers;
struct atp *dev = urb->context;
int status = atp_status_check(urb);
@@ -699,7 +708,9 @@ static void atp_complete_geyser_3_4(struct urb *urb)
dev->info->yfact, &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;
@@ -716,7 +727,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;
@@ -732,6 +743,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
* Re: [PATCH 05/15] Input: synaptics-rmi4 - remove gpio handling and polling
From: Courtney Cavin @ 2014-02-07 1:45 UTC (permalink / raw)
To: Christopher Heiny
Cc: Linus Walleij, linux-input@vger.kernel.org,
dmitry.torokhov@gmail.com, Linus WALLEIJ
In-Reply-To: <52F3EAF5.7030203@synaptics.com>
On Thu, Feb 06, 2014 at 09:05:09PM +0100, Christopher Heiny wrote:
> On 02/06/2014 01:28 AM, Linus Walleij wrote:
> > On Wed, Feb 5, 2014 at 3:31 AM, Courtney Cavin
> > <courtney.cavin@sonymobile.com> wrote:
> >> >On Wed, Feb 05, 2014 at 12:08:35AM +0100, Christopher Heiny wrote:
> >>> >>On 01/23/2014 04:00 PM, Courtney Cavin wrote:
> >>>> >> >Since all the configuration needed for an irq can be provided in other
> >>>> >> >ways, remove all gpio->irq functionality. This cleans up the code quite
> >>>> >> >a bit.
> >>> >>
> >>> >>In certain diagnostic modes, we need to be able to release the IRQ so
> >>> >>the GPIO can be monitored from userspace. This patch removes that
> >>> >>capability.
> >> >
> >> >Polling a GPIO from userspace is poor design regardless of the use-case, if
> >> >you ask me. It certainly doesn't motivate the extra gpio<->IRQ code.
> >
> > I think Courtney is right here, if you want this kind of stuff for debugging
> > it should be debug-add-on-patches, not part of the normal execution
> > path for the driver.
>
> I'm OK with replacing the current polling implementation with an
> add-on-patch, as long as the support for the debug-add-on-patches
> doesn't get torn out of the driver because "hey, I don't see what this
> is used for in the core driver code, so it must be unnecessary".
An add-on patch shouldn't force dependencies on the code which it
patches. Thus, the code shouldn't need support for debug add-ons.
Besides, try "hey, this isn't used in the core driver code, so it is
unnecessary." This whole "there's another tree which depends on things
staying the way they are," isn't conducive to actually making changes to
improve the driver.
> >>> >>As mentioned in previous patch discussions, the polling functionality is
> >>> >>quite useful during new system integration, so we need to retain that.
> >>> >>If you have a proposal for where else it could be done, we'd certainly
> >>> >>entertain a patch that implements that.
> >> >
> >> >Do you actually have systems that have these hooked up to GPIOs without
> >> >IRQ support?
> >> >
> >> >Regardless, if this is the case, implementing a GPIO polling IRQ chip
> >> >should be possible, if extremely ugly.
> > OMG that would be quite horrible.
> >
> >> >Linus may have some comments in this area, though. Linus?
> >
> > A driver may rely on polling but a driver may not rely on polling
> > done from userspace.
>
> Fortunately, the driver doesn't rely on polling from user space.
> --
> 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 05/15] Input: synaptics-rmi4 - remove gpio handling and polling
From: Courtney Cavin @ 2014-02-07 1:47 UTC (permalink / raw)
To: Christopher Heiny
Cc: linux-input@vger.kernel.org, dmitry.torokhov@gmail.com,
linus.walleij@stericsson.com, linus.walleij@linaro.org
In-Reply-To: <52F3EAF1.9000505@synaptics.com>
On Thu, Feb 06, 2014 at 09:05:05PM +0100, Christopher Heiny wrote:
> On 02/04/2014 06:31 PM, Courtney Cavin wrote:
> > On Wed, Feb 05, 2014 at 12:08:35AM +0100, Christopher Heiny wrote:
> >> On 01/23/2014 04:00 PM, Courtney Cavin wrote:
> >>> Since all the configuration needed for an irq can be provided in other
> >>> ways, remove all gpio->irq functionality. This cleans up the code quite
> >>> a bit.
> >>
> >> In certain diagnostic modes, we need to be able to release the IRQ so
> >> the GPIO can be monitored from userspace. This patch removes that
> >> capability.
> >>
> >
> > Polling a GPIO from userspace is poor design regardless of the use-case, if
> > you ask me. It certainly doesn't motivate the extra gpio<->IRQ code.
>
> Fortunately, neither the in-driver polling implementation nor the user
> space diagnostic applications are implemented that way.
I'm missing something here then. If the userspace diagnostic
applications do not poll the GPIO, then what exactly is the purpose of
them accessing the GPIO at all?
^ permalink raw reply
* Fwd: FW: Dell Latitude E7440 - ALPS touchpad keeps having state reset
From: Tommy Will @ 2014-02-07 7:46 UTC (permalink / raw)
To: allan; +Cc: linux-input@vger.kernel.org
In-Reply-To: <CA+F6ckM4gTBAE5Z3ymc=V8Oo73CV4p5tRTCZYMMVi2+3SK+U6g@mail.gmail.com>
Hi Allan,
This is Tommy from ALPS. Thanks for your contact and so sorry for my
overlooking your last week's mail.
> I've reported the problem I'm having in more detail here:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1258837
>
> In summary - I keep suffering from the mouse pointer jumping around erratically, with the following messages output to syslog:
> psmouse serio1: DualPoint TouchPad at isa0060/serio1/input0 lost sync at byte 1 psmouse serio1: DualPoint TouchPad at isa0060/serio1/input0 - driver resynced.
>
> Other people have reported the same problem in that bug report.
> Bisecting the kernel, the problem seems to have always been in place as soon as the touchpad was correctly identified; according to xinput
> - it's a "AlpsPS/2 ALPS DualPoint TouchPad". When it was being identified as a generic PS/2 mouse, there's no problem (though of course, there's also no touchpad functionality available either).
>
It seems that with the latest kernel, your touchpad became crazy
erratically and almost cannot use?
It looks like that our kernel driver did not let touchpad enter the
correct mode or did not use the correct way to decode the raw data.
>From another mail sent by Vasily, he met the same issue with touchpad
on Dell E7440 as yours.
According to his touchpad info "E7: 73-03-0A EC: 88-08-22", I think we
can start the investigation.
Please give me some time and I'll feedback you later. Thanks!
--
Best Regards,
Tommy
^ permalink raw reply
* ShuttlePROv2 jog dial reports EV_REL events with absolute position event values; pos 0 events get dropped
From: Harald Albrecht @ 2014-02-07 7:51 UTC (permalink / raw)
To: linux-input
I have a ShuttlePRO v2 (Contour Design Inc) that identifies itself as a
USB HID. The kernel properly detects its and I can receive events from
the device. The ShuttlePRO v2 is a media controller for use with video
editing software, as well as for many other applications. The controller
itself has a jog dial for frame-by-frame movement. Next, it has a
so-called shuttle, which is ring around the dial, but separate from it.
The shuttle can be turned CW and CCW up to 7 positions and a spring
ensures it returns to its center position when you take your hand off of
it. The shuttle is used to play video or audio forwards or backwards,
with varying speed. Finally, there are 15 buttons on the ShuttlePRO v2.
However, there is an annoying quirk of which I don't at this time where
it comes from. The Linux event/input system does not report when the
shuttle returns to its home/center position. It seems as if the event
sent by the device gets swallowed. I know that the device sends such an
event, as in Windows there is no such problem. Similar, when turning the
jog dial, every 256th position gets swallowed as well.
I suspect the device manufacturer to have messed up this, but I may be
wrong. Anyway, I'm looking for help to get around this annoying quirk.
When I point evtest at the ShuttlePro v2, it reports the following
information:
Input driver version is 1.0.1
Input device ID: bus 0x3 vendor 0xb33 product 0x30 version 0x110
Input device name: "Contour Design ShuttlePRO v2"
Supported events:
Event type 0 (EV_SYN)
Event type 1 (EV_KEY)
Event code 256 (BTN_0)
Event code 257 (BTN_1)
Event code 258 (BTN_2)
Event code 259 (BTN_3)
Event code 260 (BTN_4)
Event code 261 (BTN_5)
Event code 262 (BTN_6)
Event code 263 (BTN_7)
Event code 264 (BTN_8)
Event code 265 (BTN_9)
Event code 266 (?)
Event code 267 (?)
Event code 268 (?)
Event code 269 (?)
Event code 270 (?)
Event type 2 (EV_REL)
Event code 7 (REL_DIAL)
Event code 8 (REL_WHEEL)
Event type 4 (EV_MSC)
Event code 4 (MSC_SCAN)
Please notice that for whatever reason, the ShuttlePRO v2 uses relative
events for the dial and shuttle (wheel). Unfortunately, the device sends
events that contain the dial and shuttle absolute positions.
Please notice that for whatever reason, the ShuttlePRO v2 uses relative
events for the dial and shuttle (wheel). At least that is my
understanding of the output from evtest. Unfortunately, the ShuttlePRO
device sends events that contain the dial and shuttle absolute
positions. This can be clearly seen from evtest output:
Event: type 2 (EV_REL), code 8 (REL_WHEEL), value 1
Event: -------------- SYN_REPORT ------------
Event: type 2 (EV_REL), code 8 (REL_WHEEL), value 2
Event: -------------- SYN_REPORT ------------
Event: type 2 (EV_REL), code 8 (REL_WHEEL), value 1
Event: -------------- SYN_REPORT ------------
Event: type 2 (EV_REL), code 8 (REL_WHEEL), value -1
Event: -------------- SYN_REPORT ------------
My impression is that the Linux HID module correctly assumes that
relative events without any position movement can be dropped. Can
someone please confirm, if this actually is the case?
The issue for the jog dial is similar, here whenever the position
counter which is in the range of 0 to 255 doesn't get reported when it
reaches pos 0.
Event: type 2 (EV_REL), code 7 (REL_DIAL), value 2
Event: -------------- SYN_REPORT ------------
Event: type 2 (EV_REL), code 7 (REL_DIAL), value 1
Event: -------------- SYN_REPORT ------------
Event: type 2 (EV_REL), code 7 (REL_DIAL), value 255
Event: -------------- SYN_REPORT ------------
Is there any change of getting the handling of the ShuttlePRO v2 fixed
i? I asked on the linux-usb mailing list and someone pointed me in the
direction of this mailing list. As I see the situation, it would be
preferable if dropping the events could be fixed because then existing
software does not need to be changed, as it currently expects REL_DIAL
and REL_WHEEL events. In contrast, trying to fix the issue by mapping
the EV_REL to EV_ABS events, would cause all existing software that
works with the ShuttlePRO v2 to break completely.
Any help and points greatly appreciated!
-- TheDiveO
^ permalink raw reply
* Re: PROBLEM: [Dell LATITUDE E7440] ALPS touchpad events are not routed into /dev/input/mice and /dev/input/mouseX
From: Tommy Will @ 2014-02-07 8:18 UTC (permalink / raw)
To: qehgt0; +Cc: linux-input@vger.kernel.org
Hi Vasily,
Thanks for your mail & information. We would start the investigation
and give you feedback later.
Please wait.
> The issues occurs on every kernel after
> 1302bac33d9e88cd43e482191a806998f3ed43cc commit. Before the commit, this touchpad device was detected as PS/2 mouse and "cat /dev/input/mice" shows data when the touchpad is used.
> After the commit, there are no events from the touchpad in /dev/input/mice or /dev/input/mouseX files anymore.
>
> According to drivers/input/mouse/alps.c source file, all devices with signatures "ec[0] == 0x88 && ec[1] == 0x08" are treated as "Rushmore touchpads". This touchpad has E7 signature of "73-03-0A" and EC signature of "88-08-22", and it looks like that V5 protocol (not
> "Rushmore") should be used here. If V5 protocol is forced to use for this device then everything works fine.
>
> The patch is trivial, but I'm not sure how to properly separate "Rushmore" and "V5" devices. Are there any datasheets or experts to help me with it?
>
I checked the E7 & EC information and think it should be a Rushmore device.
Could you please let me know after you use V5 protocol, if touchpad
still be recognized as "AlpsPS/2 ALPS DualPoint TouchPad" or returns
to "PS/2 Generic Mouse" ?
Thanks
--
Best Regards,
Tommy
^ 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