All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Duggan <aduggan@synaptics.com>
To: Gabriele Mazzotta <gabriele.mzt@gmail.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>,
	<linux-input@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<benjamin.tissoires@redhat.com>, <jkosina@suse.cz>
Subject: Re: NULL pointer dereference in i2c-hid
Date: Fri, 9 Jan 2015 16:29:04 -0800	[thread overview]
Message-ID: <54B07250.5030803@synaptics.com> (raw)
In-Reply-To: <1940829.tOcs3CnnyV@xps13>

On 01/09/2015 12:04 AM, Gabriele Mazzotta wrote:
> On Thursday 08 January 2015 15:58:54 Andrew Duggan wrote:
>> On 12/24/2014 03:53 PM, Gabriele Mazzotta wrote:
>> [...snip...]
>>>>>> Also, if you can get the firmware id from your touchpad that would also
>>>>>> be useful.
>>>>>>
>>>>>> $ sudo ./rmihidtool -f /dev/hidraw0
>>>>> firmware id: 1522295
>>>> Thanks, I will see if I can get any additional information on this.
>>>>
>>>> Andrew
>>> Hi,
>>>
>>> I think I found the source of the problem.
>>>
>>> $ ./rmihidtool /dev/hidraw1 -r 0x50 1
>>> 0x01  #PalmDetect Interrupt Enable, right?
>> Yes, 0x50 does appear to be the address of the palm detect interrupt
>> enable register.
>>> $ ./rmihidtool /dev/hidraw1 -w 0x50 0  #Disable PalmDetect Interrupt
>>>
>>> It makes more sense now that widths greater than 12 trigger the bug.
>> That is weird behavior and I haven't seen anything like that before. I
>> will file a bug to see if firmware has any idea why this is happening.
> According to the RMI4 specification, gesture interrupts are cleared
> only once specific flag registers, F11_2D_Data8 and F11_2D_Data9, are
> read. So I tried to read those register and found that the following
> command stops the events:

It is unusual to see firmware gestures enabled for HID/I2C touchpads. In 
fact none of the touchpads I have have that functionality enabled, which 
is why I haven't been able to test. On HID touchpads there is a layer in 
the firmware which reads the RMI registers and packs them into the HID 
attention report. My guess is that the HID layer is not reading 
F11_2D_Data8 or 9 causing it to assert indefinitely. Since this isn't a 
common firmware configuration that is probably why this hasn't been 
observed before.

> $ rmihidtool /dev/hidraw1 -r 0x24 1  # I was looking for F11_2D_Data8
>
> I'm not sure I got the right address as reading any register close to
> 0x24 (such as 0x25, 0x26) has the same effect. I would have expected
> this to happen only reading one specific register.

With this firmware, F11_2D_Data8 should be at 0x3A. It's 2 bytes for 
finger state + 5 bytes per finger * 5 fingers for abs data  + 2 bytes 
per finger * 5 fingers for rel data. I'm not sure why reading 0x24 would 
stop the reports though.

>
> I also honestly don't know why palms are detected when the width is at
> least 12, PalmDetectThreshold is 0 and so the palm detection should
> be inhibited.
>

This seems to be set in the firmware config. It looks like 
PalmDetectThreshold is only used when the reporting mode is 001. The 
default reporting mode looks like it is 000.

>>> Gabriele
>>>
>> Andrew
Thanks for provide all of this detail.

Andrew

WARNING: multiple messages have this Message-ID (diff)
From: Andrew Duggan <aduggan@synaptics.com>
To: Gabriele Mazzotta <gabriele.mzt@gmail.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	benjamin.tissoires@redhat.com, jkosina@suse.cz
Subject: Re: NULL pointer dereference in i2c-hid
Date: Fri, 9 Jan 2015 16:29:04 -0800	[thread overview]
Message-ID: <54B07250.5030803@synaptics.com> (raw)
In-Reply-To: <1940829.tOcs3CnnyV@xps13>

On 01/09/2015 12:04 AM, Gabriele Mazzotta wrote:
> On Thursday 08 January 2015 15:58:54 Andrew Duggan wrote:
>> On 12/24/2014 03:53 PM, Gabriele Mazzotta wrote:
>> [...snip...]
>>>>>> Also, if you can get the firmware id from your touchpad that would also
>>>>>> be useful.
>>>>>>
>>>>>> $ sudo ./rmihidtool -f /dev/hidraw0
>>>>> firmware id: 1522295
>>>> Thanks, I will see if I can get any additional information on this.
>>>>
>>>> Andrew
>>> Hi,
>>>
>>> I think I found the source of the problem.
>>>
>>> $ ./rmihidtool /dev/hidraw1 -r 0x50 1
>>> 0x01  #PalmDetect Interrupt Enable, right?
>> Yes, 0x50 does appear to be the address of the palm detect interrupt
>> enable register.
>>> $ ./rmihidtool /dev/hidraw1 -w 0x50 0  #Disable PalmDetect Interrupt
>>>
>>> It makes more sense now that widths greater than 12 trigger the bug.
>> That is weird behavior and I haven't seen anything like that before. I
>> will file a bug to see if firmware has any idea why this is happening.
> According to the RMI4 specification, gesture interrupts are cleared
> only once specific flag registers, F11_2D_Data8 and F11_2D_Data9, are
> read. So I tried to read those register and found that the following
> command stops the events:

It is unusual to see firmware gestures enabled for HID/I2C touchpads. In 
fact none of the touchpads I have have that functionality enabled, which 
is why I haven't been able to test. On HID touchpads there is a layer in 
the firmware which reads the RMI registers and packs them into the HID 
attention report. My guess is that the HID layer is not reading 
F11_2D_Data8 or 9 causing it to assert indefinitely. Since this isn't a 
common firmware configuration that is probably why this hasn't been 
observed before.

> $ rmihidtool /dev/hidraw1 -r 0x24 1  # I was looking for F11_2D_Data8
>
> I'm not sure I got the right address as reading any register close to
> 0x24 (such as 0x25, 0x26) has the same effect. I would have expected
> this to happen only reading one specific register.

With this firmware, F11_2D_Data8 should be at 0x3A. It's 2 bytes for 
finger state + 5 bytes per finger * 5 fingers for abs data  + 2 bytes 
per finger * 5 fingers for rel data. I'm not sure why reading 0x24 would 
stop the reports though.

>
> I also honestly don't know why palms are detected when the width is at
> least 12, PalmDetectThreshold is 0 and so the palm detection should
> be inhibited.
>

This seems to be set in the firmware config. It looks like 
PalmDetectThreshold is only used when the reporting mode is 001. The 
default reporting mode looks like it is 000.

>>> Gabriele
>>>
>> Andrew
Thanks for provide all of this detail.

Andrew

  reply	other threads:[~2015-01-10  0:31 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-10 17:04 NULL pointer dereference in i2c-hid Gabriele Mazzotta
2014-12-10 17:04 ` Gabriele Mazzotta
2014-12-11  8:58 ` Mika Westerberg
2014-12-11 14:03   ` Mika Westerberg
2014-12-11 18:16     ` Gabriele Mazzotta
2014-12-11 18:40       ` Andrew Duggan
2014-12-11 18:40         ` Andrew Duggan
2014-12-11 19:11         ` Gabriele Mazzotta
2014-12-11 19:21           ` Andrew Duggan
2014-12-11 19:21             ` Andrew Duggan
2014-12-11 19:40             ` Gabriele Mazzotta
2014-12-11 20:46               ` Andrew Duggan
2014-12-11 20:46                 ` Andrew Duggan
2014-12-11 21:17                 ` Gabriele Mazzotta
2014-12-11 21:34                   ` Andrew Duggan
2014-12-11 21:34                     ` Andrew Duggan
2014-12-11 21:57                     ` Gabriele Mazzotta
2014-12-12  0:26                       ` Andrew Duggan
2014-12-12  0:26                         ` Andrew Duggan
2014-12-12  8:12                         ` Gabriele Mazzotta
2014-12-12 19:12                           ` Andrew Duggan
2014-12-12 19:12                             ` Andrew Duggan
2014-12-24 23:53                             ` Gabriele Mazzotta
2015-01-08 23:58                               ` Andrew Duggan
2015-01-08 23:58                                 ` Andrew Duggan
2015-01-09  8:04                                 ` Gabriele Mazzotta
2015-01-10  0:29                                   ` Andrew Duggan [this message]
2015-01-10  0:29                                     ` Andrew Duggan
2015-01-10  1:18                                     ` Gabriele Mazzotta
2015-02-22 21:37                                     ` Gabriele Mazzotta
2015-02-24  0:30                                       ` Andrew Duggan
2015-02-24  0:30                                         ` Andrew Duggan
2014-12-11 18:41       ` Benjamin Tissoires
2014-12-11 19:25         ` Gabriele Mazzotta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54B07250.5030803@synaptics.com \
    --to=aduggan@synaptics.com \
    --cc=benjamin.tissoires@redhat.com \
    --cc=gabriele.mzt@gmail.com \
    --cc=jkosina@suse.cz \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.