2015-07-28 09:55:58

by Chen Bough

[permalink] [raw]
Subject: [PATCH v2 0/5] Add imx6ul touch screen controller support

i.MX6UL contains a touch screen controller. This patch set add imx6ul
touch screen controller driver support.

Changes for v2:
-Add property in devicetree Documentation.
-Add tsc disable code in tsc_remove function.
-Remove some redundant code.


Haibo Chen (5):
input: touchscreen: add imx6ul_tsc driver support
Documentation: Detail permitted DT properties for the imx6ul_tsc
ARM: imx_v6_v7_defconfig: enable imx6ul_tsc
ARM: dts: imx6ul.dtsi: add TSC support
ARM: dts: imx6ul-14x14-evk.dts: add tsc support

.../bindings/input/touchscreen/imx6ul_tsc.txt | 36 ++
arch/arm/boot/dts/imx6ul-14x14-evk.dts | 18 +
arch/arm/boot/dts/imx6ul.dtsi | 11 +
arch/arm/configs/imx_v6_v7_defconfig | 1 +
drivers/input/touchscreen/Kconfig | 12 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/imx6ul_tsc.c | 504 +++++++++++++++++++++
7 files changed, 583 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
create mode 100644 drivers/input/touchscreen/imx6ul_tsc.c

--
1.9.1


2015-07-28 09:56:08

by Chen Bough

[permalink] [raw]
Subject: [PATCH v2 1/5] input: touchscreen: add imx6ul_tsc driver support

Freescale i.MX6UL contains a internal touchscreen controller,
this patch add a driver to support this controller.

Signed-off-by: Haibo Chen <[email protected]>
---
drivers/input/touchscreen/Kconfig | 12 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/imx6ul_tsc.c | 504 +++++++++++++++++++++++++++++++++
3 files changed, 517 insertions(+)
create mode 100644 drivers/input/touchscreen/imx6ul_tsc.c

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 5b272ba..32c300d 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -479,6 +479,18 @@ config TOUCHSCREEN_MTOUCH
To compile this driver as a module, choose M here: the
module will be called mtouch.

+config TOUCHSCREEN_IMX6UL_TSC
+ tristate "Freescale i.MX6UL touchscreen controller"
+ depends on OF
+ help
+ Say Y here if you have a Freescale i.MX6UL, and want to
+ use the internal touchscreen controller.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ moduel will be called imx6ul_tsc.
+
config TOUCHSCREEN_INEXIO
tristate "iNexio serial touchscreens"
select SERIO
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index c85aae2..9379b32 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_TOUCHSCREEN_EGALAX) += egalax_ts.o
obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix.o
obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
+obj-$(CONFIG_TOUCHSCREEN_IMX6UL_TSC) += imx6ul_tsc.o
obj-$(CONFIG_TOUCHSCREEN_INEXIO) += inexio.o
obj-$(CONFIG_TOUCHSCREEN_INTEL_MID) += intel-mid-touch.o
obj-$(CONFIG_TOUCHSCREEN_IPROC) += bcm_iproc_tsc.o
diff --git a/drivers/input/touchscreen/imx6ul_tsc.c b/drivers/input/touchscreen/imx6ul_tsc.c
new file mode 100644
index 0000000..807f1db
--- /dev/null
+++ b/drivers/input/touchscreen/imx6ul_tsc.c
@@ -0,0 +1,504 @@
+/*
+ * Freescale i.MX6UL touchscreen controller driver
+ *
+ * Copyright (C) 2015 Freescale Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/gpio.h>
+#include <linux/input.h>
+#include <linux/slab.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/of_irq.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+
+/* ADC configuration registers field define */
+#define ADC_AIEN (0x1 << 7)
+#define ADC_CONV_DISABLE 0x1F
+#define ADC_CAL (0x1 << 7)
+#define ADC_CALF 0x2
+#define ADC_12BIT_MODE (0x2 << 2)
+#define ADC_IPG_CLK 0x00
+#define ADC_CLK_DIV_8 (0x03 << 5)
+#define ADC_SHORT_SAMPLE_MODE (0x0 << 4)
+#define ADC_HARDWARE_TRIGGER (0x1 << 13)
+#define SELECT_CHANNEL_4 0x04
+#define SELECT_CHANNEL_1 0x01
+#define DISABLE_CONVERSION_INT (0x0 << 7)
+
+/* ADC registers */
+#define REG_ADC_HC0 0x00
+#define REG_ADC_HC1 0x04
+#define REG_ADC_HC2 0x08
+#define REG_ADC_HC3 0x0C
+#define REG_ADC_HC4 0x10
+#define REG_ADC_HS 0x14
+#define REG_ADC_R0 0x18
+#define REG_ADC_CFG 0x2C
+#define REG_ADC_GC 0x30
+#define REG_ADC_GS 0x34
+
+#define ADC_TIMEOUT msecs_to_jiffies(100)
+
+/* TSC registers */
+#define REG_TSC_BASIC_SETING 0x00
+#define REG_TSC_PRE_CHARGE_TIME 0x10
+#define REG_TSC_FLOW_CONTROL 0x20
+#define REG_TSC_MEASURE_VALUE 0x30
+#define REG_TSC_INT_EN 0x40
+#define REG_TSC_INT_SIG_EN 0x50
+#define REG_TSC_INT_STATUS 0x60
+#define REG_TSC_DEBUG_MODE 0x70
+#define REG_TSC_DEBUG_MODE2 0x80
+
+/* TSC configuration registers field define */
+#define DETECT_4_WIRE_MODE (0x0 << 4)
+#define AUTO_MEASURE 0x1
+#define MEASURE_SIGNAL 0x1
+#define DETECT_SIGNAL (0x1 << 4)
+#define VALID_SIGNAL (0x1 << 8)
+#define MEASURE_INT_EN 0x1
+#define MEASURE_SIG_EN 0x1
+#define VALID_SIG_EN (0x1 << 8)
+#define DE_GLITCH_2 (0x2 << 29)
+#define START_SENSE (0x1 << 12)
+#define TSC_DISABLE (0x1 << 16)
+#define DETECT_MODE 0x2
+
+struct imx6ul_tsc {
+ struct device *dev;
+ struct input_dev *input;
+ void __iomem *tsc_regs;
+ void __iomem *adc_regs;
+ struct clk *tsc_clk;
+ struct clk *adc_clk;
+
+ int xnur_gpio;
+ int measure_delay_time;
+ int pre_charge_time;
+
+ struct completion completion;
+};
+
+/*
+ * TSC module need ADC to get the measure value. So
+ * before config TSC, we should initialize ADC module.
+ */
+static void imx6ul_adc_init(struct imx6ul_tsc *tsc)
+{
+ int adc_hc = 0;
+ int adc_gc;
+ int adc_gs;
+ int adc_cfg;
+ int timeout;
+
+ adc_cfg = readl(tsc->adc_regs + REG_ADC_CFG);
+ adc_cfg |= ADC_12BIT_MODE | ADC_IPG_CLK;
+ adc_cfg |= ADC_CLK_DIV_8 | ADC_SHORT_SAMPLE_MODE;
+ adc_cfg &= ~ADC_HARDWARE_TRIGGER;
+ writel(adc_cfg, tsc->adc_regs + REG_ADC_CFG);
+
+ /* enable calibration interrupt */
+ adc_hc |= ADC_AIEN;
+ adc_hc |= ADC_CONV_DISABLE;
+ writel(adc_hc, tsc->adc_regs + REG_ADC_HC0);
+
+ /* start ADC calibration */
+ adc_gc = readl(tsc->adc_regs + REG_ADC_GC);
+ adc_gc |= ADC_CAL;
+ writel(adc_gc, tsc->adc_regs + REG_ADC_GC);
+
+ timeout = wait_for_completion_timeout
+ (&tsc->completion, ADC_TIMEOUT);
+ if (timeout == 0)
+ dev_err(tsc->dev, "Timeout for adc calibration\n");
+
+ adc_gs = readl(tsc->adc_regs + REG_ADC_GS);
+ if (adc_gs & ADC_CALF)
+ dev_err(tsc->dev, "ADC calibration failed\n");
+
+ /* TSC need the ADC work in hardware trigger */
+ adc_cfg = readl(tsc->adc_regs + REG_ADC_CFG);
+ adc_cfg |= ADC_HARDWARE_TRIGGER;
+ writel(adc_cfg, tsc->adc_regs + REG_ADC_CFG);
+}
+
+/*
+ * This is a TSC workaround. Currently TSC misconnect two
+ * ADC channels, this function remap channel configure for
+ * hardware trigger.
+ */
+static void imx6ul_tsc_channel_config(struct imx6ul_tsc *tsc)
+{
+ int adc_hc0, adc_hc1, adc_hc2, adc_hc3, adc_hc4;
+
+ adc_hc0 = DISABLE_CONVERSION_INT;
+ writel(adc_hc0, tsc->adc_regs + REG_ADC_HC0);
+
+ adc_hc1 = DISABLE_CONVERSION_INT | SELECT_CHANNEL_4;
+ writel(adc_hc1, tsc->adc_regs + REG_ADC_HC1);
+
+ adc_hc2 = DISABLE_CONVERSION_INT;
+ writel(adc_hc2, tsc->adc_regs + REG_ADC_HC2);
+
+ adc_hc3 = DISABLE_CONVERSION_INT | SELECT_CHANNEL_1;
+ writel(adc_hc3, tsc->adc_regs + REG_ADC_HC3);
+
+ adc_hc4 = DISABLE_CONVERSION_INT;
+ writel(adc_hc4, tsc->adc_regs + REG_ADC_HC4);
+}
+
+/*
+ * TSC setting, confige the pre-charge time and measure delay time.
+ * different touch screen may need different pre-charge time and
+ * measure delay time.
+ */
+static void imx6ul_tsc_set(struct imx6ul_tsc *tsc)
+{
+ int basic_setting = 0;
+ int start;
+
+ basic_setting |= tsc->measure_delay_time << 8;
+ basic_setting |= DETECT_4_WIRE_MODE | AUTO_MEASURE;
+ writel(basic_setting, tsc->tsc_regs + REG_TSC_BASIC_SETING);
+
+ writel(DE_GLITCH_2, tsc->tsc_regs + REG_TSC_DEBUG_MODE2);
+
+ writel(tsc->pre_charge_time, tsc->tsc_regs + REG_TSC_PRE_CHARGE_TIME);
+ writel(MEASURE_INT_EN, tsc->tsc_regs + REG_TSC_INT_EN);
+ writel(MEASURE_SIG_EN | VALID_SIG_EN,
+ tsc->tsc_regs + REG_TSC_INT_SIG_EN);
+
+ /* start sense detection */
+ start = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
+ start |= START_SENSE;
+ start &= ~TSC_DISABLE;
+ writel(start, tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
+}
+
+static void imx6ul_tsc_init(struct imx6ul_tsc *tsc)
+{
+ imx6ul_adc_init(tsc);
+ imx6ul_tsc_channel_config(tsc);
+ imx6ul_tsc_set(tsc);
+}
+
+static irqreturn_t tsc_irq_fn(int irq, void *dev_id)
+{
+ struct imx6ul_tsc *tsc = (struct imx6ul_tsc *)dev_id;
+ int status;
+ int value;
+ int x, y;
+ int xnur;
+ int debug_mode2;
+ int state_machine;
+ int start;
+ unsigned long timeout;
+
+ status = readl(tsc->tsc_regs + REG_TSC_INT_STATUS);
+
+ /* write 1 to clear the bit measure-signal */
+ writel(MEASURE_SIGNAL | DETECT_SIGNAL,
+ tsc->tsc_regs + REG_TSC_INT_STATUS);
+
+ /* It's a HW self-clean bit. Set this bit and start sense detection */
+ start = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
+ start |= START_SENSE;
+ writel(start, tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
+
+ if (status & MEASURE_SIGNAL) {
+ value = readl(tsc->tsc_regs + REG_TSC_MEASURE_VALUE);
+ x = (value >> 16) & 0x0fff;
+ y = value & 0x0fff;
+
+ /*
+ * Delay some time(max 2ms), wait the pre-charge done.
+ * After the pre-change mode, TSC go into detect mode.
+ * And in detect mode, we can get the xnur gpio value.
+ * If xnur is low, this means the touch screen still
+ * be touched. If xnur is high, this means finger leave
+ * the touch screen.
+ */
+ timeout = jiffies + HZ/500;
+ do {
+ if (time_after(jiffies, timeout)) {
+ xnur = 0;
+ goto touch_event;
+ }
+ usleep_range(200, 400);
+ debug_mode2 = readl(tsc->tsc_regs + REG_TSC_DEBUG_MODE2);
+ state_machine = (debug_mode2 >> 20) & 0x7;
+ } while (state_machine != DETECT_MODE);
+ usleep_range(200, 400);
+
+ xnur = gpio_get_value(tsc->xnur_gpio);
+touch_event:
+ if (xnur == 0) {
+ input_report_key(tsc->input, BTN_TOUCH, 1);
+ input_report_abs(tsc->input, ABS_X, x);
+ input_report_abs(tsc->input, ABS_Y, y);
+ } else
+ input_report_key(tsc->input, BTN_TOUCH, 0);
+
+ input_sync(tsc->input);
+ }
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t adc_irq_fn(int irq, void *dev_id)
+{
+ struct imx6ul_tsc *tsc = (struct imx6ul_tsc *)dev_id;
+ int coco;
+ int value;
+
+ coco = readl(tsc->adc_regs + REG_ADC_HS);
+ if (coco & 0x01) {
+ value = readl(tsc->adc_regs + REG_ADC_R0);
+ complete(&tsc->completion);
+ }
+
+ return IRQ_HANDLED;
+}
+
+static const struct of_device_id imx6ul_tsc_match[] = {
+ { .compatible = "fsl,imx6ul-tsc", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx6ul_tsc_match);
+
+static int imx6ul_tsc_probe(struct platform_device *pdev)
+{
+ struct imx6ul_tsc *tsc;
+ struct resource *tsc_mem;
+ struct resource *adc_mem;
+ struct input_dev *input_dev;
+ struct device_node *np = pdev->dev.of_node;
+ int err;
+ int tsc_irq;
+ int adc_irq;
+
+ tsc = devm_kzalloc(&pdev->dev, sizeof(struct imx6ul_tsc), GFP_KERNEL);
+ input_dev = devm_input_allocate_device(&pdev->dev);
+
+ tsc->dev = &pdev->dev;
+
+ tsc->input = input_dev;
+ tsc->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+ tsc->input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
+ input_set_abs_params(tsc->input, ABS_X, 0, 0xFFF, 0, 0);
+ input_set_abs_params(tsc->input, ABS_Y, 0, 0xFFF, 0, 0);
+
+ tsc->input->name = "iMX6UL TouchScreen Controller";
+
+ tsc->xnur_gpio = of_get_named_gpio(np, "xnur-gpio", 0);
+ err = gpio_request_one(tsc->xnur_gpio, GPIOF_IN, "tsc_X-");
+ if (err) {
+ dev_err(&pdev->dev, "failed to request GPIO tsc_X-\n");
+ return err;
+ }
+
+ tsc_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ tsc->tsc_regs = devm_ioremap_resource(&pdev->dev, tsc_mem);
+ if (IS_ERR(tsc->tsc_regs)) {
+ dev_err(&pdev->dev, "failed to remap tsc memory\n");
+ err = PTR_ERR(tsc->tsc_regs);
+ return err;
+ }
+
+ adc_mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ tsc->adc_regs = devm_ioremap_resource(&pdev->dev, adc_mem);
+ if (IS_ERR(tsc->adc_regs)) {
+ dev_err(&pdev->dev, "failed to remap adc memory\n");
+ err = PTR_ERR(tsc->adc_regs);
+ return err;
+ }
+
+ tsc->tsc_clk = devm_clk_get(&pdev->dev, "tsc");
+ if (IS_ERR(tsc->tsc_clk)) {
+ dev_err(&pdev->dev, "failed getting tsc clock\n");
+ err = PTR_ERR(tsc->tsc_clk);
+ return err;
+ }
+
+ tsc->adc_clk = devm_clk_get(&pdev->dev, "adc");
+ if (IS_ERR(tsc->adc_clk)) {
+ dev_err(&pdev->dev, "failed getting adc clock\n");
+ err = PTR_ERR(tsc->adc_clk);
+ return err;
+ }
+
+ tsc_irq = platform_get_irq(pdev, 0);
+ if (tsc_irq < 0) {
+ dev_err(&pdev->dev, "no tsc irq resource?\n");
+ return tsc_irq;
+ }
+
+ adc_irq = platform_get_irq(pdev, 1);
+ if (adc_irq <= 0) {
+ dev_err(&pdev->dev, "no adc irq resource?\n");
+ return adc_irq;
+ }
+
+ err = devm_request_threaded_irq(tsc->dev, tsc_irq,
+ NULL, tsc_irq_fn,
+ IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ dev_name(&pdev->dev), tsc);
+ if (err < 0) {
+ dev_err(&pdev->dev,
+ "failed requesting tsc irq %d\n",
+ tsc_irq);
+ return err;
+ }
+
+ err = devm_request_irq(tsc->dev, adc_irq,
+ adc_irq_fn, 0,
+ dev_name(&pdev->dev), tsc);
+ if (err < 0) {
+ dev_err(&pdev->dev,
+ "failed requesting adc irq %d\n",
+ adc_irq);
+ return err;
+ }
+
+ err = of_property_read_u32(np, "measure-delay-time",
+ &tsc->measure_delay_time);
+ if (err)
+ tsc->measure_delay_time = 0xffff;
+
+ err = of_property_read_u32(np, "pre-charge-time",
+ &tsc->pre_charge_time);
+ if (err)
+ tsc->pre_charge_time = 0xfff;
+
+ init_completion(&tsc->completion);
+
+ err = clk_prepare_enable(tsc->adc_clk);
+ if (err) {
+ dev_err(&pdev->dev,
+ "Could not prepare or enable the adc clock.\n");
+ return err;
+ }
+
+ err = clk_prepare_enable(tsc->tsc_clk);
+ if (err) {
+ dev_err(&pdev->dev,
+ "Could not prepare or enable the tsc clock.\n");
+ goto error_tsc_clk_enable;
+ }
+
+ err = input_register_device(tsc->input);
+ if (err < 0) {
+ dev_err(&pdev->dev, "failed to register input device\n");
+ err = -EIO;
+ goto err_input_register;
+ }
+
+ imx6ul_tsc_init(tsc);
+
+ platform_set_drvdata(pdev, tsc);
+ return 0;
+
+err_input_register:
+ clk_disable_unprepare(tsc->tsc_clk);
+error_tsc_clk_enable:
+ clk_disable_unprepare(tsc->adc_clk);
+
+ return err;
+}
+
+static void imx6ul_tsc_disable(struct imx6ul_tsc *tsc)
+{
+ int tsc_flow;
+ int adc_cfg;
+
+ /* TSC controller enters to idle status */
+ tsc_flow = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
+ tsc_flow |= TSC_DISABLE;
+ writel(tsc_flow, tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
+
+ /* ADC controller enters to stop mode */
+ adc_cfg = readl(tsc->adc_regs + REG_ADC_HC0);
+ adc_cfg |= ADC_CONV_DISABLE;
+ writel(adc_cfg, tsc->adc_regs + REG_ADC_HC0);
+}
+
+static int imx6ul_tsc_remove(struct platform_device *pdev)
+{
+ struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
+
+ imx6ul_tsc_disable(tsc);
+
+ clk_disable_unprepare(tsc->tsc_clk);
+ clk_disable_unprepare(tsc->adc_clk);
+ input_unregister_device(tsc->input);
+ kfree(tsc);
+
+ return 0;
+}
+
+static int __maybe_unused imx6ul_tsc_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
+
+ imx6ul_tsc_disable(tsc);
+
+ clk_disable_unprepare(tsc->tsc_clk);
+ clk_disable_unprepare(tsc->adc_clk);
+
+ return 0;
+}
+
+static int __maybe_unused imx6ul_tsc_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
+ int err;
+
+ err = clk_prepare_enable(tsc->adc_clk);
+ if (err)
+ return err;
+
+ err = clk_prepare_enable(tsc->tsc_clk);
+ if (err) {
+ clk_disable_unprepare(tsc->adc_clk);
+ return err;
+ }
+
+ imx6ul_tsc_init(tsc);
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(imx6ul_tsc_pm_ops,
+ imx6ul_tsc_suspend,
+ imx6ul_tsc_resume);
+
+static struct platform_driver imx6ul_tsc_driver = {
+ .driver = {
+ .name = "imx6ul-tsc",
+ .owner = THIS_MODULE,
+ .of_match_table = imx6ul_tsc_match,
+ .pm = &imx6ul_tsc_pm_ops,
+ },
+ .probe = imx6ul_tsc_probe,
+ .remove = imx6ul_tsc_remove,
+};
+module_platform_driver(imx6ul_tsc_driver);
+
+MODULE_AUTHOR("Haibo Chen <[email protected]>");
+MODULE_DESCRIPTION("Freescale i.MX6UL Touchscreen controller driver");
+MODULE_LICENSE("GPL v2");
--
1.9.1

2015-07-28 09:56:14

by Chen Bough

[permalink] [raw]
Subject: [PATCH v2 2/5] Documentation: Detail permitted DT properties for the imx6ul_tsc

Here we apply required documentation for the imx6ul touch screen
controller driver which describe available properties and how to
use them.

Signed-off-by: Haibo Chen <[email protected]>
---
.../bindings/input/touchscreen/imx6ul_tsc.txt | 36 ++++++++++++++++++++++
1 file changed, 36 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
new file mode 100644
index 0000000..ac41c32
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
@@ -0,0 +1,36 @@
+* Freescale i.MX6UL Touch Controller
+
+Required properties:
+- compatible: must be "fsl,imx6ul-tsc".
+- reg: this touch controller address and the ADC2 address.
+- interrupts: the interrupt of this touch controller and ADC2.
+- clocks: the root clock of touch controller and ADC2.
+- clock-names; must be "tsc" and "adc".
+- xnur-gpio: the X- gpio this controller connect to.
+ This xnur-gpio returns to high once the finger leave the touch screen (The
+ last touch event the touch controller capture).
+
+Optional properties:
+- measure-delay-time: the value of measure delay time.
+ Before X-axis or Y-axis measurement, the screen need some time before
+ even potential distribution ready.
+ This value depends on the touch screen.
+- pre-charge-time: the touch screen need some time to precharge.
+ This value depends on the touch screen.
+
+Example:
+ tsc: tsc@02040000 {
+ compatible = "fsl,imx6ul-tsc";
+ reg = <0x02040000 0x4000>, <0x0219c000 0x4000>;
+ interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks IMX6UL_CLK_IPG>,
+ <&clks IMX6UL_CLK_ADC2>;
+ clock-names = "tsc", "adc";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_tsc>;
+ xnur-gpio = <&gpio1 3 GPIO_ACTIVE_HIGH>;
+ measure-delay-time = <0xfff>;
+ pre-charge-time = <0xffff>;
+ status = "okay";
+ };
--
1.9.1

2015-07-28 09:56:21

by Chen Bough

[permalink] [raw]
Subject: [PATCH v2 3/5] ARM: imx_v6_v7_defconfig: enable imx6ul_tsc

Enable imx6ul touchscreen controller

Signed-off-by: Haibo Chen <[email protected]>
---
arch/arm/configs/imx_v6_v7_defconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
index e1ba3e2..9823ddc 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -157,6 +157,7 @@ CONFIG_MOUSE_PS2=m
CONFIG_MOUSE_PS2_ELANTECH=y
CONFIG_INPUT_TOUCHSCREEN=y
CONFIG_TOUCHSCREEN_EGALAX=y
+CONFIG_TOUCHSCREEN_IMX6UL_TSC=y
CONFIG_TOUCHSCREEN_MC13783=y
CONFIG_TOUCHSCREEN_TSC2007=y
CONFIG_TOUCHSCREEN_STMPE=y
--
1.9.1

2015-07-28 09:56:31

by Chen Bough

[permalink] [raw]
Subject: [PATCH v2 4/5] ARM: dts: imx6ul.dtsi: add TSC support

Add imx6ul touchscreen controller support.

TSC module need ADC2 module to measure the touchscreen
coordinate value. This patch put TSC and ADC2 together,
make ADC2 module only be used for TSC, can't be used as
a normal ADC.

Signed-off-by: Haibo Chen <[email protected]>
---
arch/arm/boot/dts/imx6ul.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index dc0f5b4..33aac1f 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -484,6 +484,17 @@
reg = <0x02100000 0x100000>;
ranges;

+ tsc: tsc@02040000 {
+ compatible = "fsl,imx6ul-tsc";
+ reg = <0x02040000 0x4000>, <0x0219c000 0x4000>;
+ interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks IMX6UL_CLK_IPG>,
+ <&clks IMX6UL_CLK_ADC2>;
+ clock-names = "tsc", "adc";
+ status = "disabled";
+ };
+
usdhc1: usdhc@02190000 {
compatible = "fsl,imx6ul-usdhc", "fsl,imx6sx-usdhc";
reg = <0x02190000 0x4000>;
--
1.9.1

2015-07-28 09:56:38

by Chen Bough

[permalink] [raw]
Subject: [PATCH v2 5/5] ARM: dts: imx6ul-14x14-evk.dts: add tsc support

Add touch screen surpport for i.MX6UL-EVK board.

Signed-off-by: Haibo Chen <[email protected]>
---
arch/arm/boot/dts/imx6ul-14x14-evk.dts | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/imx6ul-14x14-evk.dts b/arch/arm/boot/dts/imx6ul-14x14-evk.dts
index 61b41ee..9995656 100644
--- a/arch/arm/boot/dts/imx6ul-14x14-evk.dts
+++ b/arch/arm/boot/dts/imx6ul-14x14-evk.dts
@@ -44,6 +44,15 @@
soc-supply = <&reg_soc>;
};

+&tsc {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_tsc>;
+ xnur-gpio = <&gpio1 3 0>;
+ measure-delay-time = <0xffff>;
+ pre-charge-time = <0xfff>;
+ status = "okay";
+};
+
&uart1 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_uart1>;
@@ -212,6 +221,15 @@
>;
};

+ pinctrl_tsc: tscgrp {
+ fsl,pins = <
+ MX6UL_PAD_GPIO1_IO01__GPIO1_IO01 0xb0
+ MX6UL_PAD_GPIO1_IO02__GPIO1_IO02 0xb0
+ MX6UL_PAD_GPIO1_IO03__GPIO1_IO03 0xb0
+ MX6UL_PAD_GPIO1_IO04__GPIO1_IO04 0xb0
+ >;
+ };
+
pinctrl_uart1: uart1grp {
fsl,pins = <
MX6UL_PAD_UART1_TX_DATA__UART1_DCE_TX 0x1b0b1
--
1.9.1

2015-08-12 07:26:27

by Chen Bough

[permalink] [raw]
Subject: RE: [PATCH v2 0/5] Add imx6ul touch screen controller support


Hi all,

Any suggestion about these patch set? Since half month passed without any response.



> -----Original Message-----
> From: Haibo Chen [mailto:[email protected]]
> Sent: Tuesday, July 28, 2015 5:59 PM
> To: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: Chen Haibo-B51421; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; linux-arm-
> [email protected]; [email protected]
> Subject: [PATCH v2 0/5] Add imx6ul touch screen controller support
>
> i.MX6UL contains a touch screen controller. This patch set add imx6ul
> touch screen controller driver support.
>
> Changes for v2:
> -Add property in devicetree Documentation.
> -Add tsc disable code in tsc_remove function.
> -Remove some redundant code.
>
>
> Haibo Chen (5):
> input: touchscreen: add imx6ul_tsc driver support
> Documentation: Detail permitted DT properties for the imx6ul_tsc
> ARM: imx_v6_v7_defconfig: enable imx6ul_tsc
> ARM: dts: imx6ul.dtsi: add TSC support
> ARM: dts: imx6ul-14x14-evk.dts: add tsc support
>
> .../bindings/input/touchscreen/imx6ul_tsc.txt | 36 ++
> arch/arm/boot/dts/imx6ul-14x14-evk.dts | 18 +
> arch/arm/boot/dts/imx6ul.dtsi | 11 +
> arch/arm/configs/imx_v6_v7_defconfig | 1 +
> drivers/input/touchscreen/Kconfig | 12 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/imx6ul_tsc.c | 504
> +++++++++++++++++++++
> 7 files changed, 583 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> create mode 100644 drivers/input/touchscreen/imx6ul_tsc.c
>
> --
> 1.9.1

2015-08-19 05:11:40

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] input: touchscreen: add imx6ul_tsc driver support

Hi Haibo,

On Tue, Jul 28, 2015 at 05:58:37PM +0800, Haibo Chen wrote:
> Freescale i.MX6UL contains a internal touchscreen controller,
> this patch add a driver to support this controller.
>

This looks pretty reasonable; just a few comments below.

> Signed-off-by: Haibo Chen <[email protected]>
> ---
> drivers/input/touchscreen/Kconfig | 12 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/imx6ul_tsc.c | 504 +++++++++++++++++++++++++++++++++
> 3 files changed, 517 insertions(+)
> create mode 100644 drivers/input/touchscreen/imx6ul_tsc.c
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 5b272ba..32c300d 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -479,6 +479,18 @@ config TOUCHSCREEN_MTOUCH
> To compile this driver as a module, choose M here: the
> module will be called mtouch.
>
> +config TOUCHSCREEN_IMX6UL_TSC
> + tristate "Freescale i.MX6UL touchscreen controller"
> + depends on OF
> + help
> + Say Y here if you have a Freescale i.MX6UL, and want to
> + use the internal touchscreen controller.
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + moduel will be called imx6ul_tsc.
> +
> config TOUCHSCREEN_INEXIO
> tristate "iNexio serial touchscreens"
> select SERIO
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index c85aae2..9379b32 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -38,6 +38,7 @@ obj-$(CONFIG_TOUCHSCREEN_EGALAX) += egalax_ts.o
> obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
> obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix.o
> obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
> +obj-$(CONFIG_TOUCHSCREEN_IMX6UL_TSC) += imx6ul_tsc.o
> obj-$(CONFIG_TOUCHSCREEN_INEXIO) += inexio.o
> obj-$(CONFIG_TOUCHSCREEN_INTEL_MID) += intel-mid-touch.o
> obj-$(CONFIG_TOUCHSCREEN_IPROC) += bcm_iproc_tsc.o
> diff --git a/drivers/input/touchscreen/imx6ul_tsc.c b/drivers/input/touchscreen/imx6ul_tsc.c
> new file mode 100644
> index 0000000..807f1db
> --- /dev/null
> +++ b/drivers/input/touchscreen/imx6ul_tsc.c
> @@ -0,0 +1,504 @@
> +/*
> + * Freescale i.MX6UL touchscreen controller driver
> + *
> + * Copyright (C) 2015 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/gpio.h>
> +#include <linux/input.h>
> +#include <linux/slab.h>
> +#include <linux/completion.h>
> +#include <linux/delay.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_irq.h>

I do not think you need of_irq and of_device.

> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +
> +/* ADC configuration registers field define */
> +#define ADC_AIEN (0x1 << 7)
> +#define ADC_CONV_DISABLE 0x1F
> +#define ADC_CAL (0x1 << 7)
> +#define ADC_CALF 0x2
> +#define ADC_12BIT_MODE (0x2 << 2)
> +#define ADC_IPG_CLK 0x00
> +#define ADC_CLK_DIV_8 (0x03 << 5)
> +#define ADC_SHORT_SAMPLE_MODE (0x0 << 4)
> +#define ADC_HARDWARE_TRIGGER (0x1 << 13)
> +#define SELECT_CHANNEL_4 0x04
> +#define SELECT_CHANNEL_1 0x01
> +#define DISABLE_CONVERSION_INT (0x0 << 7)
> +
> +/* ADC registers */
> +#define REG_ADC_HC0 0x00
> +#define REG_ADC_HC1 0x04
> +#define REG_ADC_HC2 0x08
> +#define REG_ADC_HC3 0x0C
> +#define REG_ADC_HC4 0x10
> +#define REG_ADC_HS 0x14
> +#define REG_ADC_R0 0x18
> +#define REG_ADC_CFG 0x2C
> +#define REG_ADC_GC 0x30
> +#define REG_ADC_GS 0x34
> +
> +#define ADC_TIMEOUT msecs_to_jiffies(100)
> +
> +/* TSC registers */
> +#define REG_TSC_BASIC_SETING 0x00
> +#define REG_TSC_PRE_CHARGE_TIME 0x10
> +#define REG_TSC_FLOW_CONTROL 0x20
> +#define REG_TSC_MEASURE_VALUE 0x30
> +#define REG_TSC_INT_EN 0x40
> +#define REG_TSC_INT_SIG_EN 0x50
> +#define REG_TSC_INT_STATUS 0x60
> +#define REG_TSC_DEBUG_MODE 0x70
> +#define REG_TSC_DEBUG_MODE2 0x80
> +
> +/* TSC configuration registers field define */
> +#define DETECT_4_WIRE_MODE (0x0 << 4)
> +#define AUTO_MEASURE 0x1
> +#define MEASURE_SIGNAL 0x1
> +#define DETECT_SIGNAL (0x1 << 4)
> +#define VALID_SIGNAL (0x1 << 8)
> +#define MEASURE_INT_EN 0x1
> +#define MEASURE_SIG_EN 0x1
> +#define VALID_SIG_EN (0x1 << 8)
> +#define DE_GLITCH_2 (0x2 << 29)
> +#define START_SENSE (0x1 << 12)
> +#define TSC_DISABLE (0x1 << 16)
> +#define DETECT_MODE 0x2
> +
> +struct imx6ul_tsc {
> + struct device *dev;
> + struct input_dev *input;
> + void __iomem *tsc_regs;
> + void __iomem *adc_regs;
> + struct clk *tsc_clk;
> + struct clk *adc_clk;
> +
> + int xnur_gpio;
> + int measure_delay_time;
> + int pre_charge_time;
> +
> + struct completion completion;
> +};
> +
> +/*
> + * TSC module need ADC to get the measure value. So
> + * before config TSC, we should initialize ADC module.
> + */
> +static void imx6ul_adc_init(struct imx6ul_tsc *tsc)
> +{
> + int adc_hc = 0;
> + int adc_gc;
> + int adc_gs;
> + int adc_cfg;
> + int timeout;
> +
> + adc_cfg = readl(tsc->adc_regs + REG_ADC_CFG);
> + adc_cfg |= ADC_12BIT_MODE | ADC_IPG_CLK;
> + adc_cfg |= ADC_CLK_DIV_8 | ADC_SHORT_SAMPLE_MODE;
> + adc_cfg &= ~ADC_HARDWARE_TRIGGER;
> + writel(adc_cfg, tsc->adc_regs + REG_ADC_CFG);
> +
> + /* enable calibration interrupt */
> + adc_hc |= ADC_AIEN;
> + adc_hc |= ADC_CONV_DISABLE;
> + writel(adc_hc, tsc->adc_regs + REG_ADC_HC0);
> +
> + /* start ADC calibration */
> + adc_gc = readl(tsc->adc_regs + REG_ADC_GC);
> + adc_gc |= ADC_CAL;
> + writel(adc_gc, tsc->adc_regs + REG_ADC_GC);
> +

Since this is called on every resume you need to reinit completion;
probably at the beginning of this function.

> + timeout = wait_for_completion_timeout
> + (&tsc->completion, ADC_TIMEOUT);
> + if (timeout == 0)
> + dev_err(tsc->dev, "Timeout for adc calibration\n");
> +
> + adc_gs = readl(tsc->adc_regs + REG_ADC_GS);
> + if (adc_gs & ADC_CALF)
> + dev_err(tsc->dev, "ADC calibration failed\n");
> +
> + /* TSC need the ADC work in hardware trigger */
> + adc_cfg = readl(tsc->adc_regs + REG_ADC_CFG);
> + adc_cfg |= ADC_HARDWARE_TRIGGER;
> + writel(adc_cfg, tsc->adc_regs + REG_ADC_CFG);
> +}
> +
> +/*
> + * This is a TSC workaround. Currently TSC misconnect two
> + * ADC channels, this function remap channel configure for
> + * hardware trigger.
> + */
> +static void imx6ul_tsc_channel_config(struct imx6ul_tsc *tsc)
> +{
> + int adc_hc0, adc_hc1, adc_hc2, adc_hc3, adc_hc4;
> +
> + adc_hc0 = DISABLE_CONVERSION_INT;
> + writel(adc_hc0, tsc->adc_regs + REG_ADC_HC0);
> +
> + adc_hc1 = DISABLE_CONVERSION_INT | SELECT_CHANNEL_4;
> + writel(adc_hc1, tsc->adc_regs + REG_ADC_HC1);
> +
> + adc_hc2 = DISABLE_CONVERSION_INT;
> + writel(adc_hc2, tsc->adc_regs + REG_ADC_HC2);
> +
> + adc_hc3 = DISABLE_CONVERSION_INT | SELECT_CHANNEL_1;
> + writel(adc_hc3, tsc->adc_regs + REG_ADC_HC3);
> +
> + adc_hc4 = DISABLE_CONVERSION_INT;
> + writel(adc_hc4, tsc->adc_regs + REG_ADC_HC4);
> +}
> +
> +/*
> + * TSC setting, confige the pre-charge time and measure delay time.
> + * different touch screen may need different pre-charge time and
> + * measure delay time.
> + */
> +static void imx6ul_tsc_set(struct imx6ul_tsc *tsc)
> +{
> + int basic_setting = 0;
> + int start;
> +
> + basic_setting |= tsc->measure_delay_time << 8;
> + basic_setting |= DETECT_4_WIRE_MODE | AUTO_MEASURE;
> + writel(basic_setting, tsc->tsc_regs + REG_TSC_BASIC_SETING);
> +
> + writel(DE_GLITCH_2, tsc->tsc_regs + REG_TSC_DEBUG_MODE2);
> +
> + writel(tsc->pre_charge_time, tsc->tsc_regs + REG_TSC_PRE_CHARGE_TIME);
> + writel(MEASURE_INT_EN, tsc->tsc_regs + REG_TSC_INT_EN);
> + writel(MEASURE_SIG_EN | VALID_SIG_EN,
> + tsc->tsc_regs + REG_TSC_INT_SIG_EN);
> +
> + /* start sense detection */
> + start = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> + start |= START_SENSE;
> + start &= ~TSC_DISABLE;
> + writel(start, tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> +}
> +
> +static void imx6ul_tsc_init(struct imx6ul_tsc *tsc)
> +{
> + imx6ul_adc_init(tsc);
> + imx6ul_tsc_channel_config(tsc);
> + imx6ul_tsc_set(tsc);
> +}
> +
> +static irqreturn_t tsc_irq_fn(int irq, void *dev_id)
> +{
> + struct imx6ul_tsc *tsc = (struct imx6ul_tsc *)dev_id;

No need to cast void pointers.

> + int status;
> + int value;
> + int x, y;
> + int xnur;
> + int debug_mode2;
> + int state_machine;
> + int start;
> + unsigned long timeout;
> +
> + status = readl(tsc->tsc_regs + REG_TSC_INT_STATUS);
> +
> + /* write 1 to clear the bit measure-signal */
> + writel(MEASURE_SIGNAL | DETECT_SIGNAL,
> + tsc->tsc_regs + REG_TSC_INT_STATUS);
> +
> + /* It's a HW self-clean bit. Set this bit and start sense detection */
> + start = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> + start |= START_SENSE;
> + writel(start, tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> +
> + if (status & MEASURE_SIGNAL) {
> + value = readl(tsc->tsc_regs + REG_TSC_MEASURE_VALUE);
> + x = (value >> 16) & 0x0fff;
> + y = value & 0x0fff;
> +
> + /*
> + * Delay some time(max 2ms), wait the pre-charge done.
> + * After the pre-change mode, TSC go into detect mode.
> + * And in detect mode, we can get the xnur gpio value.
> + * If xnur is low, this means the touch screen still
> + * be touched. If xnur is high, this means finger leave
> + * the touch screen.
> + */
> + timeout = jiffies + HZ/500;
> + do {
> + if (time_after(jiffies, timeout)) {
> + xnur = 0;
> + goto touch_event;
> + }
> + usleep_range(200, 400);
> + debug_mode2 = readl(tsc->tsc_regs + REG_TSC_DEBUG_MODE2);
> + state_machine = (debug_mode2 >> 20) & 0x7;
> + } while (state_machine != DETECT_MODE);
> + usleep_range(200, 400);
> +
> + xnur = gpio_get_value(tsc->xnur_gpio);

It seems to me that this GPIO is actually active low: we reporting
active touch as long as we read 0.

> +touch_event:
> + if (xnur == 0) {
> + input_report_key(tsc->input, BTN_TOUCH, 1);
> + input_report_abs(tsc->input, ABS_X, x);
> + input_report_abs(tsc->input, ABS_Y, y);
> + } else
> + input_report_key(tsc->input, BTN_TOUCH, 0);

If one branch has braces all of them should have braces.

> +
> + input_sync(tsc->input);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t adc_irq_fn(int irq, void *dev_id)
> +{
> + struct imx6ul_tsc *tsc = (struct imx6ul_tsc *)dev_id;
> + int coco;
> + int value;
> +
> + coco = readl(tsc->adc_regs + REG_ADC_HS);
> + if (coco & 0x01) {
> + value = readl(tsc->adc_regs + REG_ADC_R0);
> + complete(&tsc->completion);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static const struct of_device_id imx6ul_tsc_match[] = {
> + { .compatible = "fsl,imx6ul-tsc", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imx6ul_tsc_match);
> +
> +static int imx6ul_tsc_probe(struct platform_device *pdev)
> +{
> + struct imx6ul_tsc *tsc;
> + struct resource *tsc_mem;
> + struct resource *adc_mem;
> + struct input_dev *input_dev;
> + struct device_node *np = pdev->dev.of_node;
> + int err;
> + int tsc_irq;
> + int adc_irq;
> +
> + tsc = devm_kzalloc(&pdev->dev, sizeof(struct imx6ul_tsc), GFP_KERNEL);
> + input_dev = devm_input_allocate_device(&pdev->dev);

Using devm does not mean you do not need to check results of memory
allocation.

> +
> + tsc->dev = &pdev->dev;
> +
> + tsc->input = input_dev;
> + tsc->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> + tsc->input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
> + input_set_abs_params(tsc->input, ABS_X, 0, 0xFFF, 0, 0);
> + input_set_abs_params(tsc->input, ABS_Y, 0, 0xFFF, 0, 0);
> +
> + tsc->input->name = "iMX6UL TouchScreen Controller";
> +
> + tsc->xnur_gpio = of_get_named_gpio(np, "xnur-gpio", 0);
> + err = gpio_request_one(tsc->xnur_gpio, GPIOF_IN, "tsc_X-");

Why are you not using devm for gpio? Actually, why don't you also use
gpiod? As is you are leaking that gpio in all error paths below.

> + if (err) {
> + dev_err(&pdev->dev, "failed to request GPIO tsc_X-\n");
> + return err;
> + }
> +
> + tsc_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + tsc->tsc_regs = devm_ioremap_resource(&pdev->dev, tsc_mem);
> + if (IS_ERR(tsc->tsc_regs)) {
> + dev_err(&pdev->dev, "failed to remap tsc memory\n");
> + err = PTR_ERR(tsc->tsc_regs);
> + return err;
> + }
> +
> + adc_mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + tsc->adc_regs = devm_ioremap_resource(&pdev->dev, adc_mem);
> + if (IS_ERR(tsc->adc_regs)) {
> + dev_err(&pdev->dev, "failed to remap adc memory\n");
> + err = PTR_ERR(tsc->adc_regs);
> + return err;
> + }
> +
> + tsc->tsc_clk = devm_clk_get(&pdev->dev, "tsc");
> + if (IS_ERR(tsc->tsc_clk)) {
> + dev_err(&pdev->dev, "failed getting tsc clock\n");
> + err = PTR_ERR(tsc->tsc_clk);
> + return err;
> + }
> +
> + tsc->adc_clk = devm_clk_get(&pdev->dev, "adc");
> + if (IS_ERR(tsc->adc_clk)) {
> + dev_err(&pdev->dev, "failed getting adc clock\n");
> + err = PTR_ERR(tsc->adc_clk);
> + return err;
> + }
> +
> + tsc_irq = platform_get_irq(pdev, 0);
> + if (tsc_irq < 0) {
> + dev_err(&pdev->dev, "no tsc irq resource?\n");
> + return tsc_irq;
> + }
> +
> + adc_irq = platform_get_irq(pdev, 1);
> + if (adc_irq <= 0) {
> + dev_err(&pdev->dev, "no adc irq resource?\n");
> + return adc_irq;
> + }
> +
> + err = devm_request_threaded_irq(tsc->dev, tsc_irq,
> + NULL, tsc_irq_fn,
> + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> + dev_name(&pdev->dev), tsc);
> + if (err < 0) {
> + dev_err(&pdev->dev,
> + "failed requesting tsc irq %d\n",
> + tsc_irq);
> + return err;
> + }
> +
> + err = devm_request_irq(tsc->dev, adc_irq,
> + adc_irq_fn, 0,
> + dev_name(&pdev->dev), tsc);
> + if (err < 0) {
> + dev_err(&pdev->dev,
> + "failed requesting adc irq %d\n",
> + adc_irq);
> + return err;
> + }
> +
> + err = of_property_read_u32(np, "measure-delay-time",
> + &tsc->measure_delay_time);
> + if (err)
> + tsc->measure_delay_time = 0xffff;
> +
> + err = of_property_read_u32(np, "pre-charge-time",
> + &tsc->pre_charge_time);
> + if (err)
> + tsc->pre_charge_time = 0xfff;
> +
> + init_completion(&tsc->completion);
> +
> + err = clk_prepare_enable(tsc->adc_clk);
> + if (err) {
> + dev_err(&pdev->dev,
> + "Could not prepare or enable the adc clock.\n");
> + return err;
> + }
> +
> + err = clk_prepare_enable(tsc->tsc_clk);
> + if (err) {
> + dev_err(&pdev->dev,
> + "Could not prepare or enable the tsc clock.\n");
> + goto error_tsc_clk_enable;
> + }
> +
> + err = input_register_device(tsc->input);
> + if (err < 0) {
> + dev_err(&pdev->dev, "failed to register input device\n");
> + err = -EIO;
> + goto err_input_register;
> + }
> +
> + imx6ul_tsc_init(tsc);

Enabling clock and initializing the controller is better in open()
method of input device. If you also implement close() you can get rid of
remove().

> +
> + platform_set_drvdata(pdev, tsc);
> + return 0;
> +
> +err_input_register:
> + clk_disable_unprepare(tsc->tsc_clk);
> +error_tsc_clk_enable:
> + clk_disable_unprepare(tsc->adc_clk);
> +
> + return err;
> +}
> +
> +static void imx6ul_tsc_disable(struct imx6ul_tsc *tsc)
> +{
> + int tsc_flow;
> + int adc_cfg;
> +
> + /* TSC controller enters to idle status */
> + tsc_flow = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> + tsc_flow |= TSC_DISABLE;
> + writel(tsc_flow, tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> +
> + /* ADC controller enters to stop mode */
> + adc_cfg = readl(tsc->adc_regs + REG_ADC_HC0);
> + adc_cfg |= ADC_CONV_DISABLE;
> + writel(adc_cfg, tsc->adc_regs + REG_ADC_HC0);
> +}
> +
> +static int imx6ul_tsc_remove(struct platform_device *pdev)
> +{
> + struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
> +
> + imx6ul_tsc_disable(tsc);
> +
> + clk_disable_unprepare(tsc->tsc_clk);
> + clk_disable_unprepare(tsc->adc_clk);
> + input_unregister_device(tsc->input);
> + kfree(tsc);

Tsc is allocated with devm(), you can't kfree() it.
> +
> + return 0;
> +}
> +
> +static int __maybe_unused imx6ul_tsc_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
> +
> + imx6ul_tsc_disable(tsc);
> +
> + clk_disable_unprepare(tsc->tsc_clk);
> + clk_disable_unprepare(tsc->adc_clk);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused imx6ul_tsc_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
> + int err;
> +
> + err = clk_prepare_enable(tsc->adc_clk);
> + if (err)
> + return err;
> +
> + err = clk_prepare_enable(tsc->tsc_clk);
> + if (err) {
> + clk_disable_unprepare(tsc->adc_clk);
> + return err;
> + }
> +
> + imx6ul_tsc_init(tsc);
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(imx6ul_tsc_pm_ops,
> + imx6ul_tsc_suspend,
> + imx6ul_tsc_resume);
> +
> +static struct platform_driver imx6ul_tsc_driver = {
> + .driver = {
> + .name = "imx6ul-tsc",
> + .owner = THIS_MODULE,
> + .of_match_table = imx6ul_tsc_match,
> + .pm = &imx6ul_tsc_pm_ops,
> + },
> + .probe = imx6ul_tsc_probe,
> + .remove = imx6ul_tsc_remove,
> +};
> +module_platform_driver(imx6ul_tsc_driver);
> +
> +MODULE_AUTHOR("Haibo Chen <[email protected]>");
> +MODULE_DESCRIPTION("Freescale i.MX6UL Touchscreen controller driver");
> +MODULE_LICENSE("GPL v2");
> --
> 1.9.1
>

Can you try the patch below and let me know if it still works (you'll
need to adjust your DTS for xnur gpio being active low).

Thanks!

--
Dmitry

Input: imx6ul_tsc - misc changes

From: Dmitry Torokhov <[email protected]>

Signed-off-by: Dmitry Torokhov <[email protected]>
---
.../bindings/input/touchscreen/imx6ul_tsc.txt | 2
drivers/input/touchscreen/Kconfig | 2
drivers/input/touchscreen/imx6ul_tsc.c | 262 +++++++++++---------
3 files changed, 143 insertions(+), 123 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
index ac41c32..c933588 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
@@ -29,7 +29,7 @@ Example:
clock-names = "tsc", "adc";
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_tsc>;
- xnur-gpio = <&gpio1 3 GPIO_ACTIVE_HIGH>;
+ xnur-gpio = <&gpio1 3 GPIO_ACTIVE_LOW>;
measure-delay-time = <0xfff>;
pre-charge-time = <0xffff>;
status = "okay";
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 9a1a293..5ecf19b 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -481,7 +481,7 @@ config TOUCHSCREEN_MTOUCH

config TOUCHSCREEN_IMX6UL_TSC
tristate "Freescale i.MX6UL touchscreen controller"
- depends on OF
+ depends on (OF && GPIOLIB) || COMPILE_TEST
help
Say Y here if you have a Freescale i.MX6UL, and want to
use the internal touchscreen controller.
diff --git a/drivers/input/touchscreen/imx6ul_tsc.c b/drivers/input/touchscreen/imx6ul_tsc.c
index 807f1db..1d028ed 100644
--- a/drivers/input/touchscreen/imx6ul_tsc.c
+++ b/drivers/input/touchscreen/imx6ul_tsc.c
@@ -11,15 +11,12 @@
#include <linux/errno.h>
#include <linux/kernel.h>
#include <linux/module.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/input.h>
#include <linux/slab.h>
#include <linux/completion.h>
#include <linux/delay.h>
#include <linux/of.h>
-#include <linux/of_device.h>
-#include <linux/of_gpio.h>
-#include <linux/of_irq.h>
#include <linux/interrupt.h>
#include <linux/platform_device.h>
#include <linux/clk.h>
@@ -85,8 +82,8 @@ struct imx6ul_tsc {
void __iomem *adc_regs;
struct clk *tsc_clk;
struct clk *adc_clk;
+ struct gpio_desc *xnur_gpio;

- int xnur_gpio;
int measure_delay_time;
int pre_charge_time;

@@ -105,6 +102,8 @@ static void imx6ul_adc_init(struct imx6ul_tsc *tsc)
int adc_cfg;
int timeout;

+ reinit_completion(&tsc->completion);
+
adc_cfg = readl(tsc->adc_regs + REG_ADC_CFG);
adc_cfg |= ADC_12BIT_MODE | ADC_IPG_CLK;
adc_cfg |= ADC_CLK_DIV_8 | ADC_SHORT_SAMPLE_MODE;
@@ -196,17 +195,33 @@ static void imx6ul_tsc_init(struct imx6ul_tsc *tsc)
imx6ul_tsc_set(tsc);
}

+static void imx6ul_tsc_disable(struct imx6ul_tsc *tsc)
+{
+ int tsc_flow;
+ int adc_cfg;
+
+ /* TSC controller enters to idle status */
+ tsc_flow = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
+ tsc_flow |= TSC_DISABLE;
+ writel(tsc_flow, tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
+
+ /* ADC controller enters to stop mode */
+ adc_cfg = readl(tsc->adc_regs + REG_ADC_HC0);
+ adc_cfg |= ADC_CONV_DISABLE;
+ writel(adc_cfg, tsc->adc_regs + REG_ADC_HC0);
+}
+
static irqreturn_t tsc_irq_fn(int irq, void *dev_id)
{
struct imx6ul_tsc *tsc = (struct imx6ul_tsc *)dev_id;
int status;
int value;
int x, y;
- int xnur;
int debug_mode2;
int state_machine;
int start;
unsigned long timeout;
+ bool touch;

status = readl(tsc->tsc_regs + REG_TSC_INT_STATUS);

@@ -235,8 +250,8 @@ static irqreturn_t tsc_irq_fn(int irq, void *dev_id)
timeout = jiffies + HZ/500;
do {
if (time_after(jiffies, timeout)) {
- xnur = 0;
- goto touch_event;
+ touch = true;
+ goto report_touch;
}
usleep_range(200, 400);
debug_mode2 = readl(tsc->tsc_regs + REG_TSC_DEBUG_MODE2);
@@ -244,14 +259,15 @@ static irqreturn_t tsc_irq_fn(int irq, void *dev_id)
} while (state_machine != DETECT_MODE);
usleep_range(200, 400);

- xnur = gpio_get_value(tsc->xnur_gpio);
-touch_event:
- if (xnur == 0) {
+ touch = gpiod_get_value_cansleep(tsc->xnur_gpio);
+report_touch:
+ if (touch) {
input_report_key(tsc->input, BTN_TOUCH, 1);
input_report_abs(tsc->input, ABS_X, x);
input_report_abs(tsc->input, ABS_Y, y);
- } else
+ } else {
input_report_key(tsc->input, BTN_TOUCH, 0);
+ }

input_sync(tsc->input);
}
@@ -261,7 +277,7 @@ touch_event:

static irqreturn_t adc_irq_fn(int irq, void *dev_id)
{
- struct imx6ul_tsc *tsc = (struct imx6ul_tsc *)dev_id;
+ struct imx6ul_tsc *tsc = dev_id;
int coco;
int value;

@@ -274,70 +290,113 @@ static irqreturn_t adc_irq_fn(int irq, void *dev_id)
return IRQ_HANDLED;
}

-static const struct of_device_id imx6ul_tsc_match[] = {
- { .compatible = "fsl,imx6ul-tsc", },
- { /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, imx6ul_tsc_match);
+static int imx6ul_tsc_open(struct input_dev *input_dev)
+{
+ struct imx6ul_tsc *tsc = input_get_drvdata(input_dev);
+ int err;
+
+ err = clk_prepare_enable(tsc->adc_clk);
+ if (err) {
+ dev_err(tsc->dev,
+ "Could not prepare or enable the adc clock: %d\n",
+ err);
+ return err;
+ }
+
+ err = clk_prepare_enable(tsc->tsc_clk);
+ if (err) {
+ dev_err(tsc->dev,
+ "Could not prepare or enable the tsc clock: %d\n",
+ err);
+ clk_disable_unprepare(tsc->adc_clk);
+ return err;
+ }
+
+ imx6ul_tsc_init(tsc);
+
+ return 0;
+}
+
+static void imx6ul_tsc_close(struct input_dev *input_dev)
+{
+ struct imx6ul_tsc *tsc = input_get_drvdata(input_dev);
+
+ imx6ul_tsc_disable(tsc);
+
+ clk_disable_unprepare(tsc->tsc_clk);
+ clk_disable_unprepare(tsc->adc_clk);
+}

static int imx6ul_tsc_probe(struct platform_device *pdev)
{
+ struct device_node *np = pdev->dev.of_node;
struct imx6ul_tsc *tsc;
+ struct input_dev *input_dev;
struct resource *tsc_mem;
struct resource *adc_mem;
- struct input_dev *input_dev;
- struct device_node *np = pdev->dev.of_node;
int err;
int tsc_irq;
int adc_irq;

tsc = devm_kzalloc(&pdev->dev, sizeof(struct imx6ul_tsc), GFP_KERNEL);
+ if (!tsc)
+ return -ENOMEM;
+
input_dev = devm_input_allocate_device(&pdev->dev);
+ if (!input_dev)
+ return -ENOMEM;

- tsc->dev = &pdev->dev;
+ input_dev->name = "iMX6UL TouchScreen Controller";
+ input_dev->id.bustype = BUS_HOST;

- tsc->input = input_dev;
- tsc->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
- tsc->input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
- input_set_abs_params(tsc->input, ABS_X, 0, 0xFFF, 0, 0);
- input_set_abs_params(tsc->input, ABS_Y, 0, 0xFFF, 0, 0);
+ input_dev->open = imx6ul_tsc_open;
+ input_dev->close = imx6ul_tsc_close;

- tsc->input->name = "iMX6UL TouchScreen Controller";
+ input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
+ input_set_abs_params(input_dev, ABS_X, 0, 0xFFF, 0, 0);
+ input_set_abs_params(input_dev, ABS_Y, 0, 0xFFF, 0, 0);

- tsc->xnur_gpio = of_get_named_gpio(np, "xnur-gpio", 0);
- err = gpio_request_one(tsc->xnur_gpio, GPIOF_IN, "tsc_X-");
- if (err) {
- dev_err(&pdev->dev, "failed to request GPIO tsc_X-\n");
+ input_set_drvdata(input_dev, tsc);
+
+ tsc->dev = &pdev->dev;
+ tsc->input = input_dev;
+ init_completion(&tsc->completion);
+
+ tsc->xnur_gpio = devm_gpiod_get(&pdev->dev, "xnur-gpio", GPIOD_IN);
+ if (IS_ERR(tsc->xnur_gpio)) {
+ err = PTR_ERR(tsc->xnur_gpio);
+ dev_err(&pdev->dev,
+ "failed to request GPIO tsc_X- (xnur): %d\n", err);
return err;
}

tsc_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
tsc->tsc_regs = devm_ioremap_resource(&pdev->dev, tsc_mem);
if (IS_ERR(tsc->tsc_regs)) {
- dev_err(&pdev->dev, "failed to remap tsc memory\n");
err = PTR_ERR(tsc->tsc_regs);
+ dev_err(&pdev->dev, "failed to remap tsc memory: %d\n", err);
return err;
}

adc_mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
tsc->adc_regs = devm_ioremap_resource(&pdev->dev, adc_mem);
if (IS_ERR(tsc->adc_regs)) {
- dev_err(&pdev->dev, "failed to remap adc memory\n");
err = PTR_ERR(tsc->adc_regs);
+ dev_err(&pdev->dev, "failed to remap adc memory: %d\n", err);
return err;
}

tsc->tsc_clk = devm_clk_get(&pdev->dev, "tsc");
if (IS_ERR(tsc->tsc_clk)) {
- dev_err(&pdev->dev, "failed getting tsc clock\n");
err = PTR_ERR(tsc->tsc_clk);
+ dev_err(&pdev->dev, "failed getting tsc clock: %d\n", err);
return err;
}

tsc->adc_clk = devm_clk_get(&pdev->dev, "adc");
if (IS_ERR(tsc->adc_clk)) {
- dev_err(&pdev->dev, "failed getting adc clock\n");
err = PTR_ERR(tsc->adc_clk);
+ dev_err(&pdev->dev, "failed getting adc clock: %d\n", err);
return err;
}

@@ -354,111 +413,61 @@ static int imx6ul_tsc_probe(struct platform_device *pdev)
}

err = devm_request_threaded_irq(tsc->dev, tsc_irq,
- NULL, tsc_irq_fn,
- IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ NULL, tsc_irq_fn, IRQF_ONESHOT,
dev_name(&pdev->dev), tsc);
- if (err < 0) {
+ if (err) {
dev_err(&pdev->dev,
- "failed requesting tsc irq %d\n",
- tsc_irq);
+ "failed requesting tsc irq %di: %d\n",
+ tsc_irq, err);
return err;
}

- err = devm_request_irq(tsc->dev, adc_irq,
- adc_irq_fn, 0,
+ err = devm_request_irq(tsc->dev, adc_irq, adc_irq_fn, 0,
dev_name(&pdev->dev), tsc);
- if (err < 0) {
+ if (err) {
dev_err(&pdev->dev,
- "failed requesting adc irq %d\n",
- adc_irq);
+ "failed requesting adc irq %d: %d\n",
+ adc_irq, err);
return err;
}

err = of_property_read_u32(np, "measure-delay-time",
- &tsc->measure_delay_time);
+ &tsc->measure_delay_time);
if (err)
tsc->measure_delay_time = 0xffff;

err = of_property_read_u32(np, "pre-charge-time",
- &tsc->pre_charge_time);
+ &tsc->pre_charge_time);
if (err)
tsc->pre_charge_time = 0xfff;

- init_completion(&tsc->completion);
-
- err = clk_prepare_enable(tsc->adc_clk);
+ err = input_register_device(tsc->input);
if (err) {
dev_err(&pdev->dev,
- "Could not prepare or enable the adc clock.\n");
+ "failed to register input device: %d\n", err);
return err;
}

- err = clk_prepare_enable(tsc->tsc_clk);
- if (err) {
- dev_err(&pdev->dev,
- "Could not prepare or enable the tsc clock.\n");
- goto error_tsc_clk_enable;
- }
-
- err = input_register_device(tsc->input);
- if (err < 0) {
- dev_err(&pdev->dev, "failed to register input device\n");
- err = -EIO;
- goto err_input_register;
- }
-
- imx6ul_tsc_init(tsc);
-
platform_set_drvdata(pdev, tsc);
return 0;
-
-err_input_register:
- clk_disable_unprepare(tsc->tsc_clk);
-error_tsc_clk_enable:
- clk_disable_unprepare(tsc->adc_clk);
-
- return err;
-}
-
-static void imx6ul_tsc_disable(struct imx6ul_tsc *tsc)
-{
- int tsc_flow;
- int adc_cfg;
-
- /* TSC controller enters to idle status */
- tsc_flow = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
- tsc_flow |= TSC_DISABLE;
- writel(tsc_flow, tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
-
- /* ADC controller enters to stop mode */
- adc_cfg = readl(tsc->adc_regs + REG_ADC_HC0);
- adc_cfg |= ADC_CONV_DISABLE;
- writel(adc_cfg, tsc->adc_regs + REG_ADC_HC0);
-}
-
-static int imx6ul_tsc_remove(struct platform_device *pdev)
-{
- struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
-
- imx6ul_tsc_disable(tsc);
-
- clk_disable_unprepare(tsc->tsc_clk);
- clk_disable_unprepare(tsc->adc_clk);
- input_unregister_device(tsc->input);
- kfree(tsc);
-
- return 0;
}

static int __maybe_unused imx6ul_tsc_suspend(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
+ struct input_dev *input_dev = tsc->input;

- imx6ul_tsc_disable(tsc);
+ mutex_lock(&input_dev->mutex);

- clk_disable_unprepare(tsc->tsc_clk);
- clk_disable_unprepare(tsc->adc_clk);
+ if (input_dev->users) {
+ imx6ul_tsc_disable(tsc);
+
+ clk_disable_unprepare(tsc->tsc_clk);
+ clk_disable_unprepare(tsc->adc_clk);
+ }
+
+ mutex_unlock(&input_dev->mutex);

return 0;
}
@@ -467,35 +476,46 @@ static int __maybe_unused imx6ul_tsc_resume(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
- int err;
+ struct input_dev *input_dev = tsc->input;
+ int retval = 0;

- err = clk_prepare_enable(tsc->adc_clk);
- if (err)
- return err;
+ mutex_lock(&input_dev->mutex);

- err = clk_prepare_enable(tsc->tsc_clk);
- if (err) {
- clk_disable_unprepare(tsc->adc_clk);
- return err;
+ if (input_dev->users) {
+ retval = clk_prepare_enable(tsc->adc_clk);
+ if (retval)
+ goto out;
+
+ retval = clk_prepare_enable(tsc->tsc_clk);
+ if (retval) {
+ clk_disable_unprepare(tsc->adc_clk);
+ goto out;
+ }
+
+ imx6ul_tsc_init(tsc);
}

- imx6ul_tsc_init(tsc);
- return 0;
+out:
+ mutex_unlock(&input_dev->mutex);
+ return retval;
}

static SIMPLE_DEV_PM_OPS(imx6ul_tsc_pm_ops,
- imx6ul_tsc_suspend,
- imx6ul_tsc_resume);
+ imx6ul_tsc_suspend, imx6ul_tsc_resume);
+
+static const struct of_device_id imx6ul_tsc_match[] = {
+ { .compatible = "fsl,imx6ul-tsc", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx6ul_tsc_match);

static struct platform_driver imx6ul_tsc_driver = {
.driver = {
.name = "imx6ul-tsc",
- .owner = THIS_MODULE,
.of_match_table = imx6ul_tsc_match,
.pm = &imx6ul_tsc_pm_ops,
},
.probe = imx6ul_tsc_probe,
- .remove = imx6ul_tsc_remove,
};
module_platform_driver(imx6ul_tsc_driver);

2015-08-19 05:55:30

by Markus Pargmann

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] Documentation: Detail permitted DT properties for the imx6ul_tsc

Hi,

On Tue, Jul 28, 2015 at 05:58:38PM +0800, Haibo Chen wrote:
> Here we apply required documentation for the imx6ul touch screen
> controller driver which describe available properties and how to
> use them.
>
> Signed-off-by: Haibo Chen <[email protected]>
> ---
> .../bindings/input/touchscreen/imx6ul_tsc.txt | 36 ++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> new file mode 100644
> index 0000000..ac41c32
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> @@ -0,0 +1,36 @@
> +* Freescale i.MX6UL Touch Controller
> +
> +Required properties:
> +- compatible: must be "fsl,imx6ul-tsc".
> +- reg: this touch controller address and the ADC2 address.

This suggests that this driver is using a unit ADC2. Which also means
that there are more than one ADC which are probably identical?

Shouldn't these ADCs be properly described by their own device nodes
instead of these two register ranges, two interrupts and two clocks?

Is 'ADC2' usable without tsc? Then ADC1/ADC2 should perhaps get a proper
IIO driver.

Unfortunately I don't have the reference manual to have a look how this
all works.

Best regards,

Markus

> +- interrupts: the interrupt of this touch controller and ADC2.
> +- clocks: the root clock of touch controller and ADC2.
> +- clock-names; must be "tsc" and "adc".
> +- xnur-gpio: the X- gpio this controller connect to.
> + This xnur-gpio returns to high once the finger leave the touch screen (The
> + last touch event the touch controller capture).
> +
> +Optional properties:
> +- measure-delay-time: the value of measure delay time.
> + Before X-axis or Y-axis measurement, the screen need some time before
> + even potential distribution ready.
> + This value depends on the touch screen.
> +- pre-charge-time: the touch screen need some time to precharge.
> + This value depends on the touch screen.
> +
> +Example:
> + tsc: tsc@02040000 {
> + compatible = "fsl,imx6ul-tsc";
> + reg = <0x02040000 0x4000>, <0x0219c000 0x4000>;
> + interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clks IMX6UL_CLK_IPG>,
> + <&clks IMX6UL_CLK_ADC2>;
> + clock-names = "tsc", "adc";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_tsc>;
> + xnur-gpio = <&gpio1 3 GPIO_ACTIVE_HIGH>;
> + measure-delay-time = <0xfff>;
> + pre-charge-time = <0xffff>;
> + status = "okay";
> + };
> --
> 1.9.1
>
>
>

--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |


Attachments:
(No filename) (2.94 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-08-21 08:08:59

by Chen Bough

[permalink] [raw]
Subject: RE: [PATCH v2 1/5] input: touchscreen: add imx6ul_tsc driver support

Hi Dmitry,

Thanks for your patient review, especially for the patch you attached.

I test your patch these days, with below change, touch can work normally.
(also change the xnur to active low in dts)

In probe function:

> - tsc->xnur_gpio = of_get_named_gpio(np, "xnur-gpio", 0);
> - err = gpio_request_one(tsc->xnur_gpio, GPIOF_IN, "tsc_X-");
> - if (err) {
> - dev_err(&pdev->dev, "failed to request GPIO tsc_X-\n");
> + input_set_drvdata(input_dev, tsc);
> +
> + tsc->dev = &pdev->dev;
> + tsc->input = input_dev;
> + init_completion(&tsc->completion);
> +
> + tsc->xnur_gpio = devm_gpiod_get(&pdev->dev, "xnur-gpio", GPIOD_IN);

Here, we need to change "xnur-gpio" to "xnur", otherwise the gpio request will be failed.
This is because gpiod common code already add suffix '-gpio' or 'gpios'.



For others, your patch seems normal and rational. I will add your patch and send patch-V3.

Thanks again!


Best Regards
Haibo Chen



> -----Original Message-----
> From: Dmitry Torokhov [mailto:[email protected]]
> Sent: Wednesday, August 19, 2015 1:12 PM
> To: Chen Haibo-B51421
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH v2 1/5] input: touchscreen: add imx6ul_tsc driver
> support
>
> Hi Haibo,
>
> On Tue, Jul 28, 2015 at 05:58:37PM +0800, Haibo Chen wrote:
> > Freescale i.MX6UL contains a internal touchscreen controller, this
> > patch add a driver to support this controller.
> >
>
> This looks pretty reasonable; just a few comments below.
>
> > Signed-off-by: Haibo Chen <[email protected]>
> > ---
> > drivers/input/touchscreen/Kconfig | 12 +
> > drivers/input/touchscreen/Makefile | 1 +
> > drivers/input/touchscreen/imx6ul_tsc.c | 504
> > +++++++++++++++++++++++++++++++++
> > 3 files changed, 517 insertions(+)
> > create mode 100644 drivers/input/touchscreen/imx6ul_tsc.c
> >
> > diff --git a/drivers/input/touchscreen/Kconfig
> > b/drivers/input/touchscreen/Kconfig
> > index 5b272ba..32c300d 100644
> > --- a/drivers/input/touchscreen/Kconfig
> > +++ b/drivers/input/touchscreen/Kconfig
> > @@ -479,6 +479,18 @@ config TOUCHSCREEN_MTOUCH
> > To compile this driver as a module, choose M here: the
> > module will be called mtouch.
> >
> > +config TOUCHSCREEN_IMX6UL_TSC
> > + tristate "Freescale i.MX6UL touchscreen controller"
> > + depends on OF
> > + help
> > + Say Y here if you have a Freescale i.MX6UL, and want to
> > + use the internal touchscreen controller.
> > +
> > + If unsure, say N.
> > +
> > + To compile this driver as a module, choose M here: the
> > + moduel will be called imx6ul_tsc.
> > +
> > config TOUCHSCREEN_INEXIO
> > tristate "iNexio serial touchscreens"
> > select SERIO
> > diff --git a/drivers/input/touchscreen/Makefile
> > b/drivers/input/touchscreen/Makefile
> > index c85aae2..9379b32 100644
> > --- a/drivers/input/touchscreen/Makefile
> > +++ b/drivers/input/touchscreen/Makefile
> > @@ -38,6 +38,7 @@ obj-$(CONFIG_TOUCHSCREEN_EGALAX) += egalax_ts.o
> > obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
> > obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix.o
> > obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
> > +obj-$(CONFIG_TOUCHSCREEN_IMX6UL_TSC) += imx6ul_tsc.o
> > obj-$(CONFIG_TOUCHSCREEN_INEXIO) += inexio.o
> > obj-$(CONFIG_TOUCHSCREEN_INTEL_MID) += intel-mid-touch.o
> > obj-$(CONFIG_TOUCHSCREEN_IPROC) += bcm_iproc_tsc.o
> > diff --git a/drivers/input/touchscreen/imx6ul_tsc.c
> > b/drivers/input/touchscreen/imx6ul_tsc.c
> > new file mode 100644
> > index 0000000..807f1db
> > --- /dev/null
> > +++ b/drivers/input/touchscreen/imx6ul_tsc.c
> > @@ -0,0 +1,504 @@
> > +/*
> > + * Freescale i.MX6UL touchscreen controller driver
> > + *
> > + * Copyright (C) 2015 Freescale Semiconductor, Inc.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > +modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/errno.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/gpio.h>
> > +#include <linux/input.h>
> > +#include <linux/slab.h>
> > +#include <linux/completion.h>
> > +#include <linux/delay.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_gpio.h>
> > +#include <linux/of_irq.h>
>
> I do not think you need of_irq and of_device.
>
> > +#include <linux/interrupt.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/clk.h>
> > +#include <linux/io.h>
> > +
> > +/* ADC configuration registers field define */
> > +#define ADC_AIEN (0x1 << 7)
> > +#define ADC_CONV_DISABLE 0x1F
> > +#define ADC_CAL (0x1 << 7)
> > +#define ADC_CALF 0x2
> > +#define ADC_12BIT_MODE (0x2 << 2)
> > +#define ADC_IPG_CLK 0x00
> > +#define ADC_CLK_DIV_8 (0x03 << 5)
> > +#define ADC_SHORT_SAMPLE_MODE (0x0 << 4)
> > +#define ADC_HARDWARE_TRIGGER (0x1 << 13)
> > +#define SELECT_CHANNEL_4 0x04
> > +#define SELECT_CHANNEL_1 0x01
> > +#define DISABLE_CONVERSION_INT (0x0 << 7)
> > +
> > +/* ADC registers */
> > +#define REG_ADC_HC0 0x00
> > +#define REG_ADC_HC1 0x04
> > +#define REG_ADC_HC2 0x08
> > +#define REG_ADC_HC3 0x0C
> > +#define REG_ADC_HC4 0x10
> > +#define REG_ADC_HS 0x14
> > +#define REG_ADC_R0 0x18
> > +#define REG_ADC_CFG 0x2C
> > +#define REG_ADC_GC 0x30
> > +#define REG_ADC_GS 0x34
> > +
> > +#define ADC_TIMEOUT msecs_to_jiffies(100)
> > +
> > +/* TSC registers */
> > +#define REG_TSC_BASIC_SETING 0x00
> > +#define REG_TSC_PRE_CHARGE_TIME 0x10
> > +#define REG_TSC_FLOW_CONTROL 0x20
> > +#define REG_TSC_MEASURE_VALUE 0x30
> > +#define REG_TSC_INT_EN 0x40
> > +#define REG_TSC_INT_SIG_EN 0x50
> > +#define REG_TSC_INT_STATUS 0x60
> > +#define REG_TSC_DEBUG_MODE 0x70
> > +#define REG_TSC_DEBUG_MODE2 0x80
> > +
> > +/* TSC configuration registers field define */
> > +#define DETECT_4_WIRE_MODE (0x0 << 4)
> > +#define AUTO_MEASURE 0x1
> > +#define MEASURE_SIGNAL 0x1
> > +#define DETECT_SIGNAL (0x1 << 4)
> > +#define VALID_SIGNAL (0x1 << 8)
> > +#define MEASURE_INT_EN 0x1
> > +#define MEASURE_SIG_EN 0x1
> > +#define VALID_SIG_EN (0x1 << 8)
> > +#define DE_GLITCH_2 (0x2 << 29)
> > +#define START_SENSE (0x1 << 12)
> > +#define TSC_DISABLE (0x1 << 16)
> > +#define DETECT_MODE 0x2
> > +
> > +struct imx6ul_tsc {
> > + struct device *dev;
> > + struct input_dev *input;
> > + void __iomem *tsc_regs;
> > + void __iomem *adc_regs;
> > + struct clk *tsc_clk;
> > + struct clk *adc_clk;
> > +
> > + int xnur_gpio;
> > + int measure_delay_time;
> > + int pre_charge_time;
> > +
> > + struct completion completion;
> > +};
> > +
> > +/*
> > + * TSC module need ADC to get the measure value. So
> > + * before config TSC, we should initialize ADC module.
> > + */
> > +static void imx6ul_adc_init(struct imx6ul_tsc *tsc) {
> > + int adc_hc = 0;
> > + int adc_gc;
> > + int adc_gs;
> > + int adc_cfg;
> > + int timeout;
> > +
> > + adc_cfg = readl(tsc->adc_regs + REG_ADC_CFG);
> > + adc_cfg |= ADC_12BIT_MODE | ADC_IPG_CLK;
> > + adc_cfg |= ADC_CLK_DIV_8 | ADC_SHORT_SAMPLE_MODE;
> > + adc_cfg &= ~ADC_HARDWARE_TRIGGER;
> > + writel(adc_cfg, tsc->adc_regs + REG_ADC_CFG);
> > +
> > + /* enable calibration interrupt */
> > + adc_hc |= ADC_AIEN;
> > + adc_hc |= ADC_CONV_DISABLE;
> > + writel(adc_hc, tsc->adc_regs + REG_ADC_HC0);
> > +
> > + /* start ADC calibration */
> > + adc_gc = readl(tsc->adc_regs + REG_ADC_GC);
> > + adc_gc |= ADC_CAL;
> > + writel(adc_gc, tsc->adc_regs + REG_ADC_GC);
> > +
>
> Since this is called on every resume you need to reinit completion;
> probably at the beginning of this function.
>
> > + timeout = wait_for_completion_timeout
> > + (&tsc->completion, ADC_TIMEOUT);
> > + if (timeout == 0)
> > + dev_err(tsc->dev, "Timeout for adc calibration\n");
> > +
> > + adc_gs = readl(tsc->adc_regs + REG_ADC_GS);
> > + if (adc_gs & ADC_CALF)
> > + dev_err(tsc->dev, "ADC calibration failed\n");
> > +
> > + /* TSC need the ADC work in hardware trigger */
> > + adc_cfg = readl(tsc->adc_regs + REG_ADC_CFG);
> > + adc_cfg |= ADC_HARDWARE_TRIGGER;
> > + writel(adc_cfg, tsc->adc_regs + REG_ADC_CFG); }
> > +
> > +/*
> > + * This is a TSC workaround. Currently TSC misconnect two
> > + * ADC channels, this function remap channel configure for
> > + * hardware trigger.
> > + */
> > +static void imx6ul_tsc_channel_config(struct imx6ul_tsc *tsc) {
> > + int adc_hc0, adc_hc1, adc_hc2, adc_hc3, adc_hc4;
> > +
> > + adc_hc0 = DISABLE_CONVERSION_INT;
> > + writel(adc_hc0, tsc->adc_regs + REG_ADC_HC0);
> > +
> > + adc_hc1 = DISABLE_CONVERSION_INT | SELECT_CHANNEL_4;
> > + writel(adc_hc1, tsc->adc_regs + REG_ADC_HC1);
> > +
> > + adc_hc2 = DISABLE_CONVERSION_INT;
> > + writel(adc_hc2, tsc->adc_regs + REG_ADC_HC2);
> > +
> > + adc_hc3 = DISABLE_CONVERSION_INT | SELECT_CHANNEL_1;
> > + writel(adc_hc3, tsc->adc_regs + REG_ADC_HC3);
> > +
> > + adc_hc4 = DISABLE_CONVERSION_INT;
> > + writel(adc_hc4, tsc->adc_regs + REG_ADC_HC4); }
> > +
> > +/*
> > + * TSC setting, confige the pre-charge time and measure delay time.
> > + * different touch screen may need different pre-charge time and
> > + * measure delay time.
> > + */
> > +static void imx6ul_tsc_set(struct imx6ul_tsc *tsc) {
> > + int basic_setting = 0;
> > + int start;
> > +
> > + basic_setting |= tsc->measure_delay_time << 8;
> > + basic_setting |= DETECT_4_WIRE_MODE | AUTO_MEASURE;
> > + writel(basic_setting, tsc->tsc_regs + REG_TSC_BASIC_SETING);
> > +
> > + writel(DE_GLITCH_2, tsc->tsc_regs + REG_TSC_DEBUG_MODE2);
> > +
> > + writel(tsc->pre_charge_time, tsc->tsc_regs +
> REG_TSC_PRE_CHARGE_TIME);
> > + writel(MEASURE_INT_EN, tsc->tsc_regs + REG_TSC_INT_EN);
> > + writel(MEASURE_SIG_EN | VALID_SIG_EN,
> > + tsc->tsc_regs + REG_TSC_INT_SIG_EN);
> > +
> > + /* start sense detection */
> > + start = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> > + start |= START_SENSE;
> > + start &= ~TSC_DISABLE;
> > + writel(start, tsc->tsc_regs + REG_TSC_FLOW_CONTROL); }
> > +
> > +static void imx6ul_tsc_init(struct imx6ul_tsc *tsc) {
> > + imx6ul_adc_init(tsc);
> > + imx6ul_tsc_channel_config(tsc);
> > + imx6ul_tsc_set(tsc);
> > +}
> > +
> > +static irqreturn_t tsc_irq_fn(int irq, void *dev_id) {
> > + struct imx6ul_tsc *tsc = (struct imx6ul_tsc *)dev_id;
>
> No need to cast void pointers.
>
> > + int status;
> > + int value;
> > + int x, y;
> > + int xnur;
> > + int debug_mode2;
> > + int state_machine;
> > + int start;
> > + unsigned long timeout;
> > +
> > + status = readl(tsc->tsc_regs + REG_TSC_INT_STATUS);
> > +
> > + /* write 1 to clear the bit measure-signal */
> > + writel(MEASURE_SIGNAL | DETECT_SIGNAL,
> > + tsc->tsc_regs + REG_TSC_INT_STATUS);
> > +
> > + /* It's a HW self-clean bit. Set this bit and start sense detection
> */
> > + start = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> > + start |= START_SENSE;
> > + writel(start, tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> > +
> > + if (status & MEASURE_SIGNAL) {
> > + value = readl(tsc->tsc_regs + REG_TSC_MEASURE_VALUE);
> > + x = (value >> 16) & 0x0fff;
> > + y = value & 0x0fff;
> > +
> > + /*
> > + * Delay some time(max 2ms), wait the pre-charge done.
> > + * After the pre-change mode, TSC go into detect mode.
> > + * And in detect mode, we can get the xnur gpio value.
> > + * If xnur is low, this means the touch screen still
> > + * be touched. If xnur is high, this means finger leave
> > + * the touch screen.
> > + */
> > + timeout = jiffies + HZ/500;
> > + do {
> > + if (time_after(jiffies, timeout)) {
> > + xnur = 0;
> > + goto touch_event;
> > + }
> > + usleep_range(200, 400);
> > + debug_mode2 = readl(tsc->tsc_regs +
> REG_TSC_DEBUG_MODE2);
> > + state_machine = (debug_mode2 >> 20) & 0x7;
> > + } while (state_machine != DETECT_MODE);
> > + usleep_range(200, 400);
> > +
> > + xnur = gpio_get_value(tsc->xnur_gpio);
>
> It seems to me that this GPIO is actually active low: we reporting active
> touch as long as we read 0.
>
> > +touch_event:
> > + if (xnur == 0) {
> > + input_report_key(tsc->input, BTN_TOUCH, 1);
> > + input_report_abs(tsc->input, ABS_X, x);
> > + input_report_abs(tsc->input, ABS_Y, y);
> > + } else
> > + input_report_key(tsc->input, BTN_TOUCH, 0);
>
> If one branch has braces all of them should have braces.
>
> > +
> > + input_sync(tsc->input);
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static irqreturn_t adc_irq_fn(int irq, void *dev_id) {
> > + struct imx6ul_tsc *tsc = (struct imx6ul_tsc *)dev_id;
> > + int coco;
> > + int value;
> > +
> > + coco = readl(tsc->adc_regs + REG_ADC_HS);
> > + if (coco & 0x01) {
> > + value = readl(tsc->adc_regs + REG_ADC_R0);
> > + complete(&tsc->completion);
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static const struct of_device_id imx6ul_tsc_match[] = {
> > + { .compatible = "fsl,imx6ul-tsc", },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, imx6ul_tsc_match);
> > +
> > +static int imx6ul_tsc_probe(struct platform_device *pdev) {
> > + struct imx6ul_tsc *tsc;
> > + struct resource *tsc_mem;
> > + struct resource *adc_mem;
> > + struct input_dev *input_dev;
> > + struct device_node *np = pdev->dev.of_node;
> > + int err;
> > + int tsc_irq;
> > + int adc_irq;
> > +
> > + tsc = devm_kzalloc(&pdev->dev, sizeof(struct imx6ul_tsc),
> GFP_KERNEL);
> > + input_dev = devm_input_allocate_device(&pdev->dev);
>
> Using devm does not mean you do not need to check results of memory
> allocation.
>
> > +
> > + tsc->dev = &pdev->dev;
> > +
> > + tsc->input = input_dev;
> > + tsc->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> > + tsc->input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
> > + input_set_abs_params(tsc->input, ABS_X, 0, 0xFFF, 0, 0);
> > + input_set_abs_params(tsc->input, ABS_Y, 0, 0xFFF, 0, 0);
> > +
> > + tsc->input->name = "iMX6UL TouchScreen Controller";
> > +
> > + tsc->xnur_gpio = of_get_named_gpio(np, "xnur-gpio", 0);
> > + err = gpio_request_one(tsc->xnur_gpio, GPIOF_IN, "tsc_X-");
>
> Why are you not using devm for gpio? Actually, why don't you also use
> gpiod? As is you are leaking that gpio in all error paths below.
>
> > + if (err) {
> > + dev_err(&pdev->dev, "failed to request GPIO tsc_X-\n");
> > + return err;
> > + }
> > +
> > + tsc_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + tsc->tsc_regs = devm_ioremap_resource(&pdev->dev, tsc_mem);
> > + if (IS_ERR(tsc->tsc_regs)) {
> > + dev_err(&pdev->dev, "failed to remap tsc memory\n");
> > + err = PTR_ERR(tsc->tsc_regs);
> > + return err;
> > + }
> > +
> > + adc_mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > + tsc->adc_regs = devm_ioremap_resource(&pdev->dev, adc_mem);
> > + if (IS_ERR(tsc->adc_regs)) {
> > + dev_err(&pdev->dev, "failed to remap adc memory\n");
> > + err = PTR_ERR(tsc->adc_regs);
> > + return err;
> > + }
> > +
> > + tsc->tsc_clk = devm_clk_get(&pdev->dev, "tsc");
> > + if (IS_ERR(tsc->tsc_clk)) {
> > + dev_err(&pdev->dev, "failed getting tsc clock\n");
> > + err = PTR_ERR(tsc->tsc_clk);
> > + return err;
> > + }
> > +
> > + tsc->adc_clk = devm_clk_get(&pdev->dev, "adc");
> > + if (IS_ERR(tsc->adc_clk)) {
> > + dev_err(&pdev->dev, "failed getting adc clock\n");
> > + err = PTR_ERR(tsc->adc_clk);
> > + return err;
> > + }
> > +
> > + tsc_irq = platform_get_irq(pdev, 0);
> > + if (tsc_irq < 0) {
> > + dev_err(&pdev->dev, "no tsc irq resource?\n");
> > + return tsc_irq;
> > + }
> > +
> > + adc_irq = platform_get_irq(pdev, 1);
> > + if (adc_irq <= 0) {
> > + dev_err(&pdev->dev, "no adc irq resource?\n");
> > + return adc_irq;
> > + }
> > +
> > + err = devm_request_threaded_irq(tsc->dev, tsc_irq,
> > + NULL, tsc_irq_fn,
> > + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> > + dev_name(&pdev->dev), tsc);
> > + if (err < 0) {
> > + dev_err(&pdev->dev,
> > + "failed requesting tsc irq %d\n",
> > + tsc_irq);
> > + return err;
> > + }
> > +
> > + err = devm_request_irq(tsc->dev, adc_irq,
> > + adc_irq_fn, 0,
> > + dev_name(&pdev->dev), tsc);
> > + if (err < 0) {
> > + dev_err(&pdev->dev,
> > + "failed requesting adc irq %d\n",
> > + adc_irq);
> > + return err;
> > + }
> > +
> > + err = of_property_read_u32(np, "measure-delay-time",
> > + &tsc->measure_delay_time);
> > + if (err)
> > + tsc->measure_delay_time = 0xffff;
> > +
> > + err = of_property_read_u32(np, "pre-charge-time",
> > + &tsc->pre_charge_time);
> > + if (err)
> > + tsc->pre_charge_time = 0xfff;
> > +
> > + init_completion(&tsc->completion);
> > +
> > + err = clk_prepare_enable(tsc->adc_clk);
> > + if (err) {
> > + dev_err(&pdev->dev,
> > + "Could not prepare or enable the adc clock.\n");
> > + return err;
> > + }
> > +
> > + err = clk_prepare_enable(tsc->tsc_clk);
> > + if (err) {
> > + dev_err(&pdev->dev,
> > + "Could not prepare or enable the tsc clock.\n");
> > + goto error_tsc_clk_enable;
> > + }
> > +
> > + err = input_register_device(tsc->input);
> > + if (err < 0) {
> > + dev_err(&pdev->dev, "failed to register input device\n");
> > + err = -EIO;
> > + goto err_input_register;
> > + }
> > +
> > + imx6ul_tsc_init(tsc);
>
> Enabling clock and initializing the controller is better in open() method
> of input device. If you also implement close() you can get rid of
> remove().
>
> > +
> > + platform_set_drvdata(pdev, tsc);
> > + return 0;
> > +
> > +err_input_register:
> > + clk_disable_unprepare(tsc->tsc_clk);
> > +error_tsc_clk_enable:
> > + clk_disable_unprepare(tsc->adc_clk);
> > +
> > + return err;
> > +}
> > +
> > +static void imx6ul_tsc_disable(struct imx6ul_tsc *tsc) {
> > + int tsc_flow;
> > + int adc_cfg;
> > +
> > + /* TSC controller enters to idle status */
> > + tsc_flow = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> > + tsc_flow |= TSC_DISABLE;
> > + writel(tsc_flow, tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> > +
> > + /* ADC controller enters to stop mode */
> > + adc_cfg = readl(tsc->adc_regs + REG_ADC_HC0);
> > + adc_cfg |= ADC_CONV_DISABLE;
> > + writel(adc_cfg, tsc->adc_regs + REG_ADC_HC0); }
> > +
> > +static int imx6ul_tsc_remove(struct platform_device *pdev) {
> > + struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
> > +
> > + imx6ul_tsc_disable(tsc);
> > +
> > + clk_disable_unprepare(tsc->tsc_clk);
> > + clk_disable_unprepare(tsc->adc_clk);
> > + input_unregister_device(tsc->input);
> > + kfree(tsc);
>
> Tsc is allocated with devm(), you can't kfree() it.
> > +
> > + return 0;
> > +}
> > +
> > +static int __maybe_unused imx6ul_tsc_suspend(struct device *dev) {
> > + struct platform_device *pdev = to_platform_device(dev);
> > + struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
> > +
> > + imx6ul_tsc_disable(tsc);
> > +
> > + clk_disable_unprepare(tsc->tsc_clk);
> > + clk_disable_unprepare(tsc->adc_clk);
> > +
> > + return 0;
> > +}
> > +
> > +static int __maybe_unused imx6ul_tsc_resume(struct device *dev) {
> > + struct platform_device *pdev = to_platform_device(dev);
> > + struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
> > + int err;
> > +
> > + err = clk_prepare_enable(tsc->adc_clk);
> > + if (err)
> > + return err;
> > +
> > + err = clk_prepare_enable(tsc->tsc_clk);
> > + if (err) {
> > + clk_disable_unprepare(tsc->adc_clk);
> > + return err;
> > + }
> > +
> > + imx6ul_tsc_init(tsc);
> > + return 0;
> > +}
> > +
> > +static SIMPLE_DEV_PM_OPS(imx6ul_tsc_pm_ops,
> > + imx6ul_tsc_suspend,
> > + imx6ul_tsc_resume);
> > +
> > +static struct platform_driver imx6ul_tsc_driver = {
> > + .driver = {
> > + .name = "imx6ul-tsc",
> > + .owner = THIS_MODULE,
> > + .of_match_table = imx6ul_tsc_match,
> > + .pm = &imx6ul_tsc_pm_ops,
> > + },
> > + .probe = imx6ul_tsc_probe,
> > + .remove = imx6ul_tsc_remove,
> > +};
> > +module_platform_driver(imx6ul_tsc_driver);
> > +
> > +MODULE_AUTHOR("Haibo Chen <[email protected]>");
> > +MODULE_DESCRIPTION("Freescale i.MX6UL Touchscreen controller
> > +driver"); MODULE_LICENSE("GPL v2");
> > --
> > 1.9.1
> >
>
> Can you try the patch below and let me know if it still works (you'll
> need to adjust your DTS for xnur gpio being active low).
>
> Thanks!
>
> --
> Dmitry
>
> Input: imx6ul_tsc - misc changes
>
> From: Dmitry Torokhov <[email protected]>
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
> ---
> .../bindings/input/touchscreen/imx6ul_tsc.txt | 2
> drivers/input/touchscreen/Kconfig | 2
> drivers/input/touchscreen/imx6ul_tsc.c | 262 +++++++++++---
> ------
> 3 files changed, 143 insertions(+), 123 deletions(-)
>
> diff --git
> a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> index ac41c32..c933588 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> @@ -29,7 +29,7 @@ Example:
> clock-names = "tsc", "adc";
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_tsc>;
> - xnur-gpio = <&gpio1 3 GPIO_ACTIVE_HIGH>;
> + xnur-gpio = <&gpio1 3 GPIO_ACTIVE_LOW>;
> measure-delay-time = <0xfff>;
> pre-charge-time = <0xffff>;
> status = "okay";
> diff --git a/drivers/input/touchscreen/Kconfig
> b/drivers/input/touchscreen/Kconfig
> index 9a1a293..5ecf19b 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -481,7 +481,7 @@ config TOUCHSCREEN_MTOUCH
>
> config TOUCHSCREEN_IMX6UL_TSC
> tristate "Freescale i.MX6UL touchscreen controller"
> - depends on OF
> + depends on (OF && GPIOLIB) || COMPILE_TEST
> help
> Say Y here if you have a Freescale i.MX6UL, and want to
> use the internal touchscreen controller.
> diff --git a/drivers/input/touchscreen/imx6ul_tsc.c
> b/drivers/input/touchscreen/imx6ul_tsc.c
> index 807f1db..1d028ed 100644
> --- a/drivers/input/touchscreen/imx6ul_tsc.c
> +++ b/drivers/input/touchscreen/imx6ul_tsc.c
> @@ -11,15 +11,12 @@
> #include <linux/errno.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/input.h>
> #include <linux/slab.h>
> #include <linux/completion.h>
> #include <linux/delay.h>
> #include <linux/of.h>
> -#include <linux/of_device.h>
> -#include <linux/of_gpio.h>
> -#include <linux/of_irq.h>
> #include <linux/interrupt.h>
> #include <linux/platform_device.h>
> #include <linux/clk.h>
> @@ -85,8 +82,8 @@ struct imx6ul_tsc {
> void __iomem *adc_regs;
> struct clk *tsc_clk;
> struct clk *adc_clk;
> + struct gpio_desc *xnur_gpio;
>
> - int xnur_gpio;
> int measure_delay_time;
> int pre_charge_time;
>
> @@ -105,6 +102,8 @@ static void imx6ul_adc_init(struct imx6ul_tsc *tsc)
> int adc_cfg;
> int timeout;
>
> + reinit_completion(&tsc->completion);
> +
> adc_cfg = readl(tsc->adc_regs + REG_ADC_CFG);
> adc_cfg |= ADC_12BIT_MODE | ADC_IPG_CLK;
> adc_cfg |= ADC_CLK_DIV_8 | ADC_SHORT_SAMPLE_MODE;
> @@ -196,17 +195,33 @@ static void imx6ul_tsc_init(struct imx6ul_tsc *tsc)
> imx6ul_tsc_set(tsc);
> }
>
> +static void imx6ul_tsc_disable(struct imx6ul_tsc *tsc)
> +{
> + int tsc_flow;
> + int adc_cfg;
> +
> + /* TSC controller enters to idle status */
> + tsc_flow = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> + tsc_flow |= TSC_DISABLE;
> + writel(tsc_flow, tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> +
> + /* ADC controller enters to stop mode */
> + adc_cfg = readl(tsc->adc_regs + REG_ADC_HC0);
> + adc_cfg |= ADC_CONV_DISABLE;
> + writel(adc_cfg, tsc->adc_regs + REG_ADC_HC0);
> +}
> +
> static irqreturn_t tsc_irq_fn(int irq, void *dev_id)
> {
> struct imx6ul_tsc *tsc = (struct imx6ul_tsc *)dev_id;
> int status;
> int value;
> int x, y;
> - int xnur;
> int debug_mode2;
> int state_machine;
> int start;
> unsigned long timeout;
> + bool touch;
>
> status = readl(tsc->tsc_regs + REG_TSC_INT_STATUS);
>
> @@ -235,8 +250,8 @@ static irqreturn_t tsc_irq_fn(int irq, void *dev_id)
> timeout = jiffies + HZ/500;
> do {
> if (time_after(jiffies, timeout)) {
> - xnur = 0;
> - goto touch_event;
> + touch = true;
> + goto report_touch;
> }
> usleep_range(200, 400);
> debug_mode2 = readl(tsc->tsc_regs +
> REG_TSC_DEBUG_MODE2);
> @@ -244,14 +259,15 @@ static irqreturn_t tsc_irq_fn(int irq, void *dev_id)
> } while (state_machine != DETECT_MODE);
> usleep_range(200, 400);
>
> - xnur = gpio_get_value(tsc->xnur_gpio);
> -touch_event:
> - if (xnur == 0) {
> + touch = gpiod_get_value_cansleep(tsc->xnur_gpio);
> +report_touch:
> + if (touch) {
> input_report_key(tsc->input, BTN_TOUCH, 1);
> input_report_abs(tsc->input, ABS_X, x);
> input_report_abs(tsc->input, ABS_Y, y);
> - } else
> + } else {
> input_report_key(tsc->input, BTN_TOUCH, 0);
> + }
>
> input_sync(tsc->input);
> }
> @@ -261,7 +277,7 @@ touch_event:
>
> static irqreturn_t adc_irq_fn(int irq, void *dev_id)
> {
> - struct imx6ul_tsc *tsc = (struct imx6ul_tsc *)dev_id;
> + struct imx6ul_tsc *tsc = dev_id;
> int coco;
> int value;
>
> @@ -274,70 +290,113 @@ static irqreturn_t adc_irq_fn(int irq, void
> *dev_id)
> return IRQ_HANDLED;
> }
>
> -static const struct of_device_id imx6ul_tsc_match[] = {
> - { .compatible = "fsl,imx6ul-tsc", },
> - { /* sentinel */ }
> -};
> -MODULE_DEVICE_TABLE(of, imx6ul_tsc_match);
> +static int imx6ul_tsc_open(struct input_dev *input_dev)
> +{
> + struct imx6ul_tsc *tsc = input_get_drvdata(input_dev);
> + int err;
> +
> + err = clk_prepare_enable(tsc->adc_clk);
> + if (err) {
> + dev_err(tsc->dev,
> + "Could not prepare or enable the adc clock: %d\n",
> + err);
> + return err;
> + }
> +
> + err = clk_prepare_enable(tsc->tsc_clk);
> + if (err) {
> + dev_err(tsc->dev,
> + "Could not prepare or enable the tsc clock: %d\n",
> + err);
> + clk_disable_unprepare(tsc->adc_clk);
> + return err;
> + }
> +
> + imx6ul_tsc_init(tsc);
> +
> + return 0;
> +}
> +
> +static void imx6ul_tsc_close(struct input_dev *input_dev)
> +{
> + struct imx6ul_tsc *tsc = input_get_drvdata(input_dev);
> +
> + imx6ul_tsc_disable(tsc);
> +
> + clk_disable_unprepare(tsc->tsc_clk);
> + clk_disable_unprepare(tsc->adc_clk);
> +}
>
> static int imx6ul_tsc_probe(struct platform_device *pdev)
> {
> + struct device_node *np = pdev->dev.of_node;
> struct imx6ul_tsc *tsc;
> + struct input_dev *input_dev;
> struct resource *tsc_mem;
> struct resource *adc_mem;
> - struct input_dev *input_dev;
> - struct device_node *np = pdev->dev.of_node;
> int err;
> int tsc_irq;
> int adc_irq;
>
> tsc = devm_kzalloc(&pdev->dev, sizeof(struct imx6ul_tsc),
> GFP_KERNEL);
> + if (!tsc)
> + return -ENOMEM;
> +
> input_dev = devm_input_allocate_device(&pdev->dev);
> + if (!input_dev)
> + return -ENOMEM;
>
> - tsc->dev = &pdev->dev;
> + input_dev->name = "iMX6UL TouchScreen Controller";
> + input_dev->id.bustype = BUS_HOST;
>
> - tsc->input = input_dev;
> - tsc->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> - tsc->input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
> - input_set_abs_params(tsc->input, ABS_X, 0, 0xFFF, 0, 0);
> - input_set_abs_params(tsc->input, ABS_Y, 0, 0xFFF, 0, 0);
> + input_dev->open = imx6ul_tsc_open;
> + input_dev->close = imx6ul_tsc_close;
>
> - tsc->input->name = "iMX6UL TouchScreen Controller";
> + input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
> + input_set_abs_params(input_dev, ABS_X, 0, 0xFFF, 0, 0);
> + input_set_abs_params(input_dev, ABS_Y, 0, 0xFFF, 0, 0);
>
> - tsc->xnur_gpio = of_get_named_gpio(np, "xnur-gpio", 0);
> - err = gpio_request_one(tsc->xnur_gpio, GPIOF_IN, "tsc_X-");
> - if (err) {
> - dev_err(&pdev->dev, "failed to request GPIO tsc_X-\n");
> + input_set_drvdata(input_dev, tsc);
> +
> + tsc->dev = &pdev->dev;
> + tsc->input = input_dev;
> + init_completion(&tsc->completion);
> +
> + tsc->xnur_gpio = devm_gpiod_get(&pdev->dev, "xnur-gpio", GPIOD_IN);
> + if (IS_ERR(tsc->xnur_gpio)) {
> + err = PTR_ERR(tsc->xnur_gpio);
> + dev_err(&pdev->dev,
> + "failed to request GPIO tsc_X- (xnur): %d\n", err);
> return err;
> }
>
> tsc_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> tsc->tsc_regs = devm_ioremap_resource(&pdev->dev, tsc_mem);
> if (IS_ERR(tsc->tsc_regs)) {
> - dev_err(&pdev->dev, "failed to remap tsc memory\n");
> err = PTR_ERR(tsc->tsc_regs);
> + dev_err(&pdev->dev, "failed to remap tsc memory: %d\n", err);
> return err;
> }
>
> adc_mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> tsc->adc_regs = devm_ioremap_resource(&pdev->dev, adc_mem);
> if (IS_ERR(tsc->adc_regs)) {
> - dev_err(&pdev->dev, "failed to remap adc memory\n");
> err = PTR_ERR(tsc->adc_regs);
> + dev_err(&pdev->dev, "failed to remap adc memory: %d\n", err);
> return err;
> }
>
> tsc->tsc_clk = devm_clk_get(&pdev->dev, "tsc");
> if (IS_ERR(tsc->tsc_clk)) {
> - dev_err(&pdev->dev, "failed getting tsc clock\n");
> err = PTR_ERR(tsc->tsc_clk);
> + dev_err(&pdev->dev, "failed getting tsc clock: %d\n", err);
> return err;
> }
>
> tsc->adc_clk = devm_clk_get(&pdev->dev, "adc");
> if (IS_ERR(tsc->adc_clk)) {
> - dev_err(&pdev->dev, "failed getting adc clock\n");
> err = PTR_ERR(tsc->adc_clk);
> + dev_err(&pdev->dev, "failed getting adc clock: %d\n", err);
> return err;
> }
>
> @@ -354,111 +413,61 @@ static int imx6ul_tsc_probe(struct platform_device
> *pdev)
> }
>
> err = devm_request_threaded_irq(tsc->dev, tsc_irq,
> - NULL, tsc_irq_fn,
> - IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> + NULL, tsc_irq_fn, IRQF_ONESHOT,
> dev_name(&pdev->dev), tsc);
> - if (err < 0) {
> + if (err) {
> dev_err(&pdev->dev,
> - "failed requesting tsc irq %d\n",
> - tsc_irq);
> + "failed requesting tsc irq %di: %d\n",
> + tsc_irq, err);
> return err;
> }
>
> - err = devm_request_irq(tsc->dev, adc_irq,
> - adc_irq_fn, 0,
> + err = devm_request_irq(tsc->dev, adc_irq, adc_irq_fn, 0,
> dev_name(&pdev->dev), tsc);
> - if (err < 0) {
> + if (err) {
> dev_err(&pdev->dev,
> - "failed requesting adc irq %d\n",
> - adc_irq);
> + "failed requesting adc irq %d: %d\n",
> + adc_irq, err);
> return err;
> }
>
> err = of_property_read_u32(np, "measure-delay-time",
> - &tsc->measure_delay_time);
> + &tsc->measure_delay_time);
> if (err)
> tsc->measure_delay_time = 0xffff;
>
> err = of_property_read_u32(np, "pre-charge-time",
> - &tsc->pre_charge_time);
> + &tsc->pre_charge_time);
> if (err)
> tsc->pre_charge_time = 0xfff;
>
> - init_completion(&tsc->completion);
> -
> - err = clk_prepare_enable(tsc->adc_clk);
> + err = input_register_device(tsc->input);
> if (err) {
> dev_err(&pdev->dev,
> - "Could not prepare or enable the adc clock.\n");
> + "failed to register input device: %d\n", err);
> return err;
> }
>
> - err = clk_prepare_enable(tsc->tsc_clk);
> - if (err) {
> - dev_err(&pdev->dev,
> - "Could not prepare or enable the tsc clock.\n");
> - goto error_tsc_clk_enable;
> - }
> -
> - err = input_register_device(tsc->input);
> - if (err < 0) {
> - dev_err(&pdev->dev, "failed to register input device\n");
> - err = -EIO;
> - goto err_input_register;
> - }
> -
> - imx6ul_tsc_init(tsc);
> -
> platform_set_drvdata(pdev, tsc);
> return 0;
> -
> -err_input_register:
> - clk_disable_unprepare(tsc->tsc_clk);
> -error_tsc_clk_enable:
> - clk_disable_unprepare(tsc->adc_clk);
> -
> - return err;
> -}
> -
> -static void imx6ul_tsc_disable(struct imx6ul_tsc *tsc)
> -{
> - int tsc_flow;
> - int adc_cfg;
> -
> - /* TSC controller enters to idle status */
> - tsc_flow = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> - tsc_flow |= TSC_DISABLE;
> - writel(tsc_flow, tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> -
> - /* ADC controller enters to stop mode */
> - adc_cfg = readl(tsc->adc_regs + REG_ADC_HC0);
> - adc_cfg |= ADC_CONV_DISABLE;
> - writel(adc_cfg, tsc->adc_regs + REG_ADC_HC0);
> -}
> -
> -static int imx6ul_tsc_remove(struct platform_device *pdev)
> -{
> - struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
> -
> - imx6ul_tsc_disable(tsc);
> -
> - clk_disable_unprepare(tsc->tsc_clk);
> - clk_disable_unprepare(tsc->adc_clk);
> - input_unregister_device(tsc->input);
> - kfree(tsc);
> -
> - return 0;
> }
>
> static int __maybe_unused imx6ul_tsc_suspend(struct device *dev)
> {
> struct platform_device *pdev = to_platform_device(dev);
> struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
> + struct input_dev *input_dev = tsc->input;
>
> - imx6ul_tsc_disable(tsc);
> + mutex_lock(&input_dev->mutex);
>
> - clk_disable_unprepare(tsc->tsc_clk);
> - clk_disable_unprepare(tsc->adc_clk);
> + if (input_dev->users) {
> + imx6ul_tsc_disable(tsc);
> +
> + clk_disable_unprepare(tsc->tsc_clk);
> + clk_disable_unprepare(tsc->adc_clk);
> + }
> +
> + mutex_unlock(&input_dev->mutex);
>
> return 0;
> }
> @@ -467,35 +476,46 @@ static int __maybe_unused imx6ul_tsc_resume(struct
> device *dev)
> {
> struct platform_device *pdev = to_platform_device(dev);
> struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
> - int err;
> + struct input_dev *input_dev = tsc->input;
> + int retval = 0;
>
> - err = clk_prepare_enable(tsc->adc_clk);
> - if (err)
> - return err;
> + mutex_lock(&input_dev->mutex);
>
> - err = clk_prepare_enable(tsc->tsc_clk);
> - if (err) {
> - clk_disable_unprepare(tsc->adc_clk);
> - return err;
> + if (input_dev->users) {
> + retval = clk_prepare_enable(tsc->adc_clk);
> + if (retval)
> + goto out;
> +
> + retval = clk_prepare_enable(tsc->tsc_clk);
> + if (retval) {
> + clk_disable_unprepare(tsc->adc_clk);
> + goto out;
> + }
> +
> + imx6ul_tsc_init(tsc);
> }
>
> - imx6ul_tsc_init(tsc);
> - return 0;
> +out:
> + mutex_unlock(&input_dev->mutex);
> + return retval;
> }
>
> static SIMPLE_DEV_PM_OPS(imx6ul_tsc_pm_ops,
> - imx6ul_tsc_suspend,
> - imx6ul_tsc_resume);
> + imx6ul_tsc_suspend, imx6ul_tsc_resume);
> +
> +static const struct of_device_id imx6ul_tsc_match[] = {
> + { .compatible = "fsl,imx6ul-tsc", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imx6ul_tsc_match);
>
> static struct platform_driver imx6ul_tsc_driver = {
> .driver = {
> .name = "imx6ul-tsc",
> - .owner = THIS_MODULE,
> .of_match_table = imx6ul_tsc_match,
> .pm = &imx6ul_tsc_pm_ops,
> },
> .probe = imx6ul_tsc_probe,
> - .remove = imx6ul_tsc_remove,
> };
> module_platform_driver(imx6ul_tsc_driver);
>

2015-08-21 08:30:25

by Chen Bough

[permalink] [raw]
Subject: RE: [PATCH v2 2/5] Documentation: Detail permitted DT properties for the imx6ul_tsc

Hi Markus,

> -----Original Message-----
> From: Markus Pargmann [mailto:[email protected]]
> Sent: Wednesday, August 19, 2015 1:55 PM
> To: Chen Haibo-B51421
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 2/5] Documentation: Detail permitted DT properties
> for the imx6ul_tsc
>
> Hi,
>
> On Tue, Jul 28, 2015 at 05:58:38PM +0800, Haibo Chen wrote:
> > Here we apply required documentation for the imx6ul touch screen
> > controller driver which describe available properties and how to use
> > them.
> >
> > Signed-off-by: Haibo Chen <[email protected]>
> > ---
> > .../bindings/input/touchscreen/imx6ul_tsc.txt | 36
> ++++++++++++++++++++++
> > 1 file changed, 36 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> > b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> > new file mode 100644
> > index 0000000..ac41c32
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.t
> > +++ xt
> > @@ -0,0 +1,36 @@
> > +* Freescale i.MX6UL Touch Controller
> > +
> > +Required properties:
> > +- compatible: must be "fsl,imx6ul-tsc".
> > +- reg: this touch controller address and the ADC2 address.
>
> This suggests that this driver is using a unit ADC2. Which also means
> that there are more than one ADC which are probably identical?
>
> Shouldn't these ADCs be properly described by their own device nodes
> instead of these two register ranges, two interrupts and two clocks?
>
> Is 'ADC2' usable without tsc? Then ADC1/ADC2 should perhaps get a proper
> IIO driver.

For i.MX6UL, there are two ADC. ADC1 is a normal ADC, and ADC2 can only works for
TSC, the channels of ADC2 are connected to TSC directly. TSC and ADC2 should work
together as a touch screen controller.

For ADC1, it share the driver vf610_adc.c (drivers/iio/adc/vf610_adc.c).

Best Regards

Haibo

>
> Unfortunately I don't have the reference manual to have a look how this
> all works.
>
> Best regards,
>
> Markus
>
> > +- interrupts: the interrupt of this touch controller and ADC2.
> > +- clocks: the root clock of touch controller and ADC2.
> > +- clock-names; must be "tsc" and "adc".
> > +- xnur-gpio: the X- gpio this controller connect to.
> > + This xnur-gpio returns to high once the finger leave the touch
> > +screen (The
> > + last touch event the touch controller capture).
> > +
> > +Optional properties:
> > +- measure-delay-time: the value of measure delay time.
> > + Before X-axis or Y-axis measurement, the screen need some time
> > +before
> > + even potential distribution ready.
> > + This value depends on the touch screen.
> > +- pre-charge-time: the touch screen need some time to precharge.
> > + This value depends on the touch screen.
> > +
> > +Example:
> > + tsc: tsc@02040000 {
> > + compatible = "fsl,imx6ul-tsc";
> > + reg = <0x02040000 0x4000>, <0x0219c000 0x4000>;
> > + interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clks IMX6UL_CLK_IPG>,
> > + <&clks IMX6UL_CLK_ADC2>;
> > + clock-names = "tsc", "adc";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_tsc>;
> > + xnur-gpio = <&gpio1 3 GPIO_ACTIVE_HIGH>;
> > + measure-delay-time = <0xfff>;
> > + pre-charge-time = <0xffff>;
> > + status = "okay";
> > + };
> > --
> > 1.9.1
> >
> >
> >
>
> --
> Pengutronix e.K. |
> |
> Industrial Linux Solutions | http://www.pengutronix.de/
> |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0
> |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555
> |
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2015-08-21 09:27:30

by Markus Pargmann

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] input: touchscreen: add imx6ul_tsc driver support

On Tue, Jul 28, 2015 at 05:58:37PM +0800, Haibo Chen wrote:
> Freescale i.MX6UL contains a internal touchscreen controller,
> this patch add a driver to support this controller.
>
> Signed-off-by: Haibo Chen <[email protected]>
> ---
> drivers/input/touchscreen/Kconfig | 12 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/imx6ul_tsc.c | 504 +++++++++++++++++++++++++++++++++
> 3 files changed, 517 insertions(+)
> create mode 100644 drivers/input/touchscreen/imx6ul_tsc.c
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 5b272ba..32c300d 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -479,6 +479,18 @@ config TOUCHSCREEN_MTOUCH
> To compile this driver as a module, choose M here: the
> module will be called mtouch.
>
> +config TOUCHSCREEN_IMX6UL_TSC
> + tristate "Freescale i.MX6UL touchscreen controller"
> + depends on OF
> + help
> + Say Y here if you have a Freescale i.MX6UL, and want to
> + use the internal touchscreen controller.
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + moduel will be called imx6ul_tsc.
> +
> config TOUCHSCREEN_INEXIO
> tristate "iNexio serial touchscreens"
> select SERIO
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index c85aae2..9379b32 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -38,6 +38,7 @@ obj-$(CONFIG_TOUCHSCREEN_EGALAX) += egalax_ts.o
> obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
> obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix.o
> obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
> +obj-$(CONFIG_TOUCHSCREEN_IMX6UL_TSC) += imx6ul_tsc.o
> obj-$(CONFIG_TOUCHSCREEN_INEXIO) += inexio.o
> obj-$(CONFIG_TOUCHSCREEN_INTEL_MID) += intel-mid-touch.o
> obj-$(CONFIG_TOUCHSCREEN_IPROC) += bcm_iproc_tsc.o
> diff --git a/drivers/input/touchscreen/imx6ul_tsc.c b/drivers/input/touchscreen/imx6ul_tsc.c
> new file mode 100644
> index 0000000..807f1db
> --- /dev/null
> +++ b/drivers/input/touchscreen/imx6ul_tsc.c
> @@ -0,0 +1,504 @@
> +/*
> + * Freescale i.MX6UL touchscreen controller driver
> + *
> + * Copyright (C) 2015 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/gpio.h>
> +#include <linux/input.h>
> +#include <linux/slab.h>
> +#include <linux/completion.h>
> +#include <linux/delay.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +
> +/* ADC configuration registers field define */
> +#define ADC_AIEN (0x1 << 7)
> +#define ADC_CONV_DISABLE 0x1F
> +#define ADC_CAL (0x1 << 7)
> +#define ADC_CALF 0x2
> +#define ADC_12BIT_MODE (0x2 << 2)
> +#define ADC_IPG_CLK 0x00
> +#define ADC_CLK_DIV_8 (0x03 << 5)
> +#define ADC_SHORT_SAMPLE_MODE (0x0 << 4)
> +#define ADC_HARDWARE_TRIGGER (0x1 << 13)
> +#define SELECT_CHANNEL_4 0x04
> +#define SELECT_CHANNEL_1 0x01
> +#define DISABLE_CONVERSION_INT (0x0 << 7)
> +
> +/* ADC registers */
> +#define REG_ADC_HC0 0x00
> +#define REG_ADC_HC1 0x04
> +#define REG_ADC_HC2 0x08
> +#define REG_ADC_HC3 0x0C
> +#define REG_ADC_HC4 0x10
> +#define REG_ADC_HS 0x14
> +#define REG_ADC_R0 0x18
> +#define REG_ADC_CFG 0x2C
> +#define REG_ADC_GC 0x30
> +#define REG_ADC_GS 0x34
> +
> +#define ADC_TIMEOUT msecs_to_jiffies(100)

These defines are in two drivers. Here and in
drivers/iio/adc/vf610_adc.c

> +
> +/* TSC registers */
> +#define REG_TSC_BASIC_SETING 0x00
> +#define REG_TSC_PRE_CHARGE_TIME 0x10
> +#define REG_TSC_FLOW_CONTROL 0x20
> +#define REG_TSC_MEASURE_VALUE 0x30
> +#define REG_TSC_INT_EN 0x40
> +#define REG_TSC_INT_SIG_EN 0x50
> +#define REG_TSC_INT_STATUS 0x60
> +#define REG_TSC_DEBUG_MODE 0x70
> +#define REG_TSC_DEBUG_MODE2 0x80
> +
> +/* TSC configuration registers field define */
> +#define DETECT_4_WIRE_MODE (0x0 << 4)
> +#define AUTO_MEASURE 0x1
> +#define MEASURE_SIGNAL 0x1
> +#define DETECT_SIGNAL (0x1 << 4)
> +#define VALID_SIGNAL (0x1 << 8)
> +#define MEASURE_INT_EN 0x1
> +#define MEASURE_SIG_EN 0x1
> +#define VALID_SIG_EN (0x1 << 8)
> +#define DE_GLITCH_2 (0x2 << 29)
> +#define START_SENSE (0x1 << 12)
> +#define TSC_DISABLE (0x1 << 16)
> +#define DETECT_MODE 0x2
> +
> +struct imx6ul_tsc {
> + struct device *dev;
> + struct input_dev *input;
> + void __iomem *tsc_regs;
> + void __iomem *adc_regs;
> + struct clk *tsc_clk;
> + struct clk *adc_clk;
> +
> + int xnur_gpio;
> + int measure_delay_time;
> + int pre_charge_time;
> +
> + struct completion completion;
> +};
> +
> +/*
> + * TSC module need ADC to get the measure value. So
> + * before config TSC, we should initialize ADC module.
> + */
> +static void imx6ul_adc_init(struct imx6ul_tsc *tsc)
> +{
> + int adc_hc = 0;
> + int adc_gc;
> + int adc_gs;
> + int adc_cfg;
> + int timeout;
> +
> + adc_cfg = readl(tsc->adc_regs + REG_ADC_CFG);
> + adc_cfg |= ADC_12BIT_MODE | ADC_IPG_CLK;
> + adc_cfg |= ADC_CLK_DIV_8 | ADC_SHORT_SAMPLE_MODE;
> + adc_cfg &= ~ADC_HARDWARE_TRIGGER;
> + writel(adc_cfg, tsc->adc_regs + REG_ADC_CFG);
> +
> + /* enable calibration interrupt */
> + adc_hc |= ADC_AIEN;
> + adc_hc |= ADC_CONV_DISABLE;
> + writel(adc_hc, tsc->adc_regs + REG_ADC_HC0);
> +
> + /* start ADC calibration */
> + adc_gc = readl(tsc->adc_regs + REG_ADC_GC);
> + adc_gc |= ADC_CAL;
> + writel(adc_gc, tsc->adc_regs + REG_ADC_GC);
> +
> + timeout = wait_for_completion_timeout
> + (&tsc->completion, ADC_TIMEOUT);
> + if (timeout == 0)
> + dev_err(tsc->dev, "Timeout for adc calibration\n");
> +
> + adc_gs = readl(tsc->adc_regs + REG_ADC_GS);
> + if (adc_gs & ADC_CALF)
> + dev_err(tsc->dev, "ADC calibration failed\n");
> +
> + /* TSC need the ADC work in hardware trigger */
> + adc_cfg = readl(tsc->adc_regs + REG_ADC_CFG);
> + adc_cfg |= ADC_HARDWARE_TRIGGER;
> + writel(adc_cfg, tsc->adc_regs + REG_ADC_CFG);
> +}
> +
> +/*
> + * This is a TSC workaround. Currently TSC misconnect two
> + * ADC channels, this function remap channel configure for
> + * hardware trigger.
> + */
> +static void imx6ul_tsc_channel_config(struct imx6ul_tsc *tsc)
> +{
> + int adc_hc0, adc_hc1, adc_hc2, adc_hc3, adc_hc4;
> +
> + adc_hc0 = DISABLE_CONVERSION_INT;
> + writel(adc_hc0, tsc->adc_regs + REG_ADC_HC0);
> +
> + adc_hc1 = DISABLE_CONVERSION_INT | SELECT_CHANNEL_4;
> + writel(adc_hc1, tsc->adc_regs + REG_ADC_HC1);
> +
> + adc_hc2 = DISABLE_CONVERSION_INT;
> + writel(adc_hc2, tsc->adc_regs + REG_ADC_HC2);
> +
> + adc_hc3 = DISABLE_CONVERSION_INT | SELECT_CHANNEL_1;
> + writel(adc_hc3, tsc->adc_regs + REG_ADC_HC3);
> +
> + adc_hc4 = DISABLE_CONVERSION_INT;
> + writel(adc_hc4, tsc->adc_regs + REG_ADC_HC4);
> +}

Is this code similar to initialization of the adc driver? Does it make
sense to move this code to the ADC driver?

Best regards,

Markus

> +
> +/*
> + * TSC setting, confige the pre-charge time and measure delay time.
> + * different touch screen may need different pre-charge time and
> + * measure delay time.
> + */
> +static void imx6ul_tsc_set(struct imx6ul_tsc *tsc)
> +{
> + int basic_setting = 0;
> + int start;
> +
> + basic_setting |= tsc->measure_delay_time << 8;
> + basic_setting |= DETECT_4_WIRE_MODE | AUTO_MEASURE;
> + writel(basic_setting, tsc->tsc_regs + REG_TSC_BASIC_SETING);
> +
> + writel(DE_GLITCH_2, tsc->tsc_regs + REG_TSC_DEBUG_MODE2);
> +
> + writel(tsc->pre_charge_time, tsc->tsc_regs + REG_TSC_PRE_CHARGE_TIME);
> + writel(MEASURE_INT_EN, tsc->tsc_regs + REG_TSC_INT_EN);
> + writel(MEASURE_SIG_EN | VALID_SIG_EN,
> + tsc->tsc_regs + REG_TSC_INT_SIG_EN);
> +
> + /* start sense detection */
> + start = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> + start |= START_SENSE;
> + start &= ~TSC_DISABLE;
> + writel(start, tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> +}
> +
> +static void imx6ul_tsc_init(struct imx6ul_tsc *tsc)
> +{
> + imx6ul_adc_init(tsc);
> + imx6ul_tsc_channel_config(tsc);
> + imx6ul_tsc_set(tsc);
> +}
> +
> +static irqreturn_t tsc_irq_fn(int irq, void *dev_id)
> +{
> + struct imx6ul_tsc *tsc = (struct imx6ul_tsc *)dev_id;
> + int status;
> + int value;
> + int x, y;
> + int xnur;
> + int debug_mode2;
> + int state_machine;
> + int start;
> + unsigned long timeout;
> +
> + status = readl(tsc->tsc_regs + REG_TSC_INT_STATUS);
> +
> + /* write 1 to clear the bit measure-signal */
> + writel(MEASURE_SIGNAL | DETECT_SIGNAL,
> + tsc->tsc_regs + REG_TSC_INT_STATUS);
> +
> + /* It's a HW self-clean bit. Set this bit and start sense detection */
> + start = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> + start |= START_SENSE;
> + writel(start, tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> +
> + if (status & MEASURE_SIGNAL) {
> + value = readl(tsc->tsc_regs + REG_TSC_MEASURE_VALUE);
> + x = (value >> 16) & 0x0fff;
> + y = value & 0x0fff;
> +
> + /*
> + * Delay some time(max 2ms), wait the pre-charge done.
> + * After the pre-change mode, TSC go into detect mode.
> + * And in detect mode, we can get the xnur gpio value.
> + * If xnur is low, this means the touch screen still
> + * be touched. If xnur is high, this means finger leave
> + * the touch screen.
> + */
> + timeout = jiffies + HZ/500;
> + do {
> + if (time_after(jiffies, timeout)) {
> + xnur = 0;
> + goto touch_event;
> + }
> + usleep_range(200, 400);
> + debug_mode2 = readl(tsc->tsc_regs + REG_TSC_DEBUG_MODE2);
> + state_machine = (debug_mode2 >> 20) & 0x7;
> + } while (state_machine != DETECT_MODE);
> + usleep_range(200, 400);
> +
> + xnur = gpio_get_value(tsc->xnur_gpio);
> +touch_event:
> + if (xnur == 0) {
> + input_report_key(tsc->input, BTN_TOUCH, 1);
> + input_report_abs(tsc->input, ABS_X, x);
> + input_report_abs(tsc->input, ABS_Y, y);
> + } else
> + input_report_key(tsc->input, BTN_TOUCH, 0);
> +
> + input_sync(tsc->input);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t adc_irq_fn(int irq, void *dev_id)
> +{
> + struct imx6ul_tsc *tsc = (struct imx6ul_tsc *)dev_id;
> + int coco;
> + int value;
> +
> + coco = readl(tsc->adc_regs + REG_ADC_HS);
> + if (coco & 0x01) {
> + value = readl(tsc->adc_regs + REG_ADC_R0);
> + complete(&tsc->completion);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static const struct of_device_id imx6ul_tsc_match[] = {
> + { .compatible = "fsl,imx6ul-tsc", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imx6ul_tsc_match);
> +
> +static int imx6ul_tsc_probe(struct platform_device *pdev)
> +{
> + struct imx6ul_tsc *tsc;
> + struct resource *tsc_mem;
> + struct resource *adc_mem;
> + struct input_dev *input_dev;
> + struct device_node *np = pdev->dev.of_node;
> + int err;
> + int tsc_irq;
> + int adc_irq;
> +
> + tsc = devm_kzalloc(&pdev->dev, sizeof(struct imx6ul_tsc), GFP_KERNEL);
> + input_dev = devm_input_allocate_device(&pdev->dev);
> +
> + tsc->dev = &pdev->dev;
> +
> + tsc->input = input_dev;
> + tsc->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> + tsc->input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
> + input_set_abs_params(tsc->input, ABS_X, 0, 0xFFF, 0, 0);
> + input_set_abs_params(tsc->input, ABS_Y, 0, 0xFFF, 0, 0);
> +
> + tsc->input->name = "iMX6UL TouchScreen Controller";
> +
> + tsc->xnur_gpio = of_get_named_gpio(np, "xnur-gpio", 0);
> + err = gpio_request_one(tsc->xnur_gpio, GPIOF_IN, "tsc_X-");
> + if (err) {
> + dev_err(&pdev->dev, "failed to request GPIO tsc_X-\n");
> + return err;
> + }
> +
> + tsc_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + tsc->tsc_regs = devm_ioremap_resource(&pdev->dev, tsc_mem);
> + if (IS_ERR(tsc->tsc_regs)) {
> + dev_err(&pdev->dev, "failed to remap tsc memory\n");
> + err = PTR_ERR(tsc->tsc_regs);
> + return err;
> + }
> +
> + adc_mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + tsc->adc_regs = devm_ioremap_resource(&pdev->dev, adc_mem);
> + if (IS_ERR(tsc->adc_regs)) {
> + dev_err(&pdev->dev, "failed to remap adc memory\n");
> + err = PTR_ERR(tsc->adc_regs);
> + return err;
> + }
> +
> + tsc->tsc_clk = devm_clk_get(&pdev->dev, "tsc");
> + if (IS_ERR(tsc->tsc_clk)) {
> + dev_err(&pdev->dev, "failed getting tsc clock\n");
> + err = PTR_ERR(tsc->tsc_clk);
> + return err;
> + }
> +
> + tsc->adc_clk = devm_clk_get(&pdev->dev, "adc");
> + if (IS_ERR(tsc->adc_clk)) {
> + dev_err(&pdev->dev, "failed getting adc clock\n");
> + err = PTR_ERR(tsc->adc_clk);
> + return err;
> + }
> +
> + tsc_irq = platform_get_irq(pdev, 0);
> + if (tsc_irq < 0) {
> + dev_err(&pdev->dev, "no tsc irq resource?\n");
> + return tsc_irq;
> + }
> +
> + adc_irq = platform_get_irq(pdev, 1);
> + if (adc_irq <= 0) {
> + dev_err(&pdev->dev, "no adc irq resource?\n");
> + return adc_irq;
> + }
> +
> + err = devm_request_threaded_irq(tsc->dev, tsc_irq,
> + NULL, tsc_irq_fn,
> + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> + dev_name(&pdev->dev), tsc);
> + if (err < 0) {
> + dev_err(&pdev->dev,
> + "failed requesting tsc irq %d\n",
> + tsc_irq);
> + return err;
> + }
> +
> + err = devm_request_irq(tsc->dev, adc_irq,
> + adc_irq_fn, 0,
> + dev_name(&pdev->dev), tsc);
> + if (err < 0) {
> + dev_err(&pdev->dev,
> + "failed requesting adc irq %d\n",
> + adc_irq);
> + return err;
> + }
> +
> + err = of_property_read_u32(np, "measure-delay-time",
> + &tsc->measure_delay_time);
> + if (err)
> + tsc->measure_delay_time = 0xffff;
> +
> + err = of_property_read_u32(np, "pre-charge-time",
> + &tsc->pre_charge_time);
> + if (err)
> + tsc->pre_charge_time = 0xfff;
> +
> + init_completion(&tsc->completion);
> +
> + err = clk_prepare_enable(tsc->adc_clk);
> + if (err) {
> + dev_err(&pdev->dev,
> + "Could not prepare or enable the adc clock.\n");
> + return err;
> + }
> +
> + err = clk_prepare_enable(tsc->tsc_clk);
> + if (err) {
> + dev_err(&pdev->dev,
> + "Could not prepare or enable the tsc clock.\n");
> + goto error_tsc_clk_enable;
> + }
> +
> + err = input_register_device(tsc->input);
> + if (err < 0) {
> + dev_err(&pdev->dev, "failed to register input device\n");
> + err = -EIO;
> + goto err_input_register;
> + }
> +
> + imx6ul_tsc_init(tsc);
> +
> + platform_set_drvdata(pdev, tsc);
> + return 0;
> +
> +err_input_register:
> + clk_disable_unprepare(tsc->tsc_clk);
> +error_tsc_clk_enable:
> + clk_disable_unprepare(tsc->adc_clk);
> +
> + return err;
> +}
> +
> +static void imx6ul_tsc_disable(struct imx6ul_tsc *tsc)
> +{
> + int tsc_flow;
> + int adc_cfg;
> +
> + /* TSC controller enters to idle status */
> + tsc_flow = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> + tsc_flow |= TSC_DISABLE;
> + writel(tsc_flow, tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> +
> + /* ADC controller enters to stop mode */
> + adc_cfg = readl(tsc->adc_regs + REG_ADC_HC0);
> + adc_cfg |= ADC_CONV_DISABLE;
> + writel(adc_cfg, tsc->adc_regs + REG_ADC_HC0);
> +}
> +
> +static int imx6ul_tsc_remove(struct platform_device *pdev)
> +{
> + struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
> +
> + imx6ul_tsc_disable(tsc);
> +
> + clk_disable_unprepare(tsc->tsc_clk);
> + clk_disable_unprepare(tsc->adc_clk);
> + input_unregister_device(tsc->input);
> + kfree(tsc);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused imx6ul_tsc_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
> +
> + imx6ul_tsc_disable(tsc);
> +
> + clk_disable_unprepare(tsc->tsc_clk);
> + clk_disable_unprepare(tsc->adc_clk);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused imx6ul_tsc_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
> + int err;
> +
> + err = clk_prepare_enable(tsc->adc_clk);
> + if (err)
> + return err;
> +
> + err = clk_prepare_enable(tsc->tsc_clk);
> + if (err) {
> + clk_disable_unprepare(tsc->adc_clk);
> + return err;
> + }
> +
> + imx6ul_tsc_init(tsc);
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(imx6ul_tsc_pm_ops,
> + imx6ul_tsc_suspend,
> + imx6ul_tsc_resume);
> +
> +static struct platform_driver imx6ul_tsc_driver = {
> + .driver = {
> + .name = "imx6ul-tsc",
> + .owner = THIS_MODULE,
> + .of_match_table = imx6ul_tsc_match,
> + .pm = &imx6ul_tsc_pm_ops,
> + },
> + .probe = imx6ul_tsc_probe,
> + .remove = imx6ul_tsc_remove,
> +};
> +module_platform_driver(imx6ul_tsc_driver);
> +
> +MODULE_AUTHOR("Haibo Chen <[email protected]>");
> +MODULE_DESCRIPTION("Freescale i.MX6UL Touchscreen controller driver");
> +MODULE_LICENSE("GPL v2");
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |


Attachments:
(No filename) (17.10 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-08-21 09:28:40

by Markus Pargmann

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] Documentation: Detail permitted DT properties for the imx6ul_tsc

On Fri, Aug 21, 2015 at 08:30:16AM +0000, Chen Bough wrote:
> Hi Markus,
>
> > -----Original Message-----
> > From: Markus Pargmann [mailto:[email protected]]
> > Sent: Wednesday, August 19, 2015 1:55 PM
> > To: Chen Haibo-B51421
> > Cc: [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]
> > Subject: Re: [PATCH v2 2/5] Documentation: Detail permitted DT properties
> > for the imx6ul_tsc
> >
> > Hi,
> >
> > On Tue, Jul 28, 2015 at 05:58:38PM +0800, Haibo Chen wrote:
> > > Here we apply required documentation for the imx6ul touch screen
> > > controller driver which describe available properties and how to use
> > > them.
> > >
> > > Signed-off-by: Haibo Chen <[email protected]>
> > > ---
> > > .../bindings/input/touchscreen/imx6ul_tsc.txt | 36
> > ++++++++++++++++++++++
> > > 1 file changed, 36 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> > > b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> > > new file mode 100644
> > > index 0000000..ac41c32
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.t
> > > +++ xt
> > > @@ -0,0 +1,36 @@
> > > +* Freescale i.MX6UL Touch Controller
> > > +
> > > +Required properties:
> > > +- compatible: must be "fsl,imx6ul-tsc".
> > > +- reg: this touch controller address and the ADC2 address.
> >
> > This suggests that this driver is using a unit ADC2. Which also means
> > that there are more than one ADC which are probably identical?
> >
> > Shouldn't these ADCs be properly described by their own device nodes
> > instead of these two register ranges, two interrupts and two clocks?
> >
> > Is 'ADC2' usable without tsc? Then ADC1/ADC2 should perhaps get a proper
> > IIO driver.
>
> For i.MX6UL, there are two ADC. ADC1 is a normal ADC, and ADC2 can only works for
> TSC, the channels of ADC2 are connected to TSC directly. TSC and ADC2 should work
> together as a touch screen controller.

But as I understand these are two different units. Wouldn't it be better
to abstract it that way in the DT?

Best regards,

Markus

>
> For ADC1, it share the driver vf610_adc.c (drivers/iio/adc/vf610_adc.c).
>
> Best Regards
>
> Haibo
>
> >
> > Unfortunately I don't have the reference manual to have a look how this
> > all works.
> >
> > Best regards,
> >
> > Markus
> >
> > > +- interrupts: the interrupt of this touch controller and ADC2.
> > > +- clocks: the root clock of touch controller and ADC2.
> > > +- clock-names; must be "tsc" and "adc".
> > > +- xnur-gpio: the X- gpio this controller connect to.
> > > + This xnur-gpio returns to high once the finger leave the touch
> > > +screen (The
> > > + last touch event the touch controller capture).
> > > +
> > > +Optional properties:
> > > +- measure-delay-time: the value of measure delay time.
> > > + Before X-axis or Y-axis measurement, the screen need some time
> > > +before
> > > + even potential distribution ready.
> > > + This value depends on the touch screen.
> > > +- pre-charge-time: the touch screen need some time to precharge.
> > > + This value depends on the touch screen.
> > > +
> > > +Example:
> > > + tsc: tsc@02040000 {
> > > + compatible = "fsl,imx6ul-tsc";
> > > + reg = <0x02040000 0x4000>, <0x0219c000 0x4000>;
> > > + interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
> > > + <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
> > > + clocks = <&clks IMX6UL_CLK_IPG>,
> > > + <&clks IMX6UL_CLK_ADC2>;
> > > + clock-names = "tsc", "adc";
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&pinctrl_tsc>;
> > > + xnur-gpio = <&gpio1 3 GPIO_ACTIVE_HIGH>;
> > > + measure-delay-time = <0xfff>;
> > > + pre-charge-time = <0xffff>;
> > > + status = "okay";
> > > + };
> > > --
> > > 1.9.1
> > >
> > >
> > >
> >
> > --
> > Pengutronix e.K. |
> > |
> > Industrial Linux Solutions | http://www.pengutronix.de/
> > |
> > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0
> > |
> > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555
> > |

--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |


Attachments:
(No filename) (5.04 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-08-24 08:24:23

by Chen Bough

[permalink] [raw]
Subject: RE: [PATCH v2 1/5] input: touchscreen: add imx6ul_tsc driver support

Hi Markus,



> -----Original Message-----
> From: Markus Pargmann [mailto:[email protected]]
> Sent: Friday, August 21, 2015 5:26 PM
> To: Chen Haibo-B51421
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; linux-arm-
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 1/5] input: touchscreen: add imx6ul_tsc driver
> support
>
> On Tue, Jul 28, 2015 at 05:58:37PM +0800, Haibo Chen wrote:
> > Freescale i.MX6UL contains a internal touchscreen controller, this
> > patch add a driver to support this controller.
> >
> > Signed-off-by: Haibo Chen <[email protected]>
> > ---
> > drivers/input/touchscreen/Kconfig | 12 +
> > drivers/input/touchscreen/Makefile | 1 +
> > drivers/input/touchscreen/imx6ul_tsc.c | 504
> > +++++++++++++++++++++++++++++++++
> > 3 files changed, 517 insertions(+)
> > create mode 100644 drivers/input/touchscreen/imx6ul_tsc.c
> >
> > diff --git a/drivers/input/touchscreen/Kconfig
> > b/drivers/input/touchscreen/Kconfig
> > index 5b272ba..32c300d 100644
> > --- a/drivers/input/touchscreen/Kconfig
> > +++ b/drivers/input/touchscreen/Kconfig
> > @@ -479,6 +479,18 @@ config TOUCHSCREEN_MTOUCH
> > To compile this driver as a module, choose M here: the
> > module will be called mtouch.
> >
> > +config TOUCHSCREEN_IMX6UL_TSC
> > + tristate "Freescale i.MX6UL touchscreen controller"
> > + depends on OF
> > + help
> > + Say Y here if you have a Freescale i.MX6UL, and want to
> > + use the internal touchscreen controller.
> > +
> > + If unsure, say N.
> > +
> > + To compile this driver as a module, choose M here: the
> > + moduel will be called imx6ul_tsc.
> > +
> > config TOUCHSCREEN_INEXIO
> > tristate "iNexio serial touchscreens"
> > select SERIO
> > diff --git a/drivers/input/touchscreen/Makefile
> > b/drivers/input/touchscreen/Makefile
> > index c85aae2..9379b32 100644
> > --- a/drivers/input/touchscreen/Makefile
> > +++ b/drivers/input/touchscreen/Makefile
> > @@ -38,6 +38,7 @@ obj-$(CONFIG_TOUCHSCREEN_EGALAX) += egalax_ts.o
> > obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
> > obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix.o
> > obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
> > +obj-$(CONFIG_TOUCHSCREEN_IMX6UL_TSC) += imx6ul_tsc.o
> > obj-$(CONFIG_TOUCHSCREEN_INEXIO) += inexio.o
> > obj-$(CONFIG_TOUCHSCREEN_INTEL_MID) += intel-mid-touch.o
> > obj-$(CONFIG_TOUCHSCREEN_IPROC) += bcm_iproc_tsc.o
> > diff --git a/drivers/input/touchscreen/imx6ul_tsc.c
> > b/drivers/input/touchscreen/imx6ul_tsc.c
> > new file mode 100644
> > index 0000000..807f1db
> > --- /dev/null
> > +++ b/drivers/input/touchscreen/imx6ul_tsc.c
> > @@ -0,0 +1,504 @@
> > +/*
> > + * Freescale i.MX6UL touchscreen controller driver
> > + *
> > + * Copyright (C) 2015 Freescale Semiconductor, Inc.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > +modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/errno.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/gpio.h>
> > +#include <linux/input.h>
> > +#include <linux/slab.h>
> > +#include <linux/completion.h>
> > +#include <linux/delay.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_gpio.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/clk.h>
> > +#include <linux/io.h>
> > +
> > +/* ADC configuration registers field define */
> > +#define ADC_AIEN (0x1 << 7)
> > +#define ADC_CONV_DISABLE 0x1F
> > +#define ADC_CAL (0x1 << 7)
> > +#define ADC_CALF 0x2
> > +#define ADC_12BIT_MODE (0x2 << 2)
> > +#define ADC_IPG_CLK 0x00
> > +#define ADC_CLK_DIV_8 (0x03 << 5)
> > +#define ADC_SHORT_SAMPLE_MODE (0x0 << 4)
> > +#define ADC_HARDWARE_TRIGGER (0x1 << 13)
> > +#define SELECT_CHANNEL_4 0x04
> > +#define SELECT_CHANNEL_1 0x01
> > +#define DISABLE_CONVERSION_INT (0x0 << 7)
> > +
> > +/* ADC registers */
> > +#define REG_ADC_HC0 0x00
> > +#define REG_ADC_HC1 0x04
> > +#define REG_ADC_HC2 0x08
> > +#define REG_ADC_HC3 0x0C
> > +#define REG_ADC_HC4 0x10
> > +#define REG_ADC_HS 0x14
> > +#define REG_ADC_R0 0x18
> > +#define REG_ADC_CFG 0x2C
> > +#define REG_ADC_GC 0x30
> > +#define REG_ADC_GS 0x34
> > +
> > +#define ADC_TIMEOUT msecs_to_jiffies(100)
>
> These defines are in two drivers. Here and in drivers/iio/adc/vf610_adc.c

Actually, they are different. In the TSC driver, add REG_ADC_HC2/3/4, and the offset of the
REG_ADC_HS~ REG_ADC_GS also changed.

For i.MX6UL, to support TSC, our IC team add some changes in ADC2, like add add REG_ADC_HC2/3/4,
so that we can config the channel of ADC2 to connect with X-/X+/Y+/Y- of touch screen.
And IC team suggest the ADC2 only used for TSC, should not work as a normal ADC.


>
> > +
> > +/* TSC registers */
> > +#define REG_TSC_BASIC_SETING 0x00
> > +#define REG_TSC_PRE_CHARGE_TIME 0x10
> > +#define REG_TSC_FLOW_CONTROL 0x20
> > +#define REG_TSC_MEASURE_VALUE 0x30
> > +#define REG_TSC_INT_EN 0x40
> > +#define REG_TSC_INT_SIG_EN 0x50
> > +#define REG_TSC_INT_STATUS 0x60
> > +#define REG_TSC_DEBUG_MODE 0x70
> > +#define REG_TSC_DEBUG_MODE2 0x80
> > +
> > +/* TSC configuration registers field define */
> > +#define DETECT_4_WIRE_MODE (0x0 << 4)
> > +#define AUTO_MEASURE 0x1
> > +#define MEASURE_SIGNAL 0x1
> > +#define DETECT_SIGNAL (0x1 << 4)
> > +#define VALID_SIGNAL (0x1 << 8)
> > +#define MEASURE_INT_EN 0x1
> > +#define MEASURE_SIG_EN 0x1
> > +#define VALID_SIG_EN (0x1 << 8)
> > +#define DE_GLITCH_2 (0x2 << 29)
> > +#define START_SENSE (0x1 << 12)
> > +#define TSC_DISABLE (0x1 << 16)
> > +#define DETECT_MODE 0x2
> > +
> > +struct imx6ul_tsc {
> > + struct device *dev;
> > + struct input_dev *input;
> > + void __iomem *tsc_regs;
> > + void __iomem *adc_regs;
> > + struct clk *tsc_clk;
> > + struct clk *adc_clk;
> > +
> > + int xnur_gpio;
> > + int measure_delay_time;
> > + int pre_charge_time;
> > +
> > + struct completion completion;
> > +};
> > +
> > +/*
> > + * TSC module need ADC to get the measure value. So
> > + * before config TSC, we should initialize ADC module.
> > + */
> > +static void imx6ul_adc_init(struct imx6ul_tsc *tsc) {
> > + int adc_hc = 0;
> > + int adc_gc;
> > + int adc_gs;
> > + int adc_cfg;
> > + int timeout;
> > +
> > + adc_cfg = readl(tsc->adc_regs + REG_ADC_CFG);
> > + adc_cfg |= ADC_12BIT_MODE | ADC_IPG_CLK;
> > + adc_cfg |= ADC_CLK_DIV_8 | ADC_SHORT_SAMPLE_MODE;
> > + adc_cfg &= ~ADC_HARDWARE_TRIGGER;
> > + writel(adc_cfg, tsc->adc_regs + REG_ADC_CFG);
> > +
> > + /* enable calibration interrupt */
> > + adc_hc |= ADC_AIEN;
> > + adc_hc |= ADC_CONV_DISABLE;
> > + writel(adc_hc, tsc->adc_regs + REG_ADC_HC0);
> > +
> > + /* start ADC calibration */
> > + adc_gc = readl(tsc->adc_regs + REG_ADC_GC);
> > + adc_gc |= ADC_CAL;
> > + writel(adc_gc, tsc->adc_regs + REG_ADC_GC);
> > +
> > + timeout = wait_for_completion_timeout
> > + (&tsc->completion, ADC_TIMEOUT);
> > + if (timeout == 0)
> > + dev_err(tsc->dev, "Timeout for adc calibration\n");
> > +
> > + adc_gs = readl(tsc->adc_regs + REG_ADC_GS);
> > + if (adc_gs & ADC_CALF)
> > + dev_err(tsc->dev, "ADC calibration failed\n");
> > +
> > + /* TSC need the ADC work in hardware trigger */
> > + adc_cfg = readl(tsc->adc_regs + REG_ADC_CFG);
> > + adc_cfg |= ADC_HARDWARE_TRIGGER;
> > + writel(adc_cfg, tsc->adc_regs + REG_ADC_CFG); }
> > +
> > +/*
> > + * This is a TSC workaround. Currently TSC misconnect two
> > + * ADC channels, this function remap channel configure for
> > + * hardware trigger.
> > + */
> > +static void imx6ul_tsc_channel_config(struct imx6ul_tsc *tsc) {
> > + int adc_hc0, adc_hc1, adc_hc2, adc_hc3, adc_hc4;
> > +
> > + adc_hc0 = DISABLE_CONVERSION_INT;
> > + writel(adc_hc0, tsc->adc_regs + REG_ADC_HC0);
> > +
> > + adc_hc1 = DISABLE_CONVERSION_INT | SELECT_CHANNEL_4;
> > + writel(adc_hc1, tsc->adc_regs + REG_ADC_HC1);
> > +
> > + adc_hc2 = DISABLE_CONVERSION_INT;
> > + writel(adc_hc2, tsc->adc_regs + REG_ADC_HC2);
> > +
> > + adc_hc3 = DISABLE_CONVERSION_INT | SELECT_CHANNEL_1;
> > + writel(adc_hc3, tsc->adc_regs + REG_ADC_HC3);
> > +
> > + adc_hc4 = DISABLE_CONVERSION_INT;
> > + writel(adc_hc4, tsc->adc_regs + REG_ADC_HC4); }
>
> Is this code similar to initialization of the adc driver? Does it make
> sense to move this code to the ADC driver?

Our IC team misconnect the ADC's channel1 and channel3, in the TSC logic,
ADC channel1 ---> Y-
ADC channel2 ---> Y+
ADC channel3 ---> X-
ADC channel4 ---> X+
The upper connection can make tsc work normal, but our IC team make a mistake,
in default, ADC channel1 connect to X-, ADC channel3 connect to Y-.

So these code is just a software workaround, switch the ADC channel1 and channel3,
to make the TSC work normal.

So I think we should distinguish these code and the adc initialization.

Best regards,

haibo

>
> Best regards,
>
> Markus
>
> > +
> > +/*
> > + * TSC setting, confige the pre-charge time and measure delay time.
> > + * different touch screen may need different pre-charge time and
> > + * measure delay time.
> > + */
> > +static void imx6ul_tsc_set(struct imx6ul_tsc *tsc) {
> > + int basic_setting = 0;
> > + int start;
> > +
> > + basic_setting |= tsc->measure_delay_time << 8;
> > + basic_setting |= DETECT_4_WIRE_MODE | AUTO_MEASURE;
> > + writel(basic_setting, tsc->tsc_regs + REG_TSC_BASIC_SETING);
> > +
> > + writel(DE_GLITCH_2, tsc->tsc_regs + REG_TSC_DEBUG_MODE2);
> > +
> > + writel(tsc->pre_charge_time, tsc->tsc_regs +
> REG_TSC_PRE_CHARGE_TIME);
> > + writel(MEASURE_INT_EN, tsc->tsc_regs + REG_TSC_INT_EN);
> > + writel(MEASURE_SIG_EN | VALID_SIG_EN,
> > + tsc->tsc_regs + REG_TSC_INT_SIG_EN);
> > +
> > + /* start sense detection */
> > + start = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> > + start |= START_SENSE;
> > + start &= ~TSC_DISABLE;
> > + writel(start, tsc->tsc_regs + REG_TSC_FLOW_CONTROL); }
> > +
> > +static void imx6ul_tsc_init(struct imx6ul_tsc *tsc) {
> > + imx6ul_adc_init(tsc);
> > + imx6ul_tsc_channel_config(tsc);
> > + imx6ul_tsc_set(tsc);
> > +}
> > +
> > +static irqreturn_t tsc_irq_fn(int irq, void *dev_id) {
> > + struct imx6ul_tsc *tsc = (struct imx6ul_tsc *)dev_id;
> > + int status;
> > + int value;
> > + int x, y;
> > + int xnur;
> > + int debug_mode2;
> > + int state_machine;
> > + int start;
> > + unsigned long timeout;
> > +
> > + status = readl(tsc->tsc_regs + REG_TSC_INT_STATUS);
> > +
> > + /* write 1 to clear the bit measure-signal */
> > + writel(MEASURE_SIGNAL | DETECT_SIGNAL,
> > + tsc->tsc_regs + REG_TSC_INT_STATUS);
> > +
> > + /* It's a HW self-clean bit. Set this bit and start sense detection
> */
> > + start = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> > + start |= START_SENSE;
> > + writel(start, tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> > +
> > + if (status & MEASURE_SIGNAL) {
> > + value = readl(tsc->tsc_regs + REG_TSC_MEASURE_VALUE);
> > + x = (value >> 16) & 0x0fff;
> > + y = value & 0x0fff;
> > +
> > + /*
> > + * Delay some time(max 2ms), wait the pre-charge done.
> > + * After the pre-change mode, TSC go into detect mode.
> > + * And in detect mode, we can get the xnur gpio value.
> > + * If xnur is low, this means the touch screen still
> > + * be touched. If xnur is high, this means finger leave
> > + * the touch screen.
> > + */
> > + timeout = jiffies + HZ/500;
> > + do {
> > + if (time_after(jiffies, timeout)) {
> > + xnur = 0;
> > + goto touch_event;
> > + }
> > + usleep_range(200, 400);
> > + debug_mode2 = readl(tsc->tsc_regs +
> REG_TSC_DEBUG_MODE2);
> > + state_machine = (debug_mode2 >> 20) & 0x7;
> > + } while (state_machine != DETECT_MODE);
> > + usleep_range(200, 400);
> > +
> > + xnur = gpio_get_value(tsc->xnur_gpio);
> > +touch_event:
> > + if (xnur == 0) {
> > + input_report_key(tsc->input, BTN_TOUCH, 1);
> > + input_report_abs(tsc->input, ABS_X, x);
> > + input_report_abs(tsc->input, ABS_Y, y);
> > + } else
> > + input_report_key(tsc->input, BTN_TOUCH, 0);
> > +
> > + input_sync(tsc->input);
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static irqreturn_t adc_irq_fn(int irq, void *dev_id) {
> > + struct imx6ul_tsc *tsc = (struct imx6ul_tsc *)dev_id;
> > + int coco;
> > + int value;
> > +
> > + coco = readl(tsc->adc_regs + REG_ADC_HS);
> > + if (coco & 0x01) {
> > + value = readl(tsc->adc_regs + REG_ADC_R0);
> > + complete(&tsc->completion);
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static const struct of_device_id imx6ul_tsc_match[] = {
> > + { .compatible = "fsl,imx6ul-tsc", },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, imx6ul_tsc_match);
> > +
> > +static int imx6ul_tsc_probe(struct platform_device *pdev) {
> > + struct imx6ul_tsc *tsc;
> > + struct resource *tsc_mem;
> > + struct resource *adc_mem;
> > + struct input_dev *input_dev;
> > + struct device_node *np = pdev->dev.of_node;
> > + int err;
> > + int tsc_irq;
> > + int adc_irq;
> > +
> > + tsc = devm_kzalloc(&pdev->dev, sizeof(struct imx6ul_tsc),
> GFP_KERNEL);
> > + input_dev = devm_input_allocate_device(&pdev->dev);
> > +
> > + tsc->dev = &pdev->dev;
> > +
> > + tsc->input = input_dev;
> > + tsc->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> > + tsc->input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
> > + input_set_abs_params(tsc->input, ABS_X, 0, 0xFFF, 0, 0);
> > + input_set_abs_params(tsc->input, ABS_Y, 0, 0xFFF, 0, 0);
> > +
> > + tsc->input->name = "iMX6UL TouchScreen Controller";
> > +
> > + tsc->xnur_gpio = of_get_named_gpio(np, "xnur-gpio", 0);
> > + err = gpio_request_one(tsc->xnur_gpio, GPIOF_IN, "tsc_X-");
> > + if (err) {
> > + dev_err(&pdev->dev, "failed to request GPIO tsc_X-\n");
> > + return err;
> > + }
> > +
> > + tsc_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + tsc->tsc_regs = devm_ioremap_resource(&pdev->dev, tsc_mem);
> > + if (IS_ERR(tsc->tsc_regs)) {
> > + dev_err(&pdev->dev, "failed to remap tsc memory\n");
> > + err = PTR_ERR(tsc->tsc_regs);
> > + return err;
> > + }
> > +
> > + adc_mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > + tsc->adc_regs = devm_ioremap_resource(&pdev->dev, adc_mem);
> > + if (IS_ERR(tsc->adc_regs)) {
> > + dev_err(&pdev->dev, "failed to remap adc memory\n");
> > + err = PTR_ERR(tsc->adc_regs);
> > + return err;
> > + }
> > +
> > + tsc->tsc_clk = devm_clk_get(&pdev->dev, "tsc");
> > + if (IS_ERR(tsc->tsc_clk)) {
> > + dev_err(&pdev->dev, "failed getting tsc clock\n");
> > + err = PTR_ERR(tsc->tsc_clk);
> > + return err;
> > + }
> > +
> > + tsc->adc_clk = devm_clk_get(&pdev->dev, "adc");
> > + if (IS_ERR(tsc->adc_clk)) {
> > + dev_err(&pdev->dev, "failed getting adc clock\n");
> > + err = PTR_ERR(tsc->adc_clk);
> > + return err;
> > + }
> > +
> > + tsc_irq = platform_get_irq(pdev, 0);
> > + if (tsc_irq < 0) {
> > + dev_err(&pdev->dev, "no tsc irq resource?\n");
> > + return tsc_irq;
> > + }
> > +
> > + adc_irq = platform_get_irq(pdev, 1);
> > + if (adc_irq <= 0) {
> > + dev_err(&pdev->dev, "no adc irq resource?\n");
> > + return adc_irq;
> > + }
> > +
> > + err = devm_request_threaded_irq(tsc->dev, tsc_irq,
> > + NULL, tsc_irq_fn,
> > + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> > + dev_name(&pdev->dev), tsc);
> > + if (err < 0) {
> > + dev_err(&pdev->dev,
> > + "failed requesting tsc irq %d\n",
> > + tsc_irq);
> > + return err;
> > + }
> > +
> > + err = devm_request_irq(tsc->dev, adc_irq,
> > + adc_irq_fn, 0,
> > + dev_name(&pdev->dev), tsc);
> > + if (err < 0) {
> > + dev_err(&pdev->dev,
> > + "failed requesting adc irq %d\n",
> > + adc_irq);
> > + return err;
> > + }
> > +
> > + err = of_property_read_u32(np, "measure-delay-time",
> > + &tsc->measure_delay_time);
> > + if (err)
> > + tsc->measure_delay_time = 0xffff;
> > +
> > + err = of_property_read_u32(np, "pre-charge-time",
> > + &tsc->pre_charge_time);
> > + if (err)
> > + tsc->pre_charge_time = 0xfff;
> > +
> > + init_completion(&tsc->completion);
> > +
> > + err = clk_prepare_enable(tsc->adc_clk);
> > + if (err) {
> > + dev_err(&pdev->dev,
> > + "Could not prepare or enable the adc clock.\n");
> > + return err;
> > + }
> > +
> > + err = clk_prepare_enable(tsc->tsc_clk);
> > + if (err) {
> > + dev_err(&pdev->dev,
> > + "Could not prepare or enable the tsc clock.\n");
> > + goto error_tsc_clk_enable;
> > + }
> > +
> > + err = input_register_device(tsc->input);
> > + if (err < 0) {
> > + dev_err(&pdev->dev, "failed to register input device\n");
> > + err = -EIO;
> > + goto err_input_register;
> > + }
> > +
> > + imx6ul_tsc_init(tsc);
> > +
> > + platform_set_drvdata(pdev, tsc);
> > + return 0;
> > +
> > +err_input_register:
> > + clk_disable_unprepare(tsc->tsc_clk);
> > +error_tsc_clk_enable:
> > + clk_disable_unprepare(tsc->adc_clk);
> > +
> > + return err;
> > +}
> > +
> > +static void imx6ul_tsc_disable(struct imx6ul_tsc *tsc) {
> > + int tsc_flow;
> > + int adc_cfg;
> > +
> > + /* TSC controller enters to idle status */
> > + tsc_flow = readl(tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> > + tsc_flow |= TSC_DISABLE;
> > + writel(tsc_flow, tsc->tsc_regs + REG_TSC_FLOW_CONTROL);
> > +
> > + /* ADC controller enters to stop mode */
> > + adc_cfg = readl(tsc->adc_regs + REG_ADC_HC0);
> > + adc_cfg |= ADC_CONV_DISABLE;
> > + writel(adc_cfg, tsc->adc_regs + REG_ADC_HC0); }
> > +
> > +static int imx6ul_tsc_remove(struct platform_device *pdev) {
> > + struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
> > +
> > + imx6ul_tsc_disable(tsc);
> > +
> > + clk_disable_unprepare(tsc->tsc_clk);
> > + clk_disable_unprepare(tsc->adc_clk);
> > + input_unregister_device(tsc->input);
> > + kfree(tsc);
> > +
> > + return 0;
> > +}
> > +
> > +static int __maybe_unused imx6ul_tsc_suspend(struct device *dev) {
> > + struct platform_device *pdev = to_platform_device(dev);
> > + struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
> > +
> > + imx6ul_tsc_disable(tsc);
> > +
> > + clk_disable_unprepare(tsc->tsc_clk);
> > + clk_disable_unprepare(tsc->adc_clk);
> > +
> > + return 0;
> > +}
> > +
> > +static int __maybe_unused imx6ul_tsc_resume(struct device *dev) {
> > + struct platform_device *pdev = to_platform_device(dev);
> > + struct imx6ul_tsc *tsc = platform_get_drvdata(pdev);
> > + int err;
> > +
> > + err = clk_prepare_enable(tsc->adc_clk);
> > + if (err)
> > + return err;
> > +
> > + err = clk_prepare_enable(tsc->tsc_clk);
> > + if (err) {
> > + clk_disable_unprepare(tsc->adc_clk);
> > + return err;
> > + }
> > +
> > + imx6ul_tsc_init(tsc);
> > + return 0;
> > +}
> > +
> > +static SIMPLE_DEV_PM_OPS(imx6ul_tsc_pm_ops,
> > + imx6ul_tsc_suspend,
> > + imx6ul_tsc_resume);
> > +
> > +static struct platform_driver imx6ul_tsc_driver = {
> > + .driver = {
> > + .name = "imx6ul-tsc",
> > + .owner = THIS_MODULE,
> > + .of_match_table = imx6ul_tsc_match,
> > + .pm = &imx6ul_tsc_pm_ops,
> > + },
> > + .probe = imx6ul_tsc_probe,
> > + .remove = imx6ul_tsc_remove,
> > +};
> > +module_platform_driver(imx6ul_tsc_driver);
> > +
> > +MODULE_AUTHOR("Haibo Chen <[email protected]>");
> > +MODULE_DESCRIPTION("Freescale i.MX6UL Touchscreen controller
> > +driver"); MODULE_LICENSE("GPL v2");
> > --
> > 1.9.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-kernel" in the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >
>
> --
> Pengutronix e.K. |
> |
> Industrial Linux Solutions | http://www.pengutronix.de/
> |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0
> |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555
> |
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2015-08-24 08:34:15

by Chen Bough

[permalink] [raw]
Subject: RE: [PATCH v2 2/5] Documentation: Detail permitted DT properties for the imx6ul_tsc



> -----Original Message-----
> From: Markus Pargmann [mailto:[email protected]]
> Sent: Friday, August 21, 2015 5:27 PM
> To: Chen Haibo-B51421
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 2/5] Documentation: Detail permitted DT properties
> for the imx6ul_tsc
>
> On Fri, Aug 21, 2015 at 08:30:16AM +0000, Chen Bough wrote:
> > Hi Markus,
> >
> > > -----Original Message-----
> > > From: Markus Pargmann [mailto:[email protected]]
> > > Sent: Wednesday, August 19, 2015 1:55 PM
> > > To: Chen Haibo-B51421
> > > Cc: [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]
> > > Subject: Re: [PATCH v2 2/5] Documentation: Detail permitted DT
> > > properties for the imx6ul_tsc
> > >
> > > Hi,
> > >
> > > On Tue, Jul 28, 2015 at 05:58:38PM +0800, Haibo Chen wrote:
> > > > Here we apply required documentation for the imx6ul touch screen
> > > > controller driver which describe available properties and how to
> > > > use them.
> > > >
> > > > Signed-off-by: Haibo Chen <[email protected]>
> > > > ---
> > > > .../bindings/input/touchscreen/imx6ul_tsc.txt | 36
> > > ++++++++++++++++++++++
> > > > 1 file changed, 36 insertions(+)
> > > > create mode 100644
> > > > Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.t
> > > > xt
> > > > b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.t
> > > > xt
> > > > new file mode 100644
> > > > index 0000000..ac41c32
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_t
> > > > +++ sc.t
> > > > +++ xt
> > > > @@ -0,0 +1,36 @@
> > > > +* Freescale i.MX6UL Touch Controller
> > > > +
> > > > +Required properties:
> > > > +- compatible: must be "fsl,imx6ul-tsc".
> > > > +- reg: this touch controller address and the ADC2 address.
> > >
> > > This suggests that this driver is using a unit ADC2. Which also
> > > means that there are more than one ADC which are probably identical?
> > >
> > > Shouldn't these ADCs be properly described by their own device nodes
> > > instead of these two register ranges, two interrupts and two clocks?
> > >
> > > Is 'ADC2' usable without tsc? Then ADC1/ADC2 should perhaps get a
> > > proper IIO driver.
> >
> > For i.MX6UL, there are two ADC. ADC1 is a normal ADC, and ADC2 can
> > only works for TSC, the channels of ADC2 are connected to TSC
> > directly. TSC and ADC2 should work together as a touch screen
> controller.
>
> But as I understand these are two different units. Wouldn't it be better
> to abstract it that way in the DT?

Since the ADC2 just work for TSC, no other use, why not bind ADC2 and TSC?
If separate, seems ADC2 can works as a normal ADC, but actually it can't.

Best regards,

Haibo

>
> Best regards,
>
> Markus
>
> >
> > For ADC1, it share the driver vf610_adc.c (drivers/iio/adc/vf610_adc.c).
> >
> > Best Regards
> >
> > Haibo
> >
> > >
> > > Unfortunately I don't have the reference manual to have a look how
> > > this all works.
> > >
> > > Best regards,
> > >
> > > Markus
> > >
> > > > +- interrupts: the interrupt of this touch controller and ADC2.
> > > > +- clocks: the root clock of touch controller and ADC2.
> > > > +- clock-names; must be "tsc" and "adc".
> > > > +- xnur-gpio: the X- gpio this controller connect to.
> > > > + This xnur-gpio returns to high once the finger leave the touch
> > > > +screen (The
> > > > + last touch event the touch controller capture).
> > > > +
> > > > +Optional properties:
> > > > +- measure-delay-time: the value of measure delay time.
> > > > + Before X-axis or Y-axis measurement, the screen need some time
> > > > +before
> > > > + even potential distribution ready.
> > > > + This value depends on the touch screen.
> > > > +- pre-charge-time: the touch screen need some time to precharge.
> > > > + This value depends on the touch screen.
> > > > +
> > > > +Example:
> > > > + tsc: tsc@02040000 {
> > > > + compatible = "fsl,imx6ul-tsc";
> > > > + reg = <0x02040000 0x4000>, <0x0219c000 0x4000>;
> > > > + interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
> > > > + <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
> > > > + clocks = <&clks IMX6UL_CLK_IPG>,
> > > > + <&clks IMX6UL_CLK_ADC2>;
> > > > + clock-names = "tsc", "adc";
> > > > + pinctrl-names = "default";
> > > > + pinctrl-0 = <&pinctrl_tsc>;
> > > > + xnur-gpio = <&gpio1 3 GPIO_ACTIVE_HIGH>;
> > > > + measure-delay-time = <0xfff>;
> > > > + pre-charge-time = <0xffff>;
> > > > + status = "okay";
> > > > + };
> > > > --
> > > > 1.9.1
> > > >
> > > >
> > > >
> > >
> > > --
> > > Pengutronix e.K. |
> > > |
> > > Industrial Linux Solutions |
> http://www.pengutronix.de/
> > > |
> > > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone:
> > > +49-5121-206917-0
> > > |
> > > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-
> 5555
> > > |
>
> --
> Pengutronix e.K. |
> |
> Industrial Linux Solutions | http://www.pengutronix.de/
> |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0
> |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555
> |
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?