* Re: [PATCH] HID: input: Fix TransducerSerialNumber implementation
From: Jason Gerecke @ 2014-10-28 18:31 UTC (permalink / raw)
To: Benjamin Tissoires, Jason Gerecke, linux-input@vger.kernel.org,
jkosina@suse.cz, pinglinux@gmail.com
Cc: Jason Gerecke
In-Reply-To: <1411495768-5956-1-git-send-email-killertofu@gmail.com>
On Tue, Sep 23, 2014 at 11:09 AM, Jason Gerecke <killertofu@gmail.com> wrote:
> The commit which introduced TransducerSerialNumber (368c966) is missing
> two crucial implementation details. Firstly, the commit does not set the
> type/code/bit/max fields as expected later down the code which can cause
> the driver to crash when a tablet with this usage is connected. Secondly,
> the call to 'set_bit' causes MSC_PULSELED to be sent instead of the
> expected MSC_SERIAL. This commit addreses both issues.
>
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> ---
> drivers/hid/hid-input.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 2619f7f..cb1b3fa 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -689,7 +689,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> break;
>
> case 0x5b: /* TransducerSerialNumber */
> - set_bit(MSC_SERIAL, input->mscbit);
> + usage->type = EV_MSC;
> + usage->code = MSC_SERIAL;
> + bit = input->mscbit;
> + max = MSC_MAX;
> break;
>
> default: goto unknown;
> --
> 2.1.0
>
This patch still seems to be in limbo. Anyone (Ping? Benjamin?) care
to provide an ACK or review?
Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] Input: driver for the Goodix touchpanel
From: Bastien Nocera @ 2014-10-28 18:59 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Henrik Rydberg, linux-input, Benjamin Tissoires
In-Reply-To: <20141028180538.GA22157@dtor-ws>
On Tue, 2014-10-28 at 11:05 -0700, Dmitry Torokhov wrote:
> On Tue, Oct 28, 2014 at 06:55:21PM +0100, Bastien Nocera wrote:
> > On Tue, 2014-10-07 at 13:58 -0700, Dmitry Torokhov wrote:
> > > On Wed, Sep 24, 2014 at 04:43:58PM +0200, Bastien Nocera wrote:
> > > > +static irqreturn_t goodix_ts_irq_handler(int irq, void *dev_id)
> > > > +{
> > > > + struct goodix_ts_data *ts = dev_id;
> > > > + u8 end_cmd[1] = {0};
> > > > +
> > > > + goodix_process_events(ts);
> > > > +
> > > > + if (goodix_i2c_write(ts->client,
> > > > + GOODIX_READ_COOR_ADDR, end_cmd, 1) < 0)
> > > > + dev_err(&ts->client->dev, "I2C write end_cmd error");
> > >
> > > I am not happy that we need to allocate/deallocate memory for each
> > > interrupt. We only write one command to the driver, we could simply use
> > > i2c_master_send() with a constant buffer.
> >
> > Sure. But I've split up the patch you sent us, and committed the
> > different bits separately in:
> > https://github.com/hadess/gt9xx/commits/master
> >
> > And this one commit about removing goodix_i2c_write():
> > https://github.com/hadess/gt9xx/commit/146b4cc2eed5c67bcf1cb91e845bf9f97da4be1e
> >
> > Breaks the driver.
>
> Ah, I see. In end_cmd I encoded the address as little endian, whereas it
> needs to be beg endian. Just swap "GOODIX_READ_COOR_ADDR & 0xff" and
> "GOODIX_READ_COOR_ADDR >> 8" around and I thin kit will work.
Indeed, fixed in:
https://github.com/hadess/gt9xx/commit/18e507c5c455a3d3d745380c4d0d561ea470a091
As for the various FIXMEs, could you (that includes Benjamin that
probably knows the driver more than he would like to) check whether
that's sensible?
https://github.com/hadess/gt9xx/commit/35b94f327edc04d95a7208a667553566faa871e3
If it is, I'll respin the patch and send it for merging.
Cheers
^ permalink raw reply
* Re: [PATCH 12/15] ARM: pxa: poodle: don't preallocate IRQ space for locomo
From: Robert Jarzmik @ 2014-10-28 19:13 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov
Cc: Alexandre Courbot, linux-fbdev, Bryan Wu, Russell King,
Samuel Ortiz, Liam Girdwood, Linus Walleij, Dmitry Torokhov,
alsa-devel, Haojian Zhuang, linux-spi, linux-gpio, Mark Brown,
Andrea Adami, linux-arm-kernel, linux-input, Jingoo Han,
Daniel Mack, Lee Jones, Richard Purdie, linux-leds
In-Reply-To: <1414454528-24240-13-git-send-email-dbaryshkov@gmail.com>
Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> writes:
...zip...
> diff --git a/arch/arm/mach-pxa/poodle.c b/arch/arm/mach-pxa/poodle.c
> index 3041d83..97eced9 100644
> --- a/arch/arm/mach-pxa/poodle.c
> +++ b/arch/arm/mach-pxa/poodle.c
> @@ -181,14 +181,18 @@ static struct locomo_platform_data locomo_info = {
> .gpio_data = {
> .gpio_base = POODLE_LOCOMO_GPIO_BASE,
> },
> - .bl_data = {
> + .lcd_data = {
Is that part really belonging to that patch ? It seems to me that this changes
the code included in patch 10/15 "ARM: pxa: poodle: use new LoCoMo
driver".
Therefore I'm wondering if I take the serie, halt it at patch 10, and try to
compile I would succeed ... I think I wouldn't.
> + },
> + .bl_data = {
> .gpio_fl_vr = POODLE_GPIO_FL_VR,
> },
Ditto.
> + .gpio_amp1_on = -1,
> + .gpio_amp2_on = -1,
This won't prevent compilation, but is it the target of this patch which is IRQ
space oriented ?
> @@ -453,7 +457,7 @@ static void __init poodle_init(void)
> platform_scoop_config = &poodle_pcmcia_config;
>
> if (sharpsl_param.comadj != -1)
> - locomo_info.bl_data.comadj = sharpsl_param.comadj;
> + locomo_info.lcd_data.comadj = sharpsl_param.comadj;
Ditto.
Cheers.
--
Robert
^ permalink raw reply
* Re: [PATCH v2] Input: driver for the Goodix touchpanel
From: Dmitry Torokhov @ 2014-10-28 19:14 UTC (permalink / raw)
To: Bastien Nocera; +Cc: Henrik Rydberg, linux-input, Benjamin Tissoires
In-Reply-To: <1414522745.2406.14.camel@hadess.net>
On Tue, Oct 28, 2014 at 07:59:05PM +0100, Bastien Nocera wrote:
> On Tue, 2014-10-28 at 11:05 -0700, Dmitry Torokhov wrote:
> > On Tue, Oct 28, 2014 at 06:55:21PM +0100, Bastien Nocera wrote:
> > > On Tue, 2014-10-07 at 13:58 -0700, Dmitry Torokhov wrote:
> > > > On Wed, Sep 24, 2014 at 04:43:58PM +0200, Bastien Nocera wrote:
> > > > > +static irqreturn_t goodix_ts_irq_handler(int irq, void *dev_id)
> > > > > +{
> > > > > + struct goodix_ts_data *ts = dev_id;
> > > > > + u8 end_cmd[1] = {0};
> > > > > +
> > > > > + goodix_process_events(ts);
> > > > > +
> > > > > + if (goodix_i2c_write(ts->client,
> > > > > + GOODIX_READ_COOR_ADDR, end_cmd, 1) < 0)
> > > > > + dev_err(&ts->client->dev, "I2C write end_cmd error");
> > > >
> > > > I am not happy that we need to allocate/deallocate memory for each
> > > > interrupt. We only write one command to the driver, we could simply use
> > > > i2c_master_send() with a constant buffer.
> > >
> > > Sure. But I've split up the patch you sent us, and committed the
> > > different bits separately in:
> > > https://github.com/hadess/gt9xx/commits/master
> > >
> > > And this one commit about removing goodix_i2c_write():
> > > https://github.com/hadess/gt9xx/commit/146b4cc2eed5c67bcf1cb91e845bf9f97da4be1e
> > >
> > > Breaks the driver.
> >
> > Ah, I see. In end_cmd I encoded the address as little endian, whereas it
> > needs to be beg endian. Just swap "GOODIX_READ_COOR_ADDR & 0xff" and
> > "GOODIX_READ_COOR_ADDR >> 8" around and I thin kit will work.
>
> Indeed, fixed in:
> https://github.com/hadess/gt9xx/commit/18e507c5c455a3d3d745380c4d0d561ea470a091
>
> As for the various FIXMEs, could you (that includes Benjamin that
> probably knows the driver more than he would like to) check whether
> that's sensible?
> https://github.com/hadess/gt9xx/commit/35b94f327edc04d95a7208a667553566faa871e3
>
> If it is, I'll respin the patch and send it for merging.
No, the "header" is not 10 bytes as far as I can see. IIUIC the device
sends packet with 1st byte containing number of contacts + (n_contacts *
GOODIX_CONTACT_SIZE) worth of data. The driver tries to optimize for
single-touch scenario and reads the counter and the very first contact
data and if the counter indicates more than one contact it will issue
second i2c transaction to read in the rest.
Your change did not alter the actual code, it just converted constant 10
into a define.
Please try changing FIXMEs exactly as they were and see if the driver
still works properly.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Input: driver for the Goodix touchpanel
From: Benjamin Tissoires @ 2014-10-28 19:16 UTC (permalink / raw)
To: Bastien Nocera; +Cc: Dmitry Torokhov, Henrik Rydberg, linux-input
In-Reply-To: <1414522745.2406.14.camel@hadess.net>
On Tue, Oct 28, 2014 at 2:59 PM, Bastien Nocera <hadess@hadess.net> wrote:
> On Tue, 2014-10-28 at 11:05 -0700, Dmitry Torokhov wrote:
>> On Tue, Oct 28, 2014 at 06:55:21PM +0100, Bastien Nocera wrote:
>> > On Tue, 2014-10-07 at 13:58 -0700, Dmitry Torokhov wrote:
>> > > On Wed, Sep 24, 2014 at 04:43:58PM +0200, Bastien Nocera wrote:
>> > > > +static irqreturn_t goodix_ts_irq_handler(int irq, void *dev_id)
>> > > > +{
>> > > > + struct goodix_ts_data *ts = dev_id;
>> > > > + u8 end_cmd[1] = {0};
>> > > > +
>> > > > + goodix_process_events(ts);
>> > > > +
>> > > > + if (goodix_i2c_write(ts->client,
>> > > > + GOODIX_READ_COOR_ADDR, end_cmd, 1) < 0)
>> > > > + dev_err(&ts->client->dev, "I2C write end_cmd error");
>> > >
>> > > I am not happy that we need to allocate/deallocate memory for each
>> > > interrupt. We only write one command to the driver, we could simply use
>> > > i2c_master_send() with a constant buffer.
>> >
>> > Sure. But I've split up the patch you sent us, and committed the
>> > different bits separately in:
>> > https://github.com/hadess/gt9xx/commits/master
>> >
>> > And this one commit about removing goodix_i2c_write():
>> > https://github.com/hadess/gt9xx/commit/146b4cc2eed5c67bcf1cb91e845bf9f97da4be1e
>> >
>> > Breaks the driver.
>>
>> Ah, I see. In end_cmd I encoded the address as little endian, whereas it
>> needs to be beg endian. Just swap "GOODIX_READ_COOR_ADDR & 0xff" and
>> "GOODIX_READ_COOR_ADDR >> 8" around and I thin kit will work.
>
> Indeed, fixed in:
> https://github.com/hadess/gt9xx/commit/18e507c5c455a3d3d745380c4d0d561ea470a091
>
> As for the various FIXMEs, could you (that includes Benjamin that
> probably knows the driver more than he would like to) check whether
> that's sensible?
> https://github.com/hadess/gt9xx/commit/35b94f327edc04d95a7208a667553566faa871e3
IIRC, the device is rather simple: the I2C way of accessing it is
register driven. You give the register address and read from it. The
10 contacts are all banked together starting at GOODIX_READ_COOR_ADDR
after one byte which indicates the contact count.
For efficiency purpose, the first read checks both the contact count
and the first contact data, then it reads the rest of the contact
data.
So where I wanted to be is that GOODIX_CONTACTS_HEADER_SIZE does not
make sense IMO, and using (GOODIX_CONTACT_SIZE + 1) is more reliable
(if it still works, of course).
BTW, Dmitry said the same... I could have just waited instead of
writing this up :)
Cheers,
Benjamin
>
> If it is, I'll respin the patch and send it for merging.
>
> Cheers
>
^ permalink raw reply
* Fwd: [PATCH] HID: input: Fix TransducerSerialNumber implementation
From: Ping Cheng @ 2014-10-28 19:33 UTC (permalink / raw)
To: linux-input
In-Reply-To: <CAF8JNhKYmBpgp4yRvQbxuaAPSk5dG3YW7qqZnrBAB22bRXPEJg@mail.gmail.com>
On Tue, Sep 23, 2014 at 11:09 AM, Jason Gerecke <killertofu@gmail.com> wrote:
>
> The commit which introduced TransducerSerialNumber (368c966) is missing
> two crucial implementation details. Firstly, the commit does not set the
> type/code/bit/max fields as expected later down the code which can cause
> the driver to crash when a tablet with this usage is connected. Secondly,
> the call to 'set_bit' causes MSC_PULSELED to be sent instead of the
> expected MSC_SERIAL. This commit addreses both issues.
>
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Reviewed-by: Ping Cheng <pingc@wacom.com>
Thank you, Jason, for the fix.
Ping
> ---
> drivers/hid/hid-input.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 2619f7f..cb1b3fa 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -689,7 +689,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> break;
>
> case 0x5b: /* TransducerSerialNumber */
> - set_bit(MSC_SERIAL, input->mscbit);
> + usage->type = EV_MSC;
> + usage->code = MSC_SERIAL;
> + bit = input->mscbit;
> + max = MSC_MAX;
> break;
>
> default: goto unknown;
> --
> 2.1.0
>
^ permalink raw reply
* [PATCH] Input: i8042 - add mux quirk for HP EliteBook 8470p
From: Luis Henriques @ 2014-10-28 19:48 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
This laptop requires active multiplexing to be enabled in order to be able
to separate the PS/2 mouse and the touchpad.
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
---
drivers/input/serio/i8042-x86ia64io.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
index a0bcbb64d06d..34b95e4268eb 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -220,6 +220,13 @@ static const struct dmi_system_id __initconst i8042_dmi_mux_table[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "CF-18"),
},
},
+ {
+ /* HP EliteBook 8470p */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook 8470p"),
+ },
+ },
{ }
};
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] HID: input: Fix TransducerSerialNumber implementation
From: Benjamin Tissoires @ 2014-10-28 19:58 UTC (permalink / raw)
To: Jason Gerecke
Cc: linux-input@vger.kernel.org, jkosina@suse.cz, pinglinux@gmail.com,
Jason Gerecke
In-Reply-To: <CANRwn3QbH-Jt4L7W-qOikX_JaeKs-vu_5XeK8TgzowyXLh_=qg@mail.gmail.com>
On Oct 28 2014 or thereabouts, Jason Gerecke wrote:
> On Tue, Sep 23, 2014 at 11:09 AM, Jason Gerecke <killertofu@gmail.com> wrote:
> > The commit which introduced TransducerSerialNumber (368c966) is missing
> > two crucial implementation details. Firstly, the commit does not set the
> > type/code/bit/max fields as expected later down the code which can cause
> > the driver to crash when a tablet with this usage is connected. Secondly,
> > the call to 'set_bit' causes MSC_PULSELED to be sent instead of the
> > expected MSC_SERIAL. This commit addreses both issues.
> >
> > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> > ---
> > drivers/hid/hid-input.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> > index 2619f7f..cb1b3fa 100644
> > --- a/drivers/hid/hid-input.c
> > +++ b/drivers/hid/hid-input.c
> > @@ -689,7 +689,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> > break;
> >
> > case 0x5b: /* TransducerSerialNumber */
> > - set_bit(MSC_SERIAL, input->mscbit);
> > + usage->type = EV_MSC;
> > + usage->code = MSC_SERIAL;
> > + bit = input->mscbit;
> > + max = MSC_MAX;
> > break;
> >
> > default: goto unknown;
> > --
> > 2.1.0
> >
>
> This patch still seems to be in limbo. Anyone (Ping? Benjamin?) care
> to provide an ACK or review?
>
Sorry for that, I was persuaded I already have answered to this one.
The patch makes sense, but I think it will be better to extend
hid_map_usage() with MSC bits and define map_msc_clear(). The code will
be more consistent with the rest this way.
Anyway, if Jiri does not find this to be mandatory, then this code is:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH] HID: usbhid: prevent unwanted events to be sent when re-opening the device
From: Benjamin Tissoires @ 2014-10-28 20:04 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Hans de Goede, Bastien Nocera,
linux-kernel@vger.kernel.org, linux-input
In-Reply-To: <1412101702-2752-1-git-send-email-benjamin.tissoires@redhat.com>
On Tue, Sep 30, 2014 at 2:28 PM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> When events occurs while no one is listening to the node (hid->open == 0
> and usb_kill_urb() called) some events are still stacked somewhere in
> the USB (kernel or device?) stack. When the node gets reopened, these
> events are drained, and this results in spurious touch down/up, or mouse
> button clicks.
>
> The problem was spotted with touchscreens in fdo bug #81781 [1], but it
> actually occurs with any mouse using hid-generic or touchscreen.
>
> A way to reproduce it is to call:
>
> $ xinput disable 9 ; sleep 5 ; xinput enable 9
>
> With 9 being the device ID for the touchscreen/mouse. During the "sleep",
> produce some touch events or click events. When "xinput enable" is called,
> at least one click is generated.
>
> This patch tries to fix this by draining the queue for 50 msec and
> during this time frame, not forwarding these old events to the hid layer.
>
> Hans completed the explanation:
> """
> Devices like mice (basically any hid device) will have a fifo
> on the device side, when we stop submitting urbs to get hid reports from
> it, that fifo will fill up, and when we resume we will get whatever
> is there in that fifo.
> """
>
> [1] https://bugs.freedesktop.org/show_bug.cgi?id=81781
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
My turn to raise patches lost in the mailboxes during the merge window....
Jiri, can you re-add this one to the pile of "things that you don't
want to do but some people are really relying on it so somebody still
needs to review it, sigh" list?
Cheers,
Benjamin
>
> OK, I know this is uggly, but I could not find a better way :(
>
> Cheers,
> Benjamin
>
> drivers/hid/usbhid/hid-core.c | 36 ++++++++++++++++++++++++------------
> drivers/hid/usbhid/usbhid.h | 1 +
> 2 files changed, 25 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index ca6849a..04e34b9 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -278,18 +278,20 @@ static void hid_irq_in(struct urb *urb)
> usbhid->retry_delay = 0;
> if ((hid->quirks & HID_QUIRK_ALWAYS_POLL) && !hid->open)
> break;
> - hid_input_report(urb->context, HID_INPUT_REPORT,
> - urb->transfer_buffer,
> - urb->actual_length, 1);
> - /*
> - * autosuspend refused while keys are pressed
> - * because most keyboards don't wake up when
> - * a key is released
> - */
> - if (hid_check_keys_pressed(hid))
> - set_bit(HID_KEYS_PRESSED, &usbhid->iofl);
> - else
> - clear_bit(HID_KEYS_PRESSED, &usbhid->iofl);
> + if (!test_bit(HID_RESUME_RUNNING, &usbhid->iofl)) {
> + hid_input_report(urb->context, HID_INPUT_REPORT,
> + urb->transfer_buffer,
> + urb->actual_length, 1);
> + /*
> + * autosuspend refused while keys are pressed
> + * because most keyboards don't wake up when
> + * a key is released
> + */
> + if (hid_check_keys_pressed(hid))
> + set_bit(HID_KEYS_PRESSED, &usbhid->iofl);
> + else
> + clear_bit(HID_KEYS_PRESSED, &usbhid->iofl);
> + }
> break;
> case -EPIPE: /* stall */
> usbhid_mark_busy(usbhid);
> @@ -688,6 +690,7 @@ int usbhid_open(struct hid_device *hid)
> goto done;
> }
> usbhid->intf->needs_remote_wakeup = 1;
> + set_bit(HID_RESUME_RUNNING, &usbhid->iofl);
> res = hid_start_in(hid);
> if (res) {
> if (res != -ENOSPC) {
> @@ -701,6 +704,15 @@ int usbhid_open(struct hid_device *hid)
> }
> }
> usb_autopm_put_interface(usbhid->intf);
> +
> + /*
> + * In case events are generated while nobody was listening,
> + * some are released when the device is re-opened.
> + * Wait 50 msec for the queue to empty before allowing events
> + * to go through hid.
> + */
> + msleep(50);
> + clear_bit(HID_RESUME_RUNNING, &usbhid->iofl);
> }
> done:
> mutex_unlock(&hid_open_mut);
> diff --git a/drivers/hid/usbhid/usbhid.h b/drivers/hid/usbhid/usbhid.h
> index f633c24..807922b 100644
> --- a/drivers/hid/usbhid/usbhid.h
> +++ b/drivers/hid/usbhid/usbhid.h
> @@ -52,6 +52,7 @@ struct usb_interface *usbhid_find_interface(int minor);
> #define HID_STARTED 8
> #define HID_KEYS_PRESSED 10
> #define HID_NO_BANDWIDTH 11
> +#define HID_RESUME_RUNNING 12
>
> /*
> * USB-specific HID struct, to be pointed to
> --
> 2.1.0
>
> --
> 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 00/13] HID: add support of Logitech touchpads and special devices
From: Benjamin Tissoires @ 2014-10-28 20:05 UTC (permalink / raw)
To: Jiri Kosina
Cc: Andrew de los Reyes, Benjamin Tissoires, Nestor Lopez Casado,
linux-kernel@vger.kernel.org, Linux Input
In-Reply-To: <alpine.LNX.2.00.1410031308360.14259@pobox.suse.cz>
On Fri, Oct 3, 2014 at 7:09 AM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Thu, 2 Oct 2014, Andrew de los Reyes wrote:
>
>> I've tested these patches successfully on a Chromebook Pixel with the
>> following devices:
>>
>> - T650 touchpad
>> - TK820 keyboard/touchpad
>> - Original WTP touchpad
>>
>> I also tested a touch mouse and as expected it continued to function
>> as a plain USB mouse. The TK820 was indeed broken into two devices,
>> with autorepeat enabled for the keyboard only. The other two devices
>> behaved as expected with no perceived regressions.
>>
>> In short, from a functionality perspective I'm happy with these
>> patches. I will let others discuss code structure.
>
> Thank you. Can I use
>
> Tested-by: Andrew de los Reyes <andrew-vger@gizmolabs.org>
>
> once / if I am merging this patchset?
>
I am pretty sure this is not required too, but I am raising this one
again in Jiri's pile of things to review.
Sorry Jiri, I did not forget about it :)
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH] Input: i8042 - add mux quirk for HP EliteBook 8470p
From: Dmitry Torokhov @ 2014-10-28 20:10 UTC (permalink / raw)
To: Luis Henriques; +Cc: linux-input, linux-kernel
In-Reply-To: <1414525704-8409-1-git-send-email-luis.henriques@canonical.com>
Hi Luis,
On Tue, Oct 28, 2014 at 07:48:24PM +0000, Luis Henriques wrote:
> This laptop requires active multiplexing to be enabled in order to be able
> to separate the PS/2 mouse and the touchpad.
>
> Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
> ---
> drivers/input/serio/i8042-x86ia64io.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
> index a0bcbb64d06d..34b95e4268eb 100644
> --- a/drivers/input/serio/i8042-x86ia64io.h
> +++ b/drivers/input/serio/i8042-x86ia64io.h
> @@ -220,6 +220,13 @@ static const struct dmi_system_id __initconst i8042_dmi_mux_table[] = {
> DMI_MATCH(DMI_PRODUCT_NAME, "CF-18"),
> },
> },
> + {
> + /* HP EliteBook 8470p */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook 8470p"),
Hmm, so the tide is turning the other way ;) I think I'll be comfortable
extending this to all EliteBooks as I know 840 and 1040 work as well and
I have not heard any other complaints about this model line.
Cold you resend with match on "HP EliteBook" instead of "HP EliteBook
8470p"?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] input: fix BTN_TOUCH reporting in input_mt_report_pointer_emulation
From: Benjamin Tissoires @ 2014-10-28 20:47 UTC (permalink / raw)
To: Chung-yih Wang
Cc: linux-input, linux-kernel@vger.kernel.org, Henrik Rydberg,
Dmitry Torokhov, Peter Hutterer, Hans de Goede
In-Reply-To: <1414404527-17526-1-git-send-email-cywang@chromium.org>
Hi Chung-yih,
On Mon, Oct 27, 2014 at 6:08 AM, Chung-yih Wang <cywang@chromium.org> wrote:
> From the definition of BTN_TOUCH, BTN_TOOL_<name> and BTN_TOUCH codes
> are orthogonal. BTN_TOUCH should be zero if there is no physical contact
> happened on device. With ABS_MT_DISTANCE information, the patch uses
> touch_count and finger_count to get the final reporting code for
> BTN_TOUCH and BTN_TOOL_<name>, respectively.
>
> Signed-off-by: Chung-yih Wang <cywang@chromium.org>
> ---
> drivers/input/input-mt.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
> index fbe29fc..a2ab8e3 100644
> --- a/drivers/input/input-mt.c
> +++ b/drivers/input/input-mt.c
> @@ -192,18 +192,21 @@ void input_mt_report_pointer_emulation(struct input_dev *dev, bool use_count)
> {
> struct input_mt *mt = dev->mt;
> struct input_mt_slot *oldest;
> - int oldid, count, i;
> + int oldid, i;
> + int touch_count, finger_count;
>
> if (!mt)
> return;
>
> oldest = NULL;
> oldid = mt->trkid;
> - count = 0;
> + touch_count = 0;
> + finger_count = 0;
>
> for (i = 0; i < mt->num_slots; ++i) {
> struct input_mt_slot *ps = &mt->slots[i];
> int id = input_mt_get_value(ps, ABS_MT_TRACKING_ID);
> + int distance = input_mt_get_value(ps, ABS_MT_DISTANCE);
I don't really like this statement, even if it works. Some devices do
not report ABS_MT_DISTANCE, and the value for them should not be
considered as '0' but 'undefined'.
>
> if (id < 0)
> continue;
> @@ -211,12 +214,14 @@ void input_mt_report_pointer_emulation(struct input_dev *dev, bool use_count)
> oldest = ps;
> oldid = id;
> }
> - count++;
> + finger_count++;
> + if (distance == 0)
I'd rather test here if ABS_MT_DISTANCE is used, and then if the value is 0.
> + touch_count++;
> }
>
> - input_event(dev, EV_KEY, BTN_TOUCH, count > 0);
> + input_event(dev, EV_KEY, BTN_TOUCH, touch_count > 0);
For the record, I *think* this will not break user space. This is used
in the touchpad part of libinput, and does not seemed to be impacted
by the change.
Adding Peter and Hans in CC who can tell us if I am assuming right.
Cheers,
Benjamin
> if (use_count)
> - input_mt_report_finger_count(dev, count);
> + input_mt_report_finger_count(dev, finger_count);
>
> if (oldest) {
> int x = input_mt_get_value(oldest, ABS_MT_POSITION_X);
> --
> 2.1.2
>
^ permalink raw reply
* Re: [PATCH v2] input: fix BTN_TOUCH reporting in input_mt_report_pointer_emulation
From: Dmitry Torokhov @ 2014-10-28 21:07 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Chung-yih Wang, linux-input, linux-kernel@vger.kernel.org,
Henrik Rydberg, Peter Hutterer, Hans de Goede
In-Reply-To: <CAN+gG=G+YYntYg3rpGP+rUU8wT6Ejny8nrdG7K_cfyt7BwkWbA@mail.gmail.com>
On Tue, Oct 28, 2014 at 04:47:56PM -0400, Benjamin Tissoires wrote:
> Hi Chung-yih,
>
> On Mon, Oct 27, 2014 at 6:08 AM, Chung-yih Wang <cywang@chromium.org> wrote:
> > From the definition of BTN_TOUCH, BTN_TOOL_<name> and BTN_TOUCH codes
> > are orthogonal. BTN_TOUCH should be zero if there is no physical contact
> > happened on device. With ABS_MT_DISTANCE information, the patch uses
> > touch_count and finger_count to get the final reporting code for
> > BTN_TOUCH and BTN_TOOL_<name>, respectively.
> >
> > Signed-off-by: Chung-yih Wang <cywang@chromium.org>
> > ---
> > drivers/input/input-mt.c | 15 ++++++++++-----
> > 1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
> > index fbe29fc..a2ab8e3 100644
> > --- a/drivers/input/input-mt.c
> > +++ b/drivers/input/input-mt.c
> > @@ -192,18 +192,21 @@ void input_mt_report_pointer_emulation(struct input_dev *dev, bool use_count)
> > {
> > struct input_mt *mt = dev->mt;
> > struct input_mt_slot *oldest;
> > - int oldid, count, i;
> > + int oldid, i;
> > + int touch_count, finger_count;
> >
> > if (!mt)
> > return;
> >
> > oldest = NULL;
> > oldid = mt->trkid;
> > - count = 0;
> > + touch_count = 0;
> > + finger_count = 0;
> >
> > for (i = 0; i < mt->num_slots; ++i) {
> > struct input_mt_slot *ps = &mt->slots[i];
> > int id = input_mt_get_value(ps, ABS_MT_TRACKING_ID);
> > + int distance = input_mt_get_value(ps, ABS_MT_DISTANCE);
>
> I don't really like this statement, even if it works. Some devices do
> not report ABS_MT_DISTANCE, and the value for them should not be
> considered as '0' but 'undefined'.
I think it incidentally still 0 as we zero out memory we allocate.
>
> >
> > if (id < 0)
> > continue;
> > @@ -211,12 +214,14 @@ void input_mt_report_pointer_emulation(struct input_dev *dev, bool use_count)
> > oldest = ps;
> > oldid = id;
> > }
> > - count++;
> > + finger_count++;
> > + if (distance == 0)
>
> I'd rather test here if ABS_MT_DISTANCE is used, and then if the value is 0.
>
> > + touch_count++;
I do not think we should include hovering contacts into finger_count
either. I.e if I have one finger touching and one finger hovering we
should be emitting BTN_TOOL_FINGER and not BTN_TOOL_DOUBLETAP.
> > }
> >
> > - input_event(dev, EV_KEY, BTN_TOUCH, count > 0);
> > + input_event(dev, EV_KEY, BTN_TOUCH, touch_count > 0);
>
> For the record, I *think* this will not break user space. This is used
> in the touchpad part of libinput, and does not seemed to be impacted
> by the change.
> Adding Peter and Hans in CC who can tell us if I am assuming right.
>
> Cheers,
> Benjamin
>
> > if (use_count)
> > - input_mt_report_finger_count(dev, count);
> > + input_mt_report_finger_count(dev, finger_count);
> >
> > if (oldest) {
> > int x = input_mt_get_value(oldest, ABS_MT_POSITION_X);
> > --
> > 2.1.2
> >
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: i8042 - add mux quirk for HP EliteBook 8470p
From: Luis Henriques @ 2014-10-28 22:25 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
In-Reply-To: <20141028201046.GB7461@dtor-ws>
On Tue, Oct 28, 2014 at 01:10:46PM -0700, Dmitry Torokhov wrote:
> Hi Luis,
>
> On Tue, Oct 28, 2014 at 07:48:24PM +0000, Luis Henriques wrote:
> > This laptop requires active multiplexing to be enabled in order to be able
> > to separate the PS/2 mouse and the touchpad.
> >
> > Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
> > ---
> > drivers/input/serio/i8042-x86ia64io.h | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
> > index a0bcbb64d06d..34b95e4268eb 100644
> > --- a/drivers/input/serio/i8042-x86ia64io.h
> > +++ b/drivers/input/serio/i8042-x86ia64io.h
> > @@ -220,6 +220,13 @@ static const struct dmi_system_id __initconst i8042_dmi_mux_table[] = {
> > DMI_MATCH(DMI_PRODUCT_NAME, "CF-18"),
> > },
> > },
> > + {
> > + /* HP EliteBook 8470p */
> > + .matches = {
> > + DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> > + DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook 8470p"),
>
> Hmm, so the tide is turning the other way ;) I think I'll be comfortable
> extending this to all EliteBooks as I know 840 and 1040 work as well and
> I have not heard any other complaints about this model line.
>
> Cold you resend with match on "HP EliteBook" instead of "HP EliteBook
> 8470p"?
>
Sure, sending it in a minute.
Cheers,
--
Luís
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* [PATCH v2] Input: i8042 - add mux quirk for HP EliteBook
From: Luis Henriques @ 2014-10-28 22:27 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
This laptop requires active multiplexing to be enabled in order to be able
to separate the PS/2 mouse and the touchpad.
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
---
drivers/input/serio/i8042-x86ia64io.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
index a0bcbb64d06d..2bd486e83b43 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -220,6 +220,13 @@ static const struct dmi_system_id __initconst i8042_dmi_mux_table[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "CF-18"),
},
},
+ {
+ /* HP EliteBook */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook"),
+ },
+ },
{ }
};
--
2.1.0
^ permalink raw reply related
* Re: [PATCH v2] Input: driver for the Goodix touchpanel
From: Bastien Nocera @ 2014-10-28 22:31 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Henrik Rydberg, linux-input, Benjamin Tissoires
In-Reply-To: <20141028191446.GA7461@dtor-ws>
On Tue, 2014-10-28 at 12:14 -0700, Dmitry Torokhov wrote:
> On Tue, Oct 28, 2014 at 07:59:05PM +0100, Bastien Nocera wrote:
> > On Tue, 2014-10-28 at 11:05 -0700, Dmitry Torokhov wrote:
> > > On Tue, Oct 28, 2014 at 06:55:21PM +0100, Bastien Nocera wrote:
> > > > On Tue, 2014-10-07 at 13:58 -0700, Dmitry Torokhov wrote:
> > > > > On Wed, Sep 24, 2014 at 04:43:58PM +0200, Bastien Nocera wrote:
> > > > > > +static irqreturn_t goodix_ts_irq_handler(int irq, void *dev_id)
> > > > > > +{
> > > > > > + struct goodix_ts_data *ts = dev_id;
> > > > > > + u8 end_cmd[1] = {0};
> > > > > > +
> > > > > > + goodix_process_events(ts);
> > > > > > +
> > > > > > + if (goodix_i2c_write(ts->client,
> > > > > > + GOODIX_READ_COOR_ADDR, end_cmd, 1) < 0)
> > > > > > + dev_err(&ts->client->dev, "I2C write end_cmd error");
> > > > >
> > > > > I am not happy that we need to allocate/deallocate memory for each
> > > > > interrupt. We only write one command to the driver, we could simply use
> > > > > i2c_master_send() with a constant buffer.
> > > >
> > > > Sure. But I've split up the patch you sent us, and committed the
> > > > different bits separately in:
> > > > https://github.com/hadess/gt9xx/commits/master
> > > >
> > > > And this one commit about removing goodix_i2c_write():
> > > > https://github.com/hadess/gt9xx/commit/146b4cc2eed5c67bcf1cb91e845bf9f97da4be1e
> > > >
> > > > Breaks the driver.
> > >
> > > Ah, I see. In end_cmd I encoded the address as little endian, whereas it
> > > needs to be beg endian. Just swap "GOODIX_READ_COOR_ADDR & 0xff" and
> > > "GOODIX_READ_COOR_ADDR >> 8" around and I thin kit will work.
> >
> > Indeed, fixed in:
> > https://github.com/hadess/gt9xx/commit/18e507c5c455a3d3d745380c4d0d561ea470a091
> >
> > As for the various FIXMEs, could you (that includes Benjamin that
> > probably knows the driver more than he would like to) check whether
> > that's sensible?
> > https://github.com/hadess/gt9xx/commit/35b94f327edc04d95a7208a667553566faa871e3
> >
> > If it is, I'll respin the patch and send it for merging.
>
> No, the "header" is not 10 bytes as far as I can see. IIUIC the device
> sends packet with 1st byte containing number of contacts + (n_contacts *
> GOODIX_CONTACT_SIZE) worth of data. The driver tries to optimize for
> single-touch scenario and reads the counter and the very first contact
> data and if the counter indicates more than one contact it will issue
> second i2c transaction to read in the rest.
>
> Your change did not alter the actual code, it just converted constant 10
> into a define.
Not quite, but that's besides the point.
> Please try changing FIXMEs exactly as they were and see if the driver
> still works properly.
I did, and it still works properly. I also don't understand how it could
have worked properly before...
Should I still add those linefeeds to all the print statements in the
driver?
Cheers
^ permalink raw reply
* Re: [PATCH v2] Input: driver for the Goodix touchpanel
From: Dmitry Torokhov @ 2014-10-28 23:04 UTC (permalink / raw)
To: Bastien Nocera; +Cc: Henrik Rydberg, linux-input, Benjamin Tissoires
In-Reply-To: <1414535490.2406.20.camel@hadess.net>
On Tue, Oct 28, 2014 at 11:31:30PM +0100, Bastien Nocera wrote:
> On Tue, 2014-10-28 at 12:14 -0700, Dmitry Torokhov wrote:
> > On Tue, Oct 28, 2014 at 07:59:05PM +0100, Bastien Nocera wrote:
> > > On Tue, 2014-10-28 at 11:05 -0700, Dmitry Torokhov wrote:
> > > > On Tue, Oct 28, 2014 at 06:55:21PM +0100, Bastien Nocera wrote:
> > > > > On Tue, 2014-10-07 at 13:58 -0700, Dmitry Torokhov wrote:
> > > > > > On Wed, Sep 24, 2014 at 04:43:58PM +0200, Bastien Nocera wrote:
> > > > > > > +static irqreturn_t goodix_ts_irq_handler(int irq, void *dev_id)
> > > > > > > +{
> > > > > > > + struct goodix_ts_data *ts = dev_id;
> > > > > > > + u8 end_cmd[1] = {0};
> > > > > > > +
> > > > > > > + goodix_process_events(ts);
> > > > > > > +
> > > > > > > + if (goodix_i2c_write(ts->client,
> > > > > > > + GOODIX_READ_COOR_ADDR, end_cmd, 1) < 0)
> > > > > > > + dev_err(&ts->client->dev, "I2C write end_cmd error");
> > > > > >
> > > > > > I am not happy that we need to allocate/deallocate memory for each
> > > > > > interrupt. We only write one command to the driver, we could simply use
> > > > > > i2c_master_send() with a constant buffer.
> > > > >
> > > > > Sure. But I've split up the patch you sent us, and committed the
> > > > > different bits separately in:
> > > > > https://github.com/hadess/gt9xx/commits/master
> > > > >
> > > > > And this one commit about removing goodix_i2c_write():
> > > > > https://github.com/hadess/gt9xx/commit/146b4cc2eed5c67bcf1cb91e845bf9f97da4be1e
> > > > >
> > > > > Breaks the driver.
> > > >
> > > > Ah, I see. In end_cmd I encoded the address as little endian, whereas it
> > > > needs to be beg endian. Just swap "GOODIX_READ_COOR_ADDR & 0xff" and
> > > > "GOODIX_READ_COOR_ADDR >> 8" around and I thin kit will work.
> > >
> > > Indeed, fixed in:
> > > https://github.com/hadess/gt9xx/commit/18e507c5c455a3d3d745380c4d0d561ea470a091
> > >
> > > As for the various FIXMEs, could you (that includes Benjamin that
> > > probably knows the driver more than he would like to) check whether
> > > that's sensible?
> > > https://github.com/hadess/gt9xx/commit/35b94f327edc04d95a7208a667553566faa871e3
> > >
> > > If it is, I'll respin the patch and send it for merging.
> >
> > No, the "header" is not 10 bytes as far as I can see. IIUIC the device
> > sends packet with 1st byte containing number of contacts + (n_contacts *
> > GOODIX_CONTACT_SIZE) worth of data. The driver tries to optimize for
> > single-touch scenario and reads the counter and the very first contact
> > data and if the counter indicates more than one contact it will issue
> > second i2c transaction to read in the rest.
> >
> > Your change did not alter the actual code, it just converted constant 10
> > into a define.
>
> Not quite, but that's besides the point.
>
> > Please try changing FIXMEs exactly as they were and see if the driver
> > still works properly.
>
> I did, and it still works properly. I also don't understand how it could
> have worked properly before...
>
> Should I still add those linefeeds to all the print statements in the
> driver?
Yes please.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Input: i8042 - add mux quirk for HP EliteBook
From: Dmitry Torokhov @ 2014-10-28 23:08 UTC (permalink / raw)
To: Luis Henriques; +Cc: linux-input, linux-kernel
In-Reply-To: <1414535232-4780-1-git-send-email-luis.henriques@canonical.com>
On Tue, Oct 28, 2014 at 10:27:12PM +0000, Luis Henriques wrote:
> This laptop requires active multiplexing to be enabled in order to be able
> to separate the PS/2 mouse and the touchpad.
>
> Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
Applied, thank you.
> ---
> drivers/input/serio/i8042-x86ia64io.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
> index a0bcbb64d06d..2bd486e83b43 100644
> --- a/drivers/input/serio/i8042-x86ia64io.h
> +++ b/drivers/input/serio/i8042-x86ia64io.h
> @@ -220,6 +220,13 @@ static const struct dmi_system_id __initconst i8042_dmi_mux_table[] = {
> DMI_MATCH(DMI_PRODUCT_NAME, "CF-18"),
> },
> },
> + {
> + /* HP EliteBook */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook"),
> + },
> + },
> { }
> };
>
> --
> 2.1.0
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] input: fix BTN_TOUCH reporting in input_mt_report_pointer_emulation
From: Peter Hutterer @ 2014-10-29 0:14 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Benjamin Tissoires, Chung-yih Wang, linux-input,
linux-kernel@vger.kernel.org, Henrik Rydberg, Hans de Goede
In-Reply-To: <20141028210706.GC7461@dtor-ws>
On Tue, Oct 28, 2014 at 02:07:06PM -0700, Dmitry Torokhov wrote:
> On Tue, Oct 28, 2014 at 04:47:56PM -0400, Benjamin Tissoires wrote:
> > Hi Chung-yih,
> >
> > On Mon, Oct 27, 2014 at 6:08 AM, Chung-yih Wang <cywang@chromium.org> wrote:
> > > From the definition of BTN_TOUCH, BTN_TOOL_<name> and BTN_TOUCH codes
> > > are orthogonal. BTN_TOUCH should be zero if there is no physical contact
> > > happened on device. With ABS_MT_DISTANCE information, the patch uses
> > > touch_count and finger_count to get the final reporting code for
> > > BTN_TOUCH and BTN_TOOL_<name>, respectively.
> > >
> > > Signed-off-by: Chung-yih Wang <cywang@chromium.org>
> > > ---
> > > drivers/input/input-mt.c | 15 ++++++++++-----
> > > 1 file changed, 10 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
> > > index fbe29fc..a2ab8e3 100644
> > > --- a/drivers/input/input-mt.c
> > > +++ b/drivers/input/input-mt.c
> > > @@ -192,18 +192,21 @@ void input_mt_report_pointer_emulation(struct input_dev *dev, bool use_count)
> > > {
> > > struct input_mt *mt = dev->mt;
> > > struct input_mt_slot *oldest;
> > > - int oldid, count, i;
> > > + int oldid, i;
> > > + int touch_count, finger_count;
> > >
> > > if (!mt)
> > > return;
> > >
> > > oldest = NULL;
> > > oldid = mt->trkid;
> > > - count = 0;
> > > + touch_count = 0;
> > > + finger_count = 0;
> > >
> > > for (i = 0; i < mt->num_slots; ++i) {
> > > struct input_mt_slot *ps = &mt->slots[i];
> > > int id = input_mt_get_value(ps, ABS_MT_TRACKING_ID);
> > > + int distance = input_mt_get_value(ps, ABS_MT_DISTANCE);
> >
> > I don't really like this statement, even if it works. Some devices do
> > not report ABS_MT_DISTANCE, and the value for them should not be
> > considered as '0' but 'undefined'.
>
> I think it incidentally still 0 as we zero out memory we allocate.
>
> >
> > >
> > > if (id < 0)
> > > continue;
> > > @@ -211,12 +214,14 @@ void input_mt_report_pointer_emulation(struct input_dev *dev, bool use_count)
> > > oldest = ps;
> > > oldid = id;
> > > }
> > > - count++;
> > > + finger_count++;
> > > + if (distance == 0)
> >
> > I'd rather test here if ABS_MT_DISTANCE is used, and then if the value is 0.
shouldn't you test for absinfo->minimum anyway? or does the kernel guarantee
that 0 is the minimum for distance?
> >
> > > + touch_count++;
>
> I do not think we should include hovering contacts into finger_count
> either. I.e if I have one finger touching and one finger hovering we
> should be emitting BTN_TOOL_FINGER and not BTN_TOOL_DOUBLETAP.
yes, please. DOUBLETAP for hovering would trigger things like two-finger
scrolling in some clients, not ideal.
> > > }
> > >
> > > - input_event(dev, EV_KEY, BTN_TOUCH, count > 0);
> > > + input_event(dev, EV_KEY, BTN_TOUCH, touch_count > 0);
> >
> > For the record, I *think* this will not break user space. This is used
> > in the touchpad part of libinput, and does not seemed to be impacted
> > by the change.
> > Adding Peter and Hans in CC who can tell us if I am assuming right.
should be fine, we've treated BTN_TOUCH as signal that at least one touch is
down. in the wacom driver we use it for proximity info, the rest just never
had to worry about proximity, so it's most likely just unhandled.
Cheers,
Peter
> >
> > > if (use_count)
> > > - input_mt_report_finger_count(dev, count);
> > > + input_mt_report_finger_count(dev, finger_count);
> > >
> > > if (oldest) {
> > > int x = input_mt_get_value(oldest, ABS_MT_POSITION_X);
> > > --
> > > 2.1.2
> > >
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* [PATCH] input: add gpio based irq support to Elan touchpad
From: jagadish.krishnamoorthy @ 2014-10-29 0:27 UTC (permalink / raw)
To: dmitry.torokhov, bleung, dusonlin
Cc: linux-input, linux-kernel, Jagadish Krishnamoorthy
From: Jagadish Krishnamoorthy <jagadish.krishnamoorthy@intel.com>
Adding new member 'irq' in main device structure.
On some of the platforms, gpio is passed in platform data
instead of irq. Convert the gpio to irq and store it in the
driver data.
Signed-off-by: Jagadish Krishnamoorthy <jagadish.krishnamoorthy@intel.com>
---
drivers/input/mouse/elan_i2c.h | 3 ++-
drivers/input/mouse/elan_i2c_core.c | 44 ++++++++++++++++++++++++----------
drivers/input/mouse/elan_i2c_i2c.c | 7 +++---
drivers/input/mouse/elan_i2c_smbus.c | 3 ++-
4 files changed, 39 insertions(+), 18 deletions(-)
diff --git a/drivers/input/mouse/elan_i2c.h b/drivers/input/mouse/elan_i2c.h
index 2e83862..bb94a96 100644
--- a/drivers/input/mouse/elan_i2c.h
+++ b/drivers/input/mouse/elan_i2c.h
@@ -76,7 +76,8 @@ struct elan_transport_ops {
int (*write_fw_block)(struct i2c_client *client,
const u8 *page, u16 checksum, int idx);
int (*finish_fw_update)(struct i2c_client *client,
- struct completion *reset_done);
+ struct completion *reset_done,
+ unsigned int irq);
int (*get_report)(struct i2c_client *client, u8 *report);
};
diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
index 0cb2be4..9c5c5d3 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -35,6 +35,7 @@
#include <linux/completion.h>
#include <linux/of.h>
#include <linux/regulator/consumer.h>
+#include <linux/gpio.h>
#include <asm/unaligned.h>
#include "elan_i2c.h"
@@ -75,6 +76,7 @@ struct elan_tp_data {
unsigned int width_y;
unsigned int x_res;
unsigned int y_res;
+ unsigned int irq;
u8 product_id;
u8 fw_version;
@@ -331,7 +333,8 @@ static int __elan_update_firmware(struct elan_tp_data *data,
/* Wait WDT reset and power on reset */
msleep(600);
- error = data->ops->finish_fw_update(client, &data->fw_completion);
+ error = data->ops->finish_fw_update(client, &data->fw_completion,
+ data->irq);
if (error)
return error;
@@ -356,7 +359,7 @@ static int elan_update_firmware(struct elan_tp_data *data,
dev_dbg(&client->dev, "Starting firmware update....\n");
- disable_irq(client->irq);
+ disable_irq(data->irq);
data->in_fw_update = true;
retval = __elan_update_firmware(data, fw);
@@ -370,7 +373,7 @@ static int elan_update_firmware(struct elan_tp_data *data,
}
data->in_fw_update = false;
- enable_irq(client->irq);
+ enable_irq(data->irq);
return retval;
}
@@ -482,7 +485,7 @@ static ssize_t calibrate_store(struct device *dev,
if (retval)
return retval;
- disable_irq(client->irq);
+ disable_irq(data->irq);
data->mode |= ETP_ENABLE_CALIBRATE;
retval = data->ops->set_mode(client, data->mode);
@@ -528,7 +531,7 @@ out_disable_calibrate:
retval = error;
}
out:
- enable_irq(client->irq);
+ enable_irq(data->irq);
mutex_unlock(&data->sysfs_mutex);
return retval ?: count;
}
@@ -594,7 +597,7 @@ static ssize_t acquire_store(struct device *dev, struct device_attribute *attr,
if (retval)
return retval;
- disable_irq(client->irq);
+ disable_irq(data->irq);
data->baseline_ready = false;
@@ -636,7 +639,7 @@ out_disable_calibrate:
retval = error;
}
out:
- enable_irq(client->irq);
+ enable_irq(data->irq);
mutex_unlock(&data->sysfs_mutex);
return retval ?: count;
}
@@ -900,6 +903,7 @@ static int elan_probe(struct i2c_client *client,
const struct elan_transport_ops *transport_ops;
struct device *dev = &client->dev;
struct elan_tp_data *data;
+ struct gpio_desc *desc;
unsigned long irqflags;
int error;
@@ -992,17 +996,31 @@ static int elan_probe(struct i2c_client *client,
return error;
/*
+ * Check for the irq number in platform data.
+ * If that fails check for gpio based irq.
+ */
+ if (client->irq >= 0) {
+ data->irq = client->irq;
+ } else {
+ desc = devm_gpiod_get_index(&client->dev, "ELAN_GPIO_IRQ", 0);
+ if (IS_ERR(desc) || gpiod_direction_input(desc)) {
+ dev_err(&client->dev, "failed to initialize gpio\n");
+ return error;
+ }
+ data->irq = gpiod_to_irq(desc);
+ }
+ /*
* Systems using device tree should set up interrupt via DTS,
* the rest will use the default falling edge interrupts.
*/
irqflags = client->dev.of_node ? 0 : IRQF_TRIGGER_FALLING;
- error = devm_request_threaded_irq(&client->dev, client->irq,
+ error = devm_request_threaded_irq(&client->dev, data->irq,
NULL, elan_isr,
irqflags | IRQF_ONESHOT,
client->name, data);
if (error) {
- dev_err(&client->dev, "cannot register irq=%d\n", client->irq);
+ dev_err(&client->dev, "cannot register irq=%d\n", data->irq);
return error;
}
@@ -1055,12 +1073,12 @@ static int __maybe_unused elan_suspend(struct device *dev)
if (ret)
return ret;
- disable_irq(client->irq);
+ disable_irq(data->irq);
if (device_may_wakeup(dev)) {
ret = elan_sleep(data);
/* Enable wake from IRQ */
- data->irq_wake = (enable_irq_wake(client->irq) == 0);
+ data->irq_wake = (enable_irq_wake(data->irq) == 0);
} else {
ret = elan_disable_power(data);
}
@@ -1076,7 +1094,7 @@ static int __maybe_unused elan_resume(struct device *dev)
int error;
if (device_may_wakeup(dev) && data->irq_wake) {
- disable_irq_wake(client->irq);
+ disable_irq_wake(data->irq);
data->irq_wake = false;
}
@@ -1088,7 +1106,7 @@ static int __maybe_unused elan_resume(struct device *dev)
if (error)
dev_err(dev, "initialize when resuming failed: %d\n", error);
- enable_irq(data->client->irq);
+ enable_irq(data->irq);
return 0;
}
diff --git a/drivers/input/mouse/elan_i2c_i2c.c b/drivers/input/mouse/elan_i2c_i2c.c
index 97d4937..5e897cd 100644
--- a/drivers/input/mouse/elan_i2c_i2c.c
+++ b/drivers/input/mouse/elan_i2c_i2c.c
@@ -520,7 +520,8 @@ static int elan_i2c_write_fw_block(struct i2c_client *client,
}
static int elan_i2c_finish_fw_update(struct i2c_client *client,
- struct completion *completion)
+ struct completion *completion,
+ unsigned int irq)
{
struct device *dev = &client->dev;
long ret;
@@ -529,13 +530,13 @@ static int elan_i2c_finish_fw_update(struct i2c_client *client,
u8 buffer[ETP_I2C_INF_LENGTH];
reinit_completion(completion);
- enable_irq(client->irq);
+ enable_irq(irq);
error = elan_i2c_write_cmd(client, ETP_I2C_STAND_CMD, ETP_I2C_RESET);
if (!error)
ret = wait_for_completion_interruptible_timeout(completion,
msecs_to_jiffies(300));
- disable_irq(client->irq);
+ disable_irq(irq);
if (error) {
dev_err(dev, "device reset failed: %d\n", error);
diff --git a/drivers/input/mouse/elan_i2c_smbus.c b/drivers/input/mouse/elan_i2c_smbus.c
index 359bf85..a81b8fb 100644
--- a/drivers/input/mouse/elan_i2c_smbus.c
+++ b/drivers/input/mouse/elan_i2c_smbus.c
@@ -477,7 +477,8 @@ static int elan_smbus_get_report(struct i2c_client *client, u8 *report)
}
static int elan_smbus_finish_fw_update(struct i2c_client *client,
- struct completion *fw_completion)
+ struct completion *fw_completion,
+ unsigned int irq)
{
/* No special handling unlike I2C transport */
return 0;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] input: add gpio based irq support to Elan touchpad
From: Dmitry Torokhov @ 2014-10-29 0:50 UTC (permalink / raw)
To: jagadish.krishnamoorthy; +Cc: bleung, dusonlin, linux-input, linux-kernel
In-Reply-To: <1414542444-27184-1-git-send-email-jagadish.krishnamoorthy@intel.com>
On Tue, Oct 28, 2014 at 05:27:24PM -0700, jagadish.krishnamoorthy@intel.com wrote:
> From: Jagadish Krishnamoorthy <jagadish.krishnamoorthy@intel.com>
>
> Adding new member 'irq' in main device structure.
> On some of the platforms, gpio is passed in platform data
> instead of irq. Convert the gpio to irq and store it in the
> driver data.
No, please adjust your platform to set up i2c client data properly.
There is absolutely no reason for the driver to know if it delas with
gpio or some other interrupt source.
Thanks.
>
> Signed-off-by: Jagadish Krishnamoorthy <jagadish.krishnamoorthy@intel.com>
> ---
> drivers/input/mouse/elan_i2c.h | 3 ++-
> drivers/input/mouse/elan_i2c_core.c | 44 ++++++++++++++++++++++++----------
> drivers/input/mouse/elan_i2c_i2c.c | 7 +++---
> drivers/input/mouse/elan_i2c_smbus.c | 3 ++-
> 4 files changed, 39 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/input/mouse/elan_i2c.h b/drivers/input/mouse/elan_i2c.h
> index 2e83862..bb94a96 100644
> --- a/drivers/input/mouse/elan_i2c.h
> +++ b/drivers/input/mouse/elan_i2c.h
> @@ -76,7 +76,8 @@ struct elan_transport_ops {
> int (*write_fw_block)(struct i2c_client *client,
> const u8 *page, u16 checksum, int idx);
> int (*finish_fw_update)(struct i2c_client *client,
> - struct completion *reset_done);
> + struct completion *reset_done,
> + unsigned int irq);
>
> int (*get_report)(struct i2c_client *client, u8 *report);
> };
> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> index 0cb2be4..9c5c5d3 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -35,6 +35,7 @@
> #include <linux/completion.h>
> #include <linux/of.h>
> #include <linux/regulator/consumer.h>
> +#include <linux/gpio.h>
> #include <asm/unaligned.h>
>
> #include "elan_i2c.h"
> @@ -75,6 +76,7 @@ struct elan_tp_data {
> unsigned int width_y;
> unsigned int x_res;
> unsigned int y_res;
> + unsigned int irq;
>
> u8 product_id;
> u8 fw_version;
> @@ -331,7 +333,8 @@ static int __elan_update_firmware(struct elan_tp_data *data,
> /* Wait WDT reset and power on reset */
> msleep(600);
>
> - error = data->ops->finish_fw_update(client, &data->fw_completion);
> + error = data->ops->finish_fw_update(client, &data->fw_completion,
> + data->irq);
> if (error)
> return error;
>
> @@ -356,7 +359,7 @@ static int elan_update_firmware(struct elan_tp_data *data,
>
> dev_dbg(&client->dev, "Starting firmware update....\n");
>
> - disable_irq(client->irq);
> + disable_irq(data->irq);
> data->in_fw_update = true;
>
> retval = __elan_update_firmware(data, fw);
> @@ -370,7 +373,7 @@ static int elan_update_firmware(struct elan_tp_data *data,
> }
>
> data->in_fw_update = false;
> - enable_irq(client->irq);
> + enable_irq(data->irq);
>
> return retval;
> }
> @@ -482,7 +485,7 @@ static ssize_t calibrate_store(struct device *dev,
> if (retval)
> return retval;
>
> - disable_irq(client->irq);
> + disable_irq(data->irq);
>
> data->mode |= ETP_ENABLE_CALIBRATE;
> retval = data->ops->set_mode(client, data->mode);
> @@ -528,7 +531,7 @@ out_disable_calibrate:
> retval = error;
> }
> out:
> - enable_irq(client->irq);
> + enable_irq(data->irq);
> mutex_unlock(&data->sysfs_mutex);
> return retval ?: count;
> }
> @@ -594,7 +597,7 @@ static ssize_t acquire_store(struct device *dev, struct device_attribute *attr,
> if (retval)
> return retval;
>
> - disable_irq(client->irq);
> + disable_irq(data->irq);
>
> data->baseline_ready = false;
>
> @@ -636,7 +639,7 @@ out_disable_calibrate:
> retval = error;
> }
> out:
> - enable_irq(client->irq);
> + enable_irq(data->irq);
> mutex_unlock(&data->sysfs_mutex);
> return retval ?: count;
> }
> @@ -900,6 +903,7 @@ static int elan_probe(struct i2c_client *client,
> const struct elan_transport_ops *transport_ops;
> struct device *dev = &client->dev;
> struct elan_tp_data *data;
> + struct gpio_desc *desc;
> unsigned long irqflags;
> int error;
>
> @@ -992,17 +996,31 @@ static int elan_probe(struct i2c_client *client,
> return error;
>
> /*
> + * Check for the irq number in platform data.
> + * If that fails check for gpio based irq.
> + */
> + if (client->irq >= 0) {
> + data->irq = client->irq;
> + } else {
> + desc = devm_gpiod_get_index(&client->dev, "ELAN_GPIO_IRQ", 0);
> + if (IS_ERR(desc) || gpiod_direction_input(desc)) {
> + dev_err(&client->dev, "failed to initialize gpio\n");
> + return error;
> + }
> + data->irq = gpiod_to_irq(desc);
> + }
> + /*
> * Systems using device tree should set up interrupt via DTS,
> * the rest will use the default falling edge interrupts.
> */
> irqflags = client->dev.of_node ? 0 : IRQF_TRIGGER_FALLING;
>
> - error = devm_request_threaded_irq(&client->dev, client->irq,
> + error = devm_request_threaded_irq(&client->dev, data->irq,
> NULL, elan_isr,
> irqflags | IRQF_ONESHOT,
> client->name, data);
> if (error) {
> - dev_err(&client->dev, "cannot register irq=%d\n", client->irq);
> + dev_err(&client->dev, "cannot register irq=%d\n", data->irq);
> return error;
> }
>
> @@ -1055,12 +1073,12 @@ static int __maybe_unused elan_suspend(struct device *dev)
> if (ret)
> return ret;
>
> - disable_irq(client->irq);
> + disable_irq(data->irq);
>
> if (device_may_wakeup(dev)) {
> ret = elan_sleep(data);
> /* Enable wake from IRQ */
> - data->irq_wake = (enable_irq_wake(client->irq) == 0);
> + data->irq_wake = (enable_irq_wake(data->irq) == 0);
> } else {
> ret = elan_disable_power(data);
> }
> @@ -1076,7 +1094,7 @@ static int __maybe_unused elan_resume(struct device *dev)
> int error;
>
> if (device_may_wakeup(dev) && data->irq_wake) {
> - disable_irq_wake(client->irq);
> + disable_irq_wake(data->irq);
> data->irq_wake = false;
> }
>
> @@ -1088,7 +1106,7 @@ static int __maybe_unused elan_resume(struct device *dev)
> if (error)
> dev_err(dev, "initialize when resuming failed: %d\n", error);
>
> - enable_irq(data->client->irq);
> + enable_irq(data->irq);
>
> return 0;
> }
> diff --git a/drivers/input/mouse/elan_i2c_i2c.c b/drivers/input/mouse/elan_i2c_i2c.c
> index 97d4937..5e897cd 100644
> --- a/drivers/input/mouse/elan_i2c_i2c.c
> +++ b/drivers/input/mouse/elan_i2c_i2c.c
> @@ -520,7 +520,8 @@ static int elan_i2c_write_fw_block(struct i2c_client *client,
> }
>
> static int elan_i2c_finish_fw_update(struct i2c_client *client,
> - struct completion *completion)
> + struct completion *completion,
> + unsigned int irq)
> {
> struct device *dev = &client->dev;
> long ret;
> @@ -529,13 +530,13 @@ static int elan_i2c_finish_fw_update(struct i2c_client *client,
> u8 buffer[ETP_I2C_INF_LENGTH];
>
> reinit_completion(completion);
> - enable_irq(client->irq);
> + enable_irq(irq);
>
> error = elan_i2c_write_cmd(client, ETP_I2C_STAND_CMD, ETP_I2C_RESET);
> if (!error)
> ret = wait_for_completion_interruptible_timeout(completion,
> msecs_to_jiffies(300));
> - disable_irq(client->irq);
> + disable_irq(irq);
>
> if (error) {
> dev_err(dev, "device reset failed: %d\n", error);
> diff --git a/drivers/input/mouse/elan_i2c_smbus.c b/drivers/input/mouse/elan_i2c_smbus.c
> index 359bf85..a81b8fb 100644
> --- a/drivers/input/mouse/elan_i2c_smbus.c
> +++ b/drivers/input/mouse/elan_i2c_smbus.c
> @@ -477,7 +477,8 @@ static int elan_smbus_get_report(struct i2c_client *client, u8 *report)
> }
>
> static int elan_smbus_finish_fw_update(struct i2c_client *client,
> - struct completion *fw_completion)
> + struct completion *fw_completion,
> + unsigned int irq)
> {
> /* No special handling unlike I2C transport */
> return 0;
> --
> 1.7.9.5
>
--
Dmitry
^ permalink raw reply
* [PATCH v3] Input: driver for the Goodix touchpanel
From: Bastien Nocera @ 2014-10-29 0:54 UTC (permalink / raw)
To: linux-input; +Cc: Henrik Rydberg, Benjamin Tissoires, Dmitry Torokhov
Add a driver for the Goodix touchscreen panel found in Onda v975w
tablets. The driver is based off the Android driver gt9xx.c found
in some Android code dumps, but now bears no resemblance to the original
driver.
The driver was tested on the aforementioned tablet.
Signed-off-by: Bastien Nocera <hadess@hadess.net>
Tested-by: Bastien Nocera <hadess@hadess.net>
---
MAINTAINERS | 6 +
drivers/input/touchscreen/Kconfig | 13 ++
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix.c | 395 +++++++++++++++++++++++++++++++++++++
4 files changed, 415 insertions(+)
create mode 100644 drivers/input/touchscreen/goodix.c
diff --git a/MAINTAINERS b/MAINTAINERS
index dab92a7..6886886 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4138,6 +4138,12 @@ L: linux-media@vger.kernel.org
S: Maintained
F: drivers/media/usb/go7007/
+GOODIX TOUCHSCREEN
+M: Bastien Nocera <hadess@hadess.net>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/input/touchscreen/goodix.c
+
GPIO SUBSYSTEM
M: Linus Walleij <linus.walleij@linaro.org>
M: Alexandre Courbot <gnurou@gmail.com>
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index e1d8003..568a020 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -295,6 +295,19 @@ config TOUCHSCREEN_FUJITSU
To compile this driver as a module, choose M here: the
module will be called fujitsu-ts.
+config TOUCHSCREEN_GOODIX
+ tristate "Goodix I2C touchscreen"
+ depends on I2C && ACPI
+ help
+ Say Y here if you have the Goodix touchscreen (such as one
+ installed in Onda v975w tablets) connected to your
+ system.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called goodix.
+
config TOUCHSCREEN_ILI210X
tristate "Ilitek ILI210X based touchscreen"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 090e61c..dab4a56 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_TOUCHSCREEN_EETI) += eeti_ts.o
obj-$(CONFIG_TOUCHSCREEN_ELO) += elo.o
obj-$(CONFIG_TOUCHSCREEN_EGALAX) += egalax_ts.o
obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
+obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix.o
obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
obj-$(CONFIG_TOUCHSCREEN_INEXIO) += inexio.o
obj-$(CONFIG_TOUCHSCREEN_INTEL_MID) += intel-mid-touch.o
diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
new file mode 100644
index 0000000..5b10ced
--- /dev/null
+++ b/drivers/input/touchscreen/goodix.c
@@ -0,0 +1,395 @@
+/*
+ * driver for Goodix Touchscreens
+ *
+ * Copyright (c) 2014 Red Hat Inc.
+ *
+ * This code is based on gt9xx.c authored by andrew@goodix.com:
+ *
+ * 2010 - 2012 Goodix Technology.
+ */
+
+/*
+ * 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/kernel.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/slab.h>
+#include <asm/unaligned.h>
+
+struct goodix_ts_data {
+ struct i2c_client *client;
+ struct input_dev *input_dev;
+ int abs_x_max;
+ int abs_y_max;
+ unsigned int max_touch_num;
+ unsigned int int_trigger_type;
+};
+
+#define GOODIX_MAX_HEIGHT 4096
+#define GOODIX_MAX_WIDTH 4096
+#define GOODIX_INT_TRIGGER 1
+#define GOODIX_CONTACT_SIZE 8
+#define GOODIX_MAX_CONTACTS 10
+
+#define GOODIX_CONFIG_MAX_LENGTH 240
+
+/* Register defines */
+#define GOODIX_READ_COOR_ADDR 0x814E
+#define GOODIX_REG_CONFIG_DATA 0x8047
+#define GOODIX_REG_VERSION 0x8140
+
+#define RESOLUTION_LOC 1
+#define TRIGGER_LOC 6
+
+static const unsigned long goodix_irq_flags[] = {
+ IRQ_TYPE_EDGE_RISING,
+ IRQ_TYPE_EDGE_FALLING,
+ IRQ_TYPE_LEVEL_LOW,
+ IRQ_TYPE_LEVEL_HIGH,
+};
+
+/**
+ * goodix_i2c_read - read data from a register of the i2c slave device.
+ *
+ * @client: i2c device.
+ * @reg: the register to read from.
+ * @buf: raw write data buffer.
+ * @len: length of the buffer to write
+ */
+static int goodix_i2c_read(struct i2c_client *client,
+ u16 reg, u8 *buf, int len)
+{
+ struct i2c_msg msgs[2];
+ u16 wbuf = cpu_to_be16(reg);
+ int ret;
+
+ msgs[0].flags = 0;
+ msgs[0].addr = client->addr;
+ msgs[0].len = 2;
+ msgs[0].buf = (u8 *) &wbuf;
+
+ msgs[1].flags = I2C_M_RD;
+ msgs[1].addr = client->addr;
+ msgs[1].len = len;
+ msgs[1].buf = buf;
+
+ ret = i2c_transfer(client->adapter, msgs, 2);
+ return ret < 0 ? ret : (ret != ARRAY_SIZE(msgs) ? -EIO : 0);
+}
+
+static int goodix_ts_read_input_report(struct goodix_ts_data *ts, u8 *data)
+{
+ int touch_num;
+ int error;
+
+ error = goodix_i2c_read(ts->client, GOODIX_READ_COOR_ADDR, data,
+ GOODIX_CONTACT_SIZE + 1);
+ if (error) {
+ dev_err(&ts->client->dev, "I2C transfer error: %d\n", error);
+ return error;
+ }
+
+ touch_num = data[0] & 0x0f;
+ if (touch_num > GOODIX_MAX_CONTACTS)
+ return -EPROTO;
+
+ if (touch_num > 1) {
+ data += 1 + GOODIX_CONTACT_SIZE;
+ error = goodix_i2c_read(ts->client,
+ GOODIX_READ_COOR_ADDR +
+ 1 + GOODIX_CONTACT_SIZE,
+ data,
+ GOODIX_CONTACT_SIZE * (touch_num - 1));
+ if (error)
+ return error;
+ }
+
+ return touch_num;
+}
+
+static void goodix_ts_report_touch(struct goodix_ts_data *ts, u8 *coor_data)
+{
+ int id = coor_data[0] & 0x0F;
+ int input_x = get_unaligned_le16(&coor_data[1]);
+ int input_y = get_unaligned_le16(&coor_data[3]);
+ int input_w = get_unaligned_le16(&coor_data[5]);
+
+ input_mt_slot(ts->input_dev, id);
+ input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, true);
+ input_report_abs(ts->input_dev, ABS_MT_POSITION_X, input_x);
+ input_report_abs(ts->input_dev, ABS_MT_POSITION_Y, input_y);
+ input_report_abs(ts->input_dev, ABS_MT_TOUCH_MAJOR, input_w);
+ input_report_abs(ts->input_dev, ABS_MT_WIDTH_MAJOR, input_w);
+}
+
+/**
+ * goodix_process_events - Process incoming events
+ *
+ * @ts: our goodix_ts_data pointer
+ *
+ * Called when the IRQ is triggered. Read the current device state, and push
+ * the input events to the user space.
+ */
+static void goodix_process_events(struct goodix_ts_data *ts)
+{
+ u8 point_data[1 + GOODIX_CONTACT_SIZE * GOODIX_MAX_CONTACTS];
+ int touch_num;
+ int i;
+
+ touch_num = goodix_ts_read_input_report(ts, point_data);
+ if (touch_num < 0)
+ return;
+
+ for (i = 0; i < touch_num; i++)
+ goodix_ts_report_touch(ts,
+ &point_data[1 + GOODIX_CONTACT_SIZE * i]);
+
+ input_mt_sync_frame(ts->input_dev);
+ input_sync(ts->input_dev);
+}
+
+/**
+ * goodix_ts_irq_handler - The IRQ handler
+ *
+ * @irq: interrupt number.
+ * @dev_id: private data pointer.
+ */
+static irqreturn_t goodix_ts_irq_handler(int irq, void *dev_id)
+{
+ static const u8 end_cmd[] = {
+ GOODIX_READ_COOR_ADDR >> 8,
+ GOODIX_READ_COOR_ADDR & 0xff,
+ 0
+ };
+ struct goodix_ts_data *ts = dev_id;
+
+ goodix_process_events(ts);
+
+ if (i2c_master_send(ts->client, end_cmd, sizeof(end_cmd)) < 0)
+ dev_err(&ts->client->dev, "I2C write end_cmd error\n");
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * goodix_read_config - Read the embedded configuration of the panel
+ *
+ * @ts: our goodix_ts_data pointer
+ *
+ * Must be called during probe
+ */
+static void goodix_read_config(struct goodix_ts_data *ts)
+{
+ u8 config[GOODIX_CONFIG_MAX_LENGTH];
+ int error;
+
+ error = goodix_i2c_read(ts->client, GOODIX_REG_CONFIG_DATA,
+ config,
+ GOODIX_CONFIG_MAX_LENGTH);
+ if (error) {
+ dev_warn(&ts->client->dev,
+ "Error reading config (%d), using defaults\n",
+ error);
+ ts->abs_x_max = GOODIX_MAX_WIDTH;
+ ts->abs_y_max = GOODIX_MAX_HEIGHT;
+ ts->int_trigger_type = GOODIX_INT_TRIGGER;
+ return;
+ }
+
+ ts->abs_x_max = get_unaligned_le16(&config[RESOLUTION_LOC]);
+ ts->abs_y_max = get_unaligned_le16(&config[RESOLUTION_LOC + 2]);
+ ts->int_trigger_type = (config[TRIGGER_LOC]) & 0x03;
+ if (!ts->abs_x_max || !ts->abs_y_max) {
+ dev_err(&ts->client->dev,
+ "Invalid config, using defaults\n");
+ ts->abs_x_max = GOODIX_MAX_WIDTH;
+ ts->abs_y_max = GOODIX_MAX_HEIGHT;
+ }
+}
+
+
+/**
+ * goodix_read_version - Read goodix touchscreen version
+ *
+ * @client: the i2c client
+ * @version: output buffer containing the version on success
+ */
+static int goodix_read_version(struct i2c_client *client, u16 *version)
+{
+ int error;
+ u8 buf[6];
+
+ error = goodix_i2c_read(client, GOODIX_REG_VERSION, buf, sizeof(buf));
+ if (error) {
+ dev_err(&client->dev, "read version failed: %d\n", error);
+ return error;
+ }
+
+ if (version)
+ *version = get_unaligned_le16(&buf[4]);
+
+ dev_info(&client->dev, "IC VERSION: %6ph\n", buf);
+
+ return 0;
+}
+
+/**
+ * goodix_i2c_test - I2C test function to check if the device answers.
+ *
+ * @client: the i2c client
+ */
+static int goodix_i2c_test(struct i2c_client *client)
+{
+ int retry = 0;
+ int error;
+ u8 test;
+
+ while (retry++ < 2) {
+ error = goodix_i2c_read(client, GOODIX_REG_CONFIG_DATA,
+ &test, 1);
+ if (!error)
+ return 0;
+
+ dev_err(&client->dev, "i2c test failed attempt %d: %d\n",
+ retry, error);
+ msleep(20);
+ }
+
+ return error;
+}
+
+/**
+ * goodix_request_input_dev - Allocate, populate and register the input device
+ *
+ * @ts: our goodix_ts_data pointer
+ *
+ * Must be called during probe
+ */
+static int goodix_request_input_dev(struct goodix_ts_data *ts)
+{
+ int error;
+
+ ts->input_dev = devm_input_allocate_device(&ts->client->dev);
+ if (!ts->input_dev) {
+ dev_err(&ts->client->dev, "Failed to allocate input device.");
+ return -ENOMEM;
+ }
+
+ ts->input_dev->evbit[0] = BIT_MASK(EV_SYN) |
+ BIT_MASK(EV_KEY) |
+ BIT_MASK(EV_ABS);
+
+ input_set_abs_params(ts->input_dev, ABS_MT_POSITION_X, 0,
+ ts->abs_x_max, 0, 0);
+ input_set_abs_params(ts->input_dev, ABS_MT_POSITION_Y, 0,
+ ts->abs_y_max, 0, 0);
+ input_set_abs_params(ts->input_dev, ABS_MT_WIDTH_MAJOR, 0, 255, 0, 0);
+ input_set_abs_params(ts->input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
+
+ input_mt_init_slots(ts->input_dev, GOODIX_MAX_CONTACTS,
+ INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
+
+ ts->input_dev->name = "Goodix Capacitive TouchScreen";
+ ts->input_dev->phys = "input/ts";
+ ts->input_dev->id.bustype = BUS_I2C;
+ ts->input_dev->id.vendor = 0x0416;
+ ts->input_dev->id.product = 0x1001;
+ ts->input_dev->id.version = 10427;
+
+ error = input_register_device(ts->input_dev);
+ if (error) {
+ dev_err(&ts->client->dev,
+ "Failed to register input device: %d", error);
+ return error;
+ }
+
+ return 0;
+}
+
+static int goodix_ts_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct goodix_ts_data *ts;
+ unsigned long irq_flags;
+ int error;
+ u16 version_info;
+
+ dev_dbg(&client->dev, "I2C Address: 0x%02x\n", client->addr);
+
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+ dev_err(&client->dev, "I2C check functionality failed.\n");
+ return -ENXIO;
+ }
+
+ ts = devm_kzalloc(&client->dev, sizeof(*ts), GFP_KERNEL);
+ if (!ts)
+ return -ENOMEM;
+
+ ts->client = client;
+ i2c_set_clientdata(client, ts);
+
+ error = goodix_i2c_test(client);
+ if (error) {
+ dev_err(&client->dev, "I2C communication failure: %d\n", error);
+ return error;
+ }
+
+ error = goodix_read_version(client, &version_info);
+ if (error) {
+ dev_err(&client->dev, "Read version failed.\n");
+ return error;
+ }
+
+ goodix_read_config(ts);
+
+ error = goodix_request_input_dev(ts);
+ if (error)
+ return error;
+
+ irq_flags = goodix_irq_flags[ts->int_trigger_type] | IRQF_ONESHOT;
+ error = devm_request_threaded_irq(&ts->client->dev, client->irq,
+ NULL, goodix_ts_irq_handler,
+ irq_flags, client->name, ts);
+ if (error) {
+ dev_err(&client->dev, "request IRQ failed: %d.\n", error);
+ return error;
+ }
+
+ return 0;
+}
+
+static const struct i2c_device_id goodix_ts_id[] = {
+ { "GDIX1001:00", 0 },
+ { }
+};
+
+static const struct acpi_device_id goodix_acpi_match[] = {
+ { "GDIX1001", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, goodix_acpi_match);
+
+static struct i2c_driver goodix_ts_driver = {
+ .probe = goodix_ts_probe,
+ .id_table = goodix_ts_id,
+ .driver = {
+ .name = "Goodix-TS",
+ .owner = THIS_MODULE,
+ .acpi_match_table = goodix_acpi_match,
+ },
+};
+module_i2c_driver(goodix_ts_driver);
+
+MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
+MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
+MODULE_DESCRIPTION("Goodix touchscreen driver");
+MODULE_LICENSE("GPL v2");
--
2.1.0
^ permalink raw reply related
* Re: [PATCH v2] Input: driver for the Goodix touchpanel
From: Bastien Nocera @ 2014-10-29 0:54 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Henrik Rydberg, linux-input, Benjamin Tissoires
In-Reply-To: <20141028230443.GA19675@dtor-ws>
On Tue, 2014-10-28 at 16:04 -0700, Dmitry Torokhov wrote:
> On Tue, Oct 28, 2014 at 11:31:30PM +0100, Bastien Nocera wrote:
<snip>
> > > Please try changing FIXMEs exactly as they were and see if the driver
> > > still works properly.
> >
> > I did, and it still works properly. I also don't understand how it could
> > have worked properly before...
> >
> > Should I still add those linefeeds to all the print statements in the
> > driver?
>
> Yes please.
Done, and new version sent.
Cheers
^ permalink raw reply
* About Dell Inspiron 3442 touchpad
From: Luiz Carlos Ramos @ 2014-10-29 1:00 UTC (permalink / raw)
To: linux-input
Hello,
I'm trying to make a touchpad from a Dell Inspiron I14-3442 laptop work.
Some details:
- I'm using plain Slackware64 14.1, but raised the kernel to 3.16.3 for
tests
- xinput ignores the touchpad; it shows only a USB mouse/keyboard
adapter and the laptop's keyboard:
root@pace:/sys/bus/hid/devices# xinput
Virtual core pointer id=2 [master pointer
(3)]
Virtual core XTEST pointer id=4 [slave
pointer (2)]
Generic USB K/B id=12 [slave
pointer (2)]
Virtual core keyboard id=3 [master
keyboard (2)]
Virtual core XTEST keyboard id=5 [slave
keyboard (3)]
Power Button id=6 [slave
keyboard (3)]
Video Bus id=7 [slave
keyboard (3)]
Power Button id=9 [slave
keyboard (3)]
Sleep Button id=10 [slave
keyboard (3)]
Integrated_Webcam_HD id=13 [slave
keyboard (3)]
AT Translated Set 2 keyboard id=14 [slave
keyboard (3)]
Dell WMI hotkeys id=15 [slave
keyboard (3)]
Video Bus id=8 [slave
keyboard (3)]
Generic USB K/B id=11 [slave
keyboard (3)]
- it seems Ubuntu certified this machine (check
http://www.ubuntu.com/certification/hardware/201402-14674/components/),
but it assumes the touchpad is PS/2. I haven't found it as a PS/2 thing,
even loading psmouse.ko, or doing other tricks
- some articles lists some tips for making it work (like
http://askubuntu.com/questions/134627/how-do-i-get-the-touchpad-settings-working-on-a-dell-xps-13-ultrabook,
or https://bugzilla.redhat.com/show_bug.cgi?id=1048314#c2), but I read
them carefully, made some tests, and they didn't work. One article says
I could blacklist i2c_hid or like in order to make the bring up the
touchpad in PS/2 mode, but I couldn't succeed doing so
- at Dell's site, it is offered a driver for Ubuntu 12.04, but it's
almost obsolete. It seems to be just merged into the kernel
- from Windows 8.1, which runs in the same machine (dual boot), I
concluded the proper way of making it work is to use HID over I2C. It
seems that there are two components loaded; one I2CHID, and a Synaptics
HID. This makes me hint it may be a Synaptics device
- it seems there are two I2C busses in the machine. One is related to
the Intel video graphics subsystem (i801). The other seems to be linked
to the touchpad (i2c_designware_platform). I'm not sure that latest kmod
(i2c_designware_platform) is the right one to be used in this case, but
it appears to be working:
root@pace:/sys/bus/i2c/devices# ls -l /sys/bus/i2c/devices
total 0
lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-0 ->
../../../devices/pci0000:00/INT33C2:00/i2c-0
lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-1 ->
../../../devices/pci0000:00/INT33C3:00/i2c-1
lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-2 ->
../../../devices/pci0000:00/0000:00:02.0/i2c-2
lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-3 ->
../../../devices/pci0000:00/0000:00:02.0/i2c-3
lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-4 ->
../../../devices/pci0000:00/0000:00:02.0/i2c-4
lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-5 ->
../../../devices/pci0000:00/0000:00:02.0/i2c-5
lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-6 ->
../../../devices/pci0000:00/0000:00:02.0/i2c-6
lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-7 ->
../../../devices/pci0000:00/0000:00:02.0/i2c-7
lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-8 ->
../../../devices/pci0000:00/0000:00:02.0/i2c-8
lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-DLL0652:00 ->
../../../devices/pci0000:00/INT33C3:00/i2c-1/i2c-DLL0652:00
root@pace:/sys/bus/i2c/devices# lsmod | grep i2c
i2c_hid 10682 0
hid 94632 3 i2c_hid,hid_generic,usbhid
i2c_dev 5739 0
i2c_designware_platform 3189 0
i2c_i801 13732 0
i2c_designware_core 6045 1 i2c_designware_platform
i2c_algo_bit 5351 1 i915
i2c_core 35216 11
drm,i915,i2c_i801,i2c_dev,i2c_hid,i2c_designware_platform,drm_kms_helper,i2c_algo_bit,v4l2_common,synaptics_i2c,videodev
- in the HID /sys directory, there are three devices. Two are related to
a keyboard/mouse USB adapter. The third seems to be the linked to the
touchpad:
root@pace:/sys/bus/hid/devices# ls -l /sys/bus/hid/devices
total 0
lrwxrwxrwx 1 root root 0 Out 28 22:40 0003:13BA:0017.004F ->
../../../devices/pci0000:00/0000:00:1d.0/usb3/3-1/3-1.3/3-1.3:1.0/0003:13BA:0017.004F
lrwxrwxrwx 1 root root 0 Out 28 22:40 0003:13BA:0017.0050 ->
../../../devices/pci0000:00/0000:00:1d.0/usb3/3-1/3-1.3/3-1.3:1.1/0003:13BA:0017.0050
lrwxrwxrwx 1 root root 0 Out 28 22:40 0018:06CB:2985.0052 ->
../../../devices/pci0000:00/INT33C3:00/i2c-1/i2c-DLL0652:00/0018:06CB:2985.0052
- when I load the kernel module i2c-hid.ko (with debug=1), I read this
in dmesg:
[146172.568787] i2c_hid i2c-DLL0652:00: Fetching the HID descriptor
[146172.568791] i2c_hid i2c-DLL0652:00: __i2c_hid_command: cmd=20 00
[146172.574806] i2c_hid i2c-DLL0652:00: HID Descriptor: 1e 00 00 01 85
00 21 00 24 00 20 00 25 00 17 00 22 00 23 00 cb 06 85 29 00 00 00 00 00
00
[146172.574845] i2c_hid i2c-DLL0652:00: entering i2c_hid_parse
[146172.574847] i2c_hid i2c-DLL0652:00: i2c_hid_hwreset
[146172.574849] i2c_hid i2c-DLL0652:00: i2c_hid_set_power
[146172.574850] i2c_hid i2c-DLL0652:00: __i2c_hid_command: cmd=22 00 00
08
[146172.575436] i2c_hid i2c-DLL0652:00: resetting...
[146172.575442] i2c_hid i2c-DLL0652:00: __i2c_hid_command: cmd=22 00 00
01
[146172.576113] i2c_hid i2c-DLL0652:00: __i2c_hid_command: waiting...
[146172.577414] i2c_hid i2c-DLL0652:00: __i2c_hid_command: finished.
[146172.577417] i2c_hid i2c-DLL0652:00: asking HID report descriptor
[146172.577419] i2c_hid i2c-DLL0652:00: __i2c_hid_command: cmd=21 00
[146172.581072] i2c_hid i2c-DLL0652:00: Report Descriptor: 05 01 09 02
a1 01 85 01 09 01 a1 00 05 09 19 01 29 02 15 00 25 01 75 01 95 02 81 02
95 06 81 01 05 01 09 30 09 31 15 81 25 7f 75 08 95 02 81 06 c0 c0 06 00
ff 09 01 a1 01 85 09 09 02 15 00 26
[146172.581126] i2c_hid i2c-DLL0652:00: i2c_hid_set_power
[146172.581129] i2c_hid i2c-DLL0652:00: __i2c_hid_command: cmd=22 00 01
08
I am aware this information probably is not sufficient to draw any
conclusions, but I'd appreciate to hear from someone who knows i2c_hid
in detail what steps I should take next. For me the last command timed
out or got stuck, but I haven't checked the code to see if it's the
case. Anyway, if it was a timeout case, it should have something logged
after the time expired.
I have some programming skills, and so if it's the case of applying any
patches, or recompiling the kernel or any subsystem to make tests, I'm
up to.
Many thanks,
Luiz Ramos
lramos dot prof at yahoo dot com dot br
São Paulo - Brazil
--
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: About Dell Inspiron 3442 touchpad
From: Benjamin Tissoires @ 2014-10-29 1:40 UTC (permalink / raw)
To: Luiz Carlos Ramos; +Cc: linux-input
In-Reply-To: <1414544444.169720.184461733.4C6DF47C@webmail.messagingengine.com>
Hi Luiz,
On Tue, Oct 28, 2014 at 9:00 PM, Luiz Carlos Ramos
<lramos.prof@yahoo.com.br> wrote:
> Hello,
>
> I'm trying to make a touchpad from a Dell Inspiron I14-3442 laptop work.
>
> Some details:
>
> - I'm using plain Slackware64 14.1, but raised the kernel to 3.16.3 for
> tests
>
> - xinput ignores the touchpad; it shows only a USB mouse/keyboard
> adapter and the laptop's keyboard:
>
> root@pace:/sys/bus/hid/devices# xinput
> Virtual core pointer id=2 [master pointer
> (3)]
> Virtual core XTEST pointer id=4 [slave
> pointer (2)]
> Generic USB K/B id=12 [slave
> pointer (2)]
> Virtual core keyboard id=3 [master
> keyboard (2)]
> Virtual core XTEST keyboard id=5 [slave
> keyboard (3)]
> Power Button id=6 [slave
> keyboard (3)]
> Video Bus id=7 [slave
> keyboard (3)]
> Power Button id=9 [slave
> keyboard (3)]
> Sleep Button id=10 [slave
> keyboard (3)]
> Integrated_Webcam_HD id=13 [slave
> keyboard (3)]
> AT Translated Set 2 keyboard id=14 [slave
> keyboard (3)]
> Dell WMI hotkeys id=15 [slave
> keyboard (3)]
> Video Bus id=8 [slave
> keyboard (3)]
> Generic USB K/B id=11 [slave
> keyboard (3)]
>
> - it seems Ubuntu certified this machine (check
> http://www.ubuntu.com/certification/hardware/201402-14674/components/),
> but it assumes the touchpad is PS/2. I haven't found it as a PS/2 thing,
> even loading psmouse.ko, or doing other tricks
>
> - some articles lists some tips for making it work (like
> http://askubuntu.com/questions/134627/how-do-i-get-the-touchpad-settings-working-on-a-dell-xps-13-ultrabook,
> or https://bugzilla.redhat.com/show_bug.cgi?id=1048314#c2), but I read
> them carefully, made some tests, and they didn't work. One article says
> I could blacklist i2c_hid or like in order to make the bring up the
> touchpad in PS/2 mode, but I couldn't succeed doing so
>
> - at Dell's site, it is offered a driver for Ubuntu 12.04, but it's
> almost obsolete. It seems to be just merged into the kernel
>
> - from Windows 8.1, which runs in the same machine (dual boot), I
> concluded the proper way of making it work is to use HID over I2C. It
> seems that there are two components loaded; one I2CHID, and a Synaptics
> HID. This makes me hint it may be a Synaptics device
Well, if this is a Synaptics HID over I2C device, it should be handled
by hid-rmi in recent kernels (or hid-multitouch but I would say
hid-rmi in your case).
Is the hid-rmi module loaded? Can we get a dmesg output so we can see
if there is any problem?
>
> - it seems there are two I2C busses in the machine. One is related to
> the Intel video graphics subsystem (i801). The other seems to be linked
> to the touchpad (i2c_designware_platform). I'm not sure that latest kmod
> (i2c_designware_platform) is the right one to be used in this case, but
> it appears to be working:
Yeah, i2c_designware_platform is pretty common for Haswell processors.
>
> root@pace:/sys/bus/i2c/devices# ls -l /sys/bus/i2c/devices
> total 0
> lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-0 ->
> ../../../devices/pci0000:00/INT33C2:00/i2c-0
> lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-1 ->
> ../../../devices/pci0000:00/INT33C3:00/i2c-1
> lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-2 ->
> ../../../devices/pci0000:00/0000:00:02.0/i2c-2
> lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-3 ->
> ../../../devices/pci0000:00/0000:00:02.0/i2c-3
> lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-4 ->
> ../../../devices/pci0000:00/0000:00:02.0/i2c-4
> lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-5 ->
> ../../../devices/pci0000:00/0000:00:02.0/i2c-5
> lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-6 ->
> ../../../devices/pci0000:00/0000:00:02.0/i2c-6
> lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-7 ->
> ../../../devices/pci0000:00/0000:00:02.0/i2c-7
> lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-8 ->
> ../../../devices/pci0000:00/0000:00:02.0/i2c-8
> lrwxrwxrwx 1 root root 0 Out 18 17:26 i2c-DLL0652:00 ->
> ../../../devices/pci0000:00/INT33C3:00/i2c-1/i2c-DLL0652:00
This one is the touchpad.
>
> root@pace:/sys/bus/i2c/devices# lsmod | grep i2c
> i2c_hid 10682 0
> hid 94632 3 i2c_hid,hid_generic,usbhid
> i2c_dev 5739 0
> i2c_designware_platform 3189 0
> i2c_i801 13732 0
> i2c_designware_core 6045 1 i2c_designware_platform
> i2c_algo_bit 5351 1 i915
> i2c_core 35216 11
> drm,i915,i2c_i801,i2c_dev,i2c_hid,i2c_designware_platform,drm_kms_helper,i2c_algo_bit,v4l2_common,synaptics_i2c,videodev
>
> - in the HID /sys directory, there are three devices. Two are related to
> a keyboard/mouse USB adapter. The third seems to be the linked to the
> touchpad:
>
> root@pace:/sys/bus/hid/devices# ls -l /sys/bus/hid/devices
> total 0
> lrwxrwxrwx 1 root root 0 Out 28 22:40 0003:13BA:0017.004F ->
> ../../../devices/pci0000:00/0000:00:1d.0/usb3/3-1/3-1.3/3-1.3:1.0/0003:13BA:0017.004F
> lrwxrwxrwx 1 root root 0 Out 28 22:40 0003:13BA:0017.0050 ->
> ../../../devices/pci0000:00/0000:00:1d.0/usb3/3-1/3-1.3/3-1.3:1.1/0003:13BA:0017.0050
> lrwxrwxrwx 1 root root 0 Out 28 22:40 0018:06CB:2985.0052 ->
> ../../../devices/pci0000:00/INT33C3:00/i2c-1/i2c-DLL0652:00/0018:06CB:2985.0052
This is the HID over I2C touchpad.
>
> - when I load the kernel module i2c-hid.ko (with debug=1), I read this
> in dmesg:
>
> [146172.568787] i2c_hid i2c-DLL0652:00: Fetching the HID descriptor
> [146172.568791] i2c_hid i2c-DLL0652:00: __i2c_hid_command: cmd=20 00
> [146172.574806] i2c_hid i2c-DLL0652:00: HID Descriptor: 1e 00 00 01 85
> 00 21 00 24 00 20 00 25 00 17 00 22 00 23 00 cb 06 85 29 00 00 00 00 00
> 00
> [146172.574845] i2c_hid i2c-DLL0652:00: entering i2c_hid_parse
> [146172.574847] i2c_hid i2c-DLL0652:00: i2c_hid_hwreset
> [146172.574849] i2c_hid i2c-DLL0652:00: i2c_hid_set_power
> [146172.574850] i2c_hid i2c-DLL0652:00: __i2c_hid_command: cmd=22 00 00
> 08
> [146172.575436] i2c_hid i2c-DLL0652:00: resetting...
> [146172.575442] i2c_hid i2c-DLL0652:00: __i2c_hid_command: cmd=22 00 00
> 01
> [146172.576113] i2c_hid i2c-DLL0652:00: __i2c_hid_command: waiting...
> [146172.577414] i2c_hid i2c-DLL0652:00: __i2c_hid_command: finished.
> [146172.577417] i2c_hid i2c-DLL0652:00: asking HID report descriptor
> [146172.577419] i2c_hid i2c-DLL0652:00: __i2c_hid_command: cmd=21 00
> [146172.581072] i2c_hid i2c-DLL0652:00: Report Descriptor: 05 01 09 02
> a1 01 85 01 09 01 a1 00 05 09 19 01 29 02 15 00 25 01 75 01 95 02 81 02
> 95 06 81 01 05 01 09 30 09 31 15 81 25 7f 75 08 95 02 81 06 c0 c0 06 00
> ff 09 01 a1 01 85 09 09 02 15 00 26
> [146172.581126] i2c_hid i2c-DLL0652:00: i2c_hid_set_power
> [146172.581129] i2c_hid i2c-DLL0652:00: __i2c_hid_command: cmd=22 00 01
> 08
Everything is fine, this is the normal behavior while connecting a
i2c_hid device.
Normally, we should have then hid-rmi asking for more things and then
it will eventually set up the input device.
>
> I am aware this information probably is not sufficient to draw any
> conclusions, but I'd appreciate to hear from someone who knows i2c_hid
> in detail what steps I should take next. For me the last command timed
> out or got stuck, but I haven't checked the code to see if it's the
> case. Anyway, if it was a timeout case, it should have something logged
> after the time expired.
There is no answer from the device when a SET_POWER is emitted. So
this is not a timeout problem.
If hid-rmi is compiled and is not taking the device, we have a big
problem, but for now, the symptoms look like you do not have this
driver compiled and hid-generic does not bind the device because it
waits for hid-rmi to handle it.
>
> I have some programming skills, and so if it's the case of applying any
> patches, or recompiling the kernel or any subsystem to make tests, I'm
> up to.
Cool, thanks.
Cheers,
Benjamin
^ 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