Hello,
The patchset adds support for 4 wire touchscreen on Toradex Colibri
VF50 modules. Patches are tested on top of shawn's for-next branch.
Changes since v2:
1. Fix pin multiplexing for pins in idle state. Configuration of the
pen detect pull up viz. PTA19__GPIO_9 resulted in generation of pen
irq's on a continuous basis.
2. Fix pinmux of the ADC pins as per the recommended pinmux in TRM.
3. Use a threaded irq handler instead of a irq handler plus workqueue
approach.
4. Use a low level trigger with oneshot flag specifier instead of the
previous falling edge triggered irq's. This coupled with the fix in
point 1 fixes the previous continuous spurious irq generation bug.
5. Change/fix the TS measurement logic to account for the fact that
iio_channel_read_raw might actually return an error. To be more
specific use break instead of continue and take care to close the
FET's in case of channel read error.
6. Drop the first patch "Add io-channel-cells property for ADC node"
as it has already been applied.
7. Move the iio channel get call again at the start. Having it in
the end resulted in crashes sometimes when iio was not probed and
the ts device got probed and opened earlier.
Changes since v1:
1. Fix/drop comments
2. Use an inline function for multiple gpiod_get calls in probe
3. Remove the pull up in the pinmux specified in DT for touchctrl_gpios
4. Add the io-channel-cells property before status property.
5. Add GPIOLIB as dependency in the Kconfig file
Version 2 of the patchset can be found here
https://www.mail-archive.com/[email protected]/msg18090.html
Version 1 of the patchset can be found here
https://lkml.org/lkml/2015/6/30/103
Thank you very much for the feedback till now.
Regards,
Sanchayan.
Sanchayan Maity (3):
ARM: dts: vf500-colibri: Add device tree node for touchscreen support
input: Add DT binding documentation for Colibri VF50 touchscreen
touchscreen: colibri-vf50-ts: Add touchscreen support for Colibri VF50
.../bindings/input/touchscreen/colibri-vf50-ts.txt | 32 ++
arch/arm/boot/dts/vf500-colibri-eval-v3.dts | 4 +
arch/arm/boot/dts/vf500-colibri.dtsi | 45 +++
drivers/input/touchscreen/Kconfig | 12 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/colibri-vf50-ts.c | 404 +++++++++++++++++++++
6 files changed, 498 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/colibri-vf50-ts.txt
create mode 100644 drivers/input/touchscreen/colibri-vf50-ts.c
--
2.5.0
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 <[email protected]>
---
arch/arm/boot/dts/vf500-colibri-eval-v3.dts | 4 +++
arch/arm/boot/dts/vf500-colibri.dtsi | 45 +++++++++++++++++++++++++++++
2 files changed, 49 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..14c0b00 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";
};
+
+&touchscreen {
+ status = "okay";
+};
diff --git a/arch/arm/boot/dts/vf500-colibri.dtsi b/arch/arm/boot/dts/vf500-colibri.dtsi
index cee34a3..6e795f3 100644
--- a/arch/arm/boot/dts/vf500-colibri.dtsi
+++ b/arch/arm/boot/dts/vf500-colibri.dtsi
@@ -17,4 +17,49 @@
memory {
reg = <0x80000000 0x8000000>;
};
+
+ touchscreen: vf50-touchscreen {
+ compatible = "toradex,vf50-touchscreen";
+ 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>;
+ 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 0x006d
+ VF610_PAD_PTA19__GPIO_9 0x006c
+ >;
+ };
+
+ pinctrl_touchctrl_default: touchctrl_default {
+ fsl,pins = <
+ VF610_PAD_PTA18__ADC0_SE0 0x0040
+ VF610_PAD_PTA19__ADC0_SE1 0x0040
+ VF610_PAD_PTA16__ADC1_SE0 0x0040
+ VF610_PAD_PTB2__ADC1_SE2 0x0040
+ >;
+ };
+
+ pinctrl_touchctrl_gpios: touchctrl_gpios {
+ fsl,pins = <
+ VF610_PAD_PTA23__GPIO_13 0x22e9
+ VF610_PAD_PTB23__GPIO_93 0x22e9
+ VF610_PAD_PTA22__GPIO_12 0x22e9
+ VF610_PAD_PTA11__GPIO_4 0x22e9
+ >;
+ };
+ };
};
--
2.5.0
This adds device tree binding documentation for the Colibri VF50
touchscreen driver.
Signed-off-by: Sanchayan Maity <[email protected]>
---
.../bindings/input/touchscreen/colibri-vf50-ts.txt | 32 ++++++++++++++++++++++
1 file changed, 32 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..3e0efac
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/colibri-vf50-ts.txt
@@ -0,0 +1,32 @@
+* 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
+- 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>;
+ pinctrl-names = "idle","default","gpios";
+ pinctrl-0 = <&pinctrl_touchctrl_idle>;
+ pinctrl-1 = <&pinctrl_touchctrl_default>;
+ pinctrl-2 = <&pinctrl_touchctrl_gpios>;
+ status = "disabled";
+ };
--
2.5.0
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 <[email protected]>
---
drivers/input/touchscreen/Kconfig | 12 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/colibri-vf50-ts.c | 404 ++++++++++++++++++++++++++++
3 files changed, 417 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..28948ca 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 GPIOLIB && 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..d7bc91a
--- /dev/null
+++ b/drivers/input/touchscreen/colibri-vf50-ts.c
@@ -0,0 +1,404 @@
+/* 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 <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/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 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;
+ 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);
+
+ 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) {
+ value = ret;
+ goto error_iio_read;
+ }
+
+ value += val;
+ }
+
+ value /= 5;
+
+error_iio_read:
+ 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 idle state in order to enable
+ * Pull-Up on GPIO
+ */
+ pinctrl_pm_select_idle_state(&vf50_ts->pdev->dev);
+}
+
+/*
+ * ADC touch screen sampling bottom half irq handler
+ */
+static irqreturn_t vf50_ts_irq_bh(int irq, void *private)
+{
+ struct vf50_touch_device *vf50_ts = (struct vf50_touch_device *)private;
+ 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;
+
+ /* Disable the touch detection plates */
+ gpiod_set_value(vf50_ts->gpio_ym, 0);
+
+ /* Let the platform mux to default state in order to mux as ADC */
+ pinctrl_pm_select_default_state(dev);
+
+ 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)
+ break;
+
+ /* Y-Direction */
+ val_y = adc_ts_measure(&vf50_ts->channels[1],
+ vf50_ts->gpio_yp, vf50_ts->gpio_ym);
+ if (val_y < 0)
+ break;
+
+ /*
+ * 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)
+ break;
+ val_z2 = adc_ts_measure(&vf50_ts->channels[3],
+ vf50_ts->gpio_yp, vf50_ts->gpio_xm);
+ if (val_z2 < 0)
+ break;
+
+ /* 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);
+
+ vf50_ts_enable_touch_detection(vf50_ts);
+
+ /* Wait for the pull-up to be stable on high */
+ msleep(10);
+
+ 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;
+ }
+
+ /* Mux detection before request IRQ, wait for pull-up to settle */
+ vf50_ts_enable_touch_detection(touchdev);
+ msleep(10);
+
+ 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;
+
+ touchdev->stop_touchscreen = true;
+
+ dev_dbg(dev, "Input device %s closed, disable touch detection\n",
+ dev_input->name);
+}
+
+static inline int vf50_ts_get_gpiod(struct device *dev,
+ struct gpio_desc **gpio_d, const char *con_id)
+{
+ int ret;
+
+ *gpio_d = devm_gpiod_get(dev, con_id);
+ if (IS_ERR(*gpio_d)) {
+ ret = PTR_ERR(*gpio_d);
+ dev_err(dev, "Could not get gpio_%s %d\n", con_id, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int vf50_ts_probe(struct platform_device *pdev)
+{
+ struct input_dev *input;
+ struct iio_channel *channels;
+ struct device *dev = &pdev->dev;
+ struct vf50_touch_device *touchdev;
+ int ret;
+
+ 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;
+ }
+
+ ret = vf50_ts_get_gpiod(dev, &touchdev->gpio_xp, "xp");
+ if (ret)
+ goto error_release_channels;
+
+ ret = vf50_ts_get_gpiod(dev, &touchdev->gpio_xm, "xm");
+ if (ret)
+ goto error_release_channels;
+
+ ret = vf50_ts_get_gpiod(dev, &touchdev->gpio_yp, "yp");
+ if (ret)
+ goto error_release_channels;
+
+ ret = vf50_ts_get_gpiod(dev, &touchdev->gpio_ym, "ym");
+ if (ret)
+ goto error_release_channels;
+
+ ret = vf50_ts_get_gpiod(dev, &touchdev->gpio_pen_detect,
+ "pen-detect");
+ if (ret)
+ goto error_release_channels;
+
+ touchdev->pen_irq = gpiod_to_irq(touchdev->gpio_pen_detect);
+ if (touchdev->pen_irq < 0) {
+ ret = touchdev->pen_irq;
+ dev_err(dev, "Unable to get IRQ for GPIO\n");
+ goto error_release_channels;
+ }
+
+ ret = devm_request_threaded_irq(dev, touchdev->pen_irq, NULL,
+ vf50_ts_irq_bh, IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+ "vf50 touch", touchdev);
+ if (ret < 0) {
+ dev_err(dev, "Unable to request IRQ %d\n", touchdev->pen_irq);
+ goto error_release_channels;
+ }
+
+ 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);
+
+ iio_channel_release_all(touchdev->channels);
+
+ return 0;
+}
+
+static const struct of_device_id vf50_touch_of_match[] = {
+ { .compatible = "toradex,vf50-touchscreen", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, vf50_touch_of_match);
+
+static struct platform_driver vf50_touch_driver = {
+ .driver = {
+ .name = "toradex,vf50_touchctrl",
+ .of_match_table = vf50_touch_of_match,
+ },
+ .probe = vf50_ts_probe,
+ .remove = vf50_ts_remove,
+};
+
+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.5.0
Hello,
Ping?
- Sanchayan.
On 15-08-05 14:25:48, Sanchayan Maity wrote:
> Hello,
>
> The patchset adds support for 4 wire touchscreen on Toradex Colibri
> VF50 modules. Patches are tested on top of shawn's for-next branch.
>
> Changes since v2:
> 1. Fix pin multiplexing for pins in idle state. Configuration of the
> pen detect pull up viz. PTA19__GPIO_9 resulted in generation of pen
> irq's on a continuous basis.
> 2. Fix pinmux of the ADC pins as per the recommended pinmux in TRM.
> 3. Use a threaded irq handler instead of a irq handler plus workqueue
> approach.
> 4. Use a low level trigger with oneshot flag specifier instead of the
> previous falling edge triggered irq's. This coupled with the fix in
> point 1 fixes the previous continuous spurious irq generation bug.
> 5. Change/fix the TS measurement logic to account for the fact that
> iio_channel_read_raw might actually return an error. To be more
> specific use break instead of continue and take care to close the
> FET's in case of channel read error.
> 6. Drop the first patch "Add io-channel-cells property for ADC node"
> as it has already been applied.
> 7. Move the iio channel get call again at the start. Having it in
> the end resulted in crashes sometimes when iio was not probed and
> the ts device got probed and opened earlier.
>
> Changes since v1:
> 1. Fix/drop comments
> 2. Use an inline function for multiple gpiod_get calls in probe
> 3. Remove the pull up in the pinmux specified in DT for touchctrl_gpios
> 4. Add the io-channel-cells property before status property.
> 5. Add GPIOLIB as dependency in the Kconfig file
>
> Version 2 of the patchset can be found here
> https://www.mail-archive.com/[email protected]/msg18090.html
>
> Version 1 of the patchset can be found here
> https://lkml.org/lkml/2015/6/30/103
>
> Thank you very much for the feedback till now.
>
> Regards,
> Sanchayan.
>
> Sanchayan Maity (3):
> ARM: dts: vf500-colibri: Add device tree node for touchscreen support
> input: Add DT binding documentation for Colibri VF50 touchscreen
> touchscreen: colibri-vf50-ts: Add touchscreen support for Colibri VF50
>
> .../bindings/input/touchscreen/colibri-vf50-ts.txt | 32 ++
> arch/arm/boot/dts/vf500-colibri-eval-v3.dts | 4 +
> arch/arm/boot/dts/vf500-colibri.dtsi | 45 +++
> drivers/input/touchscreen/Kconfig | 12 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/colibri-vf50-ts.c | 404 +++++++++++++++++++++
> 6 files changed, 498 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/colibri-vf50-ts.txt
> create mode 100644 drivers/input/touchscreen/colibri-vf50-ts.c
>
> --
> 2.5.0
>
Hi Sanchayan,
On Wed, Aug 05, 2015 at 02:25:51PM +0530, 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 <[email protected]>
> ---
> drivers/input/touchscreen/Kconfig | 12 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/colibri-vf50-ts.c | 404 ++++++++++++++++++++++++++++
> 3 files changed, 417 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..28948ca 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 GPIOLIB && 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..d7bc91a
> --- /dev/null
> +++ b/drivers/input/touchscreen/colibri-vf50-ts.c
> @@ -0,0 +1,404 @@
> +/* 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 <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/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 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;
> + 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);
> +
> + usleep_range(COLI_TOUCH_MIN_DELAY_US, COLI_TOUCH_MAX_DELAY_US);
> +
> + for (i = 0; i < 5; i++) {
Can we have a #define for 5?
> + ret = iio_read_channel_raw(channel, &val);
> + if (ret < 0) {
> + value = ret;
> + goto error_iio_read;
> + }
> +
> + value += val;
> + }
> +
> + value /= 5;
> +
> +error_iio_read:
> + 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 idle state in order to enable
> + * Pull-Up on GPIO
> + */
> + pinctrl_pm_select_idle_state(&vf50_ts->pdev->dev);
> +}
> +
> +/*
> + * ADC touch screen sampling bottom half irq handler
> + */
> +static irqreturn_t vf50_ts_irq_bh(int irq, void *private)
> +{
> + struct vf50_touch_device *vf50_ts = (struct vf50_touch_device *)private;
> + 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;
> +
> + /* Disable the touch detection plates */
> + gpiod_set_value(vf50_ts->gpio_ym, 0);
> +
> + /* Let the platform mux to default state in order to mux as ADC */
> + pinctrl_pm_select_default_state(dev);
> +
> + 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)
> + break;
> +
> + /* Y-Direction */
> + val_y = adc_ts_measure(&vf50_ts->channels[1],
> + vf50_ts->gpio_yp, vf50_ts->gpio_ym);
> + if (val_y < 0)
> + break;
> +
> + /*
> + * 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)
> + break;
> + val_z2 = adc_ts_measure(&vf50_ts->channels[3],
> + vf50_ts->gpio_yp, vf50_ts->gpio_xm);
> + if (val_z2 < 0)
> + break;
> +
> + /* 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);
> +
> + vf50_ts_enable_touch_detection(vf50_ts);
> +
> + /* Wait for the pull-up to be stable on high */
> + msleep(10);
> +
> + 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;
> + }
Why do we do this on every open? You can configure gpios as you request
them.
> +
> + /* Mux detection before request IRQ, wait for pull-up to settle */
> + vf50_ts_enable_touch_detection(touchdev);
> + msleep(10);
> +
> + 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;
> +
> + touchdev->stop_touchscreen = true;
I think you also need:
mb();
synchronize_irq(touchdev->irq);
to make sure interrupt routine is not running past close.
> +
> + dev_dbg(dev, "Input device %s closed, disable touch detection\n",
> + dev_input->name);
> +}
> +
> +static inline int vf50_ts_get_gpiod(struct device *dev,
Drop inline, let compiler decide whether it should be inlined or not.
> + struct gpio_desc **gpio_d, const char *con_id)
> +{
> + int ret;
Personal preference: call this variable "error".
> +
> + *gpio_d = devm_gpiod_get(dev, con_id);
I think you need use GPIOD_OUT_LOW (assuming you do not use the function
for pen detect gpio, see below).
> + if (IS_ERR(*gpio_d)) {
> + ret = PTR_ERR(*gpio_d);
> + dev_err(dev, "Could not get gpio_%s %d\n", con_id, ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int vf50_ts_probe(struct platform_device *pdev)
> +{
> + struct input_dev *input;
> + struct iio_channel *channels;
> + struct device *dev = &pdev->dev;
> + struct vf50_touch_device *touchdev;
> + int ret;
Personal preference: call this variable "error".
> +
> + channels = iio_channel_get_all(dev);
> + if (IS_ERR(channels))
> + return PTR_ERR(channels);
Please either introduce devm_iio_channel_get_all() or install
iio_channel_release_all as a custom devm action (devm_add_action).
> +
> + touchdev = devm_kzalloc(dev, sizeof(*touchdev), GFP_KERNEL);
> + if (touchdev == NULL) {
if (!touchdev)
please.
> + 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_capability(dev, EV_KEY, BTN_TOUCH);
and kill _set_bit()s above.
> + 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;
> + }
> +
> + ret = vf50_ts_get_gpiod(dev, &touchdev->gpio_xp, "xp");
> + if (ret)
> + goto error_release_channels;
> +
> + ret = vf50_ts_get_gpiod(dev, &touchdev->gpio_xm, "xm");
> + if (ret)
> + goto error_release_channels;
> +
> + ret = vf50_ts_get_gpiod(dev, &touchdev->gpio_yp, "yp");
> + if (ret)
> + goto error_release_channels;
> +
> + ret = vf50_ts_get_gpiod(dev, &touchdev->gpio_ym, "ym");
> + if (ret)
> + goto error_release_channels;
> +
> + ret = vf50_ts_get_gpiod(dev, &touchdev->gpio_pen_detect,
> + "pen-detect");
> + if (ret)
> + goto error_release_channels;
> +
> + touchdev->pen_irq = gpiod_to_irq(touchdev->gpio_pen_detect);
> + if (touchdev->pen_irq < 0) {
> + ret = touchdev->pen_irq;
> + dev_err(dev, "Unable to get IRQ for GPIO\n");
> + goto error_release_channels;
> + }
I do not think you need to specify this as gpio, since you are not
reading it's state. Map it using "interrupts" property instead and do
touchdev->pen_irq = platform_get_irq(pdev, 0);
> +
> + ret = devm_request_threaded_irq(dev, touchdev->pen_irq, NULL,
> + vf50_ts_irq_bh, IRQF_TRIGGER_LOW | IRQF_ONESHOT,
Just use IRQF_ONESHOT here, rely on the platform/OF to read and set up
the trigger properly (via "interrupts" property).
> + "vf50 touch", touchdev);
> + if (ret < 0) {
> + dev_err(dev, "Unable to request IRQ %d\n", touchdev->pen_irq);
> + goto error_release_channels;
> + }
> +
> + 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);
> +
> + iio_channel_release_all(touchdev->channels);
This is wrong order, I do not think you want to release channels while
your interrupt routine still running. With custom devm action you can
avoid this and also remove whole vf50_ts_remove.
> +
> + return 0;
> +}
> +
> +static const struct of_device_id vf50_touch_of_match[] = {
> + { .compatible = "toradex,vf50-touchscreen", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, vf50_touch_of_match);
> +
> +static struct platform_driver vf50_touch_driver = {
> + .driver = {
> + .name = "toradex,vf50_touchctrl",
> + .of_match_table = vf50_touch_of_match,
> + },
> + .probe = vf50_ts_probe,
> + .remove = vf50_ts_remove,
> +};
> +
> +module_platform_driver(vf50_touch_driver);
> +
> +module_param(min_pressure, int, 0600);
> +MODULE_PARM_DESC(min_pressure, "Minimum pressure for touch detection");
Since it is my understanding this is not user preference but rather
property of device, I'd rather this been part of DT binding.
> +MODULE_AUTHOR("Sanchayan Maity");
> +MODULE_DESCRIPTION("Colibri VF50 Touchscreen driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_VERSION(DRV_VERSION);
> --
> 2.5.0
>
Thanks.
--
Dmitry
Hello Dmitry,
Will take care of all points with the next revision.
Thank you for the feedback.
- Sanchayan.
On 15-08-14 15:53:46, Dmitry Torokhov wrote:
> Hi Sanchayan,
>
> On Wed, Aug 05, 2015 at 02:25:51PM +0530, 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 <[email protected]>
> > ---
> > drivers/input/touchscreen/Kconfig | 12 +
> > drivers/input/touchscreen/Makefile | 1 +
> > drivers/input/touchscreen/colibri-vf50-ts.c | 404 ++++++++++++++++++++++++++++
> > 3 files changed, 417 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..28948ca 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 GPIOLIB && 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..d7bc91a
> > --- /dev/null
> > +++ b/drivers/input/touchscreen/colibri-vf50-ts.c
> > @@ -0,0 +1,404 @@
> > +/* 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 <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/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 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;
> > + 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);
> > +
> > + usleep_range(COLI_TOUCH_MIN_DELAY_US, COLI_TOUCH_MAX_DELAY_US);
> > +
> > + for (i = 0; i < 5; i++) {
>
> Can we have a #define for 5?
>
> > + ret = iio_read_channel_raw(channel, &val);
> > + if (ret < 0) {
> > + value = ret;
> > + goto error_iio_read;
> > + }
> > +
> > + value += val;
> > + }
> > +
> > + value /= 5;
> > +
> > +error_iio_read:
> > + 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 idle state in order to enable
> > + * Pull-Up on GPIO
> > + */
> > + pinctrl_pm_select_idle_state(&vf50_ts->pdev->dev);
> > +}
> > +
> > +/*
> > + * ADC touch screen sampling bottom half irq handler
> > + */
> > +static irqreturn_t vf50_ts_irq_bh(int irq, void *private)
> > +{
> > + struct vf50_touch_device *vf50_ts = (struct vf50_touch_device *)private;
> > + 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;
> > +
> > + /* Disable the touch detection plates */
> > + gpiod_set_value(vf50_ts->gpio_ym, 0);
> > +
> > + /* Let the platform mux to default state in order to mux as ADC */
> > + pinctrl_pm_select_default_state(dev);
> > +
> > + 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)
> > + break;
> > +
> > + /* Y-Direction */
> > + val_y = adc_ts_measure(&vf50_ts->channels[1],
> > + vf50_ts->gpio_yp, vf50_ts->gpio_ym);
> > + if (val_y < 0)
> > + break;
> > +
> > + /*
> > + * 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)
> > + break;
> > + val_z2 = adc_ts_measure(&vf50_ts->channels[3],
> > + vf50_ts->gpio_yp, vf50_ts->gpio_xm);
> > + if (val_z2 < 0)
> > + break;
> > +
> > + /* 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);
> > +
> > + vf50_ts_enable_touch_detection(vf50_ts);
> > +
> > + /* Wait for the pull-up to be stable on high */
> > + msleep(10);
> > +
> > + 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;
> > + }
>
> Why do we do this on every open? You can configure gpios as you request
> them.
>
> > +
> > + /* Mux detection before request IRQ, wait for pull-up to settle */
> > + vf50_ts_enable_touch_detection(touchdev);
> > + msleep(10);
> > +
> > + 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;
> > +
> > + touchdev->stop_touchscreen = true;
>
> I think you also need:
>
> mb();
> synchronize_irq(touchdev->irq);
>
> to make sure interrupt routine is not running past close.
>
> > +
> > + dev_dbg(dev, "Input device %s closed, disable touch detection\n",
> > + dev_input->name);
> > +}
> > +
> > +static inline int vf50_ts_get_gpiod(struct device *dev,
>
> Drop inline, let compiler decide whether it should be inlined or not.
>
> > + struct gpio_desc **gpio_d, const char *con_id)
> > +{
> > + int ret;
>
> Personal preference: call this variable "error".
>
> > +
> > + *gpio_d = devm_gpiod_get(dev, con_id);
>
> I think you need use GPIOD_OUT_LOW (assuming you do not use the function
> for pen detect gpio, see below).
>
> > + if (IS_ERR(*gpio_d)) {
> > + ret = PTR_ERR(*gpio_d);
> > + dev_err(dev, "Could not get gpio_%s %d\n", con_id, ret);
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int vf50_ts_probe(struct platform_device *pdev)
> > +{
> > + struct input_dev *input;
> > + struct iio_channel *channels;
> > + struct device *dev = &pdev->dev;
> > + struct vf50_touch_device *touchdev;
> > + int ret;
>
> Personal preference: call this variable "error".
>
>
> > +
> > + channels = iio_channel_get_all(dev);
> > + if (IS_ERR(channels))
> > + return PTR_ERR(channels);
>
>
> Please either introduce devm_iio_channel_get_all() or install
> iio_channel_release_all as a custom devm action (devm_add_action).
>
> > +
> > + touchdev = devm_kzalloc(dev, sizeof(*touchdev), GFP_KERNEL);
> > + if (touchdev == NULL) {
>
> if (!touchdev)
>
> please.
>
> > + 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_capability(dev, EV_KEY, BTN_TOUCH);
>
> and kill _set_bit()s above.
>
> > + 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;
> > + }
> > +
> > + ret = vf50_ts_get_gpiod(dev, &touchdev->gpio_xp, "xp");
> > + if (ret)
> > + goto error_release_channels;
> > +
> > + ret = vf50_ts_get_gpiod(dev, &touchdev->gpio_xm, "xm");
> > + if (ret)
> > + goto error_release_channels;
> > +
> > + ret = vf50_ts_get_gpiod(dev, &touchdev->gpio_yp, "yp");
> > + if (ret)
> > + goto error_release_channels;
> > +
> > + ret = vf50_ts_get_gpiod(dev, &touchdev->gpio_ym, "ym");
> > + if (ret)
> > + goto error_release_channels;
> > +
> > + ret = vf50_ts_get_gpiod(dev, &touchdev->gpio_pen_detect,
> > + "pen-detect");
> > + if (ret)
> > + goto error_release_channels;
> > +
> > + touchdev->pen_irq = gpiod_to_irq(touchdev->gpio_pen_detect);
> > + if (touchdev->pen_irq < 0) {
> > + ret = touchdev->pen_irq;
> > + dev_err(dev, "Unable to get IRQ for GPIO\n");
> > + goto error_release_channels;
> > + }
>
> I do not think you need to specify this as gpio, since you are not
> reading it's state. Map it using "interrupts" property instead and do
>
> touchdev->pen_irq = platform_get_irq(pdev, 0);
>
> > +
> > + ret = devm_request_threaded_irq(dev, touchdev->pen_irq, NULL,
> > + vf50_ts_irq_bh, IRQF_TRIGGER_LOW | IRQF_ONESHOT,
>
> Just use IRQF_ONESHOT here, rely on the platform/OF to read and set up
> the trigger properly (via "interrupts" property).
>
> > + "vf50 touch", touchdev);
> > + if (ret < 0) {
> > + dev_err(dev, "Unable to request IRQ %d\n", touchdev->pen_irq);
> > + goto error_release_channels;
> > + }
> > +
> > + 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);
> > +
> > + iio_channel_release_all(touchdev->channels);
>
> This is wrong order, I do not think you want to release channels while
> your interrupt routine still running. With custom devm action you can
> avoid this and also remove whole vf50_ts_remove.
>
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id vf50_touch_of_match[] = {
> > + { .compatible = "toradex,vf50-touchscreen", },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, vf50_touch_of_match);
> > +
> > +static struct platform_driver vf50_touch_driver = {
> > + .driver = {
> > + .name = "toradex,vf50_touchctrl",
> > + .of_match_table = vf50_touch_of_match,
> > + },
> > + .probe = vf50_ts_probe,
> > + .remove = vf50_ts_remove,
> > +};
> > +
> > +module_platform_driver(vf50_touch_driver);
> > +
> > +module_param(min_pressure, int, 0600);
> > +MODULE_PARM_DESC(min_pressure, "Minimum pressure for touch detection");
>
> Since it is my understanding this is not user preference but rather
> property of device, I'd rather this been part of DT binding.
>
> > +MODULE_AUTHOR("Sanchayan Maity");
> > +MODULE_DESCRIPTION("Colibri VF50 Touchscreen driver");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_VERSION(DRV_VERSION);
> > --
> > 2.5.0
> >
>
> Thanks.
>
> --
> Dmitry