[PATCH v1 0/4] Add support for touchscreen on Colibri VF50

[PATCH v1 0/4] Add support for touchscreen on Colibri VF50

[Sanchayan Maity]

Hello,

The patchset adds support for 4 wire touchscreen on Toradex Colibri
VF50 modules. Patches are based on top of shawn's branch.

Thanks & Regards,
Sanchayan.

Sanchayan Maity (4):
  ARM: dts: vfxxx: Add io-channel-cells property for ADC node
  ARM: dts: vf500-colibri: Add device tree node for touchscreen support
  touchscreen: colibri-vf50-ts: Add touchscreen support for Colibri VF50
  input: Add DT binding documentation for Colibri VF50 touchscreen

 .../bindings/input/touchscreen/colibri-vf50-ts.txt |  34 ++
 arch/arm/boot/dts/vf500-colibri-eval-v3.dts        |   4 +
 arch/arm/boot/dts/vf500-colibri.dtsi               |  46 ++
 arch/arm/boot/dts/vfxxx.dtsi                       |   2 +
 drivers/input/touchscreen/Kconfig                  |  12 +
 drivers/input/touchscreen/Makefile                 |   1 +
 drivers/input/touchscreen/colibri-vf50-ts.c        | 466 +++++++++++++++++++++
 7 files changed, 565 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/colibri-vf50-ts.txt
 create mode 100644 drivers/input/touchscreen/colibri-vf50-ts.c

-- 
2.4.5

[PATCH v1 1/4] ARM: dts: vfxxx: Add io-channel-cells property for ADC node

[Sanchayan Maity]

This commit adds io-channel-cells property to the ADC node. This
property is required in order for an IIO consumer driver to work.
Especially required for Colibri VF50, as the touchscreen driver
uses ADC channels with the ADC driver based on IIO framework.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 arch/arm/boot/dts/vfxxx.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index 2c20f3f..f7d8fb6 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -227,6 +227,7 @@
 				clocks = <&clks VF610_CLK_ADC0>;
 				clock-names = "adc";
 				status = "disabled";
+				#io-channel-cells = <1>;
 			};
 
 			tcon0: tcon@4003d000 {
@@ -461,6 +462,7 @@
 				clocks = <&clks VF610_CLK_ADC1>;
 				clock-names = "adc";
 				status = "disabled";
+				#io-channel-cells = <1>;
 			};
 
 			esdhc1: esdhc@400b2000 {
-- 
2.4.5

[PATCH v1 2/4] ARM: dts: vf500-colibri: Add device tree node for touchscreen support

[Sanchayan Maity]

Add device tree node for touchscreen support on Colibri VF50. The
touchscreen functionality on VF50 uses the ADC channels of Vybrid
and some GPIOs. Also add pinctrl nodes for proper pinmux.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 arch/arm/boot/dts/vf500-colibri-eval-v3.dts |  4 +++
 arch/arm/boot/dts/vf500-colibri.dtsi        | 46 +++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/arch/arm/boot/dts/vf500-colibri-eval-v3.dts b/arch/arm/boot/dts/vf500-colibri-eval-v3.dts
index 7fc782c..c5efb57 100644
--- a/arch/arm/boot/dts/vf500-colibri-eval-v3.dts
+++ b/arch/arm/boot/dts/vf500-colibri-eval-v3.dts
@@ -15,3 +15,7 @@
 	model = "Toradex Colibri VF50 on Colibri Evaluation Board";
 	compatible = "toradex,vf500-colibri_vf50-on-eval", "toradex,vf500-colibri_vf50", "fsl,vf500";
 };
+
+&touchctrl {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/vf500-colibri.dtsi b/arch/arm/boot/dts/vf500-colibri.dtsi
index cee34a3..4807a32 100644
--- a/arch/arm/boot/dts/vf500-colibri.dtsi
+++ b/arch/arm/boot/dts/vf500-colibri.dtsi
@@ -17,4 +17,50 @@
 	memory {
 		reg = <0x80000000 0x8000000>;
 	};
+
+	touchctrl: vf50_touchctrl {
+		compatible = "toradex,vf50-touchctrl";
+		io-channels = <&adc1 0>,<&adc0 0>,
+				<&adc0 1>,<&adc1 2>;
+		xp-gpios = <&gpio0 13 GPIO_ACTIVE_LOW>;
+		xm-gpios = <&gpio2 29 GPIO_ACTIVE_HIGH>;
+		yp-gpios = <&gpio0 12 GPIO_ACTIVE_LOW>;
+		ym-gpios = <&gpio0 4 GPIO_ACTIVE_HIGH>;
+		pen-detect-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
+		pen-pullup-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "idle","default","gpios";
+		pinctrl-0 = <&pinctrl_touchctrl_idle>;
+		pinctrl-1 = <&pinctrl_touchctrl_default>;
+		pinctrl-2 = <&pinctrl_touchctrl_gpios>;
+		status = "disabled";
+	};
+};
+
+&iomuxc {
+	vf610-colibri {
+		pinctrl_touchctrl_idle: touchctrl_idle {
+			fsl,pins = <
+				VF610_PAD_PTA18__GPIO_8		0x206d
+				VF610_PAD_PTA19__GPIO_9		0x206d
+				>;
+		};
+
+		pinctrl_touchctrl_default: touchctrl_default {
+			fsl,pins = <
+				VF610_PAD_PTA18__ADC0_SE0	0x2060
+				VF610_PAD_PTA19__ADC0_SE1	0x2060
+				VF610_PAD_PTA16__ADC1_SE0	0x2060
+				VF610_PAD_PTB2__ADC1_SE2	0x2060
+				>;
+		};
+
+		pinctrl_touchctrl_gpios: touchctrl_gpios {
+			fsl,pins = <
+				VF610_PAD_PTA23__GPIO_13	0x22ed
+				VF610_PAD_PTB23__GPIO_93	0x22ed
+				VF610_PAD_PTA22__GPIO_12	0x22ed
+				VF610_PAD_PTA11__GPIO_4		0x22ed
+				>;
+		};
+	};
 };
-- 
2.4.5

[PATCH v1 3/4] touchscreen: colibri-vf50-ts: Add touchscreen support for Colibri VF50

[Sanchayan Maity]

The Colibri Vybrid VF50 module supports 4-wire touchscreens using
FETs and ADC inputs. This driver uses the IIO consumer interface
and relies on the vf610_adc driver based on the IIO framework.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 drivers/input/touchscreen/Kconfig           |  12 +
 drivers/input/touchscreen/Makefile          |   1 +
 drivers/input/touchscreen/colibri-vf50-ts.c | 466 ++++++++++++++++++++++++++++
 3 files changed, 479 insertions(+)
 create mode 100644 drivers/input/touchscreen/colibri-vf50-ts.c

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 80f6386..c243914 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -1027,4 +1027,16 @@ config TOUCHSCREEN_ZFORCE
 	  To compile this driver as a module, choose M here: the
 	  module will be called zforce_ts.
 
+config TOUCHSCREEN_COLIBRI_VF50
+	tristate "Toradex Colibri on board touchscreen driver"
+	depends on IIO && VF610_ADC
+	help
+	  Say Y here if you have a Colibri VF50 and plan to use
+	  the on-board provided 4-wire touchscreen driver.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called colibri_vf50_ts.
+
 endif
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 44deea7..93746a0 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -84,3 +84,4 @@ obj-$(CONFIG_TOUCHSCREEN_W90X900)	+= w90p910_ts.o
 obj-$(CONFIG_TOUCHSCREEN_SX8654)	+= sx8654.o
 obj-$(CONFIG_TOUCHSCREEN_TPS6507X)	+= tps6507x-ts.o
 obj-$(CONFIG_TOUCHSCREEN_ZFORCE)	+= zforce_ts.o
+obj-$(CONFIG_TOUCHSCREEN_COLIBRI_VF50)	+= colibri-vf50-ts.o
diff --git a/drivers/input/touchscreen/colibri-vf50-ts.c b/drivers/input/touchscreen/colibri-vf50-ts.c
new file mode 100644
index 0000000..4e884f5
--- /dev/null
+++ b/drivers/input/touchscreen/colibri-vf50-ts.c
@@ -0,0 +1,466 @@
+/* Copyright 2015 Toradex AG
+ *
+ * Toradex Colibri VF50 Touchscreen driver
+ *
+ * Originally authored by Stefan Agner for 3.0 kernel
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <dt-bindings/gpio/gpio.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/iio/consumer.h>
+#include <linux/iio/types.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#define DRIVER_NAME "colibri-vf50-ts"
+#define DRV_VERSION "1.0"
+
+#define VF_ADC_MAX ((1 << 12) - 1)
+
+#define COLI_TOUCH_MIN_DELAY_US 1000
+#define COLI_TOUCH_MAX_DELAY_US 2000
+
+static int min_pressure = 200;
+
+struct vf50_touch_device {
+	struct platform_device	*pdev;
+	struct input_dev	*ts_input;
+	struct workqueue_struct *ts_workqueue;
+	struct work_struct	ts_work;
+	struct iio_channel	*channels;
+	struct gpio_desc *gpio_xp;
+	struct gpio_desc *gpio_xm;
+	struct gpio_desc *gpio_yp;
+	struct gpio_desc *gpio_ym;
+	struct gpio_desc *gpio_pen_detect;
+	struct gpio_desc *gpio_pen_detect_pullup;
+	int pen_irq;
+	bool stop_touchscreen;
+};
+
+/*
+ * Enables given plates and measures touch parameters using ADC
+ */
+static int adc_ts_measure(struct iio_channel *channel,
+		  struct gpio_desc *plate_p, struct gpio_desc *plate_m)
+{
+	int i, value = 0, val = 0;
+	int ret;
+
+	gpiod_set_value(plate_p, 1);
+	gpiod_set_value(plate_m, 1);
+
+	/* Use hrtimer sleep since msleep sleeps 10ms+ */
+	usleep_range(COLI_TOUCH_MIN_DELAY_US, COLI_TOUCH_MAX_DELAY_US);
+
+	for (i = 0; i < 5; i++) {
+		ret = iio_read_channel_raw(channel, &val);
+		if (ret < 0)
+			return -EINVAL;
+
+		value += val;
+	}
+
+	value /= 5;
+
+	gpiod_set_value(plate_p, 0);
+	gpiod_set_value(plate_m, 0);
+
+	return value;
+}
+
+/*
+ * Enable touch detection using falling edge detection on XM
+ */
+static void vf50_ts_enable_touch_detection(struct vf50_touch_device *vf50_ts)
+{
+	/* Enable plate YM (needs to be strong GND, high active) */
+	gpiod_set_value(vf50_ts->gpio_ym, 1);
+
+	/* Let the platform mux to GPIO in order to enable Pull-Up on GPIO */
+	pinctrl_pm_select_idle_state(&vf50_ts->pdev->dev);
+}
+
+/*
+ * ADC touch screen sampling worker function
+ */
+static void vf50_ts_work(struct work_struct *ts_work)
+{
+	struct vf50_touch_device *vf50_ts = container_of(ts_work,
+				struct vf50_touch_device, ts_work);
+	struct device *dev = &vf50_ts->pdev->dev;
+	int val_x, val_y, val_z1, val_z2, val_p = 0;
+	bool discard_val_on_start = true;
+
+	while (!vf50_ts->stop_touchscreen) {
+		/* X-Direction */
+		val_x = adc_ts_measure(&vf50_ts->channels[0],
+				vf50_ts->gpio_xp, vf50_ts->gpio_xm);
+		if (val_x < 0)
+			continue;
+
+		/* Y-Direction */
+		val_y = adc_ts_measure(&vf50_ts->channels[1],
+				vf50_ts->gpio_yp, vf50_ts->gpio_ym);
+		if (val_y < 0)
+			continue;
+
+		/*
+		 * Touch pressure
+		 * Measure on XP/YM
+		 */
+		val_z1 = adc_ts_measure(&vf50_ts->channels[2],
+				vf50_ts->gpio_yp, vf50_ts->gpio_xm);
+		if (val_z1 < 0)
+			continue;
+		val_z2 = adc_ts_measure(&vf50_ts->channels[3],
+				vf50_ts->gpio_yp, vf50_ts->gpio_xm);
+		if (val_z2 < 0)
+			continue;
+
+		/*
+		 * According to datasheet of our touchscreen,
+		 * resistance on X axis is 400~1200..
+		 */
+
+		/* Validate signal (avoid calculation using noise) */
+		if (val_z1 > 64 && val_x > 64) {
+			/*
+			 * Calculate resistance between the plates
+			 * lower resistance means higher pressure
+			 */
+			int r_x = (1000 * val_x) / VF_ADC_MAX;
+
+			val_p = (r_x * val_z2) / val_z1 - r_x;
+
+		} else {
+			val_p = 2000;
+		}
+
+		val_p = 2000 - val_p;
+		dev_dbg(dev, "Measured values: x: %d, y: %d, z1: %d, z2: %d, "
+			"p: %d\n", val_x, val_y, val_z1, val_z2, val_p);
+
+		/*
+		 * If touch pressure is too low, stop measuring and reenable
+		 * touch detection
+		 */
+		if (val_p < min_pressure || val_p > 2000)
+			break;
+
+		/*
+		 * The pressure may not be enough for the first x and the
+		 * second y measurement, but, the pressure is ok when the
+		 * driver is doing the third and fourth measurement. To
+		 * take care of this, we drop the first measurement always.
+		 */
+		if (discard_val_on_start) {
+			discard_val_on_start = false;
+		} else {
+			/*
+			 * Report touch position and sleep for
+			 * next measurement
+			 */
+			input_report_abs(vf50_ts->ts_input,
+					ABS_X, VF_ADC_MAX - val_x);
+			input_report_abs(vf50_ts->ts_input,
+					ABS_Y, VF_ADC_MAX - val_y);
+			input_report_abs(vf50_ts->ts_input,
+					ABS_PRESSURE, val_p);
+			input_report_key(vf50_ts->ts_input, BTN_TOUCH, 1);
+			input_sync(vf50_ts->ts_input);
+		}
+
+		msleep(10);
+	}
+
+	/* Report no more touch, reenable touch detection */
+	input_report_abs(vf50_ts->ts_input, ABS_PRESSURE, 0);
+	input_report_key(vf50_ts->ts_input, BTN_TOUCH, 0);
+	input_sync(vf50_ts->ts_input);
+
+	/* Wait the pull-up to be stable on high */
+	vf50_ts_enable_touch_detection(vf50_ts);
+	msleep(10);
+
+	/* Reenable IRQ to detect touch */
+	enable_irq(vf50_ts->pen_irq);
+
+	dev_dbg(dev, "Reenabled touch detection interrupt\n");
+}
+
+static irqreturn_t vf50_tc_touched(int irq, void *dev_id)
+{
+	struct vf50_touch_device *vf50_ts = (struct vf50_touch_device *)dev_id;
+	struct device *dev = &vf50_ts->pdev->dev;
+
+	dev_dbg(dev, "Touch detected, start worker thread\n");
+
+	/* Stop IRQ */
+	disable_irq_nosync(irq);
+
+	/* Disable the touch detection plates */
+	gpiod_set_value(vf50_ts->gpio_ym, 0);
+
+	/* Let the platform mux to GPIO in order to enable Pull-Up on GPIO */
+	pinctrl_pm_select_default_state(dev);
+
+	/* Start worker thread */
+	queue_work(vf50_ts->ts_workqueue, &vf50_ts->ts_work);
+
+	return IRQ_HANDLED;
+}
+
+static int vf50_ts_open(struct input_dev *dev_input)
+{
+	int ret;
+	struct vf50_touch_device *touchdev = input_get_drvdata(dev_input);
+	struct device *dev = &touchdev->pdev->dev;
+
+	dev_dbg(dev, "Input device %s opened, starting touch detection\n",
+			dev_input->name);
+
+	touchdev->stop_touchscreen = false;
+
+	ret = gpiod_direction_output(touchdev->gpio_xp, 0);
+	if (ret) {
+		dev_err(dev, "Could not set gpio xp as output %d\n", ret);
+		return ret;
+	}
+
+	ret = gpiod_direction_output(touchdev->gpio_xm, 0);
+	if (ret) {
+		dev_err(dev, "Could not set gpio xm as output %d\n", ret);
+		return ret;
+	}
+
+	ret = gpiod_direction_output(touchdev->gpio_yp, 0);
+	if (ret) {
+		dev_err(dev, "Could not set gpio yp as output %d\n", ret);
+		return ret;
+	}
+
+	ret = gpiod_direction_output(touchdev->gpio_ym, 0);
+	if (ret) {
+		dev_err(dev, "Could not set gpio ym as output %d\n", ret);
+		return ret;
+	}
+
+	ret = gpiod_direction_input(touchdev->gpio_pen_detect);
+	if (ret) {
+		dev_err(dev,
+			"Could not set gpio pen detect as input %d\n", ret);
+		return ret;
+	}
+
+	ret = gpiod_direction_input(touchdev->gpio_pen_detect_pullup);
+	if (ret) {
+		dev_err(dev,
+			"Could not set pen detect pullup as input %d\n", ret);
+		return ret;
+	}
+
+	/* Mux detection before request IRQ, wait for pull-up to settle */
+	vf50_ts_enable_touch_detection(touchdev);
+	msleep(10);
+
+	touchdev->pen_irq = gpiod_to_irq(touchdev->gpio_pen_detect);
+	if (touchdev->pen_irq < 0) {
+		dev_err(dev, "Unable to get IRQ for GPIO\n");
+		return touchdev->pen_irq;
+	}
+
+	ret = request_irq(touchdev->pen_irq, vf50_tc_touched,
+			IRQF_TRIGGER_FALLING, "touch detected", touchdev);
+	if (ret < 0) {
+		dev_err(dev, "Unable to request IRQ %d\n", touchdev->pen_irq);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void vf50_ts_close(struct input_dev *dev_input)
+{
+	struct vf50_touch_device *touchdev = input_get_drvdata(dev_input);
+	struct device *dev = &touchdev->pdev->dev;
+
+	free_irq(touchdev->pen_irq, touchdev);
+
+	touchdev->stop_touchscreen = true;
+
+	/* Wait until touchscreen thread finishes any possible remnants. */
+	cancel_work_sync(&touchdev->ts_work);
+
+	dev_dbg(dev, "Input device %s closed, disable touch detection\n",
+		dev_input->name);
+}
+
+static int vf50_ts_probe(struct platform_device *pdev)
+{
+	int ret = 0;
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct vf50_touch_device *touchdev;
+	struct input_dev *input;
+	struct iio_channel *channels;
+
+	if (!node) {
+		dev_err(dev, "Device does not have associated DT data\n");
+		return -EINVAL;
+	}
+
+	channels = iio_channel_get_all(dev);
+	if (IS_ERR(channels))
+		return PTR_ERR(channels);
+
+	touchdev = devm_kzalloc(dev, sizeof(*touchdev), GFP_KERNEL);
+	if (touchdev == NULL) {
+		ret = -ENOMEM;
+		goto error_release_channels;
+	}
+
+	input = devm_input_allocate_device(dev);
+	if (!input) {
+		dev_err(dev, "Failed to allocate TS input device\n");
+		ret = -ENOMEM;
+		goto error_release_channels;
+	}
+
+	platform_set_drvdata(pdev, touchdev);
+
+	touchdev->pdev = pdev;
+	touchdev->channels = channels;
+
+	input->name = DRIVER_NAME;
+	input->id.bustype = BUS_HOST;
+	input->dev.parent = dev;
+	input->open = vf50_ts_open;
+	input->close = vf50_ts_close;
+
+	_set_bit(EV_ABS, input->evbit);
+	_set_bit(EV_KEY, input->evbit);
+	_set_bit(BTN_TOUCH, input->keybit);
+	input_set_abs_params(input, ABS_X, 0, VF_ADC_MAX, 0, 0);
+	input_set_abs_params(input, ABS_Y, 0, VF_ADC_MAX, 0, 0);
+	input_set_abs_params(input, ABS_PRESSURE, 0, VF_ADC_MAX, 0, 0);
+
+	touchdev->ts_input = input;
+	input_set_drvdata(input, touchdev);
+	ret = input_register_device(input);
+	if (ret) {
+		dev_err(dev, "Failed to register input device\n");
+		goto error_release_channels;
+	}
+
+	touchdev->gpio_xp = devm_gpiod_get(dev, "xp");
+	if (IS_ERR(touchdev->gpio_xp)) {
+		ret = PTR_ERR(touchdev->gpio_xp);
+		dev_err(dev, "Could not get gpio_xp %d\n", ret);
+		goto error_release_channels;
+	}
+
+	touchdev->gpio_xm = devm_gpiod_get(dev, "xm");
+	if (IS_ERR(touchdev->gpio_xm)) {
+		ret = PTR_ERR(touchdev->gpio_xm);
+		dev_err(dev, "Could not get gpio_xm %d\n", ret);
+		goto error_release_channels;
+	}
+
+	touchdev->gpio_yp = devm_gpiod_get(dev, "yp");
+	if (IS_ERR(touchdev->gpio_yp)) {
+		ret = PTR_ERR(touchdev->gpio_yp);
+		dev_err(dev, "Could not get gpio_yp %d\n", ret);
+		goto error_release_channels;
+	}
+
+	touchdev->gpio_ym = devm_gpiod_get(dev, "ym");
+	if (IS_ERR(touchdev->gpio_ym)) {
+		ret = PTR_ERR(touchdev->gpio_ym);
+		dev_err(dev, "Could not get gpio_ym %d\n", ret);
+		goto error_release_channels;
+	}
+
+	touchdev->gpio_pen_detect = devm_gpiod_get(dev, "pen-detect");
+	if (IS_ERR(touchdev->gpio_pen_detect)) {
+		ret = PTR_ERR(touchdev->gpio_pen_detect);
+		dev_err(dev, "Could not get gpio_pen_detect %d\n", ret);
+		goto error_release_channels;
+	}
+
+	touchdev->gpio_pen_detect_pullup = devm_gpiod_get(dev, "pen-pullup");
+	if (IS_ERR(touchdev->gpio_pen_detect_pullup)) {
+		ret = PTR_ERR(touchdev->gpio_pen_detect_pullup);
+		dev_err(dev, "Could not get gpio_pen_detect_pullup %d\n", ret);
+		goto error_release_channels;
+	}
+
+	/* Create workqueue for ADC sampling and calculation */
+	INIT_WORK(&touchdev->ts_work, vf50_ts_work);
+	touchdev->ts_workqueue = create_singlethread_workqueue("vf50-ts-touch");
+
+	if (!touchdev->ts_workqueue) {
+		ret = PTR_ERR(touchdev->ts_workqueue);
+		dev_err(dev,
+			"Failed creating vf50-ts-touch workqueue %d\n", ret);
+		goto error_release_channels;
+	}
+
+	dev_info(dev, "Attached colibri-vf50-ts driver successfully\n");
+
+	return 0;
+
+error_release_channels:
+	iio_channel_release_all(channels);
+	return ret;
+}
+
+static int vf50_ts_remove(struct platform_device *pdev)
+{
+	struct vf50_touch_device *touchdev = platform_get_drvdata(pdev);
+
+	destroy_workqueue(touchdev->ts_workqueue);
+	iio_channel_release_all(touchdev->channels);
+
+	return 0;
+}
+
+static const struct of_device_id vf50_touch_of_match[] = {
+	{ .compatible = "toradex,vf50-touchctrl", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, vf50_touch_of_match);
+
+static struct platform_driver __refdata vf50_touch_driver = {
+	.driver = {
+		.name = "toradex,vf50_touchctrl",
+		.owner = THIS_MODULE,
+		.of_match_table = vf50_touch_of_match,
+	},
+	.probe = vf50_ts_probe,
+	.remove = vf50_ts_remove,
+	.prevent_deferred_probe = false,
+};
+
+module_platform_driver(vf50_touch_driver);
+
+module_param(min_pressure, int, 0600);
+MODULE_PARM_DESC(min_pressure, "Minimum pressure for touch detection");
+MODULE_AUTHOR("Sanchayan Maity");
+MODULE_DESCRIPTION("Colibri VF50 Touchscreen driver");
+MODULE_LICENSE("GPL v2");
+MODULE_VERSION(DRV_VERSION);
-- 
2.4.5

[PATCH v1 4/4] input: Add DT binding documentation for Colibri VF50 touchscreen

[Sanchayan Maity]

This adds device tree binding documentation for the Colibri VF50
touchscreen driver.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 .../bindings/input/touchscreen/colibri-vf50-ts.txt | 34 ++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/colibri-vf50-ts.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/colibri-vf50-ts.txt b/Documentation/devicetree/bindings/input/touchscreen/colibri-vf50-ts.txt
new file mode 100644
index 0000000..caad52a
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/colibri-vf50-ts.txt
@@ -0,0 +1,34 @@
+* Toradex Colibri VF50 Touchscreen driver
+
+Required Properties:
+- compatible must be toradex,vf50-touchctrl
+- io-channels: adc channels being used by the Colibri VF50 module
+- xp-gpios: FET gate driver for input of X+
+- xm-gpios: FET gate driver for input of X-
+- yp-gpios: FET gate driver for input of Y+
+- ym-gpios: FET gate driver for input of Y-
+- pen-detect-gpios: GPIO for pen detect irq
+- pen-pullup-gpios: GPIO for pen pullup
+- pinctrl-names: "idle", "default", "gpios"
+- pinctrl-0: pinctrl node for idle state gpio pinmux
+- pinctrl-1: pinctrl node for touch detection state pinmux
+- pinctrl-2: pinctrl node for gpios functioning as FET gate drivers
+
+Example:
+
+	touchctrl: vf50_touchctrl {
+		compatible = "toradex,vf50-touchctrl";
+		io-channels = <&adc1 0>,<&adc0 0>,
+				<&adc0 1>,<&adc1 2>;
+		xp-gpios = <&gpio0 13 GPIO_ACTIVE_LOW>;
+		xm-gpios = <&gpio2 29 GPIO_ACTIVE_HIGH>;
+		yp-gpios = <&gpio0 12 GPIO_ACTIVE_LOW>;
+		ym-gpios = <&gpio0 4 GPIO_ACTIVE_HIGH>;
+		pen-detect-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
+		pen-pullup-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "idle","default","gpios";
+		pinctrl-0 = <&pinctrl_touchctrl_idle>;
+		pinctrl-1 = <&pinctrl_touchctrl_default>;
+		pinctrl-2 = <&pinctrl_touchctrl_gpios>;
+		status = "disabled";
+	};
-- 
2.4.5

Re: [PATCH v1 1/4] ARM: dts: vfxxx: Add io-channel-cells property for ADC node

[Stefan Agner]

On 2015-06-30 08:54, Sanchayan Maity wrote:
> This commit adds io-channel-cells property to the ADC node. This
> property is required in order for an IIO consumer driver to work.
> Especially required for Colibri VF50, as the touchscreen driver
> uses ADC channels with the ADC driver based on IIO framework.
> 
> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> ---
>  arch/arm/boot/dts/vfxxx.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
> index 2c20f3f..f7d8fb6 100644
> --- a/arch/arm/boot/dts/vfxxx.dtsi
> +++ b/arch/arm/boot/dts/vfxxx.dtsi
> @@ -227,6 +227,7 @@
>  				clocks = <&clks VF610_CLK_ADC0>;
>  				clock-names = "adc";
>  				status = "disabled";
> +				#io-channel-cells = <1>;
>  			};
>  
>  			tcon0: tcon@4003d000 {
> @@ -461,6 +462,7 @@
>  				clocks = <&clks VF610_CLK_ADC1>;
>  				clock-names = "adc";
>  				status = "disabled";
> +				#io-channel-cells = <1>;
>  			};
>  
>  			esdhc1: esdhc@400b2000 {

Minor nitpick: can you put the new property one line up? That way it
would be sorted alphabetically and status is conveniently still at the
end.

Otherwise:

Acked-by: Stefan Agner <stefan@agner.ch>

--
Stefan

Re: [PATCH v1 2/4] ARM: dts: vf500-colibri: Add device tree node for touchscreen support

[Stefan Agner]

On 2015-06-30 08:54, Sanchayan Maity wrote:
> Add device tree node for touchscreen support on Colibri VF50. The
> touchscreen functionality on VF50 uses the ADC channels of Vybrid
> and some GPIOs. Also add pinctrl nodes for proper pinmux.
> 
> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> ---
>  arch/arm/boot/dts/vf500-colibri-eval-v3.dts |  4 +++
>  arch/arm/boot/dts/vf500-colibri.dtsi        | 46 +++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/vf500-colibri-eval-v3.dts
> b/arch/arm/boot/dts/vf500-colibri-eval-v3.dts
> index 7fc782c..c5efb57 100644
> --- a/arch/arm/boot/dts/vf500-colibri-eval-v3.dts
> +++ b/arch/arm/boot/dts/vf500-colibri-eval-v3.dts
> @@ -15,3 +15,7 @@
>  	model = "Toradex Colibri VF50 on Colibri Evaluation Board";
>  	compatible = "toradex,vf500-colibri_vf50-on-eval",
> "toradex,vf500-colibri_vf50", "fsl,vf500";
>  };
> +
> +&touchctrl {
> +	status = "okay";
> +};
> diff --git a/arch/arm/boot/dts/vf500-colibri.dtsi
> b/arch/arm/boot/dts/vf500-colibri.dtsi
> index cee34a3..4807a32 100644
> --- a/arch/arm/boot/dts/vf500-colibri.dtsi
> +++ b/arch/arm/boot/dts/vf500-colibri.dtsi
> @@ -17,4 +17,50 @@
>  	memory {
>  		reg = <0x80000000 0x8000000>;
>  	};
> +
> +	touchctrl: vf50_touchctrl {
> +		compatible = "toradex,vf50-touchctrl";
> +		io-channels = <&adc1 0>,<&adc0 0>,
> +				<&adc0 1>,<&adc1 2>;
> +		xp-gpios = <&gpio0 13 GPIO_ACTIVE_LOW>;
> +		xm-gpios = <&gpio2 29 GPIO_ACTIVE_HIGH>;
> +		yp-gpios = <&gpio0 12 GPIO_ACTIVE_LOW>;
> +		ym-gpios = <&gpio0 4 GPIO_ACTIVE_HIGH>;
> +		pen-detect-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
> +		pen-pullup-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
> +		pinctrl-names = "idle","default","gpios";
> +		pinctrl-0 = <&pinctrl_touchctrl_idle>;
> +		pinctrl-1 = <&pinctrl_touchctrl_default>;
> +		pinctrl-2 = <&pinctrl_touchctrl_gpios>;
> +		status = "disabled";
> +	};
> +};
> +
> +&iomuxc {
> +	vf610-colibri {
> +		pinctrl_touchctrl_idle: touchctrl_idle {
> +			fsl,pins = <
> +				VF610_PAD_PTA18__GPIO_8		0x206d

I think the detect pin should be in Hi-Z, not be pulled up. Probably the
resistance over the plates is much smaller on pen down, hence it doesn't
matter too much.

> +				VF610_PAD_PTA19__GPIO_9		0x206d
> +				>;
> +		};
> +
> +		pinctrl_touchctrl_default: touchctrl_default {
> +			fsl,pins = <
> +				VF610_PAD_PTA18__ADC0_SE0	0x2060
> +				VF610_PAD_PTA19__ADC0_SE1	0x2060
> +				VF610_PAD_PTA16__ADC1_SE0	0x2060
> +				VF610_PAD_PTB2__ADC1_SE2	0x2060
> +				>;
> +		};
> +
> +		pinctrl_touchctrl_gpios: touchctrl_gpios {
> +			fsl,pins = <
> +				VF610_PAD_PTA23__GPIO_13	0x22ed
> +				VF610_PAD_PTB23__GPIO_93	0x22ed
> +				VF610_PAD_PTA22__GPIO_12	0x22ed
> +				VF610_PAD_PTA11__GPIO_4		0x22ed
> +				>;
> +		};

Hm, those have pull-ups on too, since you set them as output in the
drivers open function, I don't think this is necessary.

> +	};
>  };

--
Stefan

Re: [PATCH v1 3/4] touchscreen: colibri-vf50-ts: Add touchscreen support for Colibri VF50

[Stefan Agner]

Hi Sanchayan,

I wrote the driver some while ago, so reading it today again feels like
reading someone else's code again :-)

Some minor nits below:

On 2015-06-30 08:54, Sanchayan Maity wrote:
> The Colibri Vybrid VF50 module supports 4-wire touchscreens using
> FETs and ADC inputs. This driver uses the IIO consumer interface
> and relies on the vf610_adc driver based on the IIO framework.
> 
> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> ---
>  drivers/input/touchscreen/Kconfig           |  12 +
>  drivers/input/touchscreen/Makefile          |   1 +
>  drivers/input/touchscreen/colibri-vf50-ts.c | 466 ++++++++++++++++++++++++++++
>  3 files changed, 479 insertions(+)
>  create mode 100644 drivers/input/touchscreen/colibri-vf50-ts.c
> 
> diff --git a/drivers/input/touchscreen/Kconfig
> b/drivers/input/touchscreen/Kconfig
> index 80f6386..c243914 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -1027,4 +1027,16 @@ config TOUCHSCREEN_ZFORCE
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called zforce_ts.
>  
> +config TOUCHSCREEN_COLIBRI_VF50
> +	tristate "Toradex Colibri on board touchscreen driver"
> +	depends on IIO && VF610_ADC

Since we use GPIO's this also depends on GPIOLIB

> +	help
> +	  Say Y here if you have a Colibri VF50 and plan to use
> +	  the on-board provided 4-wire touchscreen driver.
> +
> +	  If unsure, say N.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called colibri_vf50_ts.
> +
<snip>
> +
> +	/* Use hrtimer sleep since msleep sleeps 10ms+ */

This comment is probably somewhat outdated. Depending on kernel
configuration probably wrong too. Just drop it.

> +	usleep_range(COLI_TOUCH_MIN_DELAY_US, COLI_TOUCH_MAX_DELAY_US);
> +
> +	for (i = 0; i < 5; i++) {
> +		ret = iio_read_channel_raw(channel, &val);
> +		if (ret < 0)
> +			return -EINVAL;
> +
> +		value += val;
> +	}
> +
> +	value /= 5;
> +
> +	gpiod_set_value(plate_p, 0);
> +	gpiod_set_value(plate_m, 0);
> +
> +	return value;
> +}
> +
> +/*
> + * Enable touch detection using falling edge detection on XM
> + */
> +static void vf50_ts_enable_touch_detection(struct vf50_touch_device *vf50_ts)
> +{
> +	/* Enable plate YM (needs to be strong GND, high active) */
> +	gpiod_set_value(vf50_ts->gpio_ym, 1);
> +
> +	/* Let the platform mux to GPIO in order to enable Pull-Up on GPIO */

The comment is somewhat configuing, since you use the pinctrl framework
to mux to the state "idle" I would write "Let the platform mux to
idle..."

> +	pinctrl_pm_select_idle_state(&vf50_ts->pdev->dev);
> +}
> +
> +/*
> + * ADC touch screen sampling worker function
> + */
> +static void vf50_ts_work(struct work_struct *ts_work)
> +{
> +	struct vf50_touch_device *vf50_ts = container_of(ts_work,
> +				struct vf50_touch_device, ts_work);
> +	struct device *dev = &vf50_ts->pdev->dev;
> +	int val_x, val_y, val_z1, val_z2, val_p = 0;
> +	bool discard_val_on_start = true;
> +
> +	while (!vf50_ts->stop_touchscreen) {
> +		/* X-Direction */
> +		val_x = adc_ts_measure(&vf50_ts->channels[0],
> +				vf50_ts->gpio_xp, vf50_ts->gpio_xm);
> +		if (val_x < 0)
> +			continue;
> +
> +		/* Y-Direction */
> +		val_y = adc_ts_measure(&vf50_ts->channels[1],
> +				vf50_ts->gpio_yp, vf50_ts->gpio_ym);
> +		if (val_y < 0)
> +			continue;
> +
> +		/*
> +		 * Touch pressure
> +		 * Measure on XP/YM
> +		 */
> +		val_z1 = adc_ts_measure(&vf50_ts->channels[2],
> +				vf50_ts->gpio_yp, vf50_ts->gpio_xm);
> +		if (val_z1 < 0)
> +			continue;
> +		val_z2 = adc_ts_measure(&vf50_ts->channels[3],
> +				vf50_ts->gpio_yp, vf50_ts->gpio_xm);
> +		if (val_z2 < 0)
> +			continue;
> +
> +		/*
> +		 * According to datasheet of our touchscreen,
> +		 * resistance on X axis is 400~1200..
> +		 */

This is somewhat vague and not really helpful, either mention the type
(I think it was an EDT) or drop that...

> +
> +		/* Validate signal (avoid calculation using noise) */
> +		if (val_z1 > 64 && val_x > 64) {
> +			/*
> +			 * Calculate resistance between the plates
> +			 * lower resistance means higher pressure
> +			 */
> +			int r_x = (1000 * val_x) / VF_ADC_MAX;
> +
> +			val_p = (r_x * val_z2) / val_z1 - r_x;
> +
> +		} else {
> +			val_p = 2000;
> +		}
> +
> +		val_p = 2000 - val_p;
> +		dev_dbg(dev, "Measured values: x: %d, y: %d, z1: %d, z2: %d, "
> +			"p: %d\n", val_x, val_y, val_z1, val_z2, val_p);
> +
> +		/*
> +		 * If touch pressure is too low, stop measuring and reenable
> +		 * touch detection
> +		 */
> +		if (val_p < min_pressure || val_p > 2000)
> +			break;
> +
> +		/*
> +		 * The pressure may not be enough for the first x and the
> +		 * second y measurement, but, the pressure is ok when the
> +		 * driver is doing the third and fourth measurement. To
> +		 * take care of this, we drop the first measurement always.
> +		 */
> +		if (discard_val_on_start) {
> +			discard_val_on_start = false;
> +		} else {
> +			/*
> +			 * Report touch position and sleep for
> +			 * next measurement
> +			 */
> +			input_report_abs(vf50_ts->ts_input,
> +					ABS_X, VF_ADC_MAX - val_x);
> +			input_report_abs(vf50_ts->ts_input,
> +					ABS_Y, VF_ADC_MAX - val_y);
> +			input_report_abs(vf50_ts->ts_input,
> +					ABS_PRESSURE, val_p);
> +			input_report_key(vf50_ts->ts_input, BTN_TOUCH, 1);
> +			input_sync(vf50_ts->ts_input);
> +		}
> +
> +		msleep(10);
> +	}
> +
> +	/* Report no more touch, reenable touch detection */
> +	input_report_abs(vf50_ts->ts_input, ABS_PRESSURE, 0);
> +	input_report_key(vf50_ts->ts_input, BTN_TOUCH, 0);
> +	input_sync(vf50_ts->ts_input);
> +
> +	/* Wait the pull-up to be stable on high */
> +	vf50_ts_enable_touch_detection(vf50_ts);

I would move that comment one line lower, since that is the place we
wait until the pin is high.

> +	msleep(10);
> +
> +	/* Reenable IRQ to detect touch */
> +	enable_irq(vf50_ts->pen_irq);
> +
> +	dev_dbg(dev, "Reenabled touch detection interrupt\n");
> +}
> +
> +static irqreturn_t vf50_tc_touched(int irq, void *dev_id)

tc?

> +{
> +	struct vf50_touch_device *vf50_ts = (struct vf50_touch_device *)dev_id;
> +	struct device *dev = &vf50_ts->pdev->dev;
> +
> +	dev_dbg(dev, "Touch detected, start worker thread\n");
> +
> +	/* Stop IRQ */

Some comments like this...

> +	disable_irq_nosync(irq);
> +
> +	/* Disable the touch detection plates */
> +	gpiod_set_value(vf50_ts->gpio_ym, 0);
> +
> +	/* Let the platform mux to GPIO in order to enable Pull-Up on GPIO */
> +	pinctrl_pm_select_default_state(dev);
> +
> +	/* Start worker thread */

or this are not really required since the function name called is
actually verbose enough.

> +	queue_work(vf50_ts->ts_workqueue, &vf50_ts->ts_work);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int vf50_ts_open(struct input_dev *dev_input)
> +{
> +	int ret;
> +	struct vf50_touch_device *touchdev = input_get_drvdata(dev_input);
> +	struct device *dev = &touchdev->pdev->dev;
> +
> +	dev_dbg(dev, "Input device %s opened, starting touch detection\n",
> +			dev_input->name);
> +
> +	touchdev->stop_touchscreen = false;
> +
> +	ret = gpiod_direction_output(touchdev->gpio_xp, 0);
> +	if (ret) {
> +		dev_err(dev, "Could not set gpio xp as output %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = gpiod_direction_output(touchdev->gpio_xm, 0);
> +	if (ret) {
> +		dev_err(dev, "Could not set gpio xm as output %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = gpiod_direction_output(touchdev->gpio_yp, 0);
> +	if (ret) {
> +		dev_err(dev, "Could not set gpio yp as output %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = gpiod_direction_output(touchdev->gpio_ym, 0);
> +	if (ret) {
> +		dev_err(dev, "Could not set gpio ym as output %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = gpiod_direction_input(touchdev->gpio_pen_detect);
> +	if (ret) {
> +		dev_err(dev,
> +			"Could not set gpio pen detect as input %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = gpiod_direction_input(touchdev->gpio_pen_detect_pullup);
> +	if (ret) {
> +		dev_err(dev,
> +			"Could not set pen detect pullup as input %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Mux detection before request IRQ, wait for pull-up to settle */
> +	vf50_ts_enable_touch_detection(touchdev);
> +	msleep(10);
> +
> +	touchdev->pen_irq = gpiod_to_irq(touchdev->gpio_pen_detect);
> +	if (touchdev->pen_irq < 0) {
> +		dev_err(dev, "Unable to get IRQ for GPIO\n");
> +		return touchdev->pen_irq;
> +	}
> +
> +	ret = request_irq(touchdev->pen_irq, vf50_tc_touched,
> +			IRQF_TRIGGER_FALLING, "touch detected", touchdev);
> +	if (ret < 0) {
> +		dev_err(dev, "Unable to request IRQ %d\n", touchdev->pen_irq);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void vf50_ts_close(struct input_dev *dev_input)
> +{
> +	struct vf50_touch_device *touchdev = input_get_drvdata(dev_input);
> +	struct device *dev = &touchdev->pdev->dev;
> +
> +	free_irq(touchdev->pen_irq, touchdev);
> +
> +	touchdev->stop_touchscreen = true;
> +
> +	/* Wait until touchscreen thread finishes any possible remnants. */
> +	cancel_work_sync(&touchdev->ts_work);
> +
> +	dev_dbg(dev, "Input device %s closed, disable touch detection\n",
> +		dev_input->name);
> +}
> +
> +static int vf50_ts_probe(struct platform_device *pdev)
> +{
> +	int ret = 0;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
> +	struct vf50_touch_device *touchdev;
> +	struct input_dev *input;
> +	struct iio_channel *channels;
> +
> +	if (!node) {
> +		dev_err(dev, "Device does not have associated DT data\n");
> +		return -EINVAL;
> +	}
> +
> +	channels = iio_channel_get_all(dev);
> +	if (IS_ERR(channels))
> +		return PTR_ERR(channels);
> +
> +	touchdev = devm_kzalloc(dev, sizeof(*touchdev), GFP_KERNEL);
> +	if (touchdev == NULL) {
> +		ret = -ENOMEM;
> +		goto error_release_channels;
> +	}
> +
> +	input = devm_input_allocate_device(dev);
> +	if (!input) {
> +		dev_err(dev, "Failed to allocate TS input device\n");
> +		ret = -ENOMEM;
> +		goto error_release_channels;
> +	}
> +
> +	platform_set_drvdata(pdev, touchdev);
> +
> +	touchdev->pdev = pdev;
> +	touchdev->channels = channels;
> +
> +	input->name = DRIVER_NAME;
> +	input->id.bustype = BUS_HOST;
> +	input->dev.parent = dev;
> +	input->open = vf50_ts_open;
> +	input->close = vf50_ts_close;
> +
> +	_set_bit(EV_ABS, input->evbit);
> +	_set_bit(EV_KEY, input->evbit);
> +	_set_bit(BTN_TOUCH, input->keybit);
> +	input_set_abs_params(input, ABS_X, 0, VF_ADC_MAX, 0, 0);
> +	input_set_abs_params(input, ABS_Y, 0, VF_ADC_MAX, 0, 0);
> +	input_set_abs_params(input, ABS_PRESSURE, 0, VF_ADC_MAX, 0, 0);
> +
> +	touchdev->ts_input = input;
> +	input_set_drvdata(input, touchdev);
> +	ret = input_register_device(input);
> +	if (ret) {
> +		dev_err(dev, "Failed to register input device\n");
> +		goto error_release_channels;
> +	}
> +
> +	touchdev->gpio_xp = devm_gpiod_get(dev, "xp");
> +	if (IS_ERR(touchdev->gpio_xp)) {
> +		ret = PTR_ERR(touchdev->gpio_xp);
> +		dev_err(dev, "Could not get gpio_xp %d\n", ret);
> +		goto error_release_channels;
> +	}
> +
> +	touchdev->gpio_xm = devm_gpiod_get(dev, "xm");
> +	if (IS_ERR(touchdev->gpio_xm)) {
> +		ret = PTR_ERR(touchdev->gpio_xm);
> +		dev_err(dev, "Could not get gpio_xm %d\n", ret);
> +		goto error_release_channels;
> +	}
> +
> +	touchdev->gpio_yp = devm_gpiod_get(dev, "yp");
> +	if (IS_ERR(touchdev->gpio_yp)) {
> +		ret = PTR_ERR(touchdev->gpio_yp);
> +		dev_err(dev, "Could not get gpio_yp %d\n", ret);
> +		goto error_release_channels;
> +	}
> +
> +	touchdev->gpio_ym = devm_gpiod_get(dev, "ym");
> +	if (IS_ERR(touchdev->gpio_ym)) {
> +		ret = PTR_ERR(touchdev->gpio_ym);
> +		dev_err(dev, "Could not get gpio_ym %d\n", ret);
> +		goto error_release_channels;
> +	}
> +
> +	touchdev->gpio_pen_detect = devm_gpiod_get(dev, "pen-detect");
> +	if (IS_ERR(touchdev->gpio_pen_detect)) {
> +		ret = PTR_ERR(touchdev->gpio_pen_detect);
> +		dev_err(dev, "Could not get gpio_pen_detect %d\n", ret);
> +		goto error_release_channels;
> +	}
> +
> +	touchdev->gpio_pen_detect_pullup = devm_gpiod_get(dev, "pen-pullup");
> +	if (IS_ERR(touchdev->gpio_pen_detect_pullup)) {
> +		ret = PTR_ERR(touchdev->gpio_pen_detect_pullup);
> +		dev_err(dev, "Could not get gpio_pen_detect_pullup %d\n", ret);
> +		goto error_release_channels;
> +	}

Quite some repetition here. Maybe a inline function vf50_ts_get_gpio?

static inline int vf50_ts_get_gpio(...)
{
...
		dev_err(dev, "Could not get gpio-%s: %d\n", gpio, ret);
...
}


if (ret = vf50_ts_get_gpio(&touchdev->gpio_pen_detect_pullup,
"pen-pullup")
	goto error_release_channels;

This would also save some message strings and we can reuse the GPIO name
string.

I think it should also be possible to move the channel initialization
below the GPIO's, hence saving the goto...

We could do something similar above in the open function, although its
not that huge of a deal there. But when we do here, I would probably do
it above to, just for the sake of consistency.

> +
> +	/* Create workqueue for ADC sampling and calculation */
> +	INIT_WORK(&touchdev->ts_work, vf50_ts_work);
> +	touchdev->ts_workqueue = create_singlethread_workqueue("vf50-ts-touch");
> +
> +	if (!touchdev->ts_workqueue) {
> +		ret = PTR_ERR(touchdev->ts_workqueue);
> +		dev_err(dev,
> +			"Failed creating vf50-ts-touch workqueue %d\n", ret);
> +		goto error_release_channels;
> +	}
> +
> +	dev_info(dev, "Attached colibri-vf50-ts driver successfully\n");
> +
> +	return 0;
> +
> +error_release_channels:
> +	iio_channel_release_all(channels);
> +	return ret;
> +}
> +
> +static int vf50_ts_remove(struct platform_device *pdev)
> +{
> +	struct vf50_touch_device *touchdev = platform_get_drvdata(pdev);
> +
> +	destroy_workqueue(touchdev->ts_workqueue);
> +	iio_channel_release_all(touchdev->channels);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id vf50_touch_of_match[] = {
> +	{ .compatible = "toradex,vf50-touchctrl", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, vf50_touch_of_match);
> +
> +static struct platform_driver __refdata vf50_touch_driver = {
> +	.driver = {
> +		.name = "toradex,vf50_touchctrl",
> +		.owner = THIS_MODULE,

I think the owner is not needed any more.

--
Stefan

> +		.of_match_table = vf50_touch_of_match,
> +	},
> +	.probe = vf50_ts_probe,
> +	.remove = vf50_ts_remove,
> +	.prevent_deferred_probe = false,
> +};
> +
> +module_platform_driver(vf50_touch_driver);
> +
> +module_param(min_pressure, int, 0600);
> +MODULE_PARM_DESC(min_pressure, "Minimum pressure for touch detection");
> +MODULE_AUTHOR("Sanchayan Maity");
> +MODULE_DESCRIPTION("Colibri VF50 Touchscreen driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_VERSION(DRV_VERSION);