[PATCH 0/3] STM FingerTip S touchscreen support for TM2 board

[PATCH 0/3] STM FingerTip S touchscreen support for TM2 board

[Andi Shyti]

Hi,

this patchset provides support for the ST-Microelectronics
FingerTip S device driver and enables it in the tm2 and tm2e
devices.

It's tested on top of next-20170117 kernel and applies on
Krzysztof's for-next branch.

Thanks,
Andi

Andi Shyti (3):
  Input: add STMicroelectronics FingerTip touchscreen driver
  Input: add support for the STMicroelectronics FingerTip touchscreen
  arm64: dts: exynos: Add stmfts touchscreen node for TM2 and TM2E

 .../bindings/input/touchscreen/st,stmfts.txt       |  43 ++
 .../boot/dts/exynos/exynos5433-tm2-common.dtsi     |  13 +
 arch/arm64/boot/dts/exynos/exynos5433-tm2.dts      |   5 +
 arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts     |   7 +
 drivers/input/touchscreen/Kconfig                  |  12 +
 drivers/input/touchscreen/Makefile                 |   1 +
 drivers/input/touchscreen/stmfts.c                 | 759 +++++++++++++++++++++
 7 files changed, 840 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/st,stmfts.txt
 create mode 100644 drivers/input/touchscreen/stmfts.c

-- 
2.11.0

[PATCH 1/3] Input: add STMicroelectronics FingerTip touchscreen driver

[Andi Shyti]

Add binding for the STMicroelectronics FingerTip (stmfts)
touchscreen driver.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 .../bindings/input/touchscreen/st,stmfts.txt       | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/st,stmfts.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/st,stmfts.txt b/Documentation/devicetree/bindings/input/touchscreen/st,stmfts.txt
new file mode 100644
index 000000000000..788f4ba744db
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/st,stmfts.txt
@@ -0,0 +1,43 @@
+* ST-Microelectronics FingerTip touchscreen controller
+
+The ST-Microelectronics FingerTip device provides a basic touchscreen
+functionality. Along with it the user can enable the touchkey which can work as
+a basic HOME and BACK key for phones.
+
+The driver supports also hovering as an absolute single touch event with x, y, z
+coordinates.
+
+Required properties:
+- compatible		: must be "st,stmfts"
+- reg			: I2C slave address, (e.g. 0x49)
+- interrupt-parent	: the phandle to the interrupt controller which provides
+			  the interrupt
+- interrupts		: interrupt specification
+- avdd-supply		: analogic power supply
+- vdd-supply		: power supply
+- touchscreen-size-x	: see touchscreen.txt
+- touchscreen-size-y	: see touchscreen.txt
+
+Optional properties:
+- touch-key-connected	: specifies whether the touchkey feature is connected
+- ledvdd-supply		: power supply to the touch key leds
+
+Example:
+
+i2c@00000000 {
+
+	/* ... */
+
+	stmfts@49 {
+		compatible = "st,stmfts";
+		reg = <0x49>;
+		interrupt-parent = <&gpa1>;
+		interrupts = <1 IRQ_TYPE_NONE>;
+		touchscreen-size-x = <1599>;
+		touchscreen-size-y = <2559>;
+		touch-key-connected;
+		avdd-supply = <&ldo30_reg>;
+		vdd-supply = <&ldo31_reg>;
+		ledvdd-supply = <&ldo33_reg>;
+	};
+};
-- 
2.11.0

[PATCH 2/3] Input: add support for the STMicroelectronics FingerTip touchscreen

[Andi Shyti]

The stmfts (ST-Microelectronics FingerTip S) touchscreen device
is a capacitive multi-touch controller mainly for mobile use.

It's connected through i2c bus at the address 0x49 and it
interfaces with userspace through input event interface.

At the current state it provides a touchscreen multitouch
functionality up to 10 fingers. Each finger is enumerated with a
distinctive id (from 0 to 9).

If enabled the device can support single "touch" hovering, by
providing three coordinates, x, y, z. The z coordinate is
related to the distance between the finger and the touchscreen
itself.

It is possible to select the touchkey functionality which
provides a basic two keys interface for "home" and "back" menu,
typical in mobile phones.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/input/touchscreen/Kconfig  |  12 +
 drivers/input/touchscreen/Makefile |   1 +
 drivers/input/touchscreen/stmfts.c | 759 +++++++++++++++++++++++++++++++++++++
 3 files changed, 772 insertions(+)
 create mode 100644 drivers/input/touchscreen/stmfts.c

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 8650c94e29d0..18c2f919eb79 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -1092,6 +1092,18 @@ config TOUCHSCREEN_ST1232
 	  To compile this driver as a module, choose M here: the
 	  module will be called st1232_ts.
 
+config TOUCHSCREEN_STMFTS
+	tristate "STMicroelectronics STMFTS touchscreen"
+	depends on I2C
+	depends on INPUT
+	depends on LEDS_CLASS
+	help
+	  Say Y here if you want support for STMicroelectronics
+	  STMFTS touchscreen.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called stmfts.
+
 config TOUCHSCREEN_STMPE
 	tristate "STMicroelectronics STMPE touchscreens"
 	depends on MFD_STMPE
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index e41e3c7aa427..be7673f9c71a 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -66,6 +66,7 @@ obj-$(CONFIG_TOUCHSCREEN_S3C2410)	+= s3c2410_ts.o
 obj-$(CONFIG_TOUCHSCREEN_SILEAD)	+= silead.o
 obj-$(CONFIG_TOUCHSCREEN_SIS_I2C)	+= sis_i2c.o
 obj-$(CONFIG_TOUCHSCREEN_ST1232)	+= st1232.o
+obj-$(CONFIG_TOUCHSCREEN_STMFTS)	+= stmfts.o
 obj-$(CONFIG_TOUCHSCREEN_STMPE)		+= stmpe-ts.o
 obj-$(CONFIG_TOUCHSCREEN_SUN4I)		+= sun4i-ts.o
 obj-$(CONFIG_TOUCHSCREEN_SUR40)		+= sur40.o
diff --git a/drivers/input/touchscreen/stmfts.c b/drivers/input/touchscreen/stmfts.c
new file mode 100644
index 000000000000..abccc14d06d2
--- /dev/null
+++ b/drivers/input/touchscreen/stmfts.c
@@ -0,0 +1,759 @@
+/*
+ * Copyright (c) 2017 Samsung Electronics Co., Ltd.
+ * Author: Andi Shyti <andi.shyti@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * STMicroelectronics FTS Touchscreen device driver
+ */
+
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/interrupt.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+
+/* I2C commands */
+#define STMFTS_READ_INFO			0x80
+#define STMFTS_READ_STATUS			0x84
+#define STMFTS_READ_ONE_EVENT			0x85
+#define STMFTS_SLEEP_IN				0x91
+#define STMFTS_SLEEP_OUT			0x91
+#define STMFTS_MS_MT_SENSE_OFF			0x92
+#define STMFTS_MS_MT_SENSE_ON			0x93
+#define STMFTS_SS_HOVER_SENSE_OFF		0x94
+#define STMFTS_SS_HOVER_SENSE_ON		0x95
+#define STMFTS_MS_KEY_SENSE_OFF			0x9a
+#define STMFTS_MS_KEY_SENSE_ON			0x9b
+#define STMFTS_SYSTEM_RESET			0xa0
+#define STMFTS_CLEAR_EVENT_STACK		0xa1
+#define STMFTS_FULL_FORCE_CALIBRATION		0xa2
+#define STMFTS_MS_CX_TUNING			0xa3
+#define STMFTS_SS_CX_TUNING			0xa4
+
+/* events */
+#define STMFTS_EV_NO_EVENT			0x00
+#define STMFTS_EV_MULTI_TOUCH_DETECTED		0x02
+#define STMFTS_EV_MULTI_TOUCH_ENTER		0x03
+#define STMFTS_EV_MULTI_TOUCH_LEAVE		0x04
+#define STMFTS_EV_MULTI_TOUCH_MOTION		0x05
+#define STMFTS_EV_HOVER_ENTER			0x07
+#define STMFTS_EV_HOVER_LEAVE			0x08
+#define STMFTS_EV_HOVER_MOTION			0x09
+#define STMFTS_EV_KEY_STATUS			0x0e
+#define STMFTS_EV_ERROR				0x0f
+#define STMFTS_EV_CONTROLLER_READY		0x10
+#define STMFTS_EV_SLEEP_OUT_CONTROLLER_READY	0x11
+#define STMFTS_EV_STATUS			0x16
+
+/* multi touch related event masks */
+#define STMFTS_MASK_EVENT_ID			0x0f
+#define STMFTS_MASK_TOUCH_ID			0xf0
+#define STMFTS_MASK_LEFT_EVENT			0x0f
+#define STMFTS_MASK_X_MSB			0x0f
+#define STMFTS_MASK_Y_LSB			0xf0
+
+/* key related event masks */
+#define STMFTS_MASK_KEY_NO_TOUCH		0x00
+#define STMFTS_MASK_KEY_BACK			0x01
+#define STMFTS_MASK_KEY_MENU			0x02
+
+#define STMFTS_EVENT_SIZE	8
+#define STMFTS_MAX_FINGERS	10
+#define STMFTS_DEV_NAME		"stmfts"
+
+enum stmfts_regulators {
+	STMFTS_REGULATOR_VDD,
+	STMFTS_REGULATOR_AVDD,
+};
+
+struct stmfts_data {
+	struct i2c_client *client;
+	struct input_dev *input_touch;
+	struct input_dev *input_key;
+	struct led_classdev led_cdev;
+	struct mutex mutex;
+	u32 x_size;
+	u32 y_size;
+
+	struct regulator_bulk_data regulators[2];
+
+	/* ledvdd will be used also to check
+	 * whether the LED is supported
+	 */
+	struct regulator *ledvdd;
+
+	bool use_key;
+	bool signal;
+	bool led_status;
+	u8 users;
+
+	u16 chip_id;
+	u8 chip_ver;
+	u16 fw_ver;
+	u8 config_id;
+	u8 config_ver;
+	u8 in_touch;
+
+	wait_queue_head_t wq;
+
+	bool hover_enabled;
+};
+
+static void stmfts_brightness_set(struct led_classdev *led_cdev,
+					enum led_brightness value)
+{
+	struct stmfts_data *sdata = container_of(led_cdev,
+					struct stmfts_data, led_cdev);
+
+	if (value == sdata->led_status)
+		return;
+
+	if (!value) {
+		regulator_disable(sdata->ledvdd);
+	} else {
+		int err = regulator_enable(sdata->ledvdd);
+
+		if (err)
+			dev_warn(&sdata->client->dev,
+				"failed to disable ledvdd regulator\n");
+	}
+
+	sdata->led_status = value;
+}
+
+static enum led_brightness stmfts_brightness_get(struct led_classdev *led_cdev)
+{
+	struct stmfts_data *sdata = container_of(led_cdev,
+						struct stmfts_data, led_cdev);
+
+	return !!regulator_is_enabled(sdata->ledvdd);
+}
+
+static void stmfts_parse_event(struct stmfts_data *sdata, u8 event[])
+{
+	int ret;
+	u8 id, t_id = 0;
+	u16 x, y, z, maj, min, orientation, area;
+
+	id = event[0];
+
+	do {
+		mutex_lock(&sdata->mutex);
+		if (sdata->in_touch) {
+			id = event[0] & STMFTS_MASK_EVENT_ID;
+			t_id = (event[0] & STMFTS_MASK_TOUCH_ID) >> 4;
+		} else {
+			id = event[0];
+			t_id = 0;
+		}
+
+		switch (id) {
+		case STMFTS_EV_NO_EVENT:
+			break;
+
+		case STMFTS_EV_MULTI_TOUCH_ENTER:
+		case STMFTS_EV_MULTI_TOUCH_LEAVE:
+		case STMFTS_EV_MULTI_TOUCH_MOTION:
+			if (id == STMFTS_EV_MULTI_TOUCH_ENTER) {
+				if (!(sdata->in_touch++))
+					input_mt_report_slot_state(
+							sdata->input_touch,
+							MT_TOOL_FINGER, true);
+			} else if (id == STMFTS_EV_MULTI_TOUCH_LEAVE) {
+				if (!(--sdata->in_touch))
+					input_mt_report_slot_state(
+							sdata->input_touch,
+							MT_TOOL_FINGER, false);
+			}
+
+			x = event[1] | ((event[2] & STMFTS_MASK_X_MSB) << 8);
+			y = (event[2] >> 4) | (event[3] << 4);
+
+			maj = event[4];
+			min = event[5];
+			orientation = event[6];
+			area = event[7];
+
+			input_mt_slot(sdata->input_touch, t_id);
+			input_report_abs(sdata->input_touch,
+					ABS_MT_POSITION_X, x);
+			input_report_abs(sdata->input_touch,
+					ABS_MT_POSITION_Y, y);
+			input_report_abs(sdata->input_touch,
+					ABS_MT_TOUCH_MAJOR, maj);
+			input_report_abs(sdata->input_touch,
+					ABS_MT_TOUCH_MINOR, min);
+			input_report_abs(sdata->input_touch,
+					ABS_MT_PRESSURE, area);
+			input_report_abs(sdata->input_touch,
+					ABS_MT_ORIENTATION, orientation);
+			input_sync(sdata->input_touch);
+
+			break;
+
+		case STMFTS_EV_HOVER_ENTER:
+		case STMFTS_EV_HOVER_LEAVE:
+		case STMFTS_EV_HOVER_MOTION:
+			x = (event[2] << 4) | (event[4] >> 4);
+			y = (event[3] << 4) | (event[4] & STMFTS_MASK_Y_LSB);
+			z = event[5];
+			orientation = event[6] & STMFTS_MASK_Y_LSB;
+
+			input_report_abs(sdata->input_touch, ABS_X, x);
+			input_report_abs(sdata->input_touch, ABS_Y, y);
+			input_report_abs(sdata->input_touch, ABS_Z, z);
+			input_sync(sdata->input_touch);
+
+			break;
+
+		case STMFTS_EV_KEY_STATUS:
+			if (!event[2]) {
+				input_report_key(sdata->input_key, KEY_BACK, 0);
+				input_report_key(sdata->input_key, KEY_MENU, 0);
+			} else {
+				if (event[2] == STMFTS_MASK_KEY_BACK)
+					input_report_key(sdata->input_key,
+								KEY_BACK, 1);
+				else if (event[2] == STMFTS_MASK_KEY_MENU)
+					input_report_key(sdata->input_key,
+								KEY_MENU, 1);
+				else /* quite impossible */
+					break;
+			}
+			input_sync(sdata->input_key);
+			break;
+
+		case STMFTS_EV_STATUS:
+			sdata->signal = true;
+			wake_up_interruptible(&sdata->wq);
+			break;
+
+		case STMFTS_EV_ERROR:
+			dev_err(&sdata->client->dev,
+				"error code: 0x%x%x%x%x%x%x",
+				event[6], event[5], event[4],
+				event[3], event[2], event[1]);
+			break;
+
+		default:
+			dev_err(&sdata->client->dev,
+				"unknown event 0x%x\n", event[0]);
+		}
+		mutex_unlock(&sdata->mutex);
+
+		ret = i2c_smbus_read_i2c_block_data(sdata->client,
+						STMFTS_READ_ONE_EVENT,
+						STMFTS_EVENT_SIZE, event);
+		if (ret) {
+			i2c_smbus_write_byte(sdata->client,
+						STMFTS_CLEAR_EVENT_STACK);
+			break;
+		}
+
+	} while (event[0]);
+}
+
+static irqreturn_t stmfts_irq_handler(int irq, void *dev)
+{
+	struct stmfts_data *sdata = dev;
+	int ret;
+	u8 event[STMFTS_EVENT_SIZE];
+
+	ret = i2c_smbus_read_i2c_block_data(sdata->client,
+					STMFTS_READ_ONE_EVENT,
+					STMFTS_EVENT_SIZE, event);
+
+	switch (event[0]) {
+	case STMFTS_EV_CONTROLLER_READY:
+	case STMFTS_EV_SLEEP_OUT_CONTROLLER_READY:
+		sdata->signal = true;
+		wake_up_interruptible(&sdata->wq);
+		break;
+	default:
+		stmfts_parse_event(sdata, event);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int stmfts_write_and_wait(struct stmfts_data *sdata, const u8 cmd)
+{
+	int err;
+
+	err = i2c_smbus_write_byte(sdata->client, cmd);
+	if (err)
+		return err;
+
+	sdata->signal = false;
+	err = wait_event_interruptible_timeout(sdata->wq, sdata->signal,
+						msecs_to_jiffies(1000));
+
+	return !err ? -ETIMEDOUT : 0;
+}
+
+static void stmfts_sleep_in(struct stmfts_data *sdata)
+{
+	mutex_lock(&sdata->mutex);
+
+	sdata->users--;
+	if (!sdata->users)
+		i2c_smbus_write_byte(sdata->client, STMFTS_SLEEP_IN);
+
+	mutex_unlock(&sdata->mutex);
+}
+
+static int stmfts_sleep_out(struct stmfts_data *sdata, u8 cmd)
+{
+	int ret = 0;
+
+	mutex_lock(&sdata->mutex);
+
+	if (!sdata->users)
+		ret = stmfts_write_and_wait(sdata, STMFTS_SLEEP_OUT);
+
+	/* if sleep out succeeds users increments, otherwise not */
+	sdata->users += !ret;
+
+	mutex_unlock(&sdata->mutex);
+
+	ret = i2c_smbus_write_byte(sdata->client, cmd);
+	if (ret)
+		stmfts_sleep_in(sdata);
+
+	return ret;
+}
+
+static int stmfts_input_touch_open(struct input_dev *dev)
+{
+	struct stmfts_data *sdata = input_get_drvdata(dev);
+
+	return stmfts_sleep_out(sdata, STMFTS_MS_MT_SENSE_ON);
+}
+
+static void stmfts_input_touch_close(struct input_dev *dev)
+{
+	struct stmfts_data *sdata = input_get_drvdata(dev);
+
+	i2c_smbus_write_byte(sdata->client, STMFTS_MS_MT_SENSE_OFF);
+
+	stmfts_sleep_in(sdata);
+}
+
+static int stmfts_input_key_open(struct input_dev *dev)
+{
+	struct stmfts_data *sdata = input_get_drvdata(dev);
+
+	return stmfts_sleep_out(sdata, STMFTS_MS_KEY_SENSE_ON);
+}
+
+static void stmfts_input_key_close(struct input_dev *dev)
+{
+	struct stmfts_data *sdata = input_get_drvdata(dev);
+
+	i2c_smbus_write_byte(sdata->client, STMFTS_MS_KEY_SENSE_OFF);
+	stmfts_sleep_in(sdata);
+}
+
+static ssize_t stmfts_sysfs_hwid(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct stmfts_data *sdata = dev_get_drvdata(dev);
+
+	return sprintf(buf, "ST-Microelectronics FTS 0x%x version %u\n",
+					sdata->chip_id, sdata->chip_ver);
+}
+
+static ssize_t stmfts_sysfs_fw_ver(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct stmfts_data *sdata = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%u\n", sdata->fw_ver);
+}
+
+static ssize_t stmfts_sysfs_config_id(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct stmfts_data *sdata = dev_get_drvdata(dev);
+
+	return sprintf(buf, "0x%x version %u\n",
+			sdata->config_id, sdata->config_ver);
+}
+
+static ssize_t stmfts_sysfs_read_status(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct stmfts_data *sdata = dev_get_drvdata(dev);
+	u8 status[4];
+	int ret;
+
+	ret = i2c_smbus_read_i2c_block_data(sdata->client,
+					STMFTS_READ_STATUS, 4, status);
+
+	return sprintf(buf, "0x%x\n", status[0]);
+}
+
+static ssize_t stmfts_sysfs_hover_enable_read(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct stmfts_data *sdata = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%u\n", sdata->hover_enabled);
+}
+
+static ssize_t stmfts_sysfs_hover_enable_write(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t len)
+{
+	unsigned long value;
+	int err;
+	struct stmfts_data *sdata = dev_get_drvdata(dev);
+
+	if (kstrtoul(buf, 0, &value))
+		return -EINVAL;
+
+	if (value & sdata->hover_enabled)
+		return len;
+
+	if (value) {
+		err = stmfts_sleep_out(sdata, STMFTS_SS_HOVER_SENSE_ON);
+		sdata->hover_enabled = !err;
+	} else {
+		err = i2c_smbus_write_byte(sdata->client,
+					STMFTS_SS_HOVER_SENSE_OFF);
+		stmfts_sleep_in(sdata);
+		sdata->hover_enabled = !!err;
+	}
+
+	return len;
+}
+
+static DEVICE_ATTR(hwid, 0444, stmfts_sysfs_hwid, NULL);
+static DEVICE_ATTR(fw_ver, 0444, stmfts_sysfs_fw_ver, NULL);
+static DEVICE_ATTR(config_id, 0444, stmfts_sysfs_config_id, NULL);
+static DEVICE_ATTR(status, 0444, stmfts_sysfs_read_status, NULL);
+static DEVICE_ATTR(hover_enable, 0644, stmfts_sysfs_hover_enable_read,
+					stmfts_sysfs_hover_enable_write);
+
+static struct attribute *stmfts_sysfs_attrs[] = {
+	&dev_attr_hwid.attr,
+	&dev_attr_fw_ver.attr,
+	&dev_attr_config_id.attr,
+	&dev_attr_status.attr,
+	&dev_attr_hover_enable.attr,
+	NULL
+};
+
+static struct attribute_group stmfts_attribute_group = {
+	.attrs = stmfts_sysfs_attrs
+};
+
+static int stmfts_parse_dt(struct stmfts_data *sdata)
+{
+	int ret;
+	struct device_node *np = sdata->client->dev.of_node;
+
+	if (!np)
+		return -ENOENT;
+
+	ret = of_property_read_u32(np, "touchscreen-size-x", &sdata->x_size);
+	if (ret)
+		return ret;
+
+	ret = of_property_read_u32(np, "touchscreen-size-y", &sdata->y_size);
+	if (ret)
+		return ret;
+
+	sdata->use_key = of_property_read_bool(np, "touch-key-connected");
+
+	return 0;
+}
+
+static int stmfts_power_on(struct stmfts_data *sdata)
+{
+	int err;
+	u8 reg[8];
+
+	err = regulator_bulk_enable(ARRAY_SIZE(sdata->regulators),
+							sdata->regulators);
+	if (err)
+		return err;
+
+	/*
+	 * the datasheet does not specify the power on time, but considering
+	 * that the reset time is < 10ms, I sleep 20ms to be sure
+	 */
+	msleep(20);
+
+	err = i2c_smbus_read_i2c_block_data(sdata->client,
+					STMFTS_READ_INFO, 8, reg);
+	if (err < 0)
+		return err;
+	if (err != 8)
+		return -EIO;
+
+	sdata->chip_id = (reg[6] << 8) | reg[7];
+	sdata->chip_ver = reg[0];
+	sdata->fw_ver = (reg[2] << 8) | reg[3];
+	sdata->config_id = reg[4];
+	sdata->config_ver = reg[5];
+
+	enable_irq(sdata->client->irq);
+	err = stmfts_write_and_wait(sdata, STMFTS_SYSTEM_RESET);
+	if (err)
+		return err;
+
+	err = stmfts_write_and_wait(sdata, STMFTS_SLEEP_OUT);
+	if (err)
+		return err;
+
+	/* optional tuning */
+	err = stmfts_write_and_wait(sdata, STMFTS_MS_CX_TUNING);
+	if (err)
+		dev_warn(&sdata->client->dev, "failed to perform mutual auto tune\n");
+
+	/* optional tuning */
+	err = stmfts_write_and_wait(sdata, STMFTS_SS_CX_TUNING);
+	if (err)
+		dev_warn(&sdata->client->dev, "failed to perform self auto tune\n");
+
+	err = stmfts_write_and_wait(sdata, STMFTS_FULL_FORCE_CALIBRATION);
+	if (err)
+		return err;
+
+	/* at this point no one is using the touchscreen
+	 * and I don't really care about the return value
+	 */
+	i2c_smbus_write_byte(sdata->client, STMFTS_SLEEP_IN);
+
+	return 0;
+}
+
+static void stmfts_power_off(void *data)
+{
+	struct stmfts_data *sdata = data;
+
+	disable_irq(sdata->client->irq);
+	regulator_bulk_disable(ARRAY_SIZE(sdata->regulators),
+						sdata->regulators);
+}
+
+static int stmfts_enable_key(struct stmfts_data *sdata)
+{
+	int err;
+
+	sdata->input_key = devm_input_allocate_device(&sdata->client->dev);
+	if (!sdata->input_key)
+		return -ENOMEM;
+
+	sdata->input_key->name = "stmfts_key";
+	sdata->input_key->id.bustype = BUS_I2C;
+	sdata->input_key->open = stmfts_input_key_open;
+	sdata->input_key->close = stmfts_input_key_close;
+
+	input_set_capability(sdata->input_key, EV_KEY, KEY_MENU);
+	input_set_capability(sdata->input_key, EV_KEY, KEY_BACK);
+
+	input_set_drvdata(sdata->input_key, sdata);
+	err = input_register_device(sdata->input_key);
+	if (err)
+		return err;
+
+	/* get the regulator for powering the leds on */
+	sdata->ledvdd = devm_regulator_get(&sdata->client->dev, "ledvdd");
+	if (IS_ERR(sdata->ledvdd))
+		/* there is no LED connected to the touch key */
+		sdata->ledvdd = NULL;
+
+	sdata->led_cdev.name = STMFTS_DEV_NAME;
+	sdata->led_cdev.max_brightness = LED_ON;
+	sdata->led_cdev.brightness = LED_OFF;
+	sdata->led_cdev.brightness_set = stmfts_brightness_set;
+	sdata->led_cdev.brightness_get = stmfts_brightness_get;
+
+	err = devm_led_classdev_register(&sdata->client->dev, &sdata->led_cdev);
+	if (err) {
+		dev_warn(&sdata->client->dev, "unable to register led, led might not work\n");
+		sdata->ledvdd = NULL;
+
+		/* I don't want to prevent using the touch key
+		 * only because the LEDs don't get registered
+		 */
+		err = 0;
+	}
+
+	return err;
+}
+
+static int stmfts_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	int err;
+	struct stmfts_data *sdata;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
+						I2C_FUNC_SMBUS_BYTE_DATA |
+						I2C_FUNC_SMBUS_I2C_BLOCK))
+		return -ENODEV;
+
+	sdata = devm_kzalloc(&client->dev, sizeof(*sdata), GFP_KERNEL);
+	if (!sdata)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, sdata);
+
+	mutex_init(&sdata->mutex);
+
+	sdata->regulators[STMFTS_REGULATOR_VDD].supply = "vdd";
+	sdata->regulators[STMFTS_REGULATOR_AVDD].supply = "avdd";
+	err = devm_regulator_bulk_get(&client->dev,
+			ARRAY_SIZE(sdata->regulators), sdata->regulators);
+	if (err)
+		return err;
+
+	err = devm_add_action_or_reset(&client->dev, stmfts_power_off, sdata);
+	if (err)
+		return err;
+
+	sdata->client = client;
+
+	err = stmfts_parse_dt(sdata);
+	if (err)
+		return err;
+
+	init_waitqueue_head(&sdata->wq);
+
+	err = sysfs_create_group(&sdata->client->dev.kobj,
+					&stmfts_attribute_group);
+	if (err)
+		return err;
+
+	err = devm_request_threaded_irq(&client->dev, client->irq,
+					NULL, stmfts_irq_handler,
+					IRQF_ONESHOT | IRQF_TRIGGER_LOW,
+					"stmfts_irq", sdata);
+	if (err)
+		return err;
+
+	disable_irq(sdata->client->irq);
+
+	dev_info(&client->dev, "initializing ST-Microelectronics FTS...\n");
+	err = stmfts_power_on(sdata);
+	if (err)
+		return err;
+
+	sdata->input_touch = devm_input_allocate_device(&client->dev);
+	if (!sdata->input_touch)
+		return -ENOMEM;
+
+	sdata->input_touch->name = STMFTS_DEV_NAME;
+	sdata->input_touch->id.bustype = BUS_I2C;
+	sdata->input_touch->open = stmfts_input_touch_open;
+	sdata->input_touch->close = stmfts_input_touch_close;
+
+	input_set_capability(sdata->input_touch, EV_ABS, ABS_MT_POSITION_Y);
+	input_set_capability(sdata->input_touch, EV_ABS, ABS_MT_POSITION_X);
+	input_set_capability(sdata->input_touch, EV_ABS, ABS_MT_TOUCH_MAJOR);
+	input_set_capability(sdata->input_touch, EV_ABS, ABS_MT_TOUCH_MINOR);
+	input_set_capability(sdata->input_touch, EV_ABS, ABS_MT_ORIENTATION);
+	input_set_capability(sdata->input_touch, EV_ABS, ABS_MT_PRESSURE);
+	input_set_capability(sdata->input_touch, EV_ABS, ABS_MT_POSITION_X);
+
+	input_set_abs_params(sdata->input_touch,
+				ABS_MT_POSITION_X, 0, sdata->x_size, 0, 0);
+	input_set_abs_params(sdata->input_touch,
+				ABS_MT_POSITION_Y, 0, sdata->y_size, 0, 0);
+	input_set_abs_params(sdata->input_touch,
+				ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
+	input_set_abs_params(sdata->input_touch,
+				ABS_MT_TOUCH_MINOR, 0, 255, 0, 0);
+	input_set_abs_params(sdata->input_touch,
+				ABS_MT_ORIENTATION, 0, 255, 0, 0);
+	input_set_abs_params(sdata->input_touch,
+				ABS_MT_PRESSURE, 0, 255, 0, 0);
+	input_mt_init_slots(sdata->input_touch,
+				STMFTS_MAX_FINGERS, INPUT_MT_DIRECT);
+
+	/* for hover features */
+	input_set_capability(sdata->input_touch, EV_ABS, ABS_X);
+	input_set_capability(sdata->input_touch, EV_ABS, ABS_Y);
+	input_set_capability(sdata->input_touch, EV_ABS, ABS_Z);
+	input_set_abs_params(sdata->input_touch, ABS_X, 0, sdata->x_size, 0, 0);
+	input_set_abs_params(sdata->input_touch, ABS_Y, 0, sdata->y_size, 0, 0);
+	input_set_abs_params(sdata->input_touch, ABS_Z, 0, 255, 0, 0);
+
+	input_set_drvdata(sdata->input_touch, sdata);
+	err = input_register_device(sdata->input_touch);
+	if (err)
+		return err;
+
+	if (sdata->use_key) {
+		err = stmfts_enable_key(sdata);
+		if (err)
+			dev_warn(&client->dev, "failed to enable touchkey\n");
+	}
+
+	return 0;
+}
+
+static int stmfts_remove(struct i2c_client *client)
+{
+	struct stmfts_data *sdata = i2c_get_clientdata(client);
+
+	sysfs_remove_group(&sdata->client->dev.kobj, &stmfts_attribute_group);
+
+	return 0;
+}
+
+static int __maybe_unused stmfts_suspend(struct device *dev)
+{
+	struct stmfts_data *sdata = dev_get_drvdata(dev);
+
+	stmfts_power_off(sdata);
+
+	return 0;
+}
+
+static int __maybe_unused stmfts_resume(struct device *dev)
+{
+	struct stmfts_data *sdata = dev_get_drvdata(dev);
+
+	return stmfts_power_on(sdata);
+}
+
+static SIMPLE_DEV_PM_OPS(stmfts_pm_ops, stmfts_suspend, stmfts_resume);
+
+static const struct of_device_id stmfts_of_match[] = {
+	{ .compatible = "st,stmfts", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, stmfts_of_match);
+
+static const struct i2c_device_id stmfts_id[] = {
+	{ "stmfts", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, stmfts_id);
+
+static struct i2c_driver stmfts_driver = {
+	.driver = {
+		.name = STMFTS_DEV_NAME,
+		.of_match_table = of_match_ptr(stmfts_of_match),
+		.pm = &stmfts_pm_ops,
+	},
+	.probe = stmfts_probe,
+	.remove = stmfts_remove,
+	.id_table = stmfts_id,
+};
+
+module_i2c_driver(stmfts_driver);
+
+MODULE_AUTHOR("Andi Shyti <andi.shyti@samsung.com>");
+MODULE_DESCRIPTION("STMicroelectronics FTS Touch Screen");
+MODULE_LICENSE("GPL v2");
-- 
2.11.0

[PATCH 3/3] arm64: dts: exynos: Add stmfts touchscreen node for TM2 and TM2E

[Andi Shyti]

TM2 and TM2E devices are provided with a ST-Microelectronics
Finger Tip S device with small differences:

 - screen size
 - TM2E uses the stmfts also as a touchkey for "back" and "menu"

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 13 +++++++++++++
 arch/arm64/boot/dts/exynos/exynos5433-tm2.dts         |  5 +++++
 arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts        |  7 +++++++
 3 files changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
index a5c866901e93..689d5c124a49 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
@@ -745,6 +745,19 @@
 	};
 };
 
+&hsi2c_5 {
+	status = "okay";
+
+	touchscreen: stmfts@49 {
+		compatible = "st,stmfts";
+		reg = <0x49>;
+		interrupt-parent = <&gpa1>;
+		interrupts = <1 IRQ_TYPE_NONE>;
+		avdd-supply = <&ldo30_reg>;
+		vdd-supply = <&ldo31_reg>;
+	};
+};
+
 &hsi2c_7 {
 	status = "okay";
 
diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
index 2449266b268f..d0311d34c6bd 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
@@ -29,3 +29,8 @@
 	regulator-min-microvolt = <3000000>;
 	regulator-max-microvolt = <3000000>;
 };
+
+&touchscreen {
+	touchscreen-size-x = <1439>;
+	touchscreen-size-y = <2559>;
+};
diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts
index 2fbf3a860316..388880dcb5ae 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts
@@ -29,3 +29,10 @@
 	regulator-min-microvolt = <3300000>;
 	regulator-max-microvolt = <3300000>;
 };
+
+&touchscreen {
+	touchscreen-size-x = <1599>;
+	touchscreen-size-y = <2559>;
+	touch-key-connected;
+	ledvdd-supply = <&ldo33_reg>;
+};
-- 
2.11.0

Re: [PATCH 1/3] Input: add STMicroelectronics FingerTip touchscreen driver

[Javier Martinez Canillas]

Hello Andi,

On 01/17/2017 10:54 AM, Andi Shyti wrote:
> Add binding for the STMicroelectronics FingerTip (stmfts)
> touchscreen driver.
> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---

The binding looks good to me, I've just a minor nit below.

[snip]

> +
> +i2c@00000000 {
> +
> +	/* ... */
> +
> +	stmfts@49 {

The ePAPR document says:

The name of a node should be somewhat generic, reflecting the function
of the device and not its precise programming model.

So I think this should be touchscreen@49 instead.

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 3/3] arm64: dts: exynos: Add stmfts touchscreen node for TM2 and TM2E

[Javier Martinez Canillas]

Hello Andi,

On 01/17/2017 10:54 AM, Andi Shyti wrote:
> TM2 and TM2E devices are provided with a ST-Microelectronics
> Finger Tip S device with small differences:
> 
>  - screen size
>  - TM2E uses the stmfts also as a touchkey for "back" and "menu"
> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 13 +++++++++++++
>  arch/arm64/boot/dts/exynos/exynos5433-tm2.dts         |  5 +++++
>  arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts        |  7 +++++++
>  3 files changed, 25 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> index a5c866901e93..689d5c124a49 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> @@ -745,6 +745,19 @@
>  	};
>  };
>  
> +&hsi2c_5 {
> +	status = "okay";
> +
> +	touchscreen: stmfts@49 {

Same comment than in patch 1/3, I think "stmfts: touchscreen@49" is better.

Rest looks good to me:

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 3/3] arm64: dts: exynos: Add stmfts touchscreen node for TM2 and TM2E

[Krzysztof Kozlowski]

On Tue, Jan 17, 2017 at 10:54:41PM +0900, Andi Shyti wrote:
> TM2 and TM2E devices are provided with a ST-Microelectronics
> Finger Tip S device with small differences:
> 
>  - screen size
>  - TM2E uses the stmfts also as a touchkey for "back" and "menu"
> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 13 +++++++++++++
>  arch/arm64/boot/dts/exynos/exynos5433-tm2.dts         |  5 +++++
>  arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts        |  7 +++++++
>  3 files changed, 25 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> index a5c866901e93..689d5c124a49 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> @@ -745,6 +745,19 @@
>  	};
>  };
>  
> +&hsi2c_5 {
> +	status = "okay";
> +
> +	touchscreen: stmfts@49 {
> +		compatible = "st,stmfts";
> +		reg = <0x49>;
> +		interrupt-parent = <&gpa1>;
> +		interrupts = <1 IRQ_TYPE_NONE>;

Looks fine to me, just wondering:
1. Are you sure this is TYPE_NONE interrupt? Sometimes we put it
   everywhere instead of checking real value.
2. Don't you have to configure the gpa1-1 to EINT? Any pull up/down?

Best regards,
Krzysztof

> +		avdd-supply = <&ldo30_reg>;
> +		vdd-supply = <&ldo31_reg>;
> +	};
> +};
> +
>  &hsi2c_7 {
>  	status = "okay";
>  
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> index 2449266b268f..d0311d34c6bd 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> @@ -29,3 +29,8 @@
>  	regulator-min-microvolt = <3000000>;
>  	regulator-max-microvolt = <3000000>;
>  };
> +
> +&touchscreen {
> +	touchscreen-size-x = <1439>;
> +	touchscreen-size-y = <2559>;
> +};
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts
> index 2fbf3a860316..388880dcb5ae 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts
> @@ -29,3 +29,10 @@
>  	regulator-min-microvolt = <3300000>;
>  	regulator-max-microvolt = <3300000>;
>  };
> +
> +&touchscreen {
> +	touchscreen-size-x = <1599>;
> +	touchscreen-size-y = <2559>;
> +	touch-key-connected;
> +	ledvdd-supply = <&ldo33_reg>;
> +};
> -- 
> 2.11.0
> 

Re: [PATCH 1/3] Input: add STMicroelectronics FingerTip touchscreen driver

[Krzysztof Kozlowski]

On Tue, Jan 17, 2017 at 10:54:39PM +0900, Andi Shyti wrote:
> Add binding for the STMicroelectronics FingerTip (stmfts)
> touchscreen driver.
> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  .../bindings/input/touchscreen/st,stmfts.txt       | 43 ++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/st,stmfts.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/st,stmfts.txt b/Documentation/devicetree/bindings/input/touchscreen/st,stmfts.txt
> new file mode 100644
> index 000000000000..788f4ba744db
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/st,stmfts.txt
> @@ -0,0 +1,43 @@
> +* ST-Microelectronics FingerTip touchscreen controller
> +
> +The ST-Microelectronics FingerTip device provides a basic touchscreen
> +functionality. Along with it the user can enable the touchkey which can work as
> +a basic HOME and BACK key for phones.
> +
> +The driver supports also hovering as an absolute single touch event with x, y, z
> +coordinates.
> +
> +Required properties:
> +- compatible		: must be "st,stmfts"
> +- reg			: I2C slave address, (e.g. 0x49)
> +- interrupt-parent	: the phandle to the interrupt controller which provides
> +			  the interrupt
> +- interrupts		: interrupt specification
> +- avdd-supply		: analogic power supply
> +- vdd-supply		: power supply
> +- touchscreen-size-x	: see touchscreen.txt
> +- touchscreen-size-y	: see touchscreen.txt
> +
> +Optional properties:
> +- touch-key-connected	: specifies whether the touchkey feature is connected

You are making it a generic property but it is specific to this device,
so:
	st,touch-key-connected
?

> +- ledvdd-supply		: power supply to the touch key leds

Is this really optional? If yes... how it gets the power when not
provided?

Best regards,
Krzysztof

> +
> +Example:
> +
> +i2c@00000000 {
> +
> +	/* ... */
> +
> +	stmfts@49 {
> +		compatible = "st,stmfts";
> +		reg = <0x49>;
> +		interrupt-parent = <&gpa1>;
> +		interrupts = <1 IRQ_TYPE_NONE>;
> +		touchscreen-size-x = <1599>;
> +		touchscreen-size-y = <2559>;
> +		touch-key-connected;
> +		avdd-supply = <&ldo30_reg>;
> +		vdd-supply = <&ldo31_reg>;
> +		ledvdd-supply = <&ldo33_reg>;
> +	};
> +};
> -- 
> 2.11.0
> 

Re: [PATCH 1/3] Input: add STMicroelectronics FingerTip touchscreen driver

[Dmitry Torokhov]

On Tue, Jan 17, 2017 at 08:43:32PM +0200, Krzysztof Kozlowski wrote:
> On Tue, Jan 17, 2017 at 10:54:39PM +0900, Andi Shyti wrote:
> > Add binding for the STMicroelectronics FingerTip (stmfts)
> > touchscreen driver.
> > 
> > Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> > ---
> >  .../bindings/input/touchscreen/st,stmfts.txt       | 43 ++++++++++++++++++++++
> >  1 file changed, 43 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/st,stmfts.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/st,stmfts.txt b/Documentation/devicetree/bindings/input/touchscreen/st,stmfts.txt
> > new file mode 100644
> > index 000000000000..788f4ba744db
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/st,stmfts.txt
> > @@ -0,0 +1,43 @@
> > +* ST-Microelectronics FingerTip touchscreen controller
> > +
> > +The ST-Microelectronics FingerTip device provides a basic touchscreen
> > +functionality. Along with it the user can enable the touchkey which can work as
> > +a basic HOME and BACK key for phones.
> > +
> > +The driver supports also hovering as an absolute single touch event with x, y, z
> > +coordinates.
> > +
> > +Required properties:
> > +- compatible		: must be "st,stmfts"
> > +- reg			: I2C slave address, (e.g. 0x49)
> > +- interrupt-parent	: the phandle to the interrupt controller which provides
> > +			  the interrupt
> > +- interrupts		: interrupt specification
> > +- avdd-supply		: analogic power supply
> > +- vdd-supply		: power supply
> > +- touchscreen-size-x	: see touchscreen.txt
> > +- touchscreen-size-y	: see touchscreen.txt
> > +
> > +Optional properties:
> > +- touch-key-connected	: specifies whether the touchkey feature is connected
> 
> You are making it a generic property but it is specific to this device,
> so:
> 	st,touch-key-connected
> ?
> 
> > +- ledvdd-supply		: power supply to the touch key leds
> 
> Is this really optional? If yes... how it gets the power when not
> provided?

Also, is this really a regulator? Given you are testing whether it is on
or off to report LED state it feels to me it might be a GPIO pin, not
regulator...

Thanks.

-- 
Dmitry

Re: [PATCH 2/3] Input: add support for the STMicroelectronics FingerTip touchscreen

[Dmitry Torokhov]

Hi Andi.

On Tue, Jan 17, 2017 at 10:54:40PM +0900, Andi Shyti wrote:
> The stmfts (ST-Microelectronics FingerTip S) touchscreen device
> is a capacitive multi-touch controller mainly for mobile use.
> 
> It's connected through i2c bus at the address 0x49 and it
> interfaces with userspace through input event interface.
> 
> At the current state it provides a touchscreen multitouch
> functionality up to 10 fingers. Each finger is enumerated with a
> distinctive id (from 0 to 9).
> 
> If enabled the device can support single "touch" hovering, by
> providing three coordinates, x, y, z. The z coordinate is
> related to the distance between the finger and the touchscreen
> itself.

No, we have dedicated ABS_DISTANCE event code for this, not ABS_Z.

> 
> It is possible to select the touchkey functionality which
> provides a basic two keys interface for "home" and "back" menu,
> typical in mobile phones.
> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  drivers/input/touchscreen/Kconfig  |  12 +
>  drivers/input/touchscreen/Makefile |   1 +
>  drivers/input/touchscreen/stmfts.c | 759 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 772 insertions(+)
>  create mode 100644 drivers/input/touchscreen/stmfts.c
> 
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 8650c94e29d0..18c2f919eb79 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -1092,6 +1092,18 @@ config TOUCHSCREEN_ST1232
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called st1232_ts.
>  
> +config TOUCHSCREEN_STMFTS
> +	tristate "STMicroelectronics STMFTS touchscreen"
> +	depends on I2C
> +	depends on INPUT
> +	depends on LEDS_CLASS
> +	help
> +	  Say Y here if you want support for STMicroelectronics
> +	  STMFTS touchscreen.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called stmfts.
> +
>  config TOUCHSCREEN_STMPE
>  	tristate "STMicroelectronics STMPE touchscreens"
>  	depends on MFD_STMPE
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index e41e3c7aa427..be7673f9c71a 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -66,6 +66,7 @@ obj-$(CONFIG_TOUCHSCREEN_S3C2410)	+= s3c2410_ts.o
>  obj-$(CONFIG_TOUCHSCREEN_SILEAD)	+= silead.o
>  obj-$(CONFIG_TOUCHSCREEN_SIS_I2C)	+= sis_i2c.o
>  obj-$(CONFIG_TOUCHSCREEN_ST1232)	+= st1232.o
> +obj-$(CONFIG_TOUCHSCREEN_STMFTS)	+= stmfts.o
>  obj-$(CONFIG_TOUCHSCREEN_STMPE)		+= stmpe-ts.o
>  obj-$(CONFIG_TOUCHSCREEN_SUN4I)		+= sun4i-ts.o
>  obj-$(CONFIG_TOUCHSCREEN_SUR40)		+= sur40.o
> diff --git a/drivers/input/touchscreen/stmfts.c b/drivers/input/touchscreen/stmfts.c
> new file mode 100644
> index 000000000000..abccc14d06d2
> --- /dev/null
> +++ b/drivers/input/touchscreen/stmfts.c
> @@ -0,0 +1,759 @@
> +/*
> + * Copyright (c) 2017 Samsung Electronics Co., Ltd.
> + * Author: Andi Shyti <andi.shyti@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * STMicroelectronics FTS Touchscreen device driver
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/input/mt.h>
> +#include <linux/interrupt.h>
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +
> +/* I2C commands */
> +#define STMFTS_READ_INFO			0x80
> +#define STMFTS_READ_STATUS			0x84
> +#define STMFTS_READ_ONE_EVENT			0x85
> +#define STMFTS_SLEEP_IN				0x91
> +#define STMFTS_SLEEP_OUT			0x91
> +#define STMFTS_MS_MT_SENSE_OFF			0x92
> +#define STMFTS_MS_MT_SENSE_ON			0x93
> +#define STMFTS_SS_HOVER_SENSE_OFF		0x94
> +#define STMFTS_SS_HOVER_SENSE_ON		0x95
> +#define STMFTS_MS_KEY_SENSE_OFF			0x9a
> +#define STMFTS_MS_KEY_SENSE_ON			0x9b
> +#define STMFTS_SYSTEM_RESET			0xa0
> +#define STMFTS_CLEAR_EVENT_STACK		0xa1
> +#define STMFTS_FULL_FORCE_CALIBRATION		0xa2
> +#define STMFTS_MS_CX_TUNING			0xa3
> +#define STMFTS_SS_CX_TUNING			0xa4
> +
> +/* events */
> +#define STMFTS_EV_NO_EVENT			0x00
> +#define STMFTS_EV_MULTI_TOUCH_DETECTED		0x02
> +#define STMFTS_EV_MULTI_TOUCH_ENTER		0x03
> +#define STMFTS_EV_MULTI_TOUCH_LEAVE		0x04
> +#define STMFTS_EV_MULTI_TOUCH_MOTION		0x05
> +#define STMFTS_EV_HOVER_ENTER			0x07
> +#define STMFTS_EV_HOVER_LEAVE			0x08
> +#define STMFTS_EV_HOVER_MOTION			0x09
> +#define STMFTS_EV_KEY_STATUS			0x0e
> +#define STMFTS_EV_ERROR				0x0f
> +#define STMFTS_EV_CONTROLLER_READY		0x10
> +#define STMFTS_EV_SLEEP_OUT_CONTROLLER_READY	0x11
> +#define STMFTS_EV_STATUS			0x16
> +
> +/* multi touch related event masks */
> +#define STMFTS_MASK_EVENT_ID			0x0f
> +#define STMFTS_MASK_TOUCH_ID			0xf0
> +#define STMFTS_MASK_LEFT_EVENT			0x0f
> +#define STMFTS_MASK_X_MSB			0x0f
> +#define STMFTS_MASK_Y_LSB			0xf0
> +
> +/* key related event masks */
> +#define STMFTS_MASK_KEY_NO_TOUCH		0x00
> +#define STMFTS_MASK_KEY_BACK			0x01
> +#define STMFTS_MASK_KEY_MENU			0x02
> +
> +#define STMFTS_EVENT_SIZE	8
> +#define STMFTS_MAX_FINGERS	10
> +#define STMFTS_DEV_NAME		"stmfts"
> +
> +enum stmfts_regulators {
> +	STMFTS_REGULATOR_VDD,
> +	STMFTS_REGULATOR_AVDD,
> +};
> +
> +struct stmfts_data {
> +	struct i2c_client *client;
> +	struct input_dev *input_touch;
> +	struct input_dev *input_key;
> +	struct led_classdev led_cdev;
> +	struct mutex mutex;
> +	u32 x_size;
> +	u32 y_size;
> +
> +	struct regulator_bulk_data regulators[2];
> +
> +	/* ledvdd will be used also to check
> +	 * whether the LED is supported
> +	 */
> +	struct regulator *ledvdd;
> +
> +	bool use_key;
> +	bool signal;
> +	bool led_status;
> +	u8 users;
> +
> +	u16 chip_id;
> +	u8 chip_ver;
> +	u16 fw_ver;
> +	u8 config_id;
> +	u8 config_ver;
> +	u8 in_touch;
> +
> +	wait_queue_head_t wq;
> +
> +	bool hover_enabled;
> +};
> +
> +static void stmfts_brightness_set(struct led_classdev *led_cdev,
> +					enum led_brightness value)
> +{
> +	struct stmfts_data *sdata = container_of(led_cdev,
> +					struct stmfts_data, led_cdev);
> +
> +	if (value == sdata->led_status)
> +		return;
> +
> +	if (!value) {
> +		regulator_disable(sdata->ledvdd);
> +	} else {
> +		int err = regulator_enable(sdata->ledvdd);
> +
> +		if (err)
> +			dev_warn(&sdata->client->dev,
> +				"failed to disable ledvdd regulator\n");
> +	}
> +
> +	sdata->led_status = value;
> +}
> +
> +static enum led_brightness stmfts_brightness_get(struct led_classdev *led_cdev)
> +{
> +	struct stmfts_data *sdata = container_of(led_cdev,
> +						struct stmfts_data, led_cdev);
> +
> +	return !!regulator_is_enabled(sdata->ledvdd);
> +}
> +
> +static void stmfts_parse_event(struct stmfts_data *sdata, u8 event[])
> +{
> +	int ret;
> +	u8 id, t_id = 0;
> +	u16 x, y, z, maj, min, orientation, area;
> +
> +	id = event[0];
> +
> +	do {
> +		mutex_lock(&sdata->mutex);
> +		if (sdata->in_touch) {
> +			id = event[0] & STMFTS_MASK_EVENT_ID;
> +			t_id = (event[0] & STMFTS_MASK_TOUCH_ID) >> 4;
> +		} else {
> +			id = event[0];
> +			t_id = 0;
> +		}
> +
> +		switch (id) {
> +		case STMFTS_EV_NO_EVENT:
> +			break;
> +
> +		case STMFTS_EV_MULTI_TOUCH_ENTER:
> +		case STMFTS_EV_MULTI_TOUCH_LEAVE:
> +		case STMFTS_EV_MULTI_TOUCH_MOTION:
> +			if (id == STMFTS_EV_MULTI_TOUCH_ENTER) {
> +				if (!(sdata->in_touch++))
> +					input_mt_report_slot_state(
> +							sdata->input_touch,
> +							MT_TOOL_FINGER, true);
> +			} else if (id == STMFTS_EV_MULTI_TOUCH_LEAVE) {
> +				if (!(--sdata->in_touch))
> +					input_mt_report_slot_state(
> +							sdata->input_touch,
> +							MT_TOOL_FINGER, false);
> +			}
> +
> +			x = event[1] | ((event[2] & STMFTS_MASK_X_MSB) << 8);
> +			y = (event[2] >> 4) | (event[3] << 4);
> +
> +			maj = event[4];
> +			min = event[5];
> +			orientation = event[6];
> +			area = event[7];
> +
> +			input_mt_slot(sdata->input_touch, t_id);
> +			input_report_abs(sdata->input_touch,
> +					ABS_MT_POSITION_X, x);
> +			input_report_abs(sdata->input_touch,
> +					ABS_MT_POSITION_Y, y);
> +			input_report_abs(sdata->input_touch,
> +					ABS_MT_TOUCH_MAJOR, maj);
> +			input_report_abs(sdata->input_touch,
> +					ABS_MT_TOUCH_MINOR, min);
> +			input_report_abs(sdata->input_touch,
> +					ABS_MT_PRESSURE, area);
> +			input_report_abs(sdata->input_touch,
> +					ABS_MT_ORIENTATION, orientation);
> +			input_sync(sdata->input_touch);
> +
> +			break;
> +
> +		case STMFTS_EV_HOVER_ENTER:
> +		case STMFTS_EV_HOVER_LEAVE:
> +		case STMFTS_EV_HOVER_MOTION:
> +			x = (event[2] << 4) | (event[4] >> 4);
> +			y = (event[3] << 4) | (event[4] & STMFTS_MASK_Y_LSB);
> +			z = event[5];
> +			orientation = event[6] & STMFTS_MASK_Y_LSB;
> +
> +			input_report_abs(sdata->input_touch, ABS_X, x);
> +			input_report_abs(sdata->input_touch, ABS_Y, y);
> +			input_report_abs(sdata->input_touch, ABS_Z, z);
> +			input_sync(sdata->input_touch);
> +
> +			break;
> +
> +		case STMFTS_EV_KEY_STATUS:

Please use:
			switch (event[2]) {
			case 0:
				...

			case STMFTS_MASK_KEY_BACK:
				...

			case STMFTS_MASK_KEY_MENU:
				...

			default:
				dev_warn(...);
				break;
			}

> +			if (!event[2]) {
> +				input_report_key(sdata->input_key, KEY_BACK, 0);
> +				input_report_key(sdata->input_key, KEY_MENU, 0);
> +			} else {
> +				if (event[2] == STMFTS_MASK_KEY_BACK)
> +					input_report_key(sdata->input_key,
> +								KEY_BACK, 1);
> +				else if (event[2] == STMFTS_MASK_KEY_MENU)
> +					input_report_key(sdata->input_key,
> +								KEY_MENU, 1);
> +				else /* quite impossible */
> +					break;
> +			}
> +			input_sync(sdata->input_key);
> +			break;
> +
> +		case STMFTS_EV_STATUS:
> +			sdata->signal = true;
> +			wake_up_interruptible(&sdata->wq);
> +			break;
> +
> +		case STMFTS_EV_ERROR:
> +			dev_err(&sdata->client->dev,
> +				"error code: 0x%x%x%x%x%x%x",
> +				event[6], event[5], event[4],
> +				event[3], event[2], event[1]);
> +			break;
> +
> +		default:
> +			dev_err(&sdata->client->dev,
> +				"unknown event 0x%x\n", event[0]);
> +		}
> +		mutex_unlock(&sdata->mutex);
> +
> +		ret = i2c_smbus_read_i2c_block_data(sdata->client,
> +						STMFTS_READ_ONE_EVENT,
> +						STMFTS_EVENT_SIZE, event);
> +		if (ret) {
> +			i2c_smbus_write_byte(sdata->client,
> +						STMFTS_CLEAR_EVENT_STACK);
> +			break;
> +		}
> +
> +	} while (event[0]);
> +}
> +
> +static irqreturn_t stmfts_irq_handler(int irq, void *dev)
> +{
> +	struct stmfts_data *sdata = dev;
> +	int ret;
> +	u8 event[STMFTS_EVENT_SIZE];
> +
> +	ret = i2c_smbus_read_i2c_block_data(sdata->client,
> +					STMFTS_READ_ONE_EVENT,
> +					STMFTS_EVENT_SIZE, event);
> +
> +	switch (event[0]) {
> +	case STMFTS_EV_CONTROLLER_READY:
> +	case STMFTS_EV_SLEEP_OUT_CONTROLLER_READY:
> +		sdata->signal = true;
> +		wake_up_interruptible(&sdata->wq);
> +		break;
> +	default:
> +		stmfts_parse_event(sdata, event);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int stmfts_write_and_wait(struct stmfts_data *sdata, const u8 cmd)
> +{
> +	int err;
> +
> +	err = i2c_smbus_write_byte(sdata->client, cmd);
> +	if (err)
> +		return err;
> +
> +	sdata->signal = false;

This is racy, interrupt may run before you get chance to set signal to
false. Also, please use wait_for_completion*() API.

> +	err = wait_event_interruptible_timeout(sdata->wq, sdata->signal,
> +						msecs_to_jiffies(1000));
> +
> +	return !err ? -ETIMEDOUT : 0;
> +}
> +
> +static void stmfts_sleep_in(struct stmfts_data *sdata)
> +{
> +	mutex_lock(&sdata->mutex);
> +
> +	sdata->users--;
> +	if (!sdata->users)
> +		i2c_smbus_write_byte(sdata->client, STMFTS_SLEEP_IN);

This looks like somewhat homegrown runtime PM, please replace.

> +
> +	mutex_unlock(&sdata->mutex);
> +}
> +
> +static int stmfts_sleep_out(struct stmfts_data *sdata, u8 cmd)
> +{
> +	int ret = 0;
> +
> +	mutex_lock(&sdata->mutex);
> +
> +	if (!sdata->users)
> +		ret = stmfts_write_and_wait(sdata, STMFTS_SLEEP_OUT);
> +
> +	/* if sleep out succeeds users increments, otherwise not */
> +	sdata->users += !ret;
> +
> +	mutex_unlock(&sdata->mutex);
> +
> +	ret = i2c_smbus_write_byte(sdata->client, cmd);
> +	if (ret)
> +		stmfts_sleep_in(sdata);
> +
> +	return ret;
> +}
> +
> +static int stmfts_input_touch_open(struct input_dev *dev)
> +{
> +	struct stmfts_data *sdata = input_get_drvdata(dev);
> +
> +	return stmfts_sleep_out(sdata, STMFTS_MS_MT_SENSE_ON);
> +}
> +
> +static void stmfts_input_touch_close(struct input_dev *dev)
> +{
> +	struct stmfts_data *sdata = input_get_drvdata(dev);
> +
> +	i2c_smbus_write_byte(sdata->client, STMFTS_MS_MT_SENSE_OFF);
> +
> +	stmfts_sleep_in(sdata);
> +}
> +
> +static int stmfts_input_key_open(struct input_dev *dev)
> +{
> +	struct stmfts_data *sdata = input_get_drvdata(dev);
> +
> +	return stmfts_sleep_out(sdata, STMFTS_MS_KEY_SENSE_ON);
> +}
> +
> +static void stmfts_input_key_close(struct input_dev *dev)
> +{
> +	struct stmfts_data *sdata = input_get_drvdata(dev);
> +
> +	i2c_smbus_write_byte(sdata->client, STMFTS_MS_KEY_SENSE_OFF);
> +	stmfts_sleep_in(sdata);
> +}
> +
> +static ssize_t stmfts_sysfs_hwid(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "ST-Microelectronics FTS 0x%x version %u\n",
> +					sdata->chip_id, sdata->chip_ver);

sysfs attributes should have only 1 value. Please split into chip ID and
chip version and drop static strings.

> +}
> +
> +static ssize_t stmfts_sysfs_fw_ver(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%u\n", sdata->fw_ver);
> +}
> +
> +static ssize_t stmfts_sysfs_config_id(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "0x%x version %u\n",
> +			sdata->config_id, sdata->config_ver);

Please split in 2.

> +}
> +
> +static ssize_t stmfts_sysfs_read_status(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +	u8 status[4];
> +	int ret;
> +
> +	ret = i2c_smbus_read_i2c_block_data(sdata->client,
> +					STMFTS_READ_STATUS, 4, status);
> +
> +	return sprintf(buf, "0x%x\n", status[0]);
> +}
> +
> +static ssize_t stmfts_sysfs_hover_enable_read(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%u\n", sdata->hover_enabled);
> +}
> +
> +static ssize_t stmfts_sysfs_hover_enable_write(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf, size_t len)
> +{
> +	unsigned long value;
> +	int err;
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +
> +	if (kstrtoul(buf, 0, &value))
> +		return -EINVAL;
> +
> +	if (value & sdata->hover_enabled)
> +		return len;
> +
> +	if (value) {
> +		err = stmfts_sleep_out(sdata, STMFTS_SS_HOVER_SENSE_ON);

Why setting hover explicitly wakes up controller? You shuld put it back
into sleep if nobody is listening, or postpone programming until there
are users.

> +		sdata->hover_enabled = !err;
> +	} else {
> +		err = i2c_smbus_write_byte(sdata->client,
> +					STMFTS_SS_HOVER_SENSE_OFF);
> +		stmfts_sleep_in(sdata);
> +		sdata->hover_enabled = !!err;
> +	}
> +
> +	return len;
> +}
> +
> +static DEVICE_ATTR(hwid, 0444, stmfts_sysfs_hwid, NULL);
> +static DEVICE_ATTR(fw_ver, 0444, stmfts_sysfs_fw_ver, NULL);
> +static DEVICE_ATTR(config_id, 0444, stmfts_sysfs_config_id, NULL);
> +static DEVICE_ATTR(status, 0444, stmfts_sysfs_read_status, NULL);
> +static DEVICE_ATTR(hover_enable, 0644, stmfts_sysfs_hover_enable_read,
> +					stmfts_sysfs_hover_enable_write);
> +
> +static struct attribute *stmfts_sysfs_attrs[] = {
> +	&dev_attr_hwid.attr,
> +	&dev_attr_fw_ver.attr,
> +	&dev_attr_config_id.attr,
> +	&dev_attr_status.attr,
> +	&dev_attr_hover_enable.attr,
> +	NULL
> +};
> +
> +static struct attribute_group stmfts_attribute_group = {
> +	.attrs = stmfts_sysfs_attrs
> +};
> +
> +static int stmfts_parse_dt(struct stmfts_data *sdata)
> +{
> +	int ret;
> +	struct device_node *np = sdata->client->dev.of_node;
> +
> +	if (!np)
> +		return -ENOENT;
> +
> +	ret = of_property_read_u32(np, "touchscreen-size-x", &sdata->x_size);
> +	if (ret)
> +		return ret;
> +
> +	ret = of_property_read_u32(np, "touchscreen-size-y", &sdata->y_size);
> +	if (ret)
> +		return ret;

Please use touchscreen_parse_properties().

> +
> +	sdata->use_key = of_property_read_bool(np, "touch-key-connected");
> +
> +	return 0;
> +}
> +
> +static int stmfts_power_on(struct stmfts_data *sdata)
> +{
> +	int err;
> +	u8 reg[8];
> +
> +	err = regulator_bulk_enable(ARRAY_SIZE(sdata->regulators),
> +							sdata->regulators);
> +	if (err)
> +		return err;
> +
> +	/*
> +	 * the datasheet does not specify the power on time, but considering
> +	 * that the reset time is < 10ms, I sleep 20ms to be sure
> +	 */
> +	msleep(20);
> +
> +	err = i2c_smbus_read_i2c_block_data(sdata->client,
> +					STMFTS_READ_INFO, 8, reg);
> +	if (err < 0)
> +		return err;
> +	if (err != 8)
> +		return -EIO;
> +
> +	sdata->chip_id = (reg[6] << 8) | reg[7];
> +	sdata->chip_ver = reg[0];
> +	sdata->fw_ver = (reg[2] << 8) | reg[3];
> +	sdata->config_id = reg[4];
> +	sdata->config_ver = reg[5];
> +
> +	enable_irq(sdata->client->irq);
> +	err = stmfts_write_and_wait(sdata, STMFTS_SYSTEM_RESET);
> +	if (err)
> +		return err;
> +
> +	err = stmfts_write_and_wait(sdata, STMFTS_SLEEP_OUT);
> +	if (err)
> +		return err;
> +
> +	/* optional tuning */
> +	err = stmfts_write_and_wait(sdata, STMFTS_MS_CX_TUNING);
> +	if (err)
> +		dev_warn(&sdata->client->dev, "failed to perform mutual auto tune\n");
> +
> +	/* optional tuning */
> +	err = stmfts_write_and_wait(sdata, STMFTS_SS_CX_TUNING);
> +	if (err)
> +		dev_warn(&sdata->client->dev, "failed to perform self auto tune\n");
> +
> +	err = stmfts_write_and_wait(sdata, STMFTS_FULL_FORCE_CALIBRATION);
> +	if (err)
> +		return err;

Hmm, do you need to calibrate on boot, or maybe wait for the device to
be opened first?

> +
> +	/* at this point no one is using the touchscreen
> +	 * and I don't really care about the return value
> +	 */
> +	i2c_smbus_write_byte(sdata->client, STMFTS_SLEEP_IN);
> +
> +	return 0;
> +}
> +
> +static void stmfts_power_off(void *data)
> +{
> +	struct stmfts_data *sdata = data;
> +
> +	disable_irq(sdata->client->irq);
> +	regulator_bulk_disable(ARRAY_SIZE(sdata->regulators),
> +						sdata->regulators);
> +}
> +
> +static int stmfts_enable_key(struct stmfts_data *sdata)
> +{
> +	int err;
> +
> +	sdata->input_key = devm_input_allocate_device(&sdata->client->dev);
> +	if (!sdata->input_key)
> +		return -ENOMEM;
> +
> +	sdata->input_key->name = "stmfts_key";
> +	sdata->input_key->id.bustype = BUS_I2C;
> +	sdata->input_key->open = stmfts_input_key_open;
> +	sdata->input_key->close = stmfts_input_key_close;

If you want separate input device for keys I'd recommend setting ->phys
on them. Another option is to add keys to teh touchscreen device. Do you
expect different actors listening to these devices? Even with different
actors you can program event masks so that if process is not interested
in some events (like ABS_MT_* events) it is not woken up when dveice
generates them.

> +
> +	input_set_capability(sdata->input_key, EV_KEY, KEY_MENU);
> +	input_set_capability(sdata->input_key, EV_KEY, KEY_BACK);
> +
> +	input_set_drvdata(sdata->input_key, sdata);
> +	err = input_register_device(sdata->input_key);
> +	if (err)
> +		return err;
> +
> +	/* get the regulator for powering the leds on */
> +	sdata->ledvdd = devm_regulator_get(&sdata->client->dev, "ledvdd");
> +	if (IS_ERR(sdata->ledvdd))
> +		/* there is no LED connected to the touch key */
> +		sdata->ledvdd = NULL;

As I mentioned, is it truly regulator, or just a GPIO?

> +
> +	sdata->led_cdev.name = STMFTS_DEV_NAME;
> +	sdata->led_cdev.max_brightness = LED_ON;
> +	sdata->led_cdev.brightness = LED_OFF;
> +	sdata->led_cdev.brightness_set = stmfts_brightness_set;
> +	sdata->led_cdev.brightness_get = stmfts_brightness_get;
> +
> +	err = devm_led_classdev_register(&sdata->client->dev, &sdata->led_cdev);
> +	if (err) {
> +		dev_warn(&sdata->client->dev, "unable to register led, led might not work\n");
> +		sdata->ledvdd = NULL;
> +
> +		/* I don't want to prevent using the touch key
> +		 * only because the LEDs don't get registered
> +		 */
> +		err = 0;
> +	}
> +
> +	return err;
> +}
> +
> +static int stmfts_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	int err;
> +	struct stmfts_data *sdata;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
> +						I2C_FUNC_SMBUS_BYTE_DATA |
> +						I2C_FUNC_SMBUS_I2C_BLOCK))
> +		return -ENODEV;
> +
> +	sdata = devm_kzalloc(&client->dev, sizeof(*sdata), GFP_KERNEL);
> +	if (!sdata)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, sdata);
> +
> +	mutex_init(&sdata->mutex);
> +
> +	sdata->regulators[STMFTS_REGULATOR_VDD].supply = "vdd";
> +	sdata->regulators[STMFTS_REGULATOR_AVDD].supply = "avdd";
> +	err = devm_regulator_bulk_get(&client->dev,
> +			ARRAY_SIZE(sdata->regulators), sdata->regulators);
> +	if (err)
> +		return err;
> +
> +	err = devm_add_action_or_reset(&client->dev, stmfts_power_off, sdata);
> +	if (err)
> +		return err;
> +
> +	sdata->client = client;
> +
> +	err = stmfts_parse_dt(sdata);
> +	if (err)
> +		return err;
> +
> +	init_waitqueue_head(&sdata->wq);
> +
> +	err = sysfs_create_group(&sdata->client->dev.kobj,
> +					&stmfts_attribute_group);

I do not see it being removed on errors.

> +	if (err)
> +		return err;
> +
> +	err = devm_request_threaded_irq(&client->dev, client->irq,
> +					NULL, stmfts_irq_handler,
> +					IRQF_ONESHOT | IRQF_TRIGGER_LOW,
> +					"stmfts_irq", sdata);
> +	if (err)
> +		return err;

What happens if interrupt is raised here?

> +
> +	disable_irq(sdata->client->irq);
> +
> +	dev_info(&client->dev, "initializing ST-Microelectronics FTS...\n");
> +	err = stmfts_power_on(sdata);
> +	if (err)
> +		return err;
> +
> +	sdata->input_touch = devm_input_allocate_device(&client->dev);
> +	if (!sdata->input_touch)
> +		return -ENOMEM;
> +
> +	sdata->input_touch->name = STMFTS_DEV_NAME;
> +	sdata->input_touch->id.bustype = BUS_I2C;
> +	sdata->input_touch->open = stmfts_input_touch_open;
> +	sdata->input_touch->close = stmfts_input_touch_close;
> +
> +	input_set_capability(sdata->input_touch, EV_ABS, ABS_MT_POSITION_Y);
> +	input_set_capability(sdata->input_touch, EV_ABS, ABS_MT_POSITION_X);
> +	input_set_capability(sdata->input_touch, EV_ABS, ABS_MT_TOUCH_MAJOR);
> +	input_set_capability(sdata->input_touch, EV_ABS, ABS_MT_TOUCH_MINOR);
> +	input_set_capability(sdata->input_touch, EV_ABS, ABS_MT_ORIENTATION);
> +	input_set_capability(sdata->input_touch, EV_ABS, ABS_MT_PRESSURE);
> +	input_set_capability(sdata->input_touch, EV_ABS, ABS_MT_POSITION_X);
> +
> +	input_set_abs_params(sdata->input_touch,
> +				ABS_MT_POSITION_X, 0, sdata->x_size, 0, 0);

input_set_abs_params() implies input_set_capability(), you can drop the
calls above.

> +	input_set_abs_params(sdata->input_touch,
> +				ABS_MT_POSITION_Y, 0, sdata->y_size, 0, 0);
> +	input_set_abs_params(sdata->input_touch,
> +				ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
> +	input_set_abs_params(sdata->input_touch,
> +				ABS_MT_TOUCH_MINOR, 0, 255, 0, 0);
> +	input_set_abs_params(sdata->input_touch,
> +				ABS_MT_ORIENTATION, 0, 255, 0, 0);
> +	input_set_abs_params(sdata->input_touch,
> +				ABS_MT_PRESSURE, 0, 255, 0, 0);
> +	input_mt_init_slots(sdata->input_touch,
> +				STMFTS_MAX_FINGERS, INPUT_MT_DIRECT);

Error handling please.

> +
> +	/* for hover features */
> +	input_set_capability(sdata->input_touch, EV_ABS, ABS_X);
> +	input_set_capability(sdata->input_touch, EV_ABS, ABS_Y);

Not needed, input_mt_init_slots() will copy MT axis setup into ST axis
for INPUT_MT_DIRECT devices.

> +	input_set_capability(sdata->input_touch, EV_ABS, ABS_Z);

ABS_DISTANCE

> +	input_set_abs_params(sdata->input_touch, ABS_X, 0, sdata->x_size, 0, 0);
> +	input_set_abs_params(sdata->input_touch, ABS_Y, 0, sdata->y_size, 0, 0);
> +	input_set_abs_params(sdata->input_touch, ABS_Z, 0, 255, 0, 0);
> +
> +	input_set_drvdata(sdata->input_touch, sdata);
> +	err = input_register_device(sdata->input_touch);
> +	if (err)
> +		return err;
> +
> +	if (sdata->use_key) {
> +		err = stmfts_enable_key(sdata);
> +		if (err)
> +			dev_warn(&client->dev, "failed to enable touchkey\n");
> +	}
> +
> +	return 0;
> +}
> +
> +static int stmfts_remove(struct i2c_client *client)
> +{
> +	struct stmfts_data *sdata = i2c_get_clientdata(client);
> +
> +	sysfs_remove_group(&sdata->client->dev.kobj, &stmfts_attribute_group);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused stmfts_suspend(struct device *dev)
> +{
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +
> +	stmfts_power_off(sdata);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused stmfts_resume(struct device *dev)
> +{
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +
> +	return stmfts_power_on(sdata);
> +}
> +
> +static SIMPLE_DEV_PM_OPS(stmfts_pm_ops, stmfts_suspend, stmfts_resume);
> +
> +static const struct of_device_id stmfts_of_match[] = {
> +	{ .compatible = "st,stmfts", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, stmfts_of_match);
> +
> +static const struct i2c_device_id stmfts_id[] = {
> +	{ "stmfts", 0 },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(i2c, stmfts_id);
> +
> +static struct i2c_driver stmfts_driver = {
> +	.driver = {
> +		.name = STMFTS_DEV_NAME,
> +		.of_match_table = of_match_ptr(stmfts_of_match),
> +		.pm = &stmfts_pm_ops,
> +	},
> +	.probe = stmfts_probe,
> +	.remove = stmfts_remove,
> +	.id_table = stmfts_id,
> +};
> +
> +module_i2c_driver(stmfts_driver);
> +
> +MODULE_AUTHOR("Andi Shyti <andi.shyti@samsung.com>");
> +MODULE_DESCRIPTION("STMicroelectronics FTS Touch Screen");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.11.0
> 

Thanks.

-- 
Dmitry

Re: [PATCH 1/3] Input: add STMicroelectronics FingerTip touchscreen driver

[Andi Shyti]

Hi Krzysztof,

> > +Optional properties:
> > +- touch-key-connected	: specifies whether the touchkey feature is connected
> 
> You are making it a generic property but it is specific to this device,
> so:
> 	st,touch-key-connected
> ?
> 
> > +- ledvdd-supply		: power supply to the touch key leds
> 
> Is this really optional? If yes... how it gets the power when not
> provided?

yes, it is. This is required only if (st,)touch-key-connected is
added, it's not required in any other case. Anyway, even if not
present, the device works, just without leds, which is fine for
me.

Thanks,
Andi

Re: [PATCH 1/3] Input: add STMicroelectronics FingerTip touchscreen driver

[Andi Shyti]

Hi Dmitry,

> > > +- ledvdd-supply		: power supply to the touch key leds
> > 
> > Is this really optional? If yes... how it gets the power when not
> > provided?
> 
> Also, is this really a regulator? Given you are testing whether it is on
> or off to report LED state it feels to me it might be a GPIO pin, not
> regulator...

In this case it's a real regulator, not a GPIO. But even if it
was a GPIO, I still could use gpio-regulator[*] which handles
gpio using the regulator framework. In this case, depending on
how the leds are enabled on the DTS, in the driver I would use
only one api (regulators indeed) for both the cases.

Thanks,
Andi

[*] Documentation/devicetree/bindings/regulator/gpio-regulator.txt

Re: [PATCH 2/3] Input: add support for the STMicroelectronics FingerTip touchscreen

[Andi Shyti]

Hi Dmitry,

thanks for looking into the driver and the review!
I agree with almost everything, some comments below.

> > +	/* optional tuning */
> > +	err = stmfts_write_and_wait(sdata, STMFTS_MS_CX_TUNING);
> > +	if (err)
> > +		dev_warn(&sdata->client->dev, "failed to perform mutual auto tune\n");
> > +
> > +	/* optional tuning */
> > +	err = stmfts_write_and_wait(sdata, STMFTS_SS_CX_TUNING);
> > +	if (err)
> > +		dev_warn(&sdata->client->dev, "failed to perform self auto tune\n");
> > +
> > +	err = stmfts_write_and_wait(sdata, STMFTS_FULL_FORCE_CALIBRATION);
> > +	if (err)
> > +		return err;
> 
> Hmm, do you need to calibrate on boot, or maybe wait for the device to
> be opened first?

The datasheet says that I need to calibrate during boot. Anyway,
the calibration requires some time (it can be up to a couple of
seconds), therefore I wouldn't like to calibrate everytime
someone opens the device.

> > +static int stmfts_enable_key(struct stmfts_data *sdata)
> > +{
> > +	int err;
> > +
> > +	sdata->input_key = devm_input_allocate_device(&sdata->client->dev);
> > +	if (!sdata->input_key)
> > +		return -ENOMEM;
> > +
> > +	sdata->input_key->name = "stmfts_key";
> > +	sdata->input_key->id.bustype = BUS_I2C;
> > +	sdata->input_key->open = stmfts_input_key_open;
> > +	sdata->input_key->close = stmfts_input_key_close;
> 
> If you want separate input device for keys I'd recommend setting ->phys
> on them. Another option is to add keys to teh touchscreen device. Do you
> expect different actors listening to these devices? Even with different
> actors you can program event masks so that if process is not interested
> in some events (like ABS_MT_* events) it is not woken up when dveice
> generates them.

Are you suggesting to drop one event interface?

The reason why I chose to generate two interfaces is indeed
because there might be be different processes completely
unrelated to each other that will interact with the device (for
example in Jaechul's driver that you just reviewed, touchscreen
and touchkey are different devices).

I chose it for coherency with the common userspace architecture.
But of course, I can do as you recommend.

I have one more question. This touchscreen has also a proximity
functionality that works exactly as a proximity sensor and I
planned to add it in the future. Would it make sense to use the
iio framework (as it behaves as a proximity sensor) or use the
current interface? For instance, I was going to use the iio.

> > +	/* get the regulator for powering the leds on */
> > +	sdata->ledvdd = devm_regulator_get(&sdata->client->dev, "ledvdd");
> > +	if (IS_ERR(sdata->ledvdd))
> > +		/* there is no LED connected to the touch key */
> > +		sdata->ledvdd = NULL;
> 
> As I mentioned, is it truly regulator, or just a GPIO?

As I wrote in the previous mail, this is truly a regulator. If it
was a gpio, I would still prefer to use the regulator_* functions
as those would be managed by regulator-gpio and I don't need to
change the driver for that.

Thanks a lot,
Andi

Re: [PATCH 1/3] Input: add STMicroelectronics FingerTip touchscreen driver

[Rob Herring]

On Tue, Jan 17, 2017 at 10:54:39PM +0900, Andi Shyti wrote:
> Add binding for the STMicroelectronics FingerTip (stmfts)
> touchscreen driver.

Bindings describe h/w not drivers.

> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  .../bindings/input/touchscreen/st,stmfts.txt       | 43 ++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/st,stmfts.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/st,stmfts.txt b/Documentation/devicetree/bindings/input/touchscreen/st,stmfts.txt
> new file mode 100644
> index 000000000000..788f4ba744db
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/st,stmfts.txt
> @@ -0,0 +1,43 @@
> +* ST-Microelectronics FingerTip touchscreen controller
> +
> +The ST-Microelectronics FingerTip device provides a basic touchscreen
> +functionality. Along with it the user can enable the touchkey which can work as
> +a basic HOME and BACK key for phones.
> +
> +The driver supports also hovering as an absolute single touch event with x, y, z
> +coordinates.
> +
> +Required properties:
> +- compatible		: must be "st,stmfts"

Seems too generic. Is this a single device?

> +- reg			: I2C slave address, (e.g. 0x49)
> +- interrupt-parent	: the phandle to the interrupt controller which provides
> +			  the interrupt
> +- interrupts		: interrupt specification
> +- avdd-supply		: analogic power supply
> +- vdd-supply		: power supply
> +- touchscreen-size-x	: see touchscreen.txt
> +- touchscreen-size-y	: see touchscreen.txt
> +
> +Optional properties:
> +- touch-key-connected	: specifies whether the touchkey feature is connected
> +- ledvdd-supply		: power supply to the touch key leds
> +
> +Example:
> +
> +i2c@00000000 {
> +
> +	/* ... */
> +
> +	stmfts@49 {
> +		compatible = "st,stmfts";
> +		reg = <0x49>;
> +		interrupt-parent = <&gpa1>;
> +		interrupts = <1 IRQ_TYPE_NONE>;
> +		touchscreen-size-x = <1599>;
> +		touchscreen-size-y = <2559>;
> +		touch-key-connected;
> +		avdd-supply = <&ldo30_reg>;
> +		vdd-supply = <&ldo31_reg>;
> +		ledvdd-supply = <&ldo33_reg>;
> +	};
> +};
> -- 
> 2.11.0
> 

Re: [PATCH 1/3] Input: add STMicroelectronics FingerTip touchscreen driver

[Andi Shyti]

Hi Rob,

> > +- compatible		: must be "st,stmfts"
> 
> Seems too generic. Is this a single device?

the device is precisely called ST-Microelectronics FingerTip S
(stmfts). There are variants, but they all have the same name and
same datasheet.

Maybe I can be more specific in the description, sometimes I
omitted the 'S'.

Andi

Re: [PATCH 1/3] Input: add STMicroelectronics FingerTip touchscreen driver

[Rob Herring]

On Thu, Jan 19, 2017 at 6:19 PM, Andi Shyti <andi@etezian.org> wrote:
> Hi Rob,
>
>> > +- compatible               : must be "st,stmfts"
>>
>> Seems too generic. Is this a single device?
>
> the device is precisely called ST-Microelectronics FingerTip S
> (stmfts). There are variants, but they all have the same name and
> same datasheet.

Okay, then it is fine.

>
> Maybe I can be more specific in the description, sometimes I
> omitted the 'S'.
>
> Andi
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html