Re: Input: joydev - validate axis/button maps before clobbering current ones

2015-10-07 Thread Javier Martinez Canillas
Hello Dan,

On 10/07/2015 07:46 AM, Dan Carpenter wrote:
> Oh whoops, I sent this to the wrong person.  Javier, you introduced a
> bug with 570c9a7a ('Input: joydev - use memdup_user() to duplicate
> memory from user-space')
> 
> regards,
> dan carpenter
>

Yes, thanks for reporting it but I've already mentioned in this
thread that I posted a fix for it and is already in Dmitry tree:

https://git.kernel.org/cgit/linux/kernel/git/dtor/input.git/commit/?h=next=5b21e3c740b770fb2548a5a8ea66e544d114d0a8

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] HID: wacom: Expect 'touch_max' touches if HID_DG_CONTACTCOUNT not present

2015-10-07 Thread Jason Gerecke
When introduced in commit 1b5d514, the check 'if (hid_data->cc_index >= 0)'
in 'wacom_wac_finger_pre_report' was intended to switch where the driver
got the expected number of contacts from: HID_DG_CONTACTCOUNT if the usage
was present, or 'touch_max' otherwise. Unfortunately, an oversight worthy
of a brown paper bag (specifically, that 'cc_index' could never be negative)
meant that the latter 'else' clause would never be entered.

The patch prior to this one introduced a way for 'cc_index' to be negative,
but only if HID_DG_CONTACTCOUNT is present in some report _other_ than the
one being processed. To ensure the 'else' clause is also entered for devices
which don't have HID_DG_CONTACTCOUNT on _any_ report, we add the additional
constraint that 'cc_report' be non-zero (which is true only if the usage is
present in some report).

Signed-off-by: Jason Gerecke 
---
Jiri,

Could you please queue this patch and the prior for 4.3? They fix issues
with the implementation of my "Ignore contacts in excess of declared
contact count" patch which was pulled in as part of the 4.3 merge window.

Thanks :)

 drivers/hid/wacom_wac.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 4140b1d..46bd02b 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1740,8 +1740,8 @@ static void wacom_wac_finger_pre_report(struct hid_device 
*hdev,
}
}
}
-
-   if (hid_data->cc_index >= 0) {
+   if (hid_data->cc_report != 0 &&
+   hid_data->cc_index >= 0) {
struct hid_field *field = report->field[hid_data->cc_index];
int value = field->value[hid_data->cc_value_index];
if (value)
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] HID: wacom: Tie cached HID_DG_CONTACTCOUNT indices to report ID

2015-10-07 Thread Jason Gerecke
The cached indicies 'cc_index' and 'cc_value_index' introduced in 1b5d514
are only valid for a single report ID. If a touchscreen has multiple
reports with a HID_DG_CONTACTCOUNT usage, its possible that the values
will not be correct for the report we're handling, resulting in an
incorrect value for 'num_expected'. This has been observed with the Cintiq
Companion 2.

To address this, we store the ID of the report those indicies are valid
for in a new  'cc_report' variable. Before using them to get the expected
contact count, we first check if the ID of the report we're processing
matches 'cc_report'. If it doesn't, we update the indicies to point to
the HID_DG_CONTACTCOUNT usage of the current report (if it has one).

Signed-off-by: Jason Gerecke 
---
Jiri,

Could you please queue this patch and the next for 4.3? They fix issues
with the implementation of my "Ignore contacts in excess of declared
contact count" patch which was pulled in as part of the 4.3 merge window.

Thanks :)

 drivers/hid/wacom_wac.c | 26 ++
 drivers/hid/wacom_wac.h |  1 +
 2 files changed, 27 insertions(+)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index c40a6d1..4140b1d 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1628,6 +1628,7 @@ static void wacom_wac_finger_usage_mapping(struct 
hid_device *hdev,
wacom_map_usage(input, usage, field, EV_KEY, BTN_TOUCH, 0);
break;
case HID_DG_CONTACTCOUNT:
+   wacom_wac->hid_data.cc_report = field->report->id;
wacom_wac->hid_data.cc_index = field->index;
wacom_wac->hid_data.cc_value_index = usage->usage_index;
break;
@@ -1715,6 +1716,31 @@ static void wacom_wac_finger_pre_report(struct 
hid_device *hdev,
struct wacom_wac *wacom_wac = >wacom_wac;
struct hid_data* hid_data = _wac->hid_data;
 
+   if (hid_data->cc_report != 0 &&
+   hid_data->cc_report != report->id) {
+   int i;
+
+   hid_data->cc_report = report->id;
+   hid_data->cc_index = -1;
+   hid_data->cc_value_index = -1;
+
+   for (i = 0; i < report->maxfield; i++) {
+   struct hid_field *field = report->field[i];
+   int j;
+
+   for (j = 0; j < field->maxusage; j++) {
+   if (field->usage[j].hid == HID_DG_CONTACTCOUNT) 
{
+   hid_data->cc_index = i;
+   hid_data->cc_value_index = j;
+
+   /* break */
+   i = report->maxfield;
+   j = field->maxusage;
+   }
+   }
+   }
+   }
+
if (hid_data->cc_index >= 0) {
struct hid_field *field = report->field[hid_data->cc_index];
int value = field->value[hid_data->cc_value_index];
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 1e270d4..809c03e 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -198,6 +198,7 @@ struct hid_data {
int width;
int height;
int id;
+   int cc_report;
int cc_index;
int cc_value_index;
int num_expected;
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] HID: wacom: Add support for Cintiq Companion 2

2015-10-07 Thread Bastien Nocera
On Wed, 2015-10-07 at 16:55 -0700, Jason Gerecke wrote:
> Signed-off-by: Jason Gerecke 
> Signed-off-by: Clifford Jolly 

Is that support for using the Cintiq Companion 2 as an accessory (eg.
still running Windows on the machine, and plugging it in to a Linux
machine), or for the Wacom device inside the tablet when it's running
Linux (if that's possible)?

Either way, which it is should be included in the commit message.

For those who don't know about that device:
http://www.wacom.com/en-us/products/pen-displays/cintiq-companion-2

Cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] HID: wacom: Add support for Cintiq Companion 2

2015-10-07 Thread Cliff Jolly
This applies to both. When running as a standalone tablet, all the
tablet input devices still go through the usb system. The different
modes show the same usb vendor and product IDs and seem to behave
identically in the testing I've done.

Sorry for the multiple-message spam.

On Wed, Oct 7, 2015 at 5:59 PM, Bastien Nocera  wrote:
> On Wed, 2015-10-07 at 16:55 -0700, Jason Gerecke wrote:
>> Signed-off-by: Jason Gerecke 
>> Signed-off-by: Clifford Jolly 
>
> Is that support for using the Cintiq Companion 2 as an accessory (eg.
> still running Windows on the machine, and plugging it in to a Linux
> machine), or for the Wacom device inside the tablet when it's running
> Linux (if that's possible)?
>
> Either way, which it is should be included in the commit message.
>
> For those who don't know about that device:
> http://www.wacom.com/en-us/products/pen-displays/cintiq-companion-2
>
> Cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] HID: multitouch: Fetch feature reports on demand for Win8 devices

2015-10-07 Thread Andrew Duggan



On 10/07/2015 08:56 AM, Seth Forshee wrote:

On Wed, Oct 07, 2015 at 09:34:10AM -0400, Benjamin Tissoires wrote:

[sorry for the late answer Jiri, I was on vacations last week]

On Oct 07 2015 or thereabouts, Mika Westerberg wrote:

Some newer Intel Skylake based Dell laptops with Win8 precision touchpad
fail when initial feature reports are fetched from it. Below is an example
output with some additional debug included:

  i2c_hid i2c-DLL0704:01: Fetching the HID descriptor
  i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=20 00
  i2c_hid i2c-DLL0704:01: HID Descriptor: 1e 00 00 01 99 02 21 00 24 ...
  ...
  i2c_hid i2c-DLL0704:01: i2c_hid_get_report
  i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 38 02 23 00
  i2c_hid i2c-DLL0704:01: report (len=4): 04 00 08 05
  i2c_hid i2c-DLL0704:01: report id 13
  i2c_hid i2c-DLL0704:01: i2c_hid_get_report
  i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 3d 02 23 00
  i2c_hid i2c-DLL0704:01: failed to retrieve report from device.
  i2c_hid i2c-DLL0704:01: report id 7
  i2c_hid i2c-DLL0704:01: i2c_hid_get_report
  i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 37 02 23 00
  i2c_hid i2c-DLL0704:01: report (len=259): 03 01 07 fc 28 fe 84 40 ...
  i2c_hid i2c-DLL0704:01: report id 4
  i2c_hid i2c-DLL0704:01: i2c_hid_get_report
  i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 34 02 23 00

We manage to fetch few reports but then the touchpad dies:

  i2c_designware i2c_designware.1: i2c_dw_handle_tx_abort: lost arbitration
  i2c_hid i2c-DLL0704:01: failed to retrieve report from device.

it eventually pulls the whole I2C bus low:

  i2c_designware i2c_designware.1: controller timed out
  i2c_hid i2c-DLL0704:01: failed to set a report to device.

Fix this by preventing initial feature report retrieval for Win8 devices.
Instead we fetch reports as needed in mt_feature_mapping(). This prevents
fetching reports which might cause problems with the device in question.

Suggested-by: Benjamin Tissoires 
Signed-off-by: Mika Westerberg 
---
Added check for MT_CLS_WIN_8 so that the fix is only done for real Win8
devices.

I think we are good now. I would just like to have Seth or Andrew giving
a tested-by for Windows precision touchpads that can be found on the Dell
laptops.

If the buttonpad property is set correctly, this is:
Reviewed-by: Benjamin Tissoires 

This is working fine on my XPS 13 9343.

Tested-by: Seth Forshee 
I can confirm that it is working on the Dell XPS 13 and the buttonpad 
property is set correctly. However, I also tried it on an older non 
production precision touchpad and it didn't work. It reports that it 
fails to fetch feature 8 and then doesn't report any input. I'm 
concerned that some of the early PTPs in some Dell and Acer systems 
might not work with this change. I'll test this tomorrow on a few more 
systems to see if the problem is isolated to just this one non 
production touchpad.


[ 2969.222125] usb 1-1: New USB device found, idVendor=06cb, idProduct=2393
[ 2969.222128] usb 1-1: New USB device strings: Mfr=0, Product=2, 
SerialNumber=0

[ 2969.222130] usb 1-1: Product: Synaptics T Pad V 01.31
[ 2974.223578] hid-multitouch 0003:06CB:2393.0008: failed to fetch feature 8
[ 2979.226037] hid-multitouch 0003:06CB:2393.0008: failed to fetch feature 8
[ 2979.226137] input: Synaptics T Pad V 01.31 UNKNOWN as 
/devices/pci:00/:00:14.0/usb1/1-1/1-1:1.0/0003:06CB:2393.0008/input/input34
[ 2979.226431] hid-multitouch 0003:06CB:2393.0008: 
input,hiddev0,hidraw2: USB HID v1.11 Mouse [Synaptics T Pad V 01.31] on 
usb-:00:14.0-1/input0


Feature 8 looks like this:
  FEATURE(8)[FEATURE]
Field(0)
  Application(Digitizers.TouchPad)
  Usage(2)
Digitizers.ContactMaximumNumber
Digitizers.ButtonType
  Logical Minimum(0)
  Logical Maximum(15)
  Physical Minimum(0)
  Physical Maximum(65535)
  Unit Exponent(-4)
  Unit(SI Linear : Seconds)
  Report Size(4)
  Report Count(2)
  Report Offset(0)
  Flags( Variable Absolute )

Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] HID: wacom: Add support for Cintiq Companion 2

2015-10-07 Thread Jason Gerecke
Signed-off-by: Jason Gerecke 
Signed-off-by: Clifford Jolly 
---
 drivers/hid/wacom_sys.c |  2 +-
 drivers/hid/wacom_wac.c | 41 -
 drivers/hid/wacom_wac.h |  1 +
 3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 5f6e48e..2e7a1c7 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -420,7 +420,7 @@ static int wacom_query_tablet_data(struct hid_device *hdev,
/* MT Tablet PC touch */
return wacom_set_device_mode(hdev, 3, 4, 4);
}
-   else if (features->type == WACOM_24HDT || features->type == 
CINTIQ_HYBRID) {
+   else if (features->type == WACOM_24HDT || features->type == 
CINTIQ_HYBRID || features->type == CINTIQ_COMPANION_2) {
return wacom_set_device_mode(hdev, 18, 3, 2);
}
else if (features->type == WACOM_27QHDT) {
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 46bd02b..44d4cc3 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -765,7 +765,7 @@ static void wacom_intuos_general(struct wacom_wac *wacom)
/* general pen packet */
if ((data[1] & 0xb8) == 0xa0) {
t = (data[6] << 2) | ((data[7] >> 6) & 3);
-   if (features->type >= INTUOS4S && features->type <= 
CINTIQ_HYBRID) {
+   if (features->type >= INTUOS4S && features->type <= 
CINTIQ_COMPANION_2) {
t = (t << 1) | (data[1] & 1);
}
input_report_abs(input, ABS_PRESSURE, t);
@@ -948,6 +948,27 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
} else {
input_report_abs(input, ABS_MISC, 0);
}
+
+   } else if (features->type == CINTIQ_COMPANION_2) {
+   input_report_key(input, BTN_1, (data[1] & 0x02));
+   input_report_key(input, BTN_2, (data[2] & 0x01));
+   input_report_key(input, BTN_3, (data[2] & 0x02));
+   input_report_key(input, BTN_4, (data[2] & 0x04));
+   input_report_key(input, BTN_5, (data[2] & 0x08));
+   input_report_key(input, BTN_6, (data[1] & 0x04));
+
+   input_report_key(input, BTN_7, (data[2] & 0x10));  /* 
Right  */
+   input_report_key(input, BTN_8, (data[2] & 0x20));  /* 
Up */
+   input_report_key(input, BTN_9, (data[2] & 0x40));  /* 
Left   */
+   input_report_key(input, BTN_A, (data[2] & 0x80));  /* 
Down   */
+   input_report_key(input, BTN_0, (data[1] & 0x01));  /* 
Center */
+
+   if (data[4] | (data[3] & 0x01)) {
+   input_report_abs(input, ABS_MISC, 
PAD_DEVICE_ID);
+   } else {
+   input_report_abs(input, ABS_MISC, 0);
+   }
+
} else if (features->type >= INTUOS5S && features->type <= 
INTUOSPL) {
int i;
 
@@ -2290,6 +2311,7 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t 
len)
case WACOM_27QHD:
case DTK:
case CINTIQ_HYBRID:
+   case CINTIQ_COMPANION_2:
sync = wacom_intuos_irq(wacom_wac);
break;
 
@@ -2543,6 +2565,7 @@ int wacom_setup_pen_input_capabilities(struct input_dev 
*input_dev,
case CINTIQ:
case WACOM_13HD:
case CINTIQ_HYBRID:
+   case CINTIQ_COMPANION_2:
input_set_abs_params(input_dev, ABS_Z, -900, 899, 0, 0);
input_abs_set_res(input_dev, ABS_Z, 287);
__set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
@@ -2595,6 +2618,12 @@ int wacom_setup_pen_input_capabilities(struct input_dev 
*input_dev,
__clear_bit(ABS_MISC, input_dev->absbit);
/* fall through */
 
+   case CINTIQ_COMPANION_2:
+   for (i = 0; i < 10; i++)
+   __set_bit(BTN_0 + i, input_dev->keybit);
+   __set_bit(BTN_A, input_dev->keybit);
+   break;
+
case DTUS:
case DTUSX:
case PL:
@@ -3347,6 +3376,14 @@ static const struct wacom_features wacom_features_0x318 =
 static const struct wacom_features wacom_features_0x319 =
{ "Wacom Wireless Bamboo PAD", 4095, 4095, /* Touch */
  .type = BAMBOO_PAD, 35, 48, .touch_max = 4 };
+static const struct wacom_features wacom_features_0x325 =
+   { "Wacom ISDv5 325", 59552, 33848, 2047, 63,
+ CINTIQ_COMPANION_2, WACOM_INTUOS3_RES, WACOM_INTUOS3_RES,
+ WACOM_CINTIQ_OFFSET, WACOM_CINTIQ_OFFSET,
+ .oVid = USB_VENDOR_ID_WACOM, .oPid = 0x326 };
+static const struct wacom_features 

[PATCH v2] HID: multitouch: Fetch feature reports on demand for Win8 devices

2015-10-07 Thread Mika Westerberg
Some newer Intel Skylake based Dell laptops with Win8 precision touchpad
fail when initial feature reports are fetched from it. Below is an example
output with some additional debug included:

 i2c_hid i2c-DLL0704:01: Fetching the HID descriptor
 i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=20 00
 i2c_hid i2c-DLL0704:01: HID Descriptor: 1e 00 00 01 99 02 21 00 24 ...
 ...
 i2c_hid i2c-DLL0704:01: i2c_hid_get_report
 i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 38 02 23 00
 i2c_hid i2c-DLL0704:01: report (len=4): 04 00 08 05
 i2c_hid i2c-DLL0704:01: report id 13
 i2c_hid i2c-DLL0704:01: i2c_hid_get_report
 i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 3d 02 23 00
 i2c_hid i2c-DLL0704:01: failed to retrieve report from device.
 i2c_hid i2c-DLL0704:01: report id 7
 i2c_hid i2c-DLL0704:01: i2c_hid_get_report
 i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 37 02 23 00
 i2c_hid i2c-DLL0704:01: report (len=259): 03 01 07 fc 28 fe 84 40 ...
 i2c_hid i2c-DLL0704:01: report id 4
 i2c_hid i2c-DLL0704:01: i2c_hid_get_report
 i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 34 02 23 00

We manage to fetch few reports but then the touchpad dies:

 i2c_designware i2c_designware.1: i2c_dw_handle_tx_abort: lost arbitration
 i2c_hid i2c-DLL0704:01: failed to retrieve report from device.

it eventually pulls the whole I2C bus low:

 i2c_designware i2c_designware.1: controller timed out
 i2c_hid i2c-DLL0704:01: failed to set a report to device.

Fix this by preventing initial feature report retrieval for Win8 devices.
Instead we fetch reports as needed in mt_feature_mapping(). This prevents
fetching reports which might cause problems with the device in question.

Suggested-by: Benjamin Tissoires 
Signed-off-by: Mika Westerberg 
---
Added check for MT_CLS_WIN_8 so that the fix is only done for real Win8
devices.

 drivers/hid/hid-multitouch.c | 45 +++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 661b4fce1a5d..dd64461e5e85 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -309,6 +309,41 @@ static struct attribute_group mt_attribute_group = {
.attrs = sysfs_attrs
 };
 
+static void mt_get_feature(struct hid_device *hdev, struct hid_report *report)
+{
+   struct mt_device *td = hid_get_drvdata(hdev);
+   int ret, size = hid_report_len(report);
+   u8 *buf;
+
+   /*
+* Only fetch the feature report if initial reports are not already
+* been retrieved. Currently this is only done for Windows 8 touch
+* devices.
+*/
+   if (!(hdev->quirks & HID_QUIRK_NO_INIT_REPORTS))
+   return;
+   if (td->mtclass.name != MT_CLS_WIN_8)
+   return;
+
+   buf = hid_alloc_report_buf(report, GFP_KERNEL);
+   if (!buf)
+   return;
+
+   ret = hid_hw_raw_request(hdev, report->id, buf, size,
+HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+   if (ret < 0) {
+   dev_warn(>dev, "failed to fetch feature %d\n",
+report->id);
+   } else {
+   ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, buf,
+  size, 0);
+   if (ret)
+   dev_warn(>dev, "failed to report feature\n");
+   }
+
+   kfree(buf);
+}
+
 static void mt_feature_mapping(struct hid_device *hdev,
struct hid_field *field, struct hid_usage *usage)
 {
@@ -327,6 +362,8 @@ static void mt_feature_mapping(struct hid_device *hdev,
 
break;
case HID_DG_CONTACTMAX:
+   mt_get_feature(hdev, field->report);
+
td->maxcontact_report_id = field->report->id;
td->maxcontacts = field->value[0];
if (!td->maxcontacts &&
@@ -343,6 +380,7 @@ static void mt_feature_mapping(struct hid_device *hdev,
break;
}
 
+   mt_get_feature(hdev, field->report);
if (field->value[usage->usage_index] == MT_BUTTONTYPE_CLICKPAD)
td->is_buttonpad = true;
 
@@ -1029,8 +1067,13 @@ static int mt_probe(struct hid_device *hdev, const 
struct hid_device_id *id)
 * reports. Fortunately, the Win8 spec says that all touches
 * should be sent during each report, making the initialization
 * of input reports unnecessary.
+*
+* In addition some touchpads do not behave well if we read
+* all feature reports from them. Instead we prevent
+* initial report fetching and then selectively fetch each
+* report we are interested in.
 */
-   hdev->quirks |= HID_QUIRK_NO_INIT_INPUT_REPORTS;
+   

[RFC 3/4] Input: edt-ft5x06 - Add support for FT5506

2015-10-07 Thread fcooper
From: Franklin S Cooper Jr 

FT5506 is essentially the same as other FT5x06 devices other than
supporting 10 support points.

Signed-off-by: Franklin S Cooper Jr 
---
 Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt | 2 ++
 drivers/input/touchscreen/edt-ft5x06.c | 7 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt 
b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
index bedd7dd..f99528d 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
@@ -5,6 +5,7 @@ There are 3 variants of the chip for various touch panel sizes
 FT5206GE1  2.8" .. 3.8"
 FT5306DE4  4.3" .. 7"
 FT5406EE8  7"   .. 8.9"
+FT5506EEG  7"   .. 8.9"
 
 The software interface is identical for all those chips, so that
 currently there is no need for the driver to distinguish between the
@@ -17,6 +18,7 @@ Required properties:
  - compatible:  "edt,edt-ft5206"
or:  "edt,edt-ft5306"
or:  "edt,edt-ft5406"
+   or:  "edt,edt-ft5506"
 
  - reg: I2C slave address of the chip (0x38)
  - interrupt-parent: a phandle pointing to the interrupt controller
diff --git a/drivers/input/touchscreen/edt-ft5x06.c 
b/drivers/input/touchscreen/edt-ft5x06.c
index 8031152..752da69 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -172,7 +172,7 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
struct edt_ft5x06_ts_data *tsdata = dev_id;
struct device *dev = >client->dev;
u8 cmd;
-   u8 rdbuf[29];
+   u8 rdbuf[59];
int i, type, x, y, id;
int offset, tplen, datalen;
int error;
@@ -1073,6 +1073,10 @@ MODULE_DEVICE_TABLE(i2c, edt_ft5x06_ts_id);
 
 #ifdef CONFIG_OF
 
+static const struct edt_i2c_chip_data edt_ft5506_data = {
+   .max_support_points = 10,
+};
+
 static const struct edt_i2c_chip_data edt_ft5x06_data = {
.max_support_points = 5,
 };
@@ -1081,6 +1085,7 @@ static const struct of_device_id edt_ft5x06_of_match[] = {
{ .compatible = "edt,edt-ft5206", .data = _ft5x06_data},
{ .compatible = "edt,edt-ft5306", .data = _ft5x06_data},
{ .compatible = "edt,edt-ft5406", .data = _ft5x06_data},
+   { .compatible = "edt,edt-ft5506", .data = _ft5506_data},
{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, edt_ft5x06_of_match);
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC 4/4] Input: edt-ft5x06 - Work around FT5506 firmware bug

2015-10-07 Thread fcooper
From: Franklin S Cooper Jr 

In the touchscreen controller ISR, reading the tsc starting from
register 0x2 causes the tsc to infrequently update the detected
finger's x and y coordinate. The irq pin toggles at a fast rate to
indicate touch events are happening. However, the tsc on average
updates the touch point's x and y value every ~100 ms which is much
slower than the advertised rate of 100+ Hz. This leads to multiple reads
within this ~100 ms time window returning the same value.

Example:
X: 10 , Y: 30
X: 10 , Y: 30
X: 10,  Y: 30
..
// After 100 ms
X: 300, Y: 300
X: 300, y: 300
..
// After 100 ms
X: 1743, Y: 621
X: 1743, Y: 621

For some reason if instead of starting to read at register 0x2 you
start reading at register 0x0 this issue isn't seen. This seems like
a quirk only seen in the EDT FT5506 so to fix this issue simply
adjust the code to start reading from 0x0. Technically this isn't wrong
so no regressions should be seen with other touchscreen controllers
supported by this driver.

Signed-off-by: Franklin S Cooper Jr 
---
 drivers/input/touchscreen/edt-ft5x06.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c 
b/drivers/input/touchscreen/edt-ft5x06.c
index 752da69..b3ed631 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -172,7 +172,7 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
struct edt_ft5x06_ts_data *tsdata = dev_id;
struct device *dev = >client->dev;
u8 cmd;
-   u8 rdbuf[59];
+   u8 rdbuf[61];
int i, type, x, y, id;
int offset, tplen, datalen;
int error;
@@ -188,8 +188,8 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
break;
 
case M09:
-   cmd = 0x02;
-   offset = 1;
+   cmd = 0x0;
+   offset = 3;
tplen = 6;
datalen = tplen * tsdata->max_support_points - cmd + 1;
break;
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] HID: multitouch: Fetch feature reports on demand for Win8 devices

2015-10-07 Thread Benjamin Tissoires
[sorry for the late answer Jiri, I was on vacations last week]

On Oct 07 2015 or thereabouts, Mika Westerberg wrote:
> Some newer Intel Skylake based Dell laptops with Win8 precision touchpad
> fail when initial feature reports are fetched from it. Below is an example
> output with some additional debug included:
> 
>  i2c_hid i2c-DLL0704:01: Fetching the HID descriptor
>  i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=20 00
>  i2c_hid i2c-DLL0704:01: HID Descriptor: 1e 00 00 01 99 02 21 00 24 ...
>  ...
>  i2c_hid i2c-DLL0704:01: i2c_hid_get_report
>  i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 38 02 23 00
>  i2c_hid i2c-DLL0704:01: report (len=4): 04 00 08 05
>  i2c_hid i2c-DLL0704:01: report id 13
>  i2c_hid i2c-DLL0704:01: i2c_hid_get_report
>  i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 3d 02 23 00
>  i2c_hid i2c-DLL0704:01: failed to retrieve report from device.
>  i2c_hid i2c-DLL0704:01: report id 7
>  i2c_hid i2c-DLL0704:01: i2c_hid_get_report
>  i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 37 02 23 00
>  i2c_hid i2c-DLL0704:01: report (len=259): 03 01 07 fc 28 fe 84 40 ...
>  i2c_hid i2c-DLL0704:01: report id 4
>  i2c_hid i2c-DLL0704:01: i2c_hid_get_report
>  i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 34 02 23 00
> 
> We manage to fetch few reports but then the touchpad dies:
> 
>  i2c_designware i2c_designware.1: i2c_dw_handle_tx_abort: lost arbitration
>  i2c_hid i2c-DLL0704:01: failed to retrieve report from device.
> 
> it eventually pulls the whole I2C bus low:
> 
>  i2c_designware i2c_designware.1: controller timed out
>  i2c_hid i2c-DLL0704:01: failed to set a report to device.
> 
> Fix this by preventing initial feature report retrieval for Win8 devices.
> Instead we fetch reports as needed in mt_feature_mapping(). This prevents
> fetching reports which might cause problems with the device in question.
> 
> Suggested-by: Benjamin Tissoires 
> Signed-off-by: Mika Westerberg 
> ---
> Added check for MT_CLS_WIN_8 so that the fix is only done for real Win8
> devices.

I think we are good now. I would just like to have Seth or Andrew giving 
a tested-by for Windows precision touchpads that can be found on the Dell
laptops.

If the buttonpad property is set correctly, this is:
Reviewed-by: Benjamin Tissoires 

Cheers,
Benjamin

> 
>  drivers/hid/hid-multitouch.c | 45 
> +++-
>  1 file changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 661b4fce1a5d..dd64461e5e85 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -309,6 +309,41 @@ static struct attribute_group mt_attribute_group = {
>   .attrs = sysfs_attrs
>  };
>  
> +static void mt_get_feature(struct hid_device *hdev, struct hid_report 
> *report)
> +{
> + struct mt_device *td = hid_get_drvdata(hdev);
> + int ret, size = hid_report_len(report);
> + u8 *buf;
> +
> + /*
> +  * Only fetch the feature report if initial reports are not already
> +  * been retrieved. Currently this is only done for Windows 8 touch
> +  * devices.
> +  */
> + if (!(hdev->quirks & HID_QUIRK_NO_INIT_REPORTS))
> + return;
> + if (td->mtclass.name != MT_CLS_WIN_8)
> + return;
> +
> + buf = hid_alloc_report_buf(report, GFP_KERNEL);
> + if (!buf)
> + return;
> +
> + ret = hid_hw_raw_request(hdev, report->id, buf, size,
> +  HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
> + if (ret < 0) {
> + dev_warn(>dev, "failed to fetch feature %d\n",
> +  report->id);
> + } else {
> + ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, buf,
> +size, 0);
> + if (ret)
> + dev_warn(>dev, "failed to report feature\n");
> + }
> +
> + kfree(buf);
> +}
> +
>  static void mt_feature_mapping(struct hid_device *hdev,
>   struct hid_field *field, struct hid_usage *usage)
>  {
> @@ -327,6 +362,8 @@ static void mt_feature_mapping(struct hid_device *hdev,
>  
>   break;
>   case HID_DG_CONTACTMAX:
> + mt_get_feature(hdev, field->report);
> +
>   td->maxcontact_report_id = field->report->id;
>   td->maxcontacts = field->value[0];
>   if (!td->maxcontacts &&
> @@ -343,6 +380,7 @@ static void mt_feature_mapping(struct hid_device *hdev,
>   break;
>   }
>  
> + mt_get_feature(hdev, field->report);
>   if (field->value[usage->usage_index] == MT_BUTTONTYPE_CLICKPAD)
>   td->is_buttonpad = true;
>  
> @@ -1029,8 +1067,13 @@ static int mt_probe(struct hid_device *hdev, const 
> struct hid_device_id *id)
>* reports. 

[RFC 2/4] Input: edt-ft5x06 - Add support for different max support points

2015-10-07 Thread fcooper
From: Franklin S Cooper Jr 

Update the code so that the maximum supported points aren't hard coded but
can be changed.

Set the maximum support points based on the data passed along side the
compatible field.

Signed-off-by: Franklin S Cooper Jr 
---
 drivers/input/touchscreen/edt-ft5x06.c | 56 --
 1 file changed, 47 insertions(+), 9 deletions(-)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c 
b/drivers/input/touchscreen/edt-ft5x06.c
index 1e0ed6e..8031152 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -38,8 +38,7 @@
 #include 
 #include 
 #include 
-
-#define MAX_SUPPORT_POINTS 5
+#include 
 
 #define WORK_REGISTER_THRESHOLD0x00
 #define WORK_REGISTER_REPORT_RATE  0x08
@@ -105,6 +104,7 @@ struct edt_ft5x06_ts_data {
int gain;
int offset;
int report_rate;
+   int max_support_points;
 
char name[EDT_NAME_LEN];
 
@@ -112,6 +112,10 @@ struct edt_ft5x06_ts_data {
enum edt_ver version;
 };
 
+struct edt_i2c_chip_data {
+   int  max_support_points;
+};
+
 static int edt_ft5x06_ts_readwrite(struct i2c_client *client,
   u16 wr_len, u8 *wr_buf,
   u16 rd_len, u8 *rd_buf)
@@ -180,14 +184,14 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void 
*dev_id)
tplen = 4;  /* data comes in so called frames */
 
/* how many bytes to listen for */
-   datalen = tplen * MAX_SUPPORT_POINTS + offset + 1;
+   datalen = tplen * tsdata->max_support_points + offset + 1;
break;
 
case M09:
cmd = 0x02;
offset = 1;
tplen = 6;
-   datalen = tplen * MAX_SUPPORT_POINTS - cmd + 1;
+   datalen = tplen * tsdata->max_support_points - cmd + 1;
break;
 
default:
@@ -219,7 +223,7 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
goto out;
}
 
-   for (i = 0; i < MAX_SUPPORT_POINTS; i++) {
+   for (i = 0; i < tsdata->max_support_points; i++) {
u8 *buf = [i * tplen + offset];
bool down;
 
@@ -872,6 +876,32 @@ edt_ft5x06_ts_set_regs(struct edt_ft5x06_ts_data *tsdata)
}
 }
 
+#ifdef CONFIG_OF
+
+static const struct of_device_id edt_ft5x06_of_match[];
+
+static void edt_ft5x06_ts_set_max_support_points(struct device *dev,
+   struct edt_ft5x06_ts_data *tsdata)
+{
+   struct edt_i2c_chip_data *chip_data;
+   const struct of_device_id *match;
+
+   match = of_match_device(of_match_ptr(edt_ft5x06_of_match), dev);
+
+   if (match) {
+   chip_data = (struct edt_i2c_chip_data *)match->data;
+   tsdata->max_support_points = chip_data->max_support_points;
+   } else {
+   tsdata->max_support_points = 5;
+   }
+}
+#else
+static void edt_ft5x06_ts_set_max_support_points(struct device *dev,
+   struct edt_ft5x06_ts_data *tsdata)
+{
+}
+#endif
+
 static int edt_ft5x06_ts_probe(struct i2c_client *client,
 const struct i2c_device_id *id)
 {
@@ -889,6 +919,8 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
return -ENOMEM;
}
 
+   edt_ft5x06_ts_set_max_support_points(>dev, tsdata);
+
tsdata->reset_gpio = devm_gpiod_get_optional(>dev,
 "reset", GPIOD_OUT_HIGH);
if (IS_ERR(tsdata->reset_gpio)) {
@@ -954,7 +986,8 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
 
touchscreen_parse_properties(input, true);
 
-   error = input_mt_init_slots(input, MAX_SUPPORT_POINTS, INPUT_MT_DIRECT);
+   error = input_mt_init_slots(input, tsdata->max_support_points,
+   INPUT_MT_DIRECT);
if (error) {
dev_err(>dev, "Unable to init MT slots.\n");
return error;
@@ -1039,10 +1072,15 @@ static const struct i2c_device_id edt_ft5x06_ts_id[] = {
 MODULE_DEVICE_TABLE(i2c, edt_ft5x06_ts_id);
 
 #ifdef CONFIG_OF
+
+static const struct edt_i2c_chip_data edt_ft5x06_data = {
+   .max_support_points = 5,
+};
+
 static const struct of_device_id edt_ft5x06_of_match[] = {
-   { .compatible = "edt,edt-ft5206", },
-   { .compatible = "edt,edt-ft5306", },
-   { .compatible = "edt,edt-ft5406", },
+   { .compatible = "edt,edt-ft5206", .data = _ft5x06_data},
+   { .compatible = "edt,edt-ft5306", .data = _ft5x06_data},
+   { .compatible = "edt,edt-ft5406", .data = _ft5x06_data},
{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, edt_ft5x06_of_match);
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to 

[RFC 0/4] Input: edt-ft5506 - Adding support for FT5506

2015-10-07 Thread fcooper
From: Franklin S Cooper Jr 

This series adds support for the FT5506 tsc. The biggest difference
between this tsc vs the ones currently supported by the driver is the
ability to handle upto 10 touch points.

The FT5506 tsc that I currently have seems to be based on the M09
firmware which I have documentation for. However, I haven't been able
to find documentation for the tsc that use the M06 firmware. Therefore,
the calculation I made to determine the amount of data to read for the M06
is at best an educated guess.

If anyone has the doc for the old firmware please send it to me if
possible or double check the code I use to determine the amount of bytes
to read.

Also I had to work around a weird bug that I'm only seeing on the FT5506.
I tested this patch set on another board (AM437x SK) that uses the FT5306
and didn't see any problems.

This patchset is built on top of linux-input master

Franklin S Cooper Jr (4):
  Input: edt-ft5x06 - Use max support points to determine how much to
read
  Input: edt-ft5x06 - Add support for different max support points
  Input: edt-ft5x06 - Add support for FT5506
  Input: edt-ft5x06 - Work around FT5506 firmware bug

 .../bindings/input/touchscreen/edt-ft5x06.txt  |  2 +
 drivers/input/touchscreen/edt-ft5x06.c | 69 ++
 2 files changed, 59 insertions(+), 12 deletions(-)

-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] HID: multitouch: Fetch feature reports on demand for Win8 devices

2015-10-07 Thread Seth Forshee
On Wed, Oct 07, 2015 at 09:34:10AM -0400, Benjamin Tissoires wrote:
> [sorry for the late answer Jiri, I was on vacations last week]
> 
> On Oct 07 2015 or thereabouts, Mika Westerberg wrote:
> > Some newer Intel Skylake based Dell laptops with Win8 precision touchpad
> > fail when initial feature reports are fetched from it. Below is an example
> > output with some additional debug included:
> > 
> >  i2c_hid i2c-DLL0704:01: Fetching the HID descriptor
> >  i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=20 00
> >  i2c_hid i2c-DLL0704:01: HID Descriptor: 1e 00 00 01 99 02 21 00 24 ...
> >  ...
> >  i2c_hid i2c-DLL0704:01: i2c_hid_get_report
> >  i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 38 02 23 00
> >  i2c_hid i2c-DLL0704:01: report (len=4): 04 00 08 05
> >  i2c_hid i2c-DLL0704:01: report id 13
> >  i2c_hid i2c-DLL0704:01: i2c_hid_get_report
> >  i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 3d 02 23 00
> >  i2c_hid i2c-DLL0704:01: failed to retrieve report from device.
> >  i2c_hid i2c-DLL0704:01: report id 7
> >  i2c_hid i2c-DLL0704:01: i2c_hid_get_report
> >  i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 37 02 23 00
> >  i2c_hid i2c-DLL0704:01: report (len=259): 03 01 07 fc 28 fe 84 40 ...
> >  i2c_hid i2c-DLL0704:01: report id 4
> >  i2c_hid i2c-DLL0704:01: i2c_hid_get_report
> >  i2c_hid i2c-DLL0704:01: __i2c_hid_command: cmd=22 00 34 02 23 00
> > 
> > We manage to fetch few reports but then the touchpad dies:
> > 
> >  i2c_designware i2c_designware.1: i2c_dw_handle_tx_abort: lost arbitration
> >  i2c_hid i2c-DLL0704:01: failed to retrieve report from device.
> > 
> > it eventually pulls the whole I2C bus low:
> > 
> >  i2c_designware i2c_designware.1: controller timed out
> >  i2c_hid i2c-DLL0704:01: failed to set a report to device.
> > 
> > Fix this by preventing initial feature report retrieval for Win8 devices.
> > Instead we fetch reports as needed in mt_feature_mapping(). This prevents
> > fetching reports which might cause problems with the device in question.
> > 
> > Suggested-by: Benjamin Tissoires 
> > Signed-off-by: Mika Westerberg 
> > ---
> > Added check for MT_CLS_WIN_8 so that the fix is only done for real Win8
> > devices.
> 
> I think we are good now. I would just like to have Seth or Andrew giving 
> a tested-by for Windows precision touchpads that can be found on the Dell
> laptops.
> 
> If the buttonpad property is set correctly, this is:
> Reviewed-by: Benjamin Tissoires 

This is working fine on my XPS 13 9343.

Tested-by: Seth Forshee 
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html