Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH 8/8] iio: hid-sensor-press: Introduce PM
From: Srinivas Pandruvada @ 2015-01-07  0:58 UTC (permalink / raw)
  To: jkosina, jic23; +Cc: linux-iio, linux-input, Srinivas Pandruvada
In-Reply-To: <1420592328-9942-1-git-send-email-srinivas.pandruvada@linux.intel.com>

Use common hid sensor iio pm functions. Also the poll time read and
wait is part of power up function of hid sensor iio pm function, so
remove from the client drivers.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/iio/pressure/hid-sensor-press.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
index ac150f0..7bb8d4c 100644
--- a/drivers/iio/pressure/hid-sensor-press.c
+++ b/drivers/iio/pressure/hid-sensor-press.c
@@ -79,7 +79,6 @@ static int press_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
-	s32 poll_value;
 
 	*val = 0;
 	*val2 = 0;
@@ -96,15 +95,8 @@ static int press_read_raw(struct iio_dev *indio_dev,
 			break;
 		}
 		if (report_id >= 0) {
-			poll_value = hid_sensor_read_poll_value(
-					&press_state->common_attributes);
-			if (poll_value < 0)
-				return -EINVAL;
 			hid_sensor_power_state(&press_state->common_attributes,
 						true);
-
-			msleep_interruptible(poll_value * 2);
-
 			*val = sensor_hub_input_attr_get_raw_value(
 				press_state->common_attributes.hsdev,
 				HID_USAGE_SENSOR_PRESSURE, address,
@@ -383,6 +375,7 @@ static struct platform_driver hid_press_platform_driver = {
 	.id_table = hid_press_ids,
 	.driver = {
 		.name	= KBUILD_MODNAME,
+		.pm	= &hid_sensor_pm_ops,
 	},
 	.probe		= hid_press_probe,
 	.remove		= hid_press_remove,
-- 
1.9.1


^ permalink raw reply related

* [PATCH] HID: hid-sensor-hub: Add collection device
From: Srinivas Pandruvada @ 2015-01-07  0:58 UTC (permalink / raw)
  To: jkosina, jic23; +Cc: linux-iio, linux-input, Srinivas Pandruvada
In-Reply-To: <1420592328-9942-1-git-send-email-srinivas.pandruvada@linux.intel.com>

HID sensor hub exports several sensor which are fusion sensors, where
data is interpreted from one or more sensors. Some of them can't be
exported via IIO like sysfs as the user space download some firmware,
which defines what sensor will look like. They can be a part of a
collection.
Creating a MFD cell for a collection to write a standalone driver
to manage collections. Most of the time they will be propritery drivers
may not be even upstreamed. This patch allows framework to have
capability to write such drivers.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/hid-sensor-hub.c   | 50 ++++++++++++++++++++++++++++++++++++++----
 include/linux/hid-sensor-ids.h |  2 ++
 2 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 84290e5..7403b25 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -119,11 +119,12 @@ static struct hid_sensor_hub_callbacks *sensor_hub_get_callback(
 
 	spin_lock_irqsave(&pdata->dyn_callback_lock, flags);
 	list_for_each_entry(callback, &pdata->dyn_callback_list, list)
-		if (callback->usage_id == usage_id &&
+		if (callback->usage_id == HID_USAGE_SENSOR_TYPE_COLLECTION ||
+			(callback->usage_id == usage_id &&
 			(collection_index >=
 				callback->hsdev->start_collection_index) &&
 			(collection_index <
-				callback->hsdev->end_collection_index)) {
+				callback->hsdev->end_collection_index))) {
 			*priv = callback->priv;
 			*hsdev = callback->hsdev;
 			spin_unlock_irqrestore(&pdata->dyn_callback_lock,
@@ -159,7 +160,12 @@ int sensor_hub_register_callback(struct hid_sensor_hub_device *hsdev,
 	callback->usage_callback = usage_callback;
 	callback->usage_id = usage_id;
 	callback->priv = NULL;
-	list_add_tail(&callback->list, &pdata->dyn_callback_list);
+	/* Give higher priority to collection device, so add to front */
+	if (usage_id == HID_USAGE_SENSOR_TYPE_COLLECTION)
+		list_add(&callback->list, &pdata->dyn_callback_list);
+	else
+		list_add_tail(&callback->list, &pdata->dyn_callback_list);
+
 	spin_unlock_irqrestore(&pdata->dyn_callback_lock, flags);
 
 	return 0;
@@ -590,6 +596,37 @@ static __u8 *sensor_hub_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 	return rdesc;
 }
 
+static int sensor_hub_add_collection_device(struct hid_device *hdev,
+					    struct sensor_hub_data *sd)
+{
+	struct hid_sensor_hub_device *hsdev;
+	char *name;
+
+	hsdev = devm_kzalloc(&hdev->dev, sizeof(*hsdev), GFP_KERNEL);
+	if (!hsdev)
+		return -ENOMEM;
+
+	hsdev->hdev = hdev;
+	hsdev->vendor_id = hdev->vendor;
+	hsdev->product_id = hdev->product;
+	hsdev->usage = HID_USAGE_SENSOR_TYPE_COLLECTION;
+	mutex_init(&hsdev->mutex);
+	name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "HID-SENSOR-%x",
+			      HID_USAGE_SENSOR_TYPE_COLLECTION);
+	if (name == NULL) {
+		hid_err(hdev, "Failed MFD device name\n");
+		return -ENOMEM;
+	}
+	sd->hid_sensor_hub_client_devs[sd->hid_sensor_client_cnt].name = name;
+	sd->hid_sensor_hub_client_devs[
+			sd->hid_sensor_client_cnt].platform_data = hsdev;
+	sd->hid_sensor_hub_client_devs[
+			sd->hid_sensor_client_cnt].pdata_size = sizeof(*hsdev);
+	sd->hid_sensor_client_cnt++;
+
+	return 0;
+}
+
 static int sensor_hub_probe(struct hid_device *hdev,
 				const struct hid_device_id *id)
 {
@@ -634,7 +671,8 @@ static int sensor_hub_probe(struct hid_device *hdev,
 		ret = -EINVAL;
 		goto err_stop_hw;
 	}
-	sd->hid_sensor_hub_client_devs = devm_kzalloc(&hdev->dev, dev_cnt *
+	sd->hid_sensor_hub_client_devs = devm_kzalloc(&hdev->dev,
+						      (dev_cnt + 1) *
 						      sizeof(struct mfd_cell),
 						      GFP_KERNEL);
 	if (sd->hid_sensor_hub_client_devs == NULL) {
@@ -688,6 +726,10 @@ static int sensor_hub_probe(struct hid_device *hdev,
 	if (last_hsdev)
 		last_hsdev->end_collection_index = i;
 
+	ret = sensor_hub_add_collection_device(hdev, sd);
+	if (ret)
+		goto err_stop_hw;
+
 	ret = mfd_add_hotplug_devices(&hdev->dev,
 			sd->hid_sensor_hub_client_devs,
 			sd->hid_sensor_client_cnt);
diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
index 109f0e6..300ffea 100644
--- a/include/linux/hid-sensor-ids.h
+++ b/include/linux/hid-sensor-ids.h
@@ -21,6 +21,8 @@
 
 #define HID_MAX_PHY_DEVICES					0xFF
 
+#define HID_USAGE_SENSOR_TYPE_COLLECTION			0x200001
+
 /* Accel 3D (200073) */
 #define HID_USAGE_SENSOR_ACCEL_3D				0x200073
 #define HID_USAGE_SENSOR_DATA_ACCELERATION			0x200452
-- 
1.9.1


^ permalink raw reply related

* [PATCH 4/8] iio: hid-sensor-als: Introduce PM
From: Srinivas Pandruvada @ 2015-01-07  0:58 UTC (permalink / raw)
  To: jkosina, jic23; +Cc: linux-iio, linux-input, Srinivas Pandruvada
In-Reply-To: <1420592328-9942-1-git-send-email-srinivas.pandruvada@linux.intel.com>

Use common hid sensor iio pm functions. Also the poll time read and
wait is part of power up function of hid sensor iio pm function, so
remove from the client drivers.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/iio/light/hid-sensor-als.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index 321862d..1609ecd 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -80,7 +80,6 @@ static int als_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
-	s32 poll_value;
 
 	*val = 0;
 	*val2 = 0;
@@ -97,15 +96,8 @@ static int als_read_raw(struct iio_dev *indio_dev,
 			break;
 		}
 		if (report_id >= 0) {
-			poll_value = hid_sensor_read_poll_value(
-						&als_state->common_attributes);
-			if (poll_value < 0)
-				return -EINVAL;
-
 			hid_sensor_power_state(&als_state->common_attributes,
 						true);
-			msleep_interruptible(poll_value * 2);
-
 			*val = sensor_hub_input_attr_get_raw_value(
 					als_state->common_attributes.hsdev,
 					HID_USAGE_SENSOR_ALS, address,
@@ -382,6 +374,7 @@ static struct platform_driver hid_als_platform_driver = {
 	.id_table = hid_als_ids,
 	.driver = {
 		.name	= KBUILD_MODNAME,
+		.pm	= &hid_sensor_pm_ops,
 	},
 	.probe		= hid_als_probe,
 	.remove		= hid_als_remove,
-- 
1.9.1


^ permalink raw reply related

* [PATCH 5/8] iio: hid-sensor-prox: Introduce PM
From: Srinivas Pandruvada @ 2015-01-07  0:58 UTC (permalink / raw)
  To: jkosina, jic23; +Cc: linux-iio, linux-input, Srinivas Pandruvada
In-Reply-To: <1420592328-9942-1-git-send-email-srinivas.pandruvada@linux.intel.com>

Use common hid sensor iio pm functions. Also the poll time read and
wait is part of power up function of hid sensor iio pm function, so
remove from the client drivers.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/iio/light/hid-sensor-prox.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
index db9c60e..91ecc46 100644
--- a/drivers/iio/light/hid-sensor-prox.c
+++ b/drivers/iio/light/hid-sensor-prox.c
@@ -75,7 +75,6 @@ static int prox_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
-	s32 poll_value;
 
 	*val = 0;
 	*val2 = 0;
@@ -92,16 +91,8 @@ static int prox_read_raw(struct iio_dev *indio_dev,
 			break;
 		}
 		if (report_id >= 0) {
-			poll_value = hid_sensor_read_poll_value(
-					&prox_state->common_attributes);
-			if (poll_value < 0)
-				return -EINVAL;
-
 			hid_sensor_power_state(&prox_state->common_attributes,
 						true);
-
-			msleep_interruptible(poll_value * 2);
-
 			*val = sensor_hub_input_attr_get_raw_value(
 				prox_state->common_attributes.hsdev,
 				HID_USAGE_SENSOR_PROX, address,
@@ -374,6 +365,7 @@ static struct platform_driver hid_prox_platform_driver = {
 	.id_table = hid_prox_ids,
 	.driver = {
 		.name	= KBUILD_MODNAME,
+		.pm	= &hid_sensor_pm_ops,
 	},
 	.probe		= hid_prox_probe,
 	.remove		= hid_prox_remove,
-- 
1.9.1


^ permalink raw reply related

* [PATCH 7/8] iio: hid-sensor-incl-3d: Introduce PM
From: Srinivas Pandruvada @ 2015-01-07  0:58 UTC (permalink / raw)
  To: jkosina, jic23; +Cc: linux-iio, linux-input, Srinivas Pandruvada
In-Reply-To: <1420592328-9942-1-git-send-email-srinivas.pandruvada@linux.intel.com>

Use common hid sensor iio pm functions. Also the poll time read and
wait is part of power up function of hid sensor iio pm function, so
remove from the client drivers.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/iio/orientation/hid-sensor-incl-3d.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c b/drivers/iio/orientation/hid-sensor-incl-3d.c
index 45bed08..5930fa3 100644
--- a/drivers/iio/orientation/hid-sensor-incl-3d.c
+++ b/drivers/iio/orientation/hid-sensor-incl-3d.c
@@ -111,20 +111,12 @@ static int incl_3d_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
-	s32 poll_value;
 
 	*val = 0;
 	*val2 = 0;
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		poll_value = hid_sensor_read_poll_value(
-					&incl_state->common_attributes);
-		if (poll_value < 0)
-			return -EINVAL;
-
 		hid_sensor_power_state(&incl_state->common_attributes, true);
-		msleep_interruptible(poll_value * 2);
-
 		report_id =
 			incl_state->incl[chan->scan_index].report_id;
 		address = incl_3d_addresses[chan->scan_index];
@@ -438,6 +430,7 @@ static struct platform_driver hid_incl_3d_platform_driver = {
 	.id_table = hid_incl_3d_ids,
 	.driver = {
 		.name	= KBUILD_MODNAME,
+		.pm	= &hid_sensor_pm_ops,
 	},
 	.probe		= hid_incl_3d_probe,
 	.remove		= hid_incl_3d_remove,
-- 
1.9.1


^ permalink raw reply related

* Re: [PATCH] hid: Initialize battery_no to -1 & correct its format string
From: Benjamin Tissoires @ 2015-01-07  1:50 UTC (permalink / raw)
  To: Ping Cheng
  Cc: Aniroop Mathur, Jason Gerecke, Ping Cheng, Jiri Kosina,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	Aniroop Mathur
In-Reply-To: <CAF8JNh+gzA_m-mMjjAdBz6WyyFemTU26VDOLufOJXJ5DMkK2yg@mail.gmail.com>

On Tue, Jan 6, 2015 at 7:50 PM, Ping Cheng <pinglinux@gmail.com> wrote:
> Hi Benjamin,
>
> The relevant code was introduced by
> d70420b914c98a3758674c6e9858810e0ab4ea30. Can you take a look and let
> us know if Aniroop's patch fits your original thought or not?

Hehe, indeed, that's my code.

I would agree with Jiri, I really do not care. This code is called
only once at initialization and I am pretty sure it will not change
the time to initialize the device (I am confident the compiler will do
something better than blindly following the code).

So either way it is good. But I have a slight preference to initialize
the variable to 0 and keep the unsigned in the name.

Cheers,
Benjamin

>
> Thanks,
>
> Ping
>
> On Tue, Jan 6, 2015 at 6:32 AM, Aniroop Mathur <aniroop.mathur@gmail.com> wrote:
>> Dear Mr. Jason and Mr. Ping,
>>
>> Please update about below patch.
>> Except avoiding subtraction, there is also a need to avoid negative
>> battery name which may come due to %ld, so need to change to %lu.
>>
>> Thanks,
>> Aniroop
>>
>> On Mon, Dec 29, 2014 at 5:33 PM, Jiri Kosina <jkosina@suse.cz> wrote:
>>> On Sun, 28 Dec 2014, Aniroop Mathur wrote:
>>>
>>>> From: Aniroop Mathur <a.mathur@samsung.com>
>>>>
>>>> This patch initializes battery_no to -1 to avoid extra subtraction
>>>> operation performed everytime wacom battery is initialized
>>>
>>> This is so femto-optimization, that I don't really care deeply. Adding
>>> Jason and Ping to CC. If they want this, I'll apply the patch.
>>>
>>>> and correct format string of unsigned long from %ld to %lu.
>>>>
>>>> Signed-off-by: Aniroop Mathur <a.mathur@samsung.com>
>>>> ---
>>>>  drivers/hid/wacom_sys.c | 8 ++++----
>>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
>>>> index 129fd33..4b5ff84 100644
>>>> --- a/drivers/hid/wacom_sys.c
>>>> +++ b/drivers/hid/wacom_sys.c
>>>> @@ -919,17 +919,17 @@ static int wacom_ac_get_property(struct power_supply *psy,
>>>>
>>>>  static int wacom_initialize_battery(struct wacom *wacom)
>>>>  {
>>>> -     static atomic_t battery_no = ATOMIC_INIT(0);
>>>> +     static atomic_t battery_no = ATOMIC_INIT(-1);
>>>>       int error;
>>>>       unsigned long n;
>>>>
>>>>       if (wacom->wacom_wac.features.quirks & WACOM_QUIRK_BATTERY) {
>>>> -             n = atomic_inc_return(&battery_no) - 1;
>>>> +             n = atomic_inc_return(&battery_no);
>>>>
>>>>               wacom->battery.properties = wacom_battery_props;
>>>>               wacom->battery.num_properties = ARRAY_SIZE(wacom_battery_props);
>>>>               wacom->battery.get_property = wacom_battery_get_property;
>>>> -             sprintf(wacom->wacom_wac.bat_name, "wacom_battery_%ld", n);
>>>> +             sprintf(wacom->wacom_wac.bat_name, "wacom_battery_%lu", n);
>>>>               wacom->battery.name = wacom->wacom_wac.bat_name;
>>>>               wacom->battery.type = POWER_SUPPLY_TYPE_BATTERY;
>>>>               wacom->battery.use_for_apm = 0;
>>>> @@ -937,7 +937,7 @@ static int wacom_initialize_battery(struct wacom *wacom)
>>>>               wacom->ac.properties = wacom_ac_props;
>>>>               wacom->ac.num_properties = ARRAY_SIZE(wacom_ac_props);
>>>>               wacom->ac.get_property = wacom_ac_get_property;
>>>> -             sprintf(wacom->wacom_wac.ac_name, "wacom_ac_%ld", n);
>>>> +             sprintf(wacom->wacom_wac.ac_name, "wacom_ac_%lu", n);
>>>>               wacom->ac.name = wacom->wacom_wac.ac_name;
>>>>               wacom->ac.type = POWER_SUPPLY_TYPE_MAINS;
>>>>               wacom->ac.use_for_apm = 0;
>>>> --
>>>> 1.9.1
>>>>
>>>
>>> --
>>> Jiri Kosina
>>> SUSE Labs
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH 1/1] x86: Added support for Japanese Surface Type Cover 3
From: Alan Wu @ 2015-01-07  2:32 UTC (permalink / raw)
  To: Alan Wu, Jiri Kosina, Karlis Dreizis; +Cc: linux-input, linux-kernel, linux-usb

Based on code for the US Surface Type Cover 3 
from commit be3b16341d5cd8cf2a64fcc7a604a8efe6599ff0
("HID: add support for MS Surface Pro 3 Type Cover"):

Signed-off-by: Alan Wu <alan.c.wu@gmail.com>
Tested-by: Karlis Dreizis <karlisdreizis@gmail.com> 
---
 drivers/hid/hid-core.c          | 4 +++-
 drivers/hid/hid-ids.h           | 1 +
 drivers/hid/hid-microsoft.c     | 2 ++
 drivers/hid/usbhid/hid-quirks.c | 1 +
 4 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index c3d0ac1..0cea5a0 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -704,7 +704,8 @@ static void hid_scan_collection(struct hid_parser *parser, unsigned type)
 		hid->group = HID_GROUP_SENSOR_HUB;
 
 	if (hid->vendor == USB_VENDOR_ID_MICROSOFT &&
-	    hid->product == USB_DEVICE_ID_MS_TYPE_COVER_3 &&
+	    (hid->product == USB_DEVICE_ID_MS_TYPE_COVER_3 ||
+	     hid->product == USB_DEVICE_ID_MS_TYPE_COVER_3_JP) &&
 	    hid->group == HID_GROUP_MULTITOUCH)
 		hid->group = HID_GROUP_GENERIC;
 }
@@ -1860,6 +1861,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_WIRELESS_OPTICAL_DESKTOP_3_0) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_OFFICE_KB) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3_JP) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MONTEREY, USB_DEVICE_ID_GENIUS_KB29E) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 7460f34..95ee91b 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -653,6 +653,7 @@
 #define USB_DEVICE_ID_MS_TOUCH_COVER_2   0x07a7
 #define USB_DEVICE_ID_MS_TYPE_COVER_2    0x07a9
 #define USB_DEVICE_ID_MS_TYPE_COVER_3    0x07dc
+#define USB_DEVICE_ID_MS_TYPE_COVER_3_JP 0x07dd
 
 #define USB_VENDOR_ID_MOJO		0x8282
 #define USB_DEVICE_ID_RETRO_ADAPTER	0x3201
diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c
index cacda43..fbaea6e 100644
--- a/drivers/hid/hid-microsoft.c
+++ b/drivers/hid/hid-microsoft.c
@@ -276,6 +276,8 @@ static const struct hid_device_id ms_devices[] = {
 		.driver_data = MS_DUPLICATE_USAGES },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3),
 		.driver_data = MS_HIDINPUT },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3_JP),
+		.driver_data = MS_HIDINPUT },
 
 	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_PRESENTER_8K_BT),
 		.driver_data = MS_PRESENTER },
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index dc89be9..6591b5d 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -80,6 +80,7 @@ static const struct hid_blacklist {
 	{ USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, HID_QUIRK_NOGET },
 	{ USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
 	{ USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3, HID_QUIRK_NO_INIT_REPORTS },
+	{ USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3_JP, HID_QUIRK_NO_INIT_REPORTS },
 	{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
 	{ USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750, HID_QUIRK_NO_INIT_REPORTS },
 	{ USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE, HID_QUIRK_NO_INIT_REPORTS },
-- 
2.1.0


^ permalink raw reply related

* [PATCH 1/2] Input: adxl34x - add OF support
From: Walter Lozano @ 2015-01-07  2:58 UTC (permalink / raw)
  To: michael.hennerich-OyLXuOCK7orQT0dZR+AlfA,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Walter Lozano

This patch adds the missing support for OF to the adxl34x digital
accelerometer. This is a basic version which supports the main
optional parameters. This implementation copies the default values
to the adxl34x_platform_data structure and overrides the values
that are passed from the device tree.

Signed-off-by: Walter Lozano <walter-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
---
 drivers/input/misc/adxl34x.c |  108 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 107 insertions(+), 1 deletion(-)

diff --git a/drivers/input/misc/adxl34x.c b/drivers/input/misc/adxl34x.c
index 2b2d02f..b3e06a3 100644
--- a/drivers/input/misc/adxl34x.c
+++ b/drivers/input/misc/adxl34x.c
@@ -16,6 +16,8 @@
 #include <linux/workqueue.h>
 #include <linux/input/adxl34x.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include "adxl34x.h"
 
@@ -688,13 +690,113 @@ static void adxl34x_input_close(struct input_dev *input)
 	mutex_unlock(&ac->mutex);
 }
 
+void adxl34x_parse_dt(struct device *dev, struct adxl34x_platform_data *pdata){
+	if (!dev->of_node)
+		return;
+
+	pdata =  kzalloc(sizeof(*pdata), GFP_KERNEL);
+
+	memcpy(pdata, &adxl34x_default_init, sizeof(struct adxl34x_platform_data));
+
+	of_property_read_u8(dev->of_node, "adi,tap-axis-control",
+		 &pdata->tap_axis_control);
+
+	of_property_read_u8(dev->of_node, "adi,tap-threshold",
+		 &pdata->tap_threshold);
+
+	of_property_read_u8(dev->of_node, "adi,tap-duration",
+		 &pdata->tap_duration);
+
+	of_property_read_u8(dev->of_node, "adi,tap-latency",
+		 &pdata->tap_latency);
+
+	of_property_read_u8(dev->of_node, "adi,tap-window",
+		 &pdata->tap_window);
+
+	of_property_read_u8(dev->of_node, "adi,act-axis-control",
+		 &pdata->act_axis_control);
+
+	of_property_read_u8(dev->of_node, "adi,activity-threshold",
+		 &pdata->activity_threshold);
+
+	of_property_read_u8(dev->of_node, "adi,inactivity-threshold",
+		 &pdata->inactivity_threshold);
+
+	of_property_read_u8(dev->of_node, "adi,inactivity-time",
+		 &pdata->inactivity_time);
+
+	of_property_read_u8(dev->of_node, "adi,free-fall-threshold",
+		 &pdata->free_fall_threshold);
+
+	of_property_read_u8(dev->of_node, "adi,free-fall-time",
+		 &pdata->free_fall_time);
+
+	of_property_read_u8(dev->of_node, "adi,data-rate",
+		 &pdata->data_rate);
+
+	of_property_read_u8(dev->of_node, "adi,data-range",
+		 &pdata->data_range);
+
+	of_property_read_u8(dev->of_node, "adi,low-power-mode",
+		 &pdata->low_power_mode);
+
+	of_property_read_u8(dev->of_node, "adi,power-mode",
+		 &pdata->power_mode);
+
+	of_property_read_u8(dev->of_node, "adi,fifo-mode",
+		 &pdata->fifo_mode);
+
+	of_property_read_u8(dev->of_node, "adi,watermark",
+		 &pdata->watermark);
+
+	of_property_read_u8(dev->of_node, "adi,use-int2",
+		 &pdata->use_int2);
+
+	of_property_read_u8(dev->of_node, "adi,orientation-enable",
+		 &pdata->orientation_enable);
+
+	of_property_read_u8(dev->of_node, "adi,deadzone-angle",
+		 &pdata->deadzone_angle);
+
+	of_property_read_u8(dev->of_node, "adi,divisor-length",
+		 &pdata->divisor_length);
+
+	of_property_read_u32(dev->of_node, "adi,ev-type",
+		 &pdata->ev_type);
+
+	of_property_read_u32(dev->of_node, "adi,ev-code-x",
+		 &pdata->ev_code_x);
+
+	of_property_read_u32(dev->of_node, "adi,ev-code-y",
+		 &pdata->ev_code_y);
+
+	of_property_read_u32(dev->of_node, "adi,ev-code-z",
+		 &pdata->ev_code_z);
+
+	of_property_read_u32_array(dev->of_node, "adi,ev-code-tap",
+		 pdata->ev_code_tap, 3);
+
+	of_property_read_u32(dev->of_node, "adi,ev-code-ff",
+		 &pdata->ev_code_ff);
+
+	of_property_read_u32(dev->of_node, "adi,ev-code-act-inactivity",
+		 &pdata->ev_code_act_inactivity);
+
+	of_property_read_u32_array(dev->of_node, "adi,ev-codes-orient-2d",
+		 pdata->ev_codes_orient_2d, 4);
+
+	of_property_read_u32_array(dev->of_node, "adi,ev-codes-orient-3d",
+		 pdata->ev_codes_orient_3d, 6);
+
+}
+
 struct adxl34x *adxl34x_probe(struct device *dev, int irq,
 			      bool fifo_delay_default,
 			      const struct adxl34x_bus_ops *bops)
 {
 	struct adxl34x *ac;
 	struct input_dev *input_dev;
-	const struct adxl34x_platform_data *pdata;
+	struct adxl34x_platform_data *pdata;
 	int err, range, i;
 	unsigned char revid;
 
@@ -714,6 +816,10 @@ struct adxl34x *adxl34x_probe(struct device *dev, int irq,
 	ac->fifo_delay = fifo_delay_default;
 
 	pdata = dev_get_platdata(dev);
+
+	if (!pdata && dev->of_node)
+		adxl34x_parse_dt(dev, pdata);
+
 	if (!pdata) {
 		dev_dbg(dev,
 			"No platform data: Using default initialization\n");
-- 
1.7.10.4

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

^ permalink raw reply related

* [PATCH 2/2] Input: adxl34x - add device tree documentation
From: Walter Lozano @ 2015-01-07  2:58 UTC (permalink / raw)
  To: michael.hennerich, dmitry.torokhov, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, devicetree, linux-input,
	linux-kernel
  Cc: Walter Lozano
In-Reply-To: <1420599537-28740-1-git-send-email-walter@vanguardiasur.com.ar>

This patch adds a basic documentation regarding the device tree
bindings of adxl34x digital accelerometer.

Signed-off-by: Walter Lozano <walter@vanguardiasur.com.ar>
---
 .../devicetree/bindings/input/adxl34x.txt          |   53 ++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/adxl34x.txt

diff --git a/Documentation/devicetree/bindings/input/adxl34x.txt b/Documentation/devicetree/bindings/input/adxl34x.txt
new file mode 100644
index 0000000..0774f7e
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/adxl34x.txt
@@ -0,0 +1,53 @@
+Device Tree bindings for adxl34x digital accelerometer, both spi and i2c.
+
+Depending on the interface this node should be a child of a spi or i2c bus.
+
+Required properties:
+- compatible: "adxl34x"
+- reg: SPI/I2C device address
+- interrupt-parent: a phandle for the interrupt controller
+- interrupts: interrupt to which the chip is connected
+
+Optional parameters:
+Please refer to include/linux/input/adxl34x.h for specific info. If the value
+is not specified the driver will use its default.
+
+- adi,tap-axis-control: u8 default 0
+- adi,tap-threshold: u8 default 35
+- adi,tap-duration: u8 default 3
+- adi,tap-latency: u8 default 20
+- adi,tap-window: u8 default 20
+- adi,act-axis-control: u8 default 0xFF
+- adi,activity-threshold: u8 default 6
+- adi,inactivity-threshold: u8 default 4
+- adi,inactivity-time: u8 default 3
+- adi,free-fall-threshold: u8 default 8
+- adi,free-fall-time: u8 default 20
+- adi,data-rate: u8 default 8
+- adi,data-range: u8 default 4
+- adi,low-power-mode: u8 defaut 0
+- adi,power-mode: u8 default 0x30
+- adi,fifo-mode: u8 default 2
+- adi,watermark: u8 default 0
+- adi,use-int2: u8
+- adi,orientation-enable: u8 default 0
+- adi,deadzone-angle: u8 default 0
+- divisor-length: u8 default 0
+
+Example
+
+&i2c1 {
+	status = "okay";
+
+	accel1: accelerometer@53 {
+		compatible = "adxl34x";
+		reg = <0x53>;
+
+		interrupt-parent = <&portc>;
+		interrupts = <3 2>;
+
+		adi,inactivity-time = /bits/ 8 <6>;
+		adi,data-range = /bits/ 8 <1>;
+	};
+};
+
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH v5 4/6] ARM: dts: AM335x: Make charge delay a DT parameter for TSC
From: Vignesh R @ 2015-01-07  5:47 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Jonathan Cameron, Hartmut Knaack, Dmitry Torokhov, Lee Jones,
	Sebastian Andrzej Siewior, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Benoit Cousson, Russell King,
	Lars-Peter Clausen, Peter Meerwald, Samuel Ortiz, Brad Griffis,
	Paul Gortmaker, Sanjeev Sharma, Jan Kardell, Felipe Balbi,
	Wolfram Sang, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-omap@
In-Reply-To: <20150102170933.GB3298-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>



On Friday 02 January 2015 10:39 PM, Tony Lindgren wrote:
> * Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org> [141223 23:07]:
>> The charge delay value is by default 0x400. But it can be set to lower
>> values on some boards, as long as false pen-ups are avoided. Lowering the
>> value increases the sampling rate (though current sampling rate is
>> sufficient for TSC operation). In some boards, the value has to be
>> increased to avoid false pen-up events. Hence, charge delay has been
>> made a DT parameter.
>>
>> Signed-off-by: Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org>
> 
> Assuming this binding makes sense for the input people, there should
> be no conflicts merging this patch along with the rest of the series
> via the input tree.
> 

I will post new version moving documentation to driver code and keeping
DT changes as a separate patch.

> Sounds like this value probably depends on the size of the touchscreen
> for the capacitance, so a numeric value is really needed here. 

Its not just dependent on the size of the touchscreen. It varies based
on the individual setups.

> I would personally prefer generic bindings in general, but in any case

There isn't any generic binding that corresponds to charge-delay.
Nothing as been pointed out by input maintainer either.

Regards
Vignesh

> please feel free to merge along with the other input patches:
> 
> Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> 
> 
>> ---
>> v4:
>>  - Set charge delay to 0x400 as default. Most devices function normally
>>    at this value
>>
>>  .../devicetree/bindings/input/touchscreen/ti-tsc-adc.txt  | 15 +++++++++++++++
>>  arch/arm/boot/dts/am335x-evm.dts                          |  1 +
>>  2 files changed, 16 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
>> index 878549ba814d..6df5028a4ad3 100644
>> --- a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
>> @@ -28,6 +28,20 @@ Required properties:
>>  	ti,adc-channels: List of analog inputs available for ADC.
>>  			 AIN0 = 0, AIN1 = 1 and so on till AIN7 = 7.
>>  
>> +Optional properties:
>> +- child "tsc"
>> +	ti,charge-delay: Length of touch screen charge delay step in terms of
>> +			 ADC clock cycles. Charge delay value should be large
>> +			 in order to avoid false pen-up events. This value
>> +			 affects the overall sampling speed, hence need to be
>> +			 kept as low as possible, while avoiding false pen-up
>> +			 event. Start from a lower value, say 0x400, and
>> +			 increase value until false pen-up events are avoided.
>> +			 The pen-up detection happens immediately after the
>> +			 charge step, so this does in fact function as a
>> +			 hardware knob for adjusting the amount of "settling
>> +			 time".
>> +
>>  Example:
>>  	tscadc: tscadc@44e0d000 {
>>  		compatible = "ti,am3359-tscadc";
>> @@ -36,6 +50,7 @@ Example:
>>  			ti,x-plate-resistance = <200>;
>>  			ti,coordiante-readouts = <5>;
>>  			ti,wire-config = <0x00 0x11 0x22 0x33>;
>> +			ti,charge-delay = <0x400>;
>>  		};
>>  
>>  		adc {
>> diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
>> index 54f118c08db8..66342515df20 100644
>> --- a/arch/arm/boot/dts/am335x-evm.dts
>> +++ b/arch/arm/boot/dts/am335x-evm.dts
>> @@ -648,6 +648,7 @@
>>  		ti,x-plate-resistance = <200>;
>>  		ti,coordinate-readouts = <5>;
>>  		ti,wire-config = <0x00 0x11 0x22 0x33>;
>> +		ti,charge-delay = <0x400>;
>>  	};
>>  
>>  	adc {
>> -- 
>> 1.9.1
>>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v6 0/6] Touchscreen performance related fixes
From: Vignesh R @ 2015-01-07  5:49 UTC (permalink / raw)
  To: Jonathan Cameron, Hartmut Knaack, Dmitry Torokhov, Lee Jones,
	Sebastian Andrzej Siewior
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Benoit Cousson, Tony Lindgren, Russell King, Lars-Peter Clausen,
	Peter Meerwald, Samuel Ortiz, Brad Griffis, Paul Gortmaker,
	Sanjeev Sharma, Jan Kardell, Felipe Balbi, Wolfram Sang,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Vignesh R

This series is rebase of v4 onto v3.19-rc1. It also fixes concerns
expressed on v4 wrt simultaneous use of IIO and TSC.

I have tested this patch series on am335x-evm and Beaglebone black
with lcd7-cape.

Note that, these patches do not work as expected on Beaglebone Black
with BB-View 4.3 Cape from Farnell due to a hardware issue in the cape.

Change log:
v6:
 - Reorder patch 4 and 5.

v5:
 - Rebased onto v3.19-rc1
 - minor changes in patch 2,3 and 5.

v3:
 - Replace delta filtering logic in TSC driver with median filtering
   as suggested by Richard.
 - Addressed Lee Jones comments.

v2:
 - Addressed comments by Hartmut Knaack
 - patch 2 was split into two as per Lee Jones comment

Brad Griffis (2):
  input: touchscreen: ti_am335x_tsc Interchange touchscreen and ADC
    steps
  input: touchscreen: ti_am335x_tsc: Remove udelay in interrupt handler

Vignesh R (4):
  mfd: ti_am335x_tscadc: Remove unwanted reg_se_cache save
  input: touchscreen: ti_am335x_tsc: Read charge delay from DT
  ARM: dts: AM335x: Make charge delay a DT parameter for TSC
  input: touchscreen: ti_am335x_tsc: Replace delta filtering with median
    filtering

 .../bindings/input/touchscreen/ti-tsc-adc.txt      |  15 ++
 arch/arm/boot/dts/am335x-evm.dts                   |   1 +
 drivers/iio/adc/ti_am335x_adc.c                    |   5 +-
 drivers/input/touchscreen/ti_am335x_tsc.c          | 197 ++++++++++++---------
 drivers/mfd/ti_am335x_tscadc.c                     |  13 +-
 include/linux/mfd/ti_am335x_tscadc.h               |   4 +-
 6 files changed, 137 insertions(+), 98 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH v6 1/6] input: touchscreen: ti_am335x_tsc Interchange touchscreen and ADC steps
From: Vignesh R @ 2015-01-07  5:49 UTC (permalink / raw)
  To: Jonathan Cameron, Hartmut Knaack, Dmitry Torokhov, Lee Jones,
	Sebastian Andrzej Siewior
  Cc: Mark Rutland, Vignesh R, Wolfram Sang, linux-iio, Paul Gortmaker,
	Peter Meerwald, Lars-Peter Clausen, Russell King, Jan Kardell,
	Tony Lindgren, linux-input, Sanjeev Sharma, devicetree,
	Brad Griffis, Pawel Moll, Ian Campbell, Rob Herring, linux-omap,
	linux-arm-kernel, Samuel Ortiz, linux-kernel, Felipe Balbi,
	Benoit Cousson, Kumar Gala
In-Reply-To: <1420609779-11156-1-git-send-email-vigneshr@ti.com>

From: Brad Griffis <bgriffis@ti.com>

This patch makes the initial changes required to workaround TSC-false
pen-up interrupts. It is required to implement these changes in order to
remove udelay in the TSC interrupt handler and false pen-up events.
The charge step is to be executed immediately after sampling X+. Hence
TSC is made to use higher numbered steps (steps 5 to 16 for 5 co-ordinate
readouts, 4 wire TSC configuration) and ADC to use lower ones. Further
X co-ordinate readouts must be the last to be sampled, thus co-ordinates
are sampled in the order Y-Z-X.

Signed-off-by: Brad Griffis <bgriffis@ti.com>
[vigneshr@ti.com: Ported the patch from v3.12 to v3.19rc1]

Signed-off-by: Vignesh R <vigneshr@ti.com>
Acked-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/iio/adc/ti_am335x_adc.c           |  5 ++--
 drivers/input/touchscreen/ti_am335x_tsc.c | 42 ++++++++++++++++++-------------
 2 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index b730864731e8..adba23246474 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -86,19 +86,18 @@ static void tiadc_step_config(struct iio_dev *indio_dev)
 {
 	struct tiadc_device *adc_dev = iio_priv(indio_dev);
 	unsigned int stepconfig;
-	int i, steps;
+	int i, steps = 0;
 
 	/*
 	 * There are 16 configurable steps and 8 analog input
 	 * lines available which are shared between Touchscreen and ADC.
 	 *
-	 * Steps backwards i.e. from 16 towards 0 are used by ADC
+	 * Steps forwards i.e. from 0 towards 16 are used by ADC
 	 * depending on number of input lines needed.
 	 * Channel would represent which analog input
 	 * needs to be given to ADC to digitalize data.
 	 */
 
-	steps = TOTAL_STEPS - adc_dev->channels;
 	if (iio_buffer_enabled(indio_dev))
 		stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1
 					| STEPCONFIG_MODE_SWCNT;
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 004f1346a957..dfbb9fe6a270 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -121,7 +121,7 @@ static void titsc_step_config(struct titsc *ts_dev)
 {
 	unsigned int	config;
 	int i;
-	int end_step;
+	int end_step, first_step, tsc_steps;
 	u32 stepenable;
 
 	config = STEPCONFIG_MODE_HWSYNC |
@@ -140,9 +140,11 @@ static void titsc_step_config(struct titsc *ts_dev)
 		break;
 	}
 
-	/* 1 … coordinate_readouts is for X */
-	end_step = ts_dev->coordinate_readouts;
-	for (i = 0; i < end_step; i++) {
+	tsc_steps = ts_dev->coordinate_readouts * 2 + 2;
+	first_step = TOTAL_STEPS - tsc_steps;
+	/* Steps 16 to 16-coordinate_readouts is for X */
+	end_step = first_step + tsc_steps;
+	for (i = end_step - ts_dev->coordinate_readouts; i < end_step; i++) {
 		titsc_writel(ts_dev, REG_STEPCONFIG(i), config);
 		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
 	}
@@ -164,9 +166,9 @@ static void titsc_step_config(struct titsc *ts_dev)
 		break;
 	}
 
-	/* coordinate_readouts … coordinate_readouts * 2 is for Y */
-	end_step = ts_dev->coordinate_readouts * 2;
-	for (i = ts_dev->coordinate_readouts; i < end_step; i++) {
+	/* 1 ... coordinate_readouts is for Y */
+	end_step = first_step + ts_dev->coordinate_readouts;
+	for (i = first_step; i < end_step; i++) {
 		titsc_writel(ts_dev, REG_STEPCONFIG(i), config);
 		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
 	}
@@ -179,7 +181,7 @@ static void titsc_step_config(struct titsc *ts_dev)
 	titsc_writel(ts_dev, REG_CHARGECONFIG, config);
 	titsc_writel(ts_dev, REG_CHARGEDELAY, CHARGEDLY_OPENDLY);
 
-	/* coordinate_readouts * 2 … coordinate_readouts * 2 + 2 is for Z */
+	/* coordinate_readouts + 1 ... coordinate_readouts + 2 is for Z */
 	config = STEPCONFIG_MODE_HWSYNC |
 			STEPCONFIG_AVG_16 | ts_dev->bit_yp |
 			ts_dev->bit_xn | STEPCONFIG_INM_ADCREFM |
@@ -194,8 +196,11 @@ static void titsc_step_config(struct titsc *ts_dev)
 	titsc_writel(ts_dev, REG_STEPDELAY(end_step),
 			STEPCONFIG_OPENDLY);
 
-	/* The steps1 … end and bit 0 for TS_Charge */
-	stepenable = (1 << (end_step + 2)) - 1;
+	/* The steps end ... end - readouts * 2 + 2 and bit 0 for TS_Charge */
+	stepenable = 1;
+	for (i = 0; i < tsc_steps; i++)
+		stepenable |= 1 << (first_step + i + 1);
+
 	ts_dev->step_mask = stepenable;
 	am335x_tsc_se_set_cache(ts_dev->mfd_tscadc, ts_dev->step_mask);
 }
@@ -209,6 +214,7 @@ static void titsc_read_coordinates(struct titsc *ts_dev,
 	unsigned int read, diff;
 	unsigned int i, channel;
 	unsigned int creads = ts_dev->coordinate_readouts;
+	unsigned int first_step = TOTAL_STEPS - (creads * 2 + 2);
 
 	*z1 = *z2 = 0;
 	if (fifocount % (creads * 2 + 2))
@@ -226,7 +232,7 @@ static void titsc_read_coordinates(struct titsc *ts_dev,
 
 		channel = (read & 0xf0000) >> 16;
 		read &= 0xfff;
-		if (channel < creads) {
+		if (channel > first_step + creads + 2) {
 			diff = abs(read - prev_val_x);
 			if (diff < prev_diff_x) {
 				prev_diff_x = diff;
@@ -234,19 +240,19 @@ static void titsc_read_coordinates(struct titsc *ts_dev,
 			}
 			prev_val_x = read;
 
-		} else if (channel < creads * 2) {
+		} else if (channel == first_step + creads + 1) {
+			*z1 = read;
+
+		} else if (channel == first_step + creads + 2) {
+			*z2 = read;
+
+		} else if (channel > first_step) {
 			diff = abs(read - prev_val_y);
 			if (diff < prev_diff_y) {
 				prev_diff_y = diff;
 				*y = read;
 			}
 			prev_val_y = read;
-
-		} else if (channel < creads * 2 + 1) {
-			*z1 = read;
-
-		} else if (channel < creads * 2 + 2) {
-			*z2 = read;
 		}
 	}
 }
-- 
1.9.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v6 3/6] mfd: ti_am335x_tscadc: Remove unwanted reg_se_cache save
From: Vignesh R @ 2015-01-07  5:49 UTC (permalink / raw)
  To: Jonathan Cameron, Hartmut Knaack, Dmitry Torokhov, Lee Jones,
	Sebastian Andrzej Siewior
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Benoit Cousson, Tony Lindgren, Russell King, Lars-Peter Clausen,
	Peter Meerwald, Samuel Ortiz, Brad Griffis, Paul Gortmaker,
	Sanjeev Sharma, Jan Kardell, Felipe Balbi, Wolfram Sang,
	devicetree, linux-kernel, linux-omap, linux-arm-kernel, linux-iio,
	linux-input, Vignesh R
In-Reply-To: <1420609779-11156-1-git-send-email-vigneshr@ti.com>

In one shot mode, sequencer automatically disables all enabled steps at
the end of each cycle. (both ADC steps and TSC steps) Hence these steps
need not be saved in reg_se_cache for clearing these steps at a later
stage.
Also, when ADC wakes up Sequencer should not be busy executing any of the
config steps except for the charge step. Previously charge step was 1 ADC
clock cycle and hence it was ignored.
TSC steps are always disabled at the end of each conversion cycle, hence
there is no need to explicitly disable TSC steps by writing 0 to REG_SE.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---

v5:
 - Remove unnecessary clearing of REG_SE

 drivers/mfd/ti_am335x_tscadc.c       | 13 +++++--------
 include/linux/mfd/ti_am335x_tscadc.h |  1 +
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index 467c80e1c4ae..e4e4b22eebc9 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -68,12 +68,6 @@ static void am335x_tscadc_need_adc(struct ti_tscadc_dev *tsadc)
 	DEFINE_WAIT(wait);
 	u32 reg;
 
-	/*
-	 * disable TSC steps so it does not run while the ADC is using it. If
-	 * write 0 while it is running (it just started or was already running)
-	 * then it completes all steps that were enabled and stops then.
-	 */
-	tscadc_writel(tsadc, REG_SE, 0);
 	reg = tscadc_readl(tsadc, REG_ADCFSM);
 	if (reg & SEQ_STATUS) {
 		tsadc->adc_waiting = true;
@@ -86,8 +80,12 @@ static void am335x_tscadc_need_adc(struct ti_tscadc_dev *tsadc)
 		spin_lock_irq(&tsadc->reg_lock);
 		finish_wait(&tsadc->reg_se_wait, &wait);
 
+		/*
+		 * Sequencer should either be idle or
+		 * busy applying the charge step.
+		 */
 		reg = tscadc_readl(tsadc, REG_ADCFSM);
-		WARN_ON(reg & SEQ_STATUS);
+		WARN_ON((reg & SEQ_STATUS) && !(reg & CHARGE_STEP));
 		tsadc->adc_waiting = false;
 	}
 	tsadc->adc_in_use = true;
@@ -96,7 +94,6 @@ static void am335x_tscadc_need_adc(struct ti_tscadc_dev *tsadc)
 void am335x_tsc_se_set_once(struct ti_tscadc_dev *tsadc, u32 val)
 {
 	spin_lock_irq(&tsadc->reg_lock);
-	tsadc->reg_se_cache |= val;
 	am335x_tscadc_need_adc(tsadc);
 
 	tscadc_writel(tsadc, REG_SE, val);
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index 3f4e994ace2b..1fd50dcfe47c 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -128,6 +128,7 @@
 
 /* Sequencer Status */
 #define SEQ_STATUS BIT(5)
+#define CHARGE_STEP		0x11
 
 #define ADC_CLK			3000000
 #define TOTAL_STEPS		16
-- 
1.9.1

^ permalink raw reply related

* [PATCH v6 4/6] input: touchscreen: ti_am335x_tsc: Read charge delay from DT
From: Vignesh R @ 2015-01-07  5:49 UTC (permalink / raw)
  To: Jonathan Cameron, Hartmut Knaack, Dmitry Torokhov, Lee Jones,
	Sebastian Andrzej Siewior
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Benoit Cousson, Tony Lindgren, Russell King, Lars-Peter Clausen,
	Peter Meerwald, Samuel Ortiz, Brad Griffis, Paul Gortmaker,
	Sanjeev Sharma, Jan Kardell, Felipe Balbi, Wolfram Sang,
	devicetree, linux-kernel, linux-omap, linux-arm-kernel, linux-iio,
	linux-input, Vignesh R
In-Reply-To: <1420609779-11156-1-git-send-email-vigneshr@ti.com>

This patch reads charge delay from tsc DT node and writes to
REG_CHARGEDELAY register. If the charge delay is not specified in DT
then default value of 0x400(CHARGEDLY_OPENDLY) is used.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---

v6:
 - Move Documentation from DT patch to 	driver code. 

v5:
 - print out a warning when ti,charge-delay is not specified

v4:
 - Set charge delay to 0x400 as default. Most devices function normally
   at this value

 .../devicetree/bindings/input/touchscreen/ti-tsc-adc.txt  | 15 +++++++++++++++
 drivers/input/touchscreen/ti_am335x_tsc.c                 | 14 +++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
index 878549ba814d..6c4fb34823d3 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
@@ -28,6 +28,20 @@ Required properties:
 	ti,adc-channels: List of analog inputs available for ADC.
 			 AIN0 = 0, AIN1 = 1 and so on till AIN7 = 7.
 
+Optional properties:
+- child "tsc"
+	ti,charge-delay: Length of touch screen charge delay step in terms of
+			 ADC clock cycles. Charge delay value should be large
+			 in order to avoid false pen-up events. This value
+			 effects the overall sampling speed, hence need to be
+			 kept as low as possible, while avoiding false pen-up
+			 event. Start from a lower value, say 0x400, and
+			 increase value until false pen-up events are avoided.
+			 The pen-up detection happens immediately after the
+			 charge step, so this does in fact function as a
+			 hardware knob for adjusting the amount of "settling
+			 time".
+
 Example:
 	tscadc: tscadc@44e0d000 {
 		compatible = "ti,am3359-tscadc";
@@ -36,6 +50,7 @@ Example:
 			ti,x-plate-resistance = <200>;
 			ti,coordiante-readouts = <5>;
 			ti,wire-config = <0x00 0x11 0x22 0x33>;
+			ti,charge-delay = <0x400>;
 		};
 
 		adc {
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 0625c102a1d0..7c0f6b21559d 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -52,6 +52,7 @@ struct titsc {
 	u32			bit_xp, bit_xn, bit_yp, bit_yn;
 	u32			inp_xp, inp_xn, inp_yp, inp_yn;
 	u32			step_mask;
+	u32			charge_delay;
 };
 
 static unsigned int titsc_readl(struct titsc *ts, unsigned int reg)
@@ -177,7 +178,7 @@ static void titsc_step_config(struct titsc *ts_dev)
 
 	config = titsc_readl(ts_dev, REG_IDLECONFIG);
 	titsc_writel(ts_dev, REG_CHARGECONFIG, config);
-	titsc_writel(ts_dev, REG_CHARGEDELAY, CHARGEDLY_OPENDLY);
+	titsc_writel(ts_dev, REG_CHARGEDELAY, ts_dev->charge_delay);
 
 	/* coordinate_readouts + 1 ... coordinate_readouts + 2 is for Z */
 	config = STEPCONFIG_MODE_HWSYNC |
@@ -368,6 +369,17 @@ static int titsc_parse_dt(struct platform_device *pdev,
 	if (err < 0)
 		return err;
 
+	err = of_property_read_u32(node, "ti,charge-delay",
+				   &ts_dev->charge_delay);
+	/*
+	 * If ti,charge-delay value is not specified, then use
+	 * CHARGEDLY_OPENDLY as the default value.
+	 */
+	if (err < 0) {
+		ts_dev->charge_delay = CHARGEDLY_OPENDLY;
+		dev_warn(&pdev->dev, "ti,charge-delay not specified\n");
+	}
+
 	return of_property_read_u32_array(node, "ti,wire-config",
 			ts_dev->config_inp, ARRAY_SIZE(ts_dev->config_inp));
 }
-- 
1.9.1

^ permalink raw reply related

* [PATCH v6 5/6] ARM: dts: AM335x: Make charge delay a DT parameter for TSC
From: Vignesh R @ 2015-01-07  5:49 UTC (permalink / raw)
  To: Jonathan Cameron, Hartmut Knaack, Dmitry Torokhov, Lee Jones,
	Sebastian Andrzej Siewior
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Benoit Cousson, Tony Lindgren, Russell King, Lars-Peter Clausen,
	Peter Meerwald, Samuel Ortiz, Brad Griffis, Paul Gortmaker,
	Sanjeev Sharma, Jan Kardell, Felipe Balbi, Wolfram Sang,
	devicetree, linux-kernel, linux-omap, linux-arm-kernel, linux-iio,
	linux-input, Vignesh R
In-Reply-To: <1420609779-11156-1-git-send-email-vigneshr@ti.com>

The charge delay value is by default 0x400. But it can be set to lower
values on some boards, as long as false pen-ups are avoided. Lowering the
value increases the sampling rate (though current sampling rate is
sufficient for TSC operation). In some boards, the value has to be
increased to avoid false pen-up events. Hence, charge delay has been
made a DT parameter.

Signed-off-by: Vignesh R <vigneshr@ti.com>
Acked-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/boot/dts/am335x-evm.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index 54f118c08db8..66342515df20 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -648,6 +648,7 @@
 		ti,x-plate-resistance = <200>;
 		ti,coordinate-readouts = <5>;
 		ti,wire-config = <0x00 0x11 0x22 0x33>;
+		ti,charge-delay = <0x400>;
 	};
 
 	adc {
-- 
1.9.1

^ permalink raw reply related

* [PATCH v6 2/6] input: touchscreen: ti_am335x_tsc: Remove udelay in interrupt handler
From: Vignesh R @ 2015-01-07  5:49 UTC (permalink / raw)
  To: Jonathan Cameron, Hartmut Knaack, Dmitry Torokhov, Lee Jones,
	Sebastian Andrzej Siewior
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Benoit Cousson, Tony Lindgren, Russell King, Lars-Peter Clausen,
	Peter Meerwald, Samuel Ortiz, Brad Griffis, Paul Gortmaker,
	Sanjeev Sharma, Jan Kardell, Felipe Balbi, Wolfram Sang,
	devicetree, linux-kernel, linux-omap, linux-arm-kernel, linux-iio,
	linux-input, Vignesh R
In-Reply-To: <1420609779-11156-1-git-send-email-vigneshr@ti.com>

From: Brad Griffis <bgriffis@ti.com>

TSC interrupt handler had udelay to avoid reporting of false pen-up
interrupt to user space. This patch implements workaround suggesting in
Advisory 1.0.31 of silicon errata for am335x, thus eliminating udelay
and touchscreen lag. This also improves performance of touchscreen and
eliminates sudden jump of cursor at touch release.

IDLECONFIG and CHARGECONFIG registers are to be configured
with same values in order to eliminate false pen-up events. This
workaround may result in false pen-down to be detected, hence considerable
charge step delay needs to be added. The charge delay is set to 0xB000
(in terms of ADC clock cycles) by default.

TSC steps are disabled at the end of every sampling cycle and EOS bit is
set. Once the EOS bit is set, the TSC steps need to be re-enabled to begin
next sampling cycle.

Signed-off-by: Brad Griffis <bgriffis@ti.com>
[vigneshr@ti.com: Ported the patch from v3.12 to v3.19rc1]

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
v5:
 - enable TSC steps only on EOS interrup

v4:
 - check for PEN_IRQ bit in ADCFSM to avoid false-pen when ADC
   and TSC are used together.
 - Set charge delay to 0x400 as default. Most devices function
   normally at this value.

 drivers/input/touchscreen/ti_am335x_tsc.c | 67 ++++++++++++++-----------------
 include/linux/mfd/ti_am335x_tscadc.h      |  3 +-
 2 files changed, 33 insertions(+), 37 deletions(-)

diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index dfbb9fe6a270..0625c102a1d0 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -173,11 +173,9 @@ static void titsc_step_config(struct titsc *ts_dev)
 		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
 	}
 
-	/* Charge step configuration */
-	config = ts_dev->bit_xp | ts_dev->bit_yn |
-			STEPCHARGE_RFP_XPUL | STEPCHARGE_RFM_XNUR |
-			STEPCHARGE_INM_AN1 | STEPCHARGE_INP(ts_dev->inp_yp);
+	/* Make CHARGECONFIG same as IDLECONFIG */
 
+	config = titsc_readl(ts_dev, REG_IDLECONFIG);
 	titsc_writel(ts_dev, REG_CHARGECONFIG, config);
 	titsc_writel(ts_dev, REG_CHARGEDELAY, CHARGEDLY_OPENDLY);
 
@@ -261,12 +259,34 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 {
 	struct titsc *ts_dev = dev;
 	struct input_dev *input_dev = ts_dev->input;
-	unsigned int status, irqclr = 0;
+	unsigned int fsm, status, irqclr = 0;
 	unsigned int x = 0, y = 0;
 	unsigned int z1, z2, z;
-	unsigned int fsm;
 
-	status = titsc_readl(ts_dev, REG_IRQSTATUS);
+	status = titsc_readl(ts_dev, REG_RAWIRQSTATUS);
+	if (status & IRQENB_HW_PEN) {
+		ts_dev->pen_down = true;
+		titsc_writel(ts_dev, REG_IRQWAKEUP, 0x00);
+		titsc_writel(ts_dev, REG_IRQCLR, IRQENB_HW_PEN);
+		irqclr |= IRQENB_HW_PEN;
+	}
+
+	if (status & IRQENB_PENUP) {
+		fsm = titsc_readl(ts_dev, REG_ADCFSM);
+		if (fsm == ADCFSM_STEPID) {
+			ts_dev->pen_down = false;
+			input_report_key(input_dev, BTN_TOUCH, 0);
+			input_report_abs(input_dev, ABS_PRESSURE, 0);
+			input_sync(input_dev);
+		} else {
+			ts_dev->pen_down = true;
+		}
+		irqclr |= IRQENB_PENUP;
+	}
+
+	if (status & IRQENB_EOS)
+		irqclr |= IRQENB_EOS;
+
 	/*
 	 * ADC and touchscreen share the IRQ line.
 	 * FIFO1 interrupts are used by ADC. Handle FIFO0 IRQs here only
@@ -297,37 +317,11 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 		}
 		irqclr |= IRQENB_FIFO0THRES;
 	}
-
-	/*
-	 * Time for sequencer to settle, to read
-	 * correct state of the sequencer.
-	 */
-	udelay(SEQ_SETTLE);
-
-	status = titsc_readl(ts_dev, REG_RAWIRQSTATUS);
-	if (status & IRQENB_PENUP) {
-		/* Pen up event */
-		fsm = titsc_readl(ts_dev, REG_ADCFSM);
-		if (fsm == ADCFSM_STEPID) {
-			ts_dev->pen_down = false;
-			input_report_key(input_dev, BTN_TOUCH, 0);
-			input_report_abs(input_dev, ABS_PRESSURE, 0);
-			input_sync(input_dev);
-		} else {
-			ts_dev->pen_down = true;
-		}
-		irqclr |= IRQENB_PENUP;
-	}
-
-	if (status & IRQENB_HW_PEN) {
-
-		titsc_writel(ts_dev, REG_IRQWAKEUP, 0x00);
-		titsc_writel(ts_dev, REG_IRQCLR, IRQENB_HW_PEN);
-	}
-
 	if (irqclr) {
 		titsc_writel(ts_dev, REG_IRQSTATUS, irqclr);
-		am335x_tsc_se_set_cache(ts_dev->mfd_tscadc, ts_dev->step_mask);
+		if (status & IRQENB_EOS)
+			am335x_tsc_se_set_cache(ts_dev->mfd_tscadc,
+						ts_dev->step_mask);
 		return IRQ_HANDLED;
 	}
 	return IRQ_NONE;
@@ -417,6 +411,7 @@ static int titsc_probe(struct platform_device *pdev)
 	}
 
 	titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_FIFO0THRES);
+	titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_EOS);
 	err = titsc_config_wires(ts_dev);
 	if (err) {
 		dev_err(&pdev->dev, "wrong i/p wire configuration\n");
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index e2e70053470e..3f4e994ace2b 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -52,6 +52,7 @@
 
 /* IRQ enable */
 #define IRQENB_HW_PEN		BIT(0)
+#define IRQENB_EOS		BIT(1)
 #define IRQENB_FIFO0THRES	BIT(2)
 #define IRQENB_FIFO0OVRRUN	BIT(3)
 #define IRQENB_FIFO0UNDRFLW	BIT(4)
@@ -107,7 +108,7 @@
 /* Charge delay */
 #define CHARGEDLY_OPEN_MASK	(0x3FFFF << 0)
 #define CHARGEDLY_OPEN(val)	((val) << 0)
-#define CHARGEDLY_OPENDLY	CHARGEDLY_OPEN(1)
+#define CHARGEDLY_OPENDLY	CHARGEDLY_OPEN(0x400)
 
 /* Control register */
 #define CNTRLREG_TSCSSENB	BIT(0)
-- 
1.9.1


^ permalink raw reply related

* [PATCH v6 6/6] input: touchscreen: ti_am335x_tsc: Replace delta filtering with median filtering
From: Vignesh R @ 2015-01-07  5:49 UTC (permalink / raw)
  To: Jonathan Cameron, Hartmut Knaack, Dmitry Torokhov, Lee Jones,
	Sebastian Andrzej Siewior
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Benoit Cousson, Tony Lindgren, Russell King, Lars-Peter Clausen,
	Peter Meerwald, Samuel Ortiz, Brad Griffis, Paul Gortmaker,
	Sanjeev Sharma, Jan Kardell, Felipe Balbi, Wolfram Sang,
	devicetree, linux-kernel, linux-omap, linux-arm-kernel, linux-iio,
	linux-input, Vignesh R
In-Reply-To: <1420609779-11156-1-git-send-email-vigneshr@ti.com>

Previously, delta filtering was applied TSC co-ordinate readouts before
reporting a single value to user space. This patch replaces delta filtering
with median filtering. Median filtering sorts co-ordinate readouts, drops min
and max values, and reports the average of remaining values. This method is
more sensible than delta filering. Median filtering is applied only if number
of readouts is greater than 3 else just average of co-ordinate readouts is
reported.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/input/touchscreen/ti_am335x_tsc.c | 92 +++++++++++++++++--------------
 1 file changed, 52 insertions(+), 40 deletions(-)

diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 7c0f6b21559d..191a1b87895f 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -26,6 +26,7 @@
 #include <linux/delay.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/sort.h>
 
 #include <linux/mfd/ti_am335x_tscadc.h>
 
@@ -204,56 +205,61 @@ static void titsc_step_config(struct titsc *ts_dev)
 	am335x_tsc_se_set_cache(ts_dev->mfd_tscadc, ts_dev->step_mask);
 }
 
+static int titsc_cmp_coord(const void *a, const void *b)
+{
+	return *(int *)a - *(int *)b;
+}
+
 static void titsc_read_coordinates(struct titsc *ts_dev,
 		u32 *x, u32 *y, u32 *z1, u32 *z2)
 {
-	unsigned int fifocount = titsc_readl(ts_dev, REG_FIFO0CNT);
-	unsigned int prev_val_x = ~0, prev_val_y = ~0;
-	unsigned int prev_diff_x = ~0, prev_diff_y = ~0;
-	unsigned int read, diff;
-	unsigned int i, channel;
+	unsigned int yvals[7], xvals[7];
+	unsigned int i, xsum = 0, ysum = 0;
 	unsigned int creads = ts_dev->coordinate_readouts;
-	unsigned int first_step = TOTAL_STEPS - (creads * 2 + 2);
 
-	*z1 = *z2 = 0;
-	if (fifocount % (creads * 2 + 2))
-		fifocount -= fifocount % (creads * 2 + 2);
-	/*
-	 * Delta filter is used to remove large variations in sampled
-	 * values from ADC. The filter tries to predict where the next
-	 * coordinate could be. This is done by taking a previous
-	 * coordinate and subtracting it form current one. Further the
-	 * algorithm compares the difference with that of a present value,
-	 * if true the value is reported to the sub system.
-	 */
-	for (i = 0; i < fifocount; i++) {
-		read = titsc_readl(ts_dev, REG_FIFO0);
-
-		channel = (read & 0xf0000) >> 16;
-		read &= 0xfff;
-		if (channel > first_step + creads + 2) {
-			diff = abs(read - prev_val_x);
-			if (diff < prev_diff_x) {
-				prev_diff_x = diff;
-				*x = read;
-			}
-			prev_val_x = read;
+	for (i = 0; i < creads; i++) {
+		yvals[i] = titsc_readl(ts_dev, REG_FIFO0);
+		yvals[i] &= 0xfff;
+	}
 
-		} else if (channel == first_step + creads + 1) {
-			*z1 = read;
+	*z1 = titsc_readl(ts_dev, REG_FIFO0);
+	*z1 &= 0xfff;
+	*z2 = titsc_readl(ts_dev, REG_FIFO0);
+	*z2 &= 0xfff;
 
-		} else if (channel == first_step + creads + 2) {
-			*z2 = read;
+	for (i = 0; i < creads; i++) {
+		xvals[i] = titsc_readl(ts_dev, REG_FIFO0);
+		xvals[i] &= 0xfff;
+	}
 
-		} else if (channel > first_step) {
-			diff = abs(read - prev_val_y);
-			if (diff < prev_diff_y) {
-				prev_diff_y = diff;
-				*y = read;
-			}
-			prev_val_y = read;
+	/*
+	 * If co-ordinates readouts is less than 4 then
+	 * report the average. In case of 4 or more
+	 * readouts, sort the co-ordinate samples, drop
+	 * min and max values and report the average of
+	 * remaining values.
+	 */
+	if (creads <=  3) {
+		for (i = 0; i < creads; i++) {
+			ysum += yvals[i];
+			xsum += xvals[i];
 		}
+		ysum /= creads;
+		xsum /= creads;
+	} else {
+		sort(yvals, creads, sizeof(unsigned int),
+		     titsc_cmp_coord, NULL);
+		sort(xvals, creads, sizeof(unsigned int),
+		     titsc_cmp_coord, NULL);
+		for (i = 1; i < creads - 1; i++) {
+			ysum += yvals[i];
+			xsum += xvals[i];
+		}
+		ysum /= creads - 2;
+		xsum /= creads - 2;
 	}
+	*y = ysum;
+	*x = xsum;
 }
 
 static irqreturn_t titsc_irq(int irq, void *dev)
@@ -369,6 +375,12 @@ static int titsc_parse_dt(struct platform_device *pdev,
 	if (err < 0)
 		return err;
 
+	if (ts_dev->coordinate_readouts <= 0) {
+		dev_warn(&pdev->dev,
+			 "invalid co-ordinate readouts, resetting it to 5\n");
+		ts_dev->coordinate_readouts = 5;
+	}
+
 	err = of_property_read_u32(node, "ti,charge-delay",
 				   &ts_dev->charge_delay);
 	/*
-- 
1.9.1


^ permalink raw reply related

* Re: [PATCH 1/2] input: synaptics - make image sensors report ABS_MT_TOUCH_MAJOR
From: Peter Hutterer @ 2015-01-07  6:02 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Gabriele Mazzotta, Hans de Goede, Dmitry Torokhov, Henrik Rydberg,
	linux-input, linux-kernel@vger.kernel.org, Maxwell Anselm
In-Reply-To: <CAN+gG=Fhaj2aQM7gxXZBaXx3MZQbdMdCZ7KO=nmhzufp4YqnmA@mail.gmail.com>

On Mon, Jan 05, 2015 at 02:24:30PM -0500, Benjamin Tissoires wrote:
> Hi Gabriele,
> 
> [Adding Peter and Hans as this change will impact both
> xf86-input-synaptics and libinput]
> 
> On Sat, Dec 27, 2014 at 6:31 AM, Gabriele Mazzotta
> <gabriele.mzt@gmail.com> wrote:
> > Despite claiming to be able to report ABS_TOOL_WIDTH, image sensors
> > were not doing it. Make them report widths and use ABS_MT_TOUCH_MAJOR
> > instead ABS_TOOL_WIDTH.
> 
> It looks like the current xorg-synaptics code already handles
> ABS_MT_TOUCH_MAJOR as finger_width. So I think we should be good in
> replacing the ABS_TOOL_WIDTH event. However, I'd prefer having Peter
> confirm this because xorg-synaptics still relies a lot on the single
> touch emulation.

synaptics doesn't actually use TOUCH_MAJOR for finger width, we just forward
it as axis information (not that anyone uses it).
dropping TOOL_WIDTH would only affect palm detection which unreliable, off
by default and mostly broken. so this is fine with me.

Cheers,
   Peter

> 
> >
> > Since the 'w' slot is used to report the finger count when two or more
> > fingers are on the touchpad, make sure that only meaningful values are
> > emitted, i.e. values greater than or equal to 4, and assign the correct
> > range to ABS_MT_TOUCH_MAJOR.
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=77161
> > Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> > ---
> >  drivers/input/mouse/synaptics.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> > index f947292..ea0563e 100644
> > --- a/drivers/input/mouse/synaptics.c
> > +++ b/drivers/input/mouse/synaptics.c
> > @@ -814,6 +814,8 @@ static void synaptics_report_slot(struct input_dev *dev, int slot,
> 
> Just FYI, this does not apply anymore on top of Dmitry's tree.
> synaptics_report_slot() has been removed, and you should now report
> the slot state in synaptics_report_mt_data().
> 
> >         input_report_abs(dev, ABS_MT_POSITION_X, hw->x);
> >         input_report_abs(dev, ABS_MT_POSITION_Y, synaptics_invert_y(hw->y));
> >         input_report_abs(dev, ABS_MT_PRESSURE, hw->z);
> > +       if (hw->w >= 4)
> 
> That I don't like. IMO, at this point, .w should only contain the
> finger width, unconditionally.
> Also, with 2/2, .w is computed accurately for the second finger, but
> not for the first.
> 
> I tried to figure out a way to properly extract the actual width
> information from the sgm packet when the w is 0 or 1, and the only way
> I found was to do the fix in synaptics_image_sensor_process(). I would
> have preferred dealing with that in synaptics_parse_hw_state()
> directly, but I think the final code would be more and more ugly.
> Dealing with the true finger width in synaptics_image_sensor_process()
> is not a problem for cr48 sensors, because they will not have the
> ABS_MT_TOUCH_MAJOR event exported.
> 
> Anyway, something like that "should" work (sorry for the tab/space,
> gmail in use), of course, this is untested when I send the mail :) :
> 
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index 95c15451..caf7ccd 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -852,6 +852,17 @@ static void synaptics_image_sensor_process(struct
> psmouse *psmouse,
>         else
>                 num_fingers = 4;
> 
> +       /* When multiple fingers are on the TouchPad, the width is encoded in
> +        * X, Y and Z */
> +       if (sgm->w == 0 || sgm->w == 1) {
> +               sgm->w = ((sgm->x & BIT(1)) >> 1) |
> +                         (sgm->y & BIT(1)) |
> +                        ((sgm->z & BIT(0)) << 1);
> +               sgm->x &= ~BIT(1);
> +               sgm->y &= ~BIT(1);
> +               sgm->z &= ~BIT(0);
> +       }
> +
>         /* Send resulting input events to user space */
>         synaptics_report_mt_data(psmouse, sgm, num_fingers);
>  }
> 
> 
> > +               input_report_abs(dev, ABS_MT_TOUCH_MAJOR, hw->w);
> >  }
> >
> >  static void synaptics_report_mt_data(struct psmouse *psmouse,
> > @@ -1462,8 +1464,13 @@ static void set_input_params(struct psmouse *psmouse,
> >                                         INPUT_MT_TRACK : INPUT_MT_SEMI_MT));
> >         }
> >
> > -       if (SYN_CAP_PALMDETECT(priv->capabilities))
> > -               input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
> > +       if (SYN_CAP_PALMDETECT(priv->capabilities)) {
> > +               if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c))
> > +                       input_set_abs_params(dev,
> > +                                            ABS_MT_TOUCH_MAJOR, 4, 15, 0, 0);
> > +               else
> > +                       input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
> > +       }
> 
> Again, I'd prefer having Peter's point of view here.
> 
> If we really need both, we will need to change input-mt to also
> emulate ABS_TOOL_WIDTH from ABS_MT_TOUCH_MAJOR like we do between
> ABS_MT_POSITION_X|Y and ABS_X|Y.
> 
> Cheers,
> Benjamin
> 
> >
> >         __set_bit(BTN_TOUCH, dev->keybit);
> >         __set_bit(BTN_TOOL_FINGER, dev->keybit);
> > --
> > 2.1.4
> >
> > --
> > 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 1/2] input: synaptics - make image sensors report ABS_MT_TOUCH_MAJOR
From: Peter Hutterer @ 2015-01-07  6:10 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Gabriele Mazzotta, Hans de Goede, Dmitry Torokhov, Henrik Rydberg,
	linux-input, linux-kernel@vger.kernel.org, Maxwell Anselm
In-Reply-To: <CAN+gG=FQY4V8K6k-mVrYRNSLoJDZW5bX1T=rD04MkEedQRdfUg@mail.gmail.com>

On Mon, Jan 05, 2015 at 05:04:55PM -0500, Benjamin Tissoires wrote:
> On Mon, Jan 5, 2015 at 5:00 PM, Gabriele Mazzotta
> <gabriele.mzt@gmail.com> wrote:
> > On Monday 05 January 2015 14:24:30 Benjamin Tissoires wrote:
> >> Hi Gabriele,
> >>
> >> [Adding Peter and Hans as this change will impact both
> >> xf86-input-synaptics and libinput]
> >>
> >> On Sat, Dec 27, 2014 at 6:31 AM, Gabriele Mazzotta
> >> <gabriele.mzt@gmail.com> wrote:
> >> > Despite claiming to be able to report ABS_TOOL_WIDTH, image sensors
> >> > were not doing it. Make them report widths and use ABS_MT_TOUCH_MAJOR
> >> > instead ABS_TOOL_WIDTH.
> >>
> >> It looks like the current xorg-synaptics code already handles
> >> ABS_MT_TOUCH_MAJOR as finger_width. So I think we should be good in
> >> replacing the ABS_TOOL_WIDTH event. However, I'd prefer having Peter
> >> confirm this because xorg-synaptics still relies a lot on the single
> >> touch emulation.
> >>
> >> >
> >> > Since the 'w' slot is used to report the finger count when two or more
> >> > fingers are on the touchpad, make sure that only meaningful values are
> >> > emitted, i.e. values greater than or equal to 4, and assign the correct
> >> > range to ABS_MT_TOUCH_MAJOR.
> >> >
> >> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=77161
> >> > Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> >> > ---
> >> >  drivers/input/mouse/synaptics.c | 11 +++++++++--
> >> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> >> > index f947292..ea0563e 100644
> >> > --- a/drivers/input/mouse/synaptics.c
> >> > +++ b/drivers/input/mouse/synaptics.c
> >> > @@ -814,6 +814,8 @@ static void synaptics_report_slot(struct input_dev *dev, int slot,
> >>
> >> Just FYI, this does not apply anymore on top of Dmitry's tree.
> >> synaptics_report_slot() has been removed, and you should now report
> >> the slot state in synaptics_report_mt_data().
> >>
> >> >         input_report_abs(dev, ABS_MT_POSITION_X, hw->x);
> >> >         input_report_abs(dev, ABS_MT_POSITION_Y, synaptics_invert_y(hw->y));
> >> >         input_report_abs(dev, ABS_MT_PRESSURE, hw->z);
> >> > +       if (hw->w >= 4)
> >>
> >> That I don't like. IMO, at this point, .w should only contain the
> >> finger width, unconditionally.
> >> Also, with 2/2, .w is computed accurately for the second finger, but
> >> not for the first.
> >>
> >> I tried to figure out a way to properly extract the actual width
> >> information from the sgm packet when the w is 0 or 1, and the only way
> >> I found was to do the fix in synaptics_image_sensor_process(). I would
> >> have preferred dealing with that in synaptics_parse_hw_state()
> >> directly, but I think the final code would be more and more ugly.
> >> Dealing with the true finger width in synaptics_image_sensor_process()
> >> is not a problem for cr48 sensors, because they will not have the
> >> ABS_MT_TOUCH_MAJOR event exported.
> >
> > Regarding the last part on cr48 sensors.
> > Currently these sensors are not reporting widths through ABS_TOOL_WIDTH
> > and I don't see what could go wrong if they start reporting
> > ABS_MT_TOUCH_MAJOR. If I understood correctly, they can report widths
> > only when one finger is on the touchpad. This means that they will
> > report widths through slot 0, but they won't through slot 1. Nothing
> > bad should happen.
> 
> I am not entirely sure. The entire purpose of having widht for palm
> detection is to filter palm from true finger events. So if we only
> have the width info on the first slot, it would be useless IMO.
> Still I agree with "nothing bad should happen" :)

fwiw, I found finger width _very_ unreliable for palm detection. from what I
recorded, both finger width and palm width ranges overlap to a large amount,
making width useless for anything that's not the whole hand on the
touchpad.

Cheers,
   Peter

> > I've just rebased my patches and included the support for cr48 sensors.
> > The only change I made was to change the default width value from
> > 5 to 4 since this is the minimum values these sensors can report
> > (according to the existing code).
> >
> > Anyway, should I simply include the changes you suggested to handle
> > widths of sgm packets in what will be the new 2/2?
> 
> Go ahead and include my changes in your 2/2. It's better to have a
> consistent commit.
> 
> Cheers,
> Benjamin

^ permalink raw reply

* Re: [PATCH] input: Add soft kill switch for input devices
From: Tristan Lelong @ 2015-01-07  6:40 UTC (permalink / raw)
  To: Bruno Prémont; +Cc: linux-input, dmitry.torokhov, linux-kernel
In-Reply-To: <20150106221732.1cbbe3ae@neptune.home>

On Tue, Jan 06, 2015 at 10:17:32PM +0100, Bruno Prémont wrote:
> 
> One big issue I see here is that you start dropping all events at a random
> point in time.
> You may end up within a gesture or just while a button is down and remain
> so until that same button gets pressed again on unmute.
> 
> On your touchscreen, how do things behave if you trigger the muting while
> you are dragging or doing some gesture (and unmute when noone touches
> the touchscreen)?

That is right, I thought of it for pointing devices, and it seemed that it should not be
too much of a problem, even thought it would be better to handle it properly.

> 
> If applied to a keyboard, what happens when you mute while keys are pressed
> and unmute when they have long been released?

This case is actually a bigger problem and I didn't think of it.
> 
> 
> If you could send a "reset" event to input listeners when muting you could
> work around most issues as any application making use of events would know
> that after this "reset" event things should not depend on any past state.

I could definitely do something like this and make sure all the events are handled properly.

> 
> 
> Possibly a better way to achieve your aim is to just unbind and re-bind
> the device from device driver, causing software hotplug.
> That way you are sure no application will perform incorrect guesses while
> device is "muted".

This is another good solution that would work in my case.

If somebody think this patch can be of any interest, I'll be happy to
submit a v2 with correct handling of current events at the time of mutting.

Best regards
--
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] [ADD] 1.support new ic type for version 4.
From: Dmitry Torokhov @ 2015-01-07  7:21 UTC (permalink / raw)
  To: Sam hung; +Cc: linux-input, dusonlin
In-Reply-To: <1420531816-4744-1-git-send-email-sam.hung@emc.com.tw>

Hi Sam,

On Tue, Jan 06, 2015 at 04:10:16PM +0800, Sam hung wrote:
> ---
>  drivers/input/mouse/elantech.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index f2b9780..4891d07 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -700,14 +700,14 @@ static int elantech_packet_check_v1(struct psmouse *psmouse)
>  
>  static int elantech_debounce_check_v2(struct psmouse *psmouse)
>  {
> -        /*
> -         * When we encounter packet that matches this exactly, it means the
> -         * hardware is in debounce status. Just ignore the whole packet.
> -         */
> -        const u8 debounce_packet[] = { 0x84, 0xff, 0xff, 0x02, 0xff, 0xff };
> -        unsigned char *packet = psmouse->packet;
> -
> -        return !memcmp(packet, debounce_packet, sizeof(debounce_packet));
> +	/*
> +	* When we encounter packet that matches this exactly, it means the
> +	* hardware is in debounce status. Just ignore the whole packet.
> +	*/
> +	const u8 debounce_packet[] = { 0x84, 0xff, 0xff, 0x02, 0xff, 0xff };
> +	unsigned char *packet = psmouse->packet;
> +
> +	return !memcmp(packet, debounce_packet, sizeof(debounce_packet));
>  }

Hmm, this chunk seems to be replacing spaces with tabs... I'd rather had
it separate from the hardware version changes.

I'll pick up the chunk below though.

>  
>  static int elantech_packet_check_v2(struct psmouse *psmouse)
> @@ -1499,7 +1499,7 @@ static const struct dmi_system_id no_hw_res_dmi_table[] = {
>  /*
>   * determine hardware version and set some properties according to it.
>   */
> -static int elantech_set_properties(struct elantech_data *etd)
> +static int elantech_set_properties(struct psmouse *psmouse, struct elantech_data *etd)
>  {
>  	/* This represents the version of IC body. */
>  	int ver = (etd->fw_version & 0x0f0000) >> 16;
> @@ -1520,9 +1520,12 @@ static int elantech_set_properties(struct elantech_data *etd)
>  		case 7:
>  		case 8:
>  		case 9:
> +		case 10:
> +		case 13:
>  			etd->hw_version = 4;
>  			break;
>  		default:
> +			psmouse_err(psmouse, "failed to setting hw version.\n");
>  			return -1;
>  		}
>  	}
> @@ -1593,7 +1596,7 @@ int elantech_init(struct psmouse *psmouse)
>  	}
>  	etd->fw_version = (param[0] << 16) | (param[1] << 8) | param[2];
>  
> -	if (elantech_set_properties(etd)) {
> +	if (elantech_set_properties(psmouse, etd)) {
>  		psmouse_err(psmouse, "unknown hardware version, aborting...\n");
>  		goto init_fail;
>  	}
> -- 
> 1.8.3.2
> 

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] [ADD] 1.support new ic type for version 4.
From: Dmitry Torokhov @ 2015-01-07  7:25 UTC (permalink / raw)
  To: Sam hung; +Cc: linux-input, dusonlin
In-Reply-To: <20150107072136.GA5256@dtor-ws>

On Tue, Jan 06, 2015 at 11:21:36PM -0800, Dmitry Torokhov wrote:
> Hi Sam,
> 
> On Tue, Jan 06, 2015 at 04:10:16PM +0800, Sam hung wrote:
> > ---
> >  drivers/input/mouse/elantech.c | 23 +++++++++++++----------
> >  1 file changed, 13 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> > index f2b9780..4891d07 100644
> > --- a/drivers/input/mouse/elantech.c
> > +++ b/drivers/input/mouse/elantech.c
> > @@ -700,14 +700,14 @@ static int elantech_packet_check_v1(struct psmouse *psmouse)
> >  
> >  static int elantech_debounce_check_v2(struct psmouse *psmouse)
> >  {
> > -        /*
> > -         * When we encounter packet that matches this exactly, it means the
> > -         * hardware is in debounce status. Just ignore the whole packet.
> > -         */
> > -        const u8 debounce_packet[] = { 0x84, 0xff, 0xff, 0x02, 0xff, 0xff };
> > -        unsigned char *packet = psmouse->packet;
> > -
> > -        return !memcmp(packet, debounce_packet, sizeof(debounce_packet));
> > +	/*
> > +	* When we encounter packet that matches this exactly, it means the
> > +	* hardware is in debounce status. Just ignore the whole packet.
> > +	*/
> > +	const u8 debounce_packet[] = { 0x84, 0xff, 0xff, 0x02, 0xff, 0xff };
> > +	unsigned char *packet = psmouse->packet;
> > +
> > +	return !memcmp(packet, debounce_packet, sizeof(debounce_packet));
> >  }
> 
> Hmm, this chunk seems to be replacing spaces with tabs... I'd rather had
> it separate from the hardware version changes.
> 
> I'll pick up the chunk below though.

Ah, I missed the fact that the patch does not have your "signed-off-by"
signature (see Documentation/SubmittingPatches). Please resend with it.

> 
> >  
> >  static int elantech_packet_check_v2(struct psmouse *psmouse)
> > @@ -1499,7 +1499,7 @@ static const struct dmi_system_id no_hw_res_dmi_table[] = {
> >  /*
> >   * determine hardware version and set some properties according to it.
> >   */
> > -static int elantech_set_properties(struct elantech_data *etd)
> > +static int elantech_set_properties(struct psmouse *psmouse, struct elantech_data *etd)
> >  {
> >  	/* This represents the version of IC body. */
> >  	int ver = (etd->fw_version & 0x0f0000) >> 16;
> > @@ -1520,9 +1520,12 @@ static int elantech_set_properties(struct elantech_data *etd)
> >  		case 7:
> >  		case 8:
> >  		case 9:
> > +		case 10:
> > +		case 13:
> >  			etd->hw_version = 4;
> >  			break;
> >  		default:
> > +			psmouse_err(psmouse, "failed to setting hw version.\n");

The caller of elantech_set_properties() already issues message about
unknown hardware version, why do we need to have it here as well?

> >  			return -1;
> >  		}
> >  	}
> > @@ -1593,7 +1596,7 @@ int elantech_init(struct psmouse *psmouse)
> >  	}
> >  	etd->fw_version = (param[0] << 16) | (param[1] << 8) | param[2];
> >  
> > -	if (elantech_set_properties(etd)) {
> > +	if (elantech_set_properties(psmouse, etd)) {
> >  		psmouse_err(psmouse, "unknown hardware version, aborting...\n");
> >  		goto init_fail;
> >  	}
> > -- 
> > 1.8.3.2
> > 

Thanks.

-- 
Dmitry

^ permalink raw reply

* [PATCH v1] input: make use of the input_set_capability helper
From: Olliver Schinagl @ 2015-01-07  7:31 UTC (permalink / raw)
  To: Dmitry Torokhov, Wolfram Sang, Paul Gortmaker, Jingoo Han,
	David S. Miller, Sam Ravnborg
  Cc: Olliver Schinagl, linux-input, linux-kernel

From: Olliver Schinagl <oliver@schinagl.nl>

Almost all of the speaker drivers under input manipulate the ev bits
directly, which is not needed, as there is a helper available.

This patch makes use of the helper for the speaker drivers.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
---
 drivers/input/misc/cm109.c         | 4 ++--
 drivers/input/misc/ixp4xx-beeper.c | 5 ++---
 drivers/input/misc/m68kspkr.c      | 5 ++---
 drivers/input/misc/pcspkr.c        | 5 ++---
 drivers/input/misc/pwm-beeper.c    | 5 +----
 drivers/input/misc/sparcspkr.c     | 6 ++----
 6 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/drivers/input/misc/cm109.c b/drivers/input/misc/cm109.c
index 9365535..8e41070 100644
--- a/drivers/input/misc/cm109.c
+++ b/drivers/input/misc/cm109.c
@@ -767,10 +767,10 @@ static int cm109_usb_probe(struct usb_interface *intf,
 	input_dev->keycodesize = sizeof(unsigned char);
 	input_dev->keycodemax = ARRAY_SIZE(dev->keymap);
 
-	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_SND);
-	input_dev->sndbit[0] = BIT_MASK(SND_BELL) | BIT_MASK(SND_TONE);
+	input_set_capability(input_dev, EV_SND, SND_BELL | SND_TONE);
 
 	/* register available key events */
+	input_dev->evbit[0] = BIT_MASK(EV_KEY);
 	for (i = 0; i < KEYMAP_SIZE; i++) {
 		unsigned short k = keymap(i);
 		dev->keymap[i] = k;
diff --git a/drivers/input/misc/ixp4xx-beeper.c b/drivers/input/misc/ixp4xx-beeper.c
index 1fe149f..befccd0 100644
--- a/drivers/input/misc/ixp4xx-beeper.c
+++ b/drivers/input/misc/ixp4xx-beeper.c
@@ -105,11 +105,10 @@ static int ixp4xx_spkr_probe(struct platform_device *dev)
 	input_dev->id.product = 0x0001;
 	input_dev->id.version = 0x0100;
 	input_dev->dev.parent = &dev->dev;
-
-	input_dev->evbit[0] = BIT_MASK(EV_SND);
-	input_dev->sndbit[0] = BIT_MASK(SND_BELL) | BIT_MASK(SND_TONE);
 	input_dev->event = ixp4xx_spkr_event;
 
+	input_set_capability(input_dev, EV_SND, SND_BELL | SND_TONE);
+
 	err = gpio_request(dev->id, "ixp4-beeper");
 	if (err)
 		goto err_free_device;
diff --git a/drivers/input/misc/m68kspkr.c b/drivers/input/misc/m68kspkr.c
index 312d636..721f1dc 100644
--- a/drivers/input/misc/m68kspkr.c
+++ b/drivers/input/misc/m68kspkr.c
@@ -64,11 +64,10 @@ static int m68kspkr_probe(struct platform_device *dev)
 	input_dev->id.product = 0x0001;
 	input_dev->id.version = 0x0100;
 	input_dev->dev.parent = &dev->dev;
-
-	input_dev->evbit[0] = BIT_MASK(EV_SND);
-	input_dev->sndbit[0] = BIT_MASK(SND_BELL) | BIT_MASK(SND_TONE);
 	input_dev->event = m68kspkr_event;
 
+	input_set_capability(input_dev, EV_SND, SND_BELL | SND_TONE);
+
 	err = input_register_device(input_dev);
 	if (err) {
 		input_free_device(input_dev);
diff --git a/drivers/input/misc/pcspkr.c b/drivers/input/misc/pcspkr.c
index 72b1fc3..5374a01 100644
--- a/drivers/input/misc/pcspkr.c
+++ b/drivers/input/misc/pcspkr.c
@@ -78,11 +78,10 @@ static int pcspkr_probe(struct platform_device *dev)
 	pcspkr_dev->id.product = 0x0001;
 	pcspkr_dev->id.version = 0x0100;
 	pcspkr_dev->dev.parent = &dev->dev;
-
-	pcspkr_dev->evbit[0] = BIT_MASK(EV_SND);
-	pcspkr_dev->sndbit[0] = BIT_MASK(SND_BELL) | BIT_MASK(SND_TONE);
 	pcspkr_dev->event = pcspkr_event;
 
+	input_set_capability(pcspr_dev, ENV_SND, SND_BELL | SND_TONE);
+
 	err = input_register_device(pcspkr_dev);
 	if (err) {
 		input_free_device(pcspkr_dev);
diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index a28ee70..e8facbd 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -102,12 +102,9 @@ static int pwm_beeper_probe(struct platform_device *pdev)
 	beeper->input->id.vendor = 0x001f;
 	beeper->input->id.product = 0x0001;
 	beeper->input->id.version = 0x0100;
-
-	beeper->input->evbit[0] = BIT(EV_SND);
-	beeper->input->sndbit[0] = BIT(SND_TONE) | BIT(SND_BELL);
-
 	beeper->input->event = pwm_beeper_event;
 
+	input_set_capability(beeper->input, EV_SND, SND_TONE | SND_BELL);
 	input_set_drvdata(beeper->input, beeper);
 
 	error = input_register_device(beeper->input);
diff --git a/drivers/input/misc/sparcspkr.c b/drivers/input/misc/sparcspkr.c
index 54116e5..7b55c97 100644
--- a/drivers/input/misc/sparcspkr.c
+++ b/drivers/input/misc/sparcspkr.c
@@ -156,12 +156,10 @@ static int sparcspkr_probe(struct device *dev)
 	input_dev->id.product = 0x0001;
 	input_dev->id.version = 0x0100;
 	input_dev->dev.parent = dev;
-
-	input_dev->evbit[0] = BIT_MASK(EV_SND);
-	input_dev->sndbit[0] = BIT_MASK(SND_BELL) | BIT_MASK(SND_TONE);
-
 	input_dev->event = state->event;
 
+	input_set_capability(input_dev, EV_SND, SND_BELL | SND_TONE);
+
 	error = input_register_device(input_dev);
 	if (error) {
 		input_free_device(input_dev);
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCH 1/1] Elantech touchpad detection fix for laptops
From: Dmitry Torokhov @ 2015-01-07  7:44 UTC (permalink / raw)
  To: Srihari Vijayaraghavan
  Cc: linux-input, Mateusz Jończyk, Zakariya Dehlawi,
	guillaum.bouchard
In-Reply-To: <CAEK4c-8CsrBaf8gaH2B2kuoMTHrLpLMgGQPrXTZAX3W5At0uoA@mail.gmail.com>

Hi Srihari,

On Mon, Jan 05, 2015 at 07:45:39PM +1100, Srihari Vijayaraghavan wrote:
> Hello Folks & Dmitry,
> 
> Further to the previous patch, here is the current version (against
> mainline) updated with the DMI info of another laptop model where this
> fix had helped a user.
> 
> Can you please review it & give some feedback? If it looks good as it
> is, then on behalf of all affected users (including yours truly), may
> I request for it to be merged upstream (and possibly stable too)?
> 
> (If the patch gets spoiled by gmail, please ref to this bugzilla URL,
> where this patch is there as a file attachment:
> https://bugzilla.kernel.org/show_bug.cgi?id=81331)

I am not terribly happy with having fiddling with keyboard from i8042
core, but I guess that's what we have to do unless someone has better
idea...

I'll drop mention of Elantech from the option though as it may well be
needed for other devices in the future.

Thanks!


> 
> Thanks

> commit c77a39a799b51fbb68b72330267c2d32a8e20542
> Author: Srihari Vijayaraghavan <linux.bug.reporting@gmail.com>
> Date:   Mon Jan 5 18:53:04 2015 +1100
> 
>     As reported in kernel bugzilla 81331, on many laptops keyboard needs to be reset
>     for the detection of Elantech touchpad. Based on the original patch by Mateusz Jończyk
>     this version has been expanded to include DMI based detection & application of the fix
>     automatically. Confirmed to fix the problem by three users already.
>     
>     Signed-off-by: Srihari Vijayaraghavan <linux.bug.reporting@gmail.com>
>     Acked-by: Mateusz Jończyk <mat.jonczyk@o2.pl>
>     Tested-by: Srihari Vijayaraghavan <linux.bug.reporting@gmail.com>
>     Tested by: Zakariya Dehlawi <zdehlawi@gmail.com>
>     Tested-by: Guillaum Bouchard <guillaum.bouchard@gmail.com>
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 4df73da..c6e8cbc 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1277,6 +1277,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  	i8042.notimeout	[HW] Ignore timeout condition signalled by controller
>  	i8042.reset	[HW] Reset the controller during init and cleanup
>  	i8042.unlock	[HW] Unlock (ignore) the keylock
> +	i8042.kbdreset  [HW] Reset keyboard to detect Elantech touchpad
>  
>  	i810=		[HW,DRM]
>  
> diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
> index c66d1b5..734360b 100644
> --- a/drivers/input/serio/i8042-x86ia64io.h
> +++ b/drivers/input/serio/i8042-x86ia64io.h
> @@ -745,6 +745,36 @@ static const struct dmi_system_id __initconst i8042_dmi_dritek_table[] = {
>  	{ }
>  };
>  
> +/*
> + * Just as per kernel bugzilla #81331, some Elantech touchpad based laptops
> + * need keyboard reset while probing for the tochpad to get detected,
> + * initialised & finally work.
> + */
> +static const struct dmi_system_id __initconst i8042_dmi_elantech_kbdreset_table[] = {
> +        {
> +                /* Gigabyte P35 v2 */
> +                .matches = {
> +                        DMI_MATCH(DMI_SYS_VENDOR, "GIGABYTE"),
> +                        DMI_MATCH(DMI_PRODUCT_NAME, "P35V2"),
> +                },
> +        },
> +	{
> +		/* Aorus branded Gigabyte X3 Plus */
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "GIGABYTE"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "X3"),
> +		},
> +	},
> +        {
> +                /* Gigabyte P34 */
> +                .matches = {
> +                        DMI_MATCH(DMI_SYS_VENDOR, "GIGABYTE"),
> +                        DMI_MATCH(DMI_PRODUCT_NAME, "P34"),
> +                },
> +        },
> +	{ }
> +};
> +
>  #endif /* CONFIG_X86 */
>  
>  #ifdef CONFIG_PNP
> @@ -1040,6 +1070,9 @@ static int __init i8042_platform_init(void)
>  	if (dmi_check_system(i8042_dmi_dritek_table))
>  		i8042_dritek = true;
>  
> +	if (dmi_check_system(i8042_dmi_elantech_kbdreset_table))
> +		i8042_kbdreset = true;
> +
>  	/*
>  	 * A20 was already enabled during early kernel init. But some buggy
>  	 * BIOSes (in MSI Laptops) require A20 to be enabled using 8042 to
> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
> index 924e4bf..92d0aa8 100644
> --- a/drivers/input/serio/i8042.c
> +++ b/drivers/input/serio/i8042.c
> @@ -67,6 +67,10 @@ static bool i8042_notimeout;
>  module_param_named(notimeout, i8042_notimeout, bool, 0);
>  MODULE_PARM_DESC(notimeout, "Ignore timeouts signalled by i8042");
>  
> +static bool i8042_kbdreset;
> +module_param_named(kbdreset, i8042_kbdreset, bool, 0);
> +MODULE_PARM_DESC(kbdreset, "Reset keyboard to detect Elantech touchpad on some laptops");
> +
>  #ifdef CONFIG_X86
>  static bool i8042_dritek;
>  module_param_named(dritek, i8042_dritek, bool, 0);
> @@ -790,6 +794,14 @@ static int __init i8042_check_aux(void)
>  		return -1;
>  
>  /*
> + * Reset keyboard to detect touchpad on some laptops having Elantech touchpad
> + */
> +	if (i8042_kbdreset) {
> +		pr_warn("Resetting keyboard to detect Elantech touchpad on some laptops\n");
> +		i8042_kbd_write(NULL, (unsigned char) 0xff);
> +	}
> +
> +/*
>   * Test AUX IRQ delivery to make sure BIOS did not grab the IRQ and
>   * used it for a PCI card or somethig else.
>   */


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

^ permalink raw reply

* Re: [PATCH 1/2] input: synaptics - make image sensors report ABS_MT_TOUCH_MAJOR
From: Dmitry Torokhov @ 2015-01-07  7:49 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Gabriele Mazzotta, Peter Hutterer, Hans de Goede, Henrik Rydberg,
	linux-input, linux-kernel@vger.kernel.org, Maxwell Anselm
In-Reply-To: <CAN+gG=FQY4V8K6k-mVrYRNSLoJDZW5bX1T=rD04MkEedQRdfUg@mail.gmail.com>

On Mon, Jan 05, 2015 at 05:04:55PM -0500, Benjamin Tissoires wrote:
> On Mon, Jan 5, 2015 at 5:00 PM, Gabriele Mazzotta
> <gabriele.mzt@gmail.com> wrote:
> > On Monday 05 January 2015 14:24:30 Benjamin Tissoires wrote:
> >> Hi Gabriele,
> >>
> >> [Adding Peter and Hans as this change will impact both
> >> xf86-input-synaptics and libinput]
> >>
> >> On Sat, Dec 27, 2014 at 6:31 AM, Gabriele Mazzotta
> >> <gabriele.mzt@gmail.com> wrote:
> >> > Despite claiming to be able to report ABS_TOOL_WIDTH, image sensors
> >> > were not doing it. Make them report widths and use ABS_MT_TOUCH_MAJOR
> >> > instead ABS_TOOL_WIDTH.
> >>
> >> It looks like the current xorg-synaptics code already handles
> >> ABS_MT_TOUCH_MAJOR as finger_width. So I think we should be good in
> >> replacing the ABS_TOOL_WIDTH event. However, I'd prefer having Peter
> >> confirm this because xorg-synaptics still relies a lot on the single
> >> touch emulation.
> >>
> >> >
> >> > Since the 'w' slot is used to report the finger count when two or more
> >> > fingers are on the touchpad, make sure that only meaningful values are
> >> > emitted, i.e. values greater than or equal to 4, and assign the correct
> >> > range to ABS_MT_TOUCH_MAJOR.
> >> >
> >> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=77161
> >> > Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> >> > ---
> >> >  drivers/input/mouse/synaptics.c | 11 +++++++++--
> >> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> >> > index f947292..ea0563e 100644
> >> > --- a/drivers/input/mouse/synaptics.c
> >> > +++ b/drivers/input/mouse/synaptics.c
> >> > @@ -814,6 +814,8 @@ static void synaptics_report_slot(struct input_dev *dev, int slot,
> >>
> >> Just FYI, this does not apply anymore on top of Dmitry's tree.
> >> synaptics_report_slot() has been removed, and you should now report
> >> the slot state in synaptics_report_mt_data().
> >>
> >> >         input_report_abs(dev, ABS_MT_POSITION_X, hw->x);
> >> >         input_report_abs(dev, ABS_MT_POSITION_Y, synaptics_invert_y(hw->y));
> >> >         input_report_abs(dev, ABS_MT_PRESSURE, hw->z);
> >> > +       if (hw->w >= 4)
> >>
> >> That I don't like. IMO, at this point, .w should only contain the
> >> finger width, unconditionally.
> >> Also, with 2/2, .w is computed accurately for the second finger, but
> >> not for the first.
> >>
> >> I tried to figure out a way to properly extract the actual width
> >> information from the sgm packet when the w is 0 or 1, and the only way
> >> I found was to do the fix in synaptics_image_sensor_process(). I would
> >> have preferred dealing with that in synaptics_parse_hw_state()
> >> directly, but I think the final code would be more and more ugly.
> >> Dealing with the true finger width in synaptics_image_sensor_process()
> >> is not a problem for cr48 sensors, because they will not have the
> >> ABS_MT_TOUCH_MAJOR event exported.
> >
> > Regarding the last part on cr48 sensors.
> > Currently these sensors are not reporting widths through ABS_TOOL_WIDTH
> > and I don't see what could go wrong if they start reporting
> > ABS_MT_TOUCH_MAJOR. If I understood correctly, they can report widths
> > only when one finger is on the touchpad. This means that they will
> > report widths through slot 0, but they won't through slot 1. Nothing
> > bad should happen.
> 
> I am not entirely sure. The entire purpose of having widht for palm
> detection is to filter palm from true finger events. So if we only
> have the width info on the first slot, it would be useless IMO.
> Still I agree with "nothing bad should happen" :)

>From conceptual perspective if device is not capable of reporting
contact size for each contact then it should not be sending
ABS_MT_TOUCH_MAJOR, same as we do not send ABS_MT_PRESSURE if we can't
provide per-contact pressure. For such devices we revert to ST-events
ABS_PRESSURE and I guess we'll have to continue with ABS_TOOL_WIDTH for
contact sizes in Synaptics.

Thanks.

-- 
Dmitry

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox