2014-12-19 22:18:27

by Jonathan Richardson

[permalink] [raw]
Subject: [PATCH v2 0/2] Add support for Broadcom iProc touchscreen

This patchset contains initial support for the touchscreen on the Broadcom
iProc family of SoCs. This driver has been validated with Cygnus and is expected
to work on other iProc family of SoCs that use the same touchscreen controller.

Changes from v1:
- Add missing newlines to debug messages
- Use BIT macro for defines
- Fix logic in get_tsc_config to improve readability
- Get rid of unnecessary remove() function

Jonathan Richardson (2):
Input: touchscreen-iproc: Add Broadcom iProc touchscreen driver
Input: touchscreen-iproc: add device tree bindings

.../input/touchscreen/brcm,iproc-touchscreen.txt | 70 +++
drivers/input/touchscreen/Kconfig | 11 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/bcm_iproc_tsc.c | 535 ++++++++++++++++++++
4 files changed, 617 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/brcm,iproc-touchscreen.txt
create mode 100644 drivers/input/touchscreen/bcm_iproc_tsc.c

--
1.7.9.5


2014-12-19 22:17:56

by Jonathan Richardson

[permalink] [raw]
Subject: [PATCH v2 1/2] Input: touchscreen-iproc: Add Broadcom iProc touchscreen driver

Add initial version of the Broadcom touchscreen driver.

Reviewed-by: Ray Jui <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
Tested-by: Scott Branden <[email protected]>
Signed-off-by: Jonathan Richardson <[email protected]>
---
drivers/input/touchscreen/Kconfig | 11 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/bcm_iproc_tsc.c | 535 +++++++++++++++++++++++++++++
3 files changed, 547 insertions(+)
create mode 100644 drivers/input/touchscreen/bcm_iproc_tsc.c

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index e1d8003..77ff531 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -310,6 +310,17 @@ config TOUCHSCREEN_ILI210X
To compile this driver as a module, choose M here: the
module will be called ili210x.

+config TOUCHSCREEN_IPROC
+ tristate "IPROC touch panel driver support"
+ help
+ Say Y here if you want to add support for the IPROC touch
+ controller to your system.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called iproc-ts.
+
config TOUCHSCREEN_S3C2410
tristate "Samsung S3C2410/generic touchscreen input driver"
depends on ARCH_S3C24XX || SAMSUNG_DEV_TS
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 090e61c..f7e6de9 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -37,6 +37,7 @@ obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
obj-$(CONFIG_TOUCHSCREEN_INEXIO) += inexio.o
obj-$(CONFIG_TOUCHSCREEN_INTEL_MID) += intel-mid-touch.o
+obj-$(CONFIG_TOUCHSCREEN_IPROC) += bcm_iproc_tsc.o
obj-$(CONFIG_TOUCHSCREEN_LPC32XX) += lpc32xx_ts.o
obj-$(CONFIG_TOUCHSCREEN_MAX11801) += max11801_ts.o
obj-$(CONFIG_TOUCHSCREEN_MC13783) += mc13783_ts.o
diff --git a/drivers/input/touchscreen/bcm_iproc_tsc.c b/drivers/input/touchscreen/bcm_iproc_tsc.c
new file mode 100644
index 0000000..bf6eb7f
--- /dev/null
+++ b/drivers/input/touchscreen/bcm_iproc_tsc.c
@@ -0,0 +1,535 @@
+/*
+* Copyright (C) 2014 Broadcom Corporation
+*
+* This program is free software; you can redistribute it and/or
+* modify it under the terms of the GNU General Public License as
+* published by the Free Software Foundation version 2.
+*
+* This program is distributed "as is" WITHOUT ANY WARRANTY of any
+* kind, whether express or implied; without even the implied warranty
+* of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+* GNU General Public License for more details.
+*/
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/keyboard.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <asm/irq.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/serio.h>
+
+#define IPROC_TS_NAME "iproc-ts"
+
+#define PEN_DOWN_STATUS 1
+#define PEN_UP_STATUS 0
+
+#define X_MIN 0
+#define Y_MIN 0
+#define X_MAX 0xFFF
+#define Y_MAX 0xFFF
+
+/* Value given by controller for invalid coordinate. */
+#define INVALID_COORD 0xFFFFFFFF
+
+/* Register offsets */
+#define REGCTL1 0x00
+#define REGCTL2 0x04
+#define INTERRUPT_THRES 0x08
+#define INTERRUPT_MASK 0x0c
+
+#define INTERRUPT_STATUS 0x10
+#define CONTROLLER_STATUS 0x14
+#define FIFO_DATA 0x18
+#define FIFO_DATA_X_Y_MASK 0xFFFF
+#define ANALOG_CONTROL 0x1c
+
+#define AUX_DATA 0x20
+#define DEBOUNCE_CNTR_STAT 0x24
+#define SCAN_CNTR_STAT 0x28
+#define REM_CNTR_STAT 0x2c
+
+#define SETTLING_TIMER_STAT 0x30
+#define SPARE_REG 0x34
+#define SOFT_BYPASS_CONTROL 0x38
+#define SOFT_BYPASS_DATA 0x3c
+
+
+/* Bit values for INTERRUPT_MASK and INTERRUPT_STATUS regs */
+#define TS_PEN_INTR_MASK BIT(0)
+#define TS_FIFO_INTR_MASK BIT(2)
+
+/* Bit values for CONTROLLER_STATUS reg1 */
+#define TS_PEN_DOWN BIT(0)
+
+/* Shift values for control reg1 */
+#define SCANNING_PERIOD_SHIFT 24
+#define DEBOUNCE_TIMEOUT_SHIFT 16
+#define SETTLING_TIMEOUT_SHIFT 8
+#define TOUCH_TIMEOUT_SHIFT 0
+
+/* Shift values for coordinates from fifo */
+#define X_COORD_SHIFT 0
+#define Y_COORD_SHIFT 16
+
+/* Bit values for REGCTL2 */
+#define TS_CONTROLLER_EN_BIT BIT(16)
+#define TS_CONTROLLER_AVGDATA_SHIFT 8
+#define TS_CONTROLLER_AVGDATA_MASK (0x7 << TS_CONTROLLER_AVGDATA_SHIFT)
+#define TS_CONTROLLER_PWR_LDO BIT(5)
+#define TS_CONTROLLER_PWR_ADC BIT(4)
+#define TS_CONTROLLER_PWR_BGP BIT(3)
+#define TS_CONTROLLER_PWR_TS BIT(2)
+#define TS_WIRE_MODE_BIT BIT(1)
+
+/*
+ * Touch screen mount angles w.r.t LCD panel left side top corner
+ * TS (x_min,y_min) placed at LCD (x_min,y_min) rotation angle is 0
+ * TS (x_min,y_max) placed at LCD (x_min,y_min) rotation angle is 90
+ * TS (x_max,y_max) placed at LCD (x_min,y_min) rotation angle is 180
+ * TS (x_max,y_min) placed at LCD (x_min,y_min) rotation angle is 270
+ */
+enum ts_rotation_angles {
+ TS_ROTATION_0,
+ TS_ROTATION_90,
+ TS_ROTATION_180,
+ TS_ROTATION_270,
+};
+
+struct tsc_param {
+ /* Each step is 1024 us. Valid 1-256 */
+ u32 scanning_period;
+
+ /* Each step is 512 us. Valid 0-255 */
+ u32 debounce_timeout;
+
+ /*
+ * The settling duration (in ms) is the amount of time the tsc
+ * waits to allow the voltage to settle after turning on the
+ * drivers in detection mode. Valid values: 0-11
+ * 0 = 0.008 ms
+ * 1 = 0.01 ms
+ * 2 = 0.02 ms
+ * 3 = 0.04 ms
+ * 4 = 0.08 ms
+ * 5 = 0.16 ms
+ * 6 = 0.32 ms
+ * 7 = 0.64 ms
+ * 8 = 1.28 ms
+ * 9 = 2.56 ms
+ * 10 = 5.12 ms
+ * 11 = 10.24 ms
+ */
+ u32 settling_timeout;
+
+ /* touch timeout in sample counts */
+ u32 touch_timeout;
+
+ /*
+ * Number of data samples which are averaged before a final data point
+ * is placed into the FIFO
+ */
+ u32 average_data;
+
+ /* FIFO threshold */
+ u32 fifo_threshold;
+};
+
+struct iproc_ts_priv {
+ struct platform_device *pdev;
+ struct input_dev *idev;
+
+ void __iomem *regs;
+ struct clk *tsc_clk;
+
+ int pen_status;
+ int ts_rotation;
+ struct tsc_param cfg_params;
+};
+
+/*
+ * Set default values the same as hardware reset values
+ * except for fifo_threshold with is set to 1.
+ */
+static struct tsc_param default_config = {
+ .scanning_period = 0x5, /* 1 to 256 */
+ .debounce_timeout = 0x28, /* 0 to 255 */
+ .settling_timeout = 0x7, /* 0 to 11 */
+ .touch_timeout = 0xa, /* 0 to 255 */
+ .average_data = 5, /* entry 5 = 32 pts */
+ .fifo_threshold = 1, /* 0 to 31 */
+};
+
+static void ts_reg_dump(struct iproc_ts_priv *priv)
+{
+ struct device *dev = &priv->pdev->dev;
+
+ dev_dbg(dev, "regCtl1 = 0x%08x\n",
+ readl(priv->regs + REGCTL1));
+ dev_dbg(dev, "regCtl2 = 0x%08x\n",
+ readl(priv->regs + REGCTL2));
+ dev_dbg(dev, "interrupt_Thres = 0x%08x\n",
+ readl(priv->regs + INTERRUPT_THRES));
+ dev_dbg(dev, "interrupt_Mask = 0x%08x\n",
+ readl(priv->regs + INTERRUPT_MASK));
+ dev_dbg(dev, "interrupt_Status = 0x%08x\n",
+ readl(priv->regs + INTERRUPT_STATUS));
+ dev_dbg(dev, "controller_Status = 0x%08x\n",
+ readl(priv->regs + CONTROLLER_STATUS));
+ dev_dbg(dev, "FIFO_Data = 0x%08x\n",
+ readl(priv->regs + FIFO_DATA));
+ dev_dbg(dev, "analog_Control = 0x%08x\n",
+ readl(priv->regs + ANALOG_CONTROL));
+ dev_dbg(dev, "aux_Data = 0x%08x\n",
+ readl(priv->regs + AUX_DATA));
+ dev_dbg(dev, "debounce_Cntr_Stat = 0x%08x\n",
+ readl(priv->regs + DEBOUNCE_CNTR_STAT));
+ dev_dbg(dev, "scan_Cntr_Stat = 0x%08x\n",
+ readl(priv->regs + SCAN_CNTR_STAT));
+ dev_dbg(dev, "rem_Cntr_Stat = 0x%08x\n",
+ readl(priv->regs + REM_CNTR_STAT));
+ dev_dbg(dev, "settling_Timer_Stat = 0x%08x\n",
+ readl(priv->regs + SETTLING_TIMER_STAT));
+ dev_dbg(dev, "spare_Reg = 0x%08x\n",
+ readl(priv->regs + SPARE_REG));
+ dev_dbg(dev, "soft_Bypass_Control = 0x%08x\n",
+ readl(priv->regs + SOFT_BYPASS_CONTROL));
+ dev_dbg(dev, "soft_Bypass_Data = 0x%08x\n",
+ readl(priv->regs + SOFT_BYPASS_DATA));
+}
+
+static irqreturn_t iproc_touchscreen_interrupt(int irq, void *data)
+{
+ struct platform_device *pdev = (struct platform_device *)data;
+ struct iproc_ts_priv *priv;
+ u32 intr_status = 0;
+ u32 raw_coordinate = 0;
+ u16 x = 0;
+ u16 y = 0;
+ int i;
+
+ priv = (struct iproc_ts_priv *)platform_get_drvdata(pdev);
+
+ intr_status = readl(priv->regs + INTERRUPT_STATUS);
+ intr_status &= (TS_PEN_INTR_MASK | TS_FIFO_INTR_MASK);
+ if (intr_status == 0)
+ return IRQ_NONE;
+
+ /* Clear all interrupt status bits, write-1-clear */
+ writel(intr_status, priv->regs + INTERRUPT_STATUS);
+
+ /* Pen up/down */
+ if (intr_status & TS_PEN_INTR_MASK) {
+ if (readl(priv->regs + CONTROLLER_STATUS) & TS_PEN_DOWN) {
+ priv->pen_status = PEN_DOWN_STATUS;
+ input_report_key(priv->idev, BTN_TOUCH,
+ priv->pen_status);
+ } else {
+ priv->pen_status = PEN_UP_STATUS;
+ input_report_key(priv->idev, BTN_TOUCH,
+ priv->pen_status);
+ input_sync(priv->idev);
+ }
+
+ dev_dbg(&priv->pdev->dev,
+ "pen up-down (%d)\n", priv->pen_status);
+ }
+
+ /* coordinates in FIFO exceed the theshold */
+ if (intr_status & TS_FIFO_INTR_MASK) {
+ for (i = 0; i < priv->cfg_params.fifo_threshold; i++) {
+ raw_coordinate = readl(priv->regs + FIFO_DATA);
+ if (raw_coordinate == INVALID_COORD)
+ continue;
+
+ /*
+ * The x and y coordinate are 16 bits each
+ * with the x in the lower 16 bits and y in the
+ * upper 16 bits.
+ */
+ x = (raw_coordinate >> X_COORD_SHIFT) &
+ FIFO_DATA_X_Y_MASK;
+ y = (raw_coordinate >> Y_COORD_SHIFT) &
+ FIFO_DATA_X_Y_MASK;
+
+ /* We only want to retain the 12 msb of the 16 */
+ x = (x >> 4) & 0x0FFF;
+ y = (y >> 4) & 0x0FFF;
+
+ /* adjust x y according to lcd tsc mount angle */
+ if (priv->ts_rotation == TS_ROTATION_90) {
+ y = Y_MAX - y;
+ } else if (priv->ts_rotation == TS_ROTATION_180) {
+ x = X_MAX - x;
+ y = Y_MAX - y;
+ } else if (priv->ts_rotation == TS_ROTATION_270) {
+ x = X_MAX - x;
+ }
+
+ input_report_abs(priv->idev, ABS_X, x);
+ input_report_abs(priv->idev, ABS_Y, y);
+ input_sync(priv->idev);
+
+ dev_dbg(&priv->pdev->dev, "xy (0x%x 0x%x)\n", x, y);
+ }
+ }
+
+ return IRQ_HANDLED;
+}
+
+static int iproc_ts_start(struct input_dev *idev)
+{
+ u32 val;
+ int ret;
+ struct iproc_ts_priv *priv;
+
+ priv = input_get_drvdata(idev);
+ if (priv == NULL)
+ return -ENODEV;
+
+ /* Enable clock */
+ ret = clk_prepare_enable(priv->tsc_clk);
+ if (ret) {
+ dev_err(&priv->pdev->dev, "%s clk_prepare_enable failed %d\n",
+ __func__, ret);
+ return ret;
+ }
+
+ /*
+ * Interrupt is generated when:
+ * FIFO reaches the int_th value, and pen event(up/down)
+ */
+ val = TS_PEN_INTR_MASK | TS_FIFO_INTR_MASK;
+ writel(val, priv->regs + INTERRUPT_MASK);
+
+ writel(priv->cfg_params.fifo_threshold, priv->regs + INTERRUPT_THRES);
+
+ /* Initialize control reg1 */
+ val = 0;
+ val |= (priv->cfg_params.scanning_period << SCANNING_PERIOD_SHIFT);
+ val |= (priv->cfg_params.debounce_timeout << DEBOUNCE_TIMEOUT_SHIFT);
+ val |= (priv->cfg_params.settling_timeout << SETTLING_TIMEOUT_SHIFT);
+ val |= (priv->cfg_params.touch_timeout << TOUCH_TIMEOUT_SHIFT);
+ writel(val, priv->regs + REGCTL1);
+
+ /* Try to clear all interrupt status */
+ val = readl(priv->regs + INTERRUPT_STATUS);
+ val |= (TS_FIFO_INTR_MASK | TS_PEN_INTR_MASK);
+ writel(val, priv->regs + INTERRUPT_STATUS);
+
+ /* Initialize control reg2 */
+ val = readl(priv->regs + REGCTL2);
+ val |= (TS_CONTROLLER_EN_BIT | TS_WIRE_MODE_BIT);
+
+ val &= ~(TS_CONTROLLER_AVGDATA_MASK);
+ val |= (priv->cfg_params.average_data << TS_CONTROLLER_AVGDATA_SHIFT);
+
+ val &= ~(TS_CONTROLLER_PWR_LDO | /* PWR up LDO */
+ TS_CONTROLLER_PWR_ADC | /* PWR up ADC */
+ TS_CONTROLLER_PWR_BGP | /* PWR up BGP */
+ TS_CONTROLLER_PWR_TS); /* PWR up TS */
+
+ writel(val, priv->regs + REGCTL2);
+
+ ts_reg_dump(priv);
+
+ return 0;
+}
+
+static void iproc_ts_stop(struct input_dev *dev)
+{
+ u32 val;
+ struct iproc_ts_priv *priv;
+
+ priv = input_get_drvdata(dev);
+ if (priv == NULL)
+ return;
+
+ writel(0, priv->regs + INTERRUPT_MASK); /* Disable all interrupts */
+
+ /* Only power down touch screen controller */
+ val = readl(priv->regs + REGCTL2);
+ val |= TS_CONTROLLER_PWR_TS;
+ writel(val, priv->regs + REGCTL2);
+
+ clk_disable(priv->tsc_clk);
+}
+
+static int get_tsc_config(struct device_node *np, struct iproc_ts_priv *priv)
+{
+ u32 val;
+ struct device *dev = &priv->pdev->dev;
+
+ priv->cfg_params = default_config;
+
+ if (of_property_read_u32(np, "scanning_period", &val) >= 0) {
+ if (val < 1 || val > 256) {
+ dev_err(dev, "scanning_period must be [1-256]\n");
+ return -EINVAL;
+ }
+ priv->cfg_params.scanning_period = val;
+ }
+
+ if (of_property_read_u32(np, "debounce_timeout", &val) >= 0) {
+ if (val < 0 || val > 255) {
+ dev_err(dev, "debounce_timeout must be [0-255]\n");
+ return -EINVAL;
+ }
+ priv->cfg_params.debounce_timeout = val;
+ }
+
+ if (of_property_read_u32(np, "settling_timeout", &val) >= 0) {
+ if (val < 0 || val > 11) {
+ dev_err(dev, "settling_timeout must be [0-11]\n");
+ return -EINVAL;
+ }
+ priv->cfg_params.settling_timeout = val;
+ }
+
+ if (of_property_read_u32(np, "touch_timeout", &val) >= 0) {
+ if (val < 0 || val > 255) {
+ dev_err(dev, "touch_timeout must be [0-255]\n");
+ return -EINVAL;
+ }
+ priv->cfg_params.touch_timeout = val;
+ }
+
+ if (of_property_read_u32(np, "average_data", &val) >= 0) {
+ if (val < 0 || val > 8) {
+ dev_err(dev, "average_data must be [0-8]\n");
+ return -EINVAL;
+ }
+ priv->cfg_params.average_data = val;
+ }
+
+ if (of_property_read_u32(np, "fifo_threshold", &val) >= 0) {
+ if (val < 0 || val > 31) {
+ dev_err(dev, "fifo_threshold must be [0-31]\n");
+ return -EINVAL;
+ }
+ priv->cfg_params.fifo_threshold = val;
+ }
+
+ priv->ts_rotation = TS_ROTATION_0;
+ if (of_property_read_u32(np, "ts-rotation", &val) >= 0) {
+ priv->ts_rotation = val;
+ dev_dbg(dev, "ts rotation [%d] degrees\n",
+ 90 * priv->ts_rotation);
+ }
+
+ return 0;
+}
+
+static int iproc_ts_probe(struct platform_device *pdev)
+{
+ struct iproc_ts_priv *priv;
+ struct input_dev *idev;
+ struct resource *res;
+ int ret;
+ int irq;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ /* touchscreen controller memory mapped regs */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ priv->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(priv->regs)) {
+ dev_err(&pdev->dev, "unable to map I/O memory\n");
+ return PTR_ERR(priv->regs);
+ }
+
+ priv->tsc_clk = devm_clk_get(&pdev->dev, "tsc_clk");
+ if (IS_ERR(priv->tsc_clk)) {
+ dev_err(&pdev->dev,
+ "%s Failed getting clock tsc_clk\n", __func__);
+ return PTR_ERR(priv->tsc_clk);
+ }
+
+ ret = get_tsc_config(pdev->dev.of_node, priv);
+ if (ret != 0) {
+ dev_err(&pdev->dev, "%s get_tsc_config failed\n", __func__);
+ return ret;
+ }
+
+ idev = devm_input_allocate_device(&pdev->dev);
+ if (!idev) {
+ dev_err(&pdev->dev,
+ "%s Allocate input device failed\n", __func__);
+ return -ENOMEM;
+ }
+
+ priv->idev = idev;
+ priv->pdev = pdev;
+
+ priv->pen_status = PEN_UP_STATUS;
+
+ /* Set input device info */
+ idev->name = IPROC_TS_NAME;
+ idev->dev.parent = &pdev->dev;
+
+ idev->id.bustype = BUS_HOST;
+ idev->id.vendor = SERIO_UNKNOWN;
+ idev->id.product = 0;
+ idev->id.version = 0;
+
+ idev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+ set_bit(BTN_TOUCH, idev->keybit);
+
+ input_set_abs_params(idev, ABS_X, X_MIN, X_MAX, 0, 0);
+ input_set_abs_params(idev, ABS_Y, Y_MIN, Y_MAX, 0, 0);
+
+ idev->open = iproc_ts_start;
+ idev->close = iproc_ts_stop;
+
+ input_set_drvdata(idev, priv);
+ platform_set_drvdata(pdev, priv);
+
+ /* get interrupt */
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "%s platform_get_irq failed\n", __func__);
+ return irq;
+ }
+
+ ret = devm_request_irq(&pdev->dev, irq,
+ iproc_touchscreen_interrupt,
+ IRQF_SHARED, IPROC_TS_NAME, pdev);
+ if (ret)
+ return ret;
+
+ ret = input_register_device(priv->idev);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "%s register input device failed %d\n", __func__, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id iproc_ts_of_match[] = {
+ {.compatible = "brcm,iproc-touchscreen", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, iproc_ts_of_match);
+
+static struct platform_driver iproc_ts_driver = {
+ .probe = iproc_ts_probe,
+ .driver = {
+ .name = IPROC_TS_NAME,
+ .of_match_table = of_match_ptr(iproc_ts_of_match),
+ },
+};
+
+module_platform_driver(iproc_ts_driver);
+
+MODULE_DESCRIPTION("IPROC Touchscreen driver");
+MODULE_AUTHOR("Broadcom");
+MODULE_LICENSE("GPL v2");
--
1.7.9.5

2014-12-19 22:18:29

by Jonathan Richardson

[permalink] [raw]
Subject: [PATCH v2 2/2] Input: touchscreen-iproc: add device tree bindings

Documents the touchscreen device tree binding for Broadcom iProc family
of SoCs.

Reviewed-by: Scott Branden <[email protected]>
Tested-by: Scott Branden <[email protected]>
Signed-off-by: Jonathan Richardson <[email protected]>
---
.../input/touchscreen/brcm,iproc-touchscreen.txt | 70 ++++++++++++++++++++
1 file changed, 70 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/brcm,iproc-touchscreen.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/brcm,iproc-touchscreen.txt b/Documentation/devicetree/bindings/input/touchscreen/brcm,iproc-touchscreen.txt
new file mode 100644
index 0000000..eb28822
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/brcm,iproc-touchscreen.txt
@@ -0,0 +1,70 @@
+* Broadcom's IPROC Touchscreen Controller
+
+Required properties:
+- compatible: must be "brcm,iproc-touchscreen"
+- reg: physical base address of the controller and length of memory mapped
+ region.
+- clocks: The clock provided by the SOC to driver the tsc
+- clock-name: name for the clock
+- interrupts: The touchscreen controller's interrupt
+
+Optional properties:
+- scanning_period: Time between scans. Each step is 1024 us. Valid 1-256.
+- debounce_timeout: Each step is 512 us. Valid 0-255
+- settling_timeout: The settling duration (in ms) is the amount of time
+ the tsc waits to allow the voltage to settle after
+ turning on the drivers in detection mode.
+ Valid values: 0-11
+ 0 = 0.008 ms
+ 1 = 0.01 ms
+ 2 = 0.02 ms
+ 3 = 0.04 ms
+ 4 = 0.08 ms
+ 5 = 0.16 ms
+ 6 = 0.32 ms
+ 7 = 0.64 ms
+ 8 = 1.28 ms
+ 9 = 2.56 ms
+ 10 = 5.12 ms
+ 11 = 10.24 ms
+- touch_timeout: The continuous number of scan periods in which touch is
+ not detected before the controller returns to idle state.
+ Valid values 0-255.
+- average_data: Number of data samples which are averaged before a final
+ data point is placed into the FIFO
+ Valid values 0-7
+ 0 = 1 sample
+ 1 = 2 samples
+ 2 = 4 samples
+ 3 = 8 samples
+ 4 = 16 samples
+ 5 = 32 samples
+ 6 = 64 samples
+ 7 = 128 samples
+- fifo_threshold: Interrupt is generated whenever the number of fifo
+ entries exceeds this value
+ Valid values 0-31
+- ts-rotation: Touch screen controller and display panel orientation angle.
+ 0 = Tsc x_min y_min placed at LCD x_min y_min. None
+ 1 = Tsc x_min y_max placed at LCD x_min y_min. 90 degrees
+ 2 = Tsc x_max y_max placed at LCD x_min y_min. 180 degrees
+ 3 = Tsc x_max y_min placed at LCD x_min y_min. 270 degrees
+Example:
+
+ touchscreen: tsc@0x180A6000 {
+ compatible = "brcm,iproc-touchscreen";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = <0x180A6000 0x40>;
+ clocks = <&adc_clk>;
+ clock-names = "tsc_clk";
+ interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
+
+ scanning_period = <5>;
+ debounce_timeout = <40>;
+ settling_timeout = <7>;
+ touch_timeout = <10>;
+ average_data = <5>;
+ fifo_threshold = <1>;
+ ts-rotation = <2>;
+ };
--
1.7.9.5

2014-12-19 22:27:01

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] Input: touchscreen-iproc: Add Broadcom iProc touchscreen driver

On Fri, 2014-12-19 at 14:17 -0800, Jonathan Richardson wrote:
> Add initial version of the Broadcom touchscreen driver.

more trivia:

> diff --git a/drivers/input/touchscreen/bcm_iproc_tsc.c b/drivers/input/touchscreen/bcm_iproc_tsc.c
[]
> +static int get_tsc_config(struct device_node *np, struct iproc_ts_priv *priv)
> +{
> + u32 val;
[]
> + if (of_property_read_u32(np, "debounce_timeout", &val) >= 0) {
> + if (val < 0 || val > 255) {
> + dev_err(dev, "debounce_timeout must be [0-255]\n");
> + return -EINVAL;
> + }
> + priv->cfg_params.debounce_timeout = val;

Doesn't the compiler generate a warning message
about an impossible "unsigned < 0" test for all
of these "val < 0" uses?

> + }
> +
> + if (of_property_read_u32(np, "settling_timeout", &val) >= 0) {
> + if (val < 0 || val > 11) {
[]
> + if (of_property_read_u32(np, "touch_timeout", &val) >= 0) {
> + if (val < 0 || val > 255) {
[]
> + if (of_property_read_u32(np, "average_data", &val) >= 0) {
> + if (val < 0 || val > 8) {
[]
> + if (of_property_read_u32(np, "fifo_threshold", &val) >= 0) {
> + if (val < 0 || val > 31) {

2014-12-19 23:03:25

by Jonathan Richardson

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] Input: touchscreen-iproc: Add Broadcom iProc touchscreen driver

On 14-12-19 02:26 PM, Joe Perches wrote:
> On Fri, 2014-12-19 at 14:17 -0800, Jonathan Richardson wrote:
>> Add initial version of the Broadcom touchscreen driver.
>
> more trivia:
>
>> diff --git a/drivers/input/touchscreen/bcm_iproc_tsc.c b/drivers/input/touchscreen/bcm_iproc_tsc.c
> []
>> +static int get_tsc_config(struct device_node *np, struct iproc_ts_priv *priv)
>> +{
>> + u32 val;
> []
>> + if (of_property_read_u32(np, "debounce_timeout", &val) >= 0) {
>> + if (val < 0 || val > 255) {
>> + dev_err(dev, "debounce_timeout must be [0-255]\n");
>> + return -EINVAL;
>> + }
>> + priv->cfg_params.debounce_timeout = val;
>
> Doesn't the compiler generate a warning message
> about an impossible "unsigned < 0" test for all
> of these "val < 0" uses?
>

Actually no it doesn't. The gcc output shows that neither -Wtype-limits
nor -Wextra are used to compile that file. I assume this is because
there would be just too many warnings.


>> + }
>> +
>> + if (of_property_read_u32(np, "settling_timeout", &val) >= 0) {
>> + if (val < 0 || val > 11) {
> []
>> + if (of_property_read_u32(np, "touch_timeout", &val) >= 0) {
>> + if (val < 0 || val > 255) {
> []
>> + if (of_property_read_u32(np, "average_data", &val) >= 0) {
>> + if (val < 0 || val > 8) {
> []
>> + if (of_property_read_u32(np, "fifo_threshold", &val) >= 0) {
>> + if (val < 0 || val > 31) {
>
>

2015-01-01 00:55:32

by Jonathan Richardson

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] Input: touchscreen-iproc: Add Broadcom iProc touchscreen driver

Hi,

Would anyone be able to look at this to help us move forward? Thanks.

On 14-12-19 03:03 PM, Jonathan Richardson wrote:
> On 14-12-19 02:26 PM, Joe Perches wrote:
>> On Fri, 2014-12-19 at 14:17 -0800, Jonathan Richardson wrote:
>>> Add initial version of the Broadcom touchscreen driver.
>>
>> more trivia:
>>
>>> diff --git a/drivers/input/touchscreen/bcm_iproc_tsc.c b/drivers/input/touchscreen/bcm_iproc_tsc.c
>> []
>>> +static int get_tsc_config(struct device_node *np, struct iproc_ts_priv *priv)
>>> +{
>>> + u32 val;
>> []
>>> + if (of_property_read_u32(np, "debounce_timeout", &val) >= 0) {
>>> + if (val < 0 || val > 255) {
>>> + dev_err(dev, "debounce_timeout must be [0-255]\n");
>>> + return -EINVAL;
>>> + }
>>> + priv->cfg_params.debounce_timeout = val;
>>
>> Doesn't the compiler generate a warning message
>> about an impossible "unsigned < 0" test for all
>> of these "val < 0" uses?
>>
>
> Actually no it doesn't. The gcc output shows that neither -Wtype-limits
> nor -Wextra are used to compile that file. I assume this is because
> there would be just too many warnings.
>
>
>>> + }
>>> +
>>> + if (of_property_read_u32(np, "settling_timeout", &val) >= 0) {
>>> + if (val < 0 || val > 11) {
>> []
>>> + if (of_property_read_u32(np, "touch_timeout", &val) >= 0) {
>>> + if (val < 0 || val > 255) {
>> []
>>> + if (of_property_read_u32(np, "average_data", &val) >= 0) {
>>> + if (val < 0 || val > 8) {
>> []
>>> + if (of_property_read_u32(np, "fifo_threshold", &val) >= 0) {
>>> + if (val < 0 || val > 31) {
>>
>>
>

2015-02-11 18:45:40

by Jonathan Richardson

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] Input: touchscreen-iproc: Add Broadcom iProc touchscreen driver

Pinging maintainers... Am I ok to go ahead with the current rotation
implementation? I haven't heard anything further. Any feedback on naming
conventions from DT people?

Thanks!

On 15-01-15 11:51 AM, Jonathan Richardson wrote:
> Hi Dmitry,
>
> On 15-01-14 10:07 PM, Dmitry Torokhov wrote:
>> On Wed, Jan 14, 2015 at 09:44:39PM -0800, Scott Branden wrote:
>>> On 15-01-14 05:02 PM, Dmitry Torokhov wrote:
>>>> Hi Jonathan,
>>>>
>>>> On Fri, Dec 19, 2014 at 02:17:49PM -0800, Jonathan Richardson wrote:
>>>>> + if (of_property_read_u32(np, "scanning_period", &val) >= 0) {
>>>>> + if (val < 1 || val > 256) {
>>>>> + dev_err(dev, "scanning_period must be [1-256]\n");
>>>>> + return -EINVAL;
>>>>> + }
>>>>> + priv->cfg_params.scanning_period = val;
>>>>> + }
>>>>> +
>>>>> + if (of_property_read_u32(np, "debounce_timeout", &val) >= 0) {
>>>>> + if (val < 0 || val > 255) {
>>>>> + dev_err(dev, "debounce_timeout must be [0-255]\n");
>>>>> + return -EINVAL;
>>>>> + }
>>>>> + priv->cfg_params.debounce_timeout = val;
>>>>> + }
>>>>> +
>>>>> + if (of_property_read_u32(np, "settling_timeout", &val) >= 0) {
>>>>> + if (val < 0 || val > 11) {
>>>>> + dev_err(dev, "settling_timeout must be [0-11]\n");
>>>>> + return -EINVAL;
>>>>> + }
>>>>> + priv->cfg_params.settling_timeout = val;
>>>>> + }
>>>>> +
>>>>> + if (of_property_read_u32(np, "touch_timeout", &val) >= 0) {
>>>>> + if (val < 0 || val > 255) {
>>>>> + dev_err(dev, "touch_timeout must be [0-255]\n");
>>>>> + return -EINVAL;
>>>>> + }
>>>>> + priv->cfg_params.touch_timeout = val;
>>>>> + }
>>>>> +
>>>>> + if (of_property_read_u32(np, "average_data", &val) >= 0) {
>>>>> + if (val < 0 || val > 8) {
>>>>> + dev_err(dev, "average_data must be [0-8]\n");
>>>>> + return -EINVAL;
>>>>> + }
>>>>> + priv->cfg_params.average_data = val;
>>>>> + }
>>>>> +
>>>>> + if (of_property_read_u32(np, "fifo_threshold", &val) >= 0) {
>>>>> + if (val < 0 || val > 31) {
>>>>> + dev_err(dev, "fifo_threshold must be [0-31]\n");
>>>>> + return -EINVAL;
>>>>> + }
>>>>> + priv->cfg_params.fifo_threshold = val;
>>>>> + }
>>>>
>>>> I think these are dveice specific and thus should have "brcm," prefix.
>>> I'm confused as to why we need the brcm prefix? Other device tree
>>> bindings we have for other drivers do not need such prefix.
>>
>> Properties that are not standard on the system (reg, interrupts,
>> clkocks, etc) or subsystem level customarily carry the vendor prefix so
>> that they do not clash with newly added global or subsystem properties.
>>
>>> Is this
>>> convention documented somewhere?
>>
>> Not sure. I glanced through Documentation/devicetree and do not see it
>> spelled out. Device tree overlords, what say you?
>
> Let me know. I haven't seen this before either. I will change the
> entries to use dashes though instead of underscores but will wait until
> these other issues are decided on before sending out another patch.
>
>>
>>>>
>>>>> +
>>>>> + priv->ts_rotation = TS_ROTATION_0;
>>>>> + if (of_property_read_u32(np, "ts-rotation", &val) >= 0) {
>>>>> + priv->ts_rotation = val;
>>>>> + dev_dbg(dev, "ts rotation [%d] degrees\n",
>>>>> + 90 * priv->ts_rotation);
>>>>> + }
>>>>
>>>> This I am not quite sure about - if we want rotation or swap+invert. You
>>>> are CCed on another email (tsc2007) that talks about need of generic
>>>> touchscreen transforms in input core/of bindings.
>>> Does such generic binding exist today? If not, I would like to go
>>> with this implementation and update to the new binding if/when it
>>> exists?
>>
>> Not yet but there several people interested. I think we have enough time
>> till 3.20 to hash it out properly.
>
> I think the rotation is simpler personally. Everyone would understand
> rotation refers to how it's oriented but I'm not sure everyone would
> immediately know how it is wired. Let me know what is decided and I'll
> make any changes required.
>
> Thanks,
> Jon
>
>
>>
>> Thanks.
>>
>

2015-02-24 23:19:24

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] Input: touchscreen-iproc: Add Broadcom iProc touchscreen driver

Jonathan,


On Wed, Feb 11, 2015 at 10:45:34AM -0800, Jonathan Richardson wrote:
> Pinging maintainers... Am I ok to go ahead with the current rotation
> implementation? I haven't heard anything further. Any feedback on naming
> conventions from DT people?
>

I believe we should go with touchscreen-inverted-x,
touchscreen-inverted-y and touchscreen-swapped-x-y properties since
rotation can't really describe all permutations of potential
connections.

I'll be taking Hans de Goede's patch adding this new property to the
bindings (adjusting the name slightly).

Thanks.

> Thanks!
>
> On 15-01-15 11:51 AM, Jonathan Richardson wrote:
> > Hi Dmitry,
> >
> > On 15-01-14 10:07 PM, Dmitry Torokhov wrote:
> >> On Wed, Jan 14, 2015 at 09:44:39PM -0800, Scott Branden wrote:
> >>> On 15-01-14 05:02 PM, Dmitry Torokhov wrote:
> >>>> Hi Jonathan,
> >>>>
> >>>> On Fri, Dec 19, 2014 at 02:17:49PM -0800, Jonathan Richardson wrote:
> >>>>> + if (of_property_read_u32(np, "scanning_period", &val) >= 0) {
> >>>>> + if (val < 1 || val > 256) {
> >>>>> + dev_err(dev, "scanning_period must be [1-256]\n");
> >>>>> + return -EINVAL;
> >>>>> + }
> >>>>> + priv->cfg_params.scanning_period = val;
> >>>>> + }
> >>>>> +
> >>>>> + if (of_property_read_u32(np, "debounce_timeout", &val) >= 0) {
> >>>>> + if (val < 0 || val > 255) {
> >>>>> + dev_err(dev, "debounce_timeout must be [0-255]\n");
> >>>>> + return -EINVAL;
> >>>>> + }
> >>>>> + priv->cfg_params.debounce_timeout = val;
> >>>>> + }
> >>>>> +
> >>>>> + if (of_property_read_u32(np, "settling_timeout", &val) >= 0) {
> >>>>> + if (val < 0 || val > 11) {
> >>>>> + dev_err(dev, "settling_timeout must be [0-11]\n");
> >>>>> + return -EINVAL;
> >>>>> + }
> >>>>> + priv->cfg_params.settling_timeout = val;
> >>>>> + }
> >>>>> +
> >>>>> + if (of_property_read_u32(np, "touch_timeout", &val) >= 0) {
> >>>>> + if (val < 0 || val > 255) {
> >>>>> + dev_err(dev, "touch_timeout must be [0-255]\n");
> >>>>> + return -EINVAL;
> >>>>> + }
> >>>>> + priv->cfg_params.touch_timeout = val;
> >>>>> + }
> >>>>> +
> >>>>> + if (of_property_read_u32(np, "average_data", &val) >= 0) {
> >>>>> + if (val < 0 || val > 8) {
> >>>>> + dev_err(dev, "average_data must be [0-8]\n");
> >>>>> + return -EINVAL;
> >>>>> + }
> >>>>> + priv->cfg_params.average_data = val;
> >>>>> + }
> >>>>> +
> >>>>> + if (of_property_read_u32(np, "fifo_threshold", &val) >= 0) {
> >>>>> + if (val < 0 || val > 31) {
> >>>>> + dev_err(dev, "fifo_threshold must be [0-31]\n");
> >>>>> + return -EINVAL;
> >>>>> + }
> >>>>> + priv->cfg_params.fifo_threshold = val;
> >>>>> + }
> >>>>
> >>>> I think these are dveice specific and thus should have "brcm," prefix.
> >>> I'm confused as to why we need the brcm prefix? Other device tree
> >>> bindings we have for other drivers do not need such prefix.
> >>
> >> Properties that are not standard on the system (reg, interrupts,
> >> clkocks, etc) or subsystem level customarily carry the vendor prefix so
> >> that they do not clash with newly added global or subsystem properties.
> >>
> >>> Is this
> >>> convention documented somewhere?
> >>
> >> Not sure. I glanced through Documentation/devicetree and do not see it
> >> spelled out. Device tree overlords, what say you?
> >
> > Let me know. I haven't seen this before either. I will change the
> > entries to use dashes though instead of underscores but will wait until
> > these other issues are decided on before sending out another patch.
> >
> >>
> >>>>
> >>>>> +
> >>>>> + priv->ts_rotation = TS_ROTATION_0;
> >>>>> + if (of_property_read_u32(np, "ts-rotation", &val) >= 0) {
> >>>>> + priv->ts_rotation = val;
> >>>>> + dev_dbg(dev, "ts rotation [%d] degrees\n",
> >>>>> + 90 * priv->ts_rotation);
> >>>>> + }
> >>>>
> >>>> This I am not quite sure about - if we want rotation or swap+invert. You
> >>>> are CCed on another email (tsc2007) that talks about need of generic
> >>>> touchscreen transforms in input core/of bindings.
> >>> Does such generic binding exist today? If not, I would like to go
> >>> with this implementation and update to the new binding if/when it
> >>> exists?
> >>
> >> Not yet but there several people interested. I think we have enough time
> >> till 3.20 to hash it out properly.
> >
> > I think the rotation is simpler personally. Everyone would understand
> > rotation refers to how it's oriented but I'm not sure everyone would
> > immediately know how it is wired. Let me know what is decided and I'll
> > make any changes required.
> >
> > Thanks,
> > Jon
> >
> >
> >>
> >> Thanks.
> >>
> >
>

--
Dmitry

2015-02-24 23:30:05

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] Input: touchscreen-iproc: Add Broadcom iProc touchscreen driver

Hi Jonathan,

On Fri, Dec 19, 2014 at 02:17:49PM -0800, Jonathan Richardson wrote:
> Add initial version of the Broadcom touchscreen driver.
>
> Reviewed-by: Ray Jui <[email protected]>
> Reviewed-by: Scott Branden <[email protected]>
> Tested-by: Scott Branden <[email protected]>
> Signed-off-by: Jonathan Richardson <[email protected]>
> ---
> drivers/input/touchscreen/Kconfig | 11 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/bcm_iproc_tsc.c | 535 +++++++++++++++++++++++++++++
> 3 files changed, 547 insertions(+)
> create mode 100644 drivers/input/touchscreen/bcm_iproc_tsc.c
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index e1d8003..77ff531 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -310,6 +310,17 @@ config TOUCHSCREEN_ILI210X
> To compile this driver as a module, choose M here: the
> module will be called ili210x.
>
> +config TOUCHSCREEN_IPROC
> + tristate "IPROC touch panel driver support"
> + help
> + Say Y here if you want to add support for the IPROC touch
> + controller to your system.
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called iproc-ts.
> +
> config TOUCHSCREEN_S3C2410
> tristate "Samsung S3C2410/generic touchscreen input driver"
> depends on ARCH_S3C24XX || SAMSUNG_DEV_TS
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 090e61c..f7e6de9 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -37,6 +37,7 @@ obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
> obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
> obj-$(CONFIG_TOUCHSCREEN_INEXIO) += inexio.o
> obj-$(CONFIG_TOUCHSCREEN_INTEL_MID) += intel-mid-touch.o
> +obj-$(CONFIG_TOUCHSCREEN_IPROC) += bcm_iproc_tsc.o
> obj-$(CONFIG_TOUCHSCREEN_LPC32XX) += lpc32xx_ts.o
> obj-$(CONFIG_TOUCHSCREEN_MAX11801) += max11801_ts.o
> obj-$(CONFIG_TOUCHSCREEN_MC13783) += mc13783_ts.o
> diff --git a/drivers/input/touchscreen/bcm_iproc_tsc.c b/drivers/input/touchscreen/bcm_iproc_tsc.c
> new file mode 100644
> index 0000000..bf6eb7f
> --- /dev/null
> +++ b/drivers/input/touchscreen/bcm_iproc_tsc.c
> @@ -0,0 +1,535 @@
> +/*
> +* Copyright (C) 2014 Broadcom Corporation
> +*
> +* This program is free software; you can redistribute it and/or
> +* modify it under the terms of the GNU General Public License as
> +* published by the Free Software Foundation version 2.
> +*
> +* This program is distributed "as is" WITHOUT ANY WARRANTY of any
> +* kind, whether express or implied; without even the implied warranty
> +* of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +* GNU General Public License for more details.
> +*/
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/input.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/keyboard.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <asm/irq.h>
> +#include <linux/io.h>
> +#include <linux/clk.h>
> +#include <linux/serio.h>
> +
> +#define IPROC_TS_NAME "iproc-ts"
> +
> +#define PEN_DOWN_STATUS 1
> +#define PEN_UP_STATUS 0
> +
> +#define X_MIN 0
> +#define Y_MIN 0
> +#define X_MAX 0xFFF
> +#define Y_MAX 0xFFF
> +
> +/* Value given by controller for invalid coordinate. */
> +#define INVALID_COORD 0xFFFFFFFF
> +
> +/* Register offsets */
> +#define REGCTL1 0x00
> +#define REGCTL2 0x04
> +#define INTERRUPT_THRES 0x08
> +#define INTERRUPT_MASK 0x0c
> +
> +#define INTERRUPT_STATUS 0x10
> +#define CONTROLLER_STATUS 0x14
> +#define FIFO_DATA 0x18
> +#define FIFO_DATA_X_Y_MASK 0xFFFF
> +#define ANALOG_CONTROL 0x1c
> +
> +#define AUX_DATA 0x20
> +#define DEBOUNCE_CNTR_STAT 0x24
> +#define SCAN_CNTR_STAT 0x28
> +#define REM_CNTR_STAT 0x2c
> +
> +#define SETTLING_TIMER_STAT 0x30
> +#define SPARE_REG 0x34
> +#define SOFT_BYPASS_CONTROL 0x38
> +#define SOFT_BYPASS_DATA 0x3c
> +
> +
> +/* Bit values for INTERRUPT_MASK and INTERRUPT_STATUS regs */
> +#define TS_PEN_INTR_MASK BIT(0)
> +#define TS_FIFO_INTR_MASK BIT(2)
> +
> +/* Bit values for CONTROLLER_STATUS reg1 */
> +#define TS_PEN_DOWN BIT(0)
> +
> +/* Shift values for control reg1 */
> +#define SCANNING_PERIOD_SHIFT 24
> +#define DEBOUNCE_TIMEOUT_SHIFT 16
> +#define SETTLING_TIMEOUT_SHIFT 8
> +#define TOUCH_TIMEOUT_SHIFT 0
> +
> +/* Shift values for coordinates from fifo */
> +#define X_COORD_SHIFT 0
> +#define Y_COORD_SHIFT 16
> +
> +/* Bit values for REGCTL2 */
> +#define TS_CONTROLLER_EN_BIT BIT(16)
> +#define TS_CONTROLLER_AVGDATA_SHIFT 8
> +#define TS_CONTROLLER_AVGDATA_MASK (0x7 << TS_CONTROLLER_AVGDATA_SHIFT)
> +#define TS_CONTROLLER_PWR_LDO BIT(5)
> +#define TS_CONTROLLER_PWR_ADC BIT(4)
> +#define TS_CONTROLLER_PWR_BGP BIT(3)
> +#define TS_CONTROLLER_PWR_TS BIT(2)
> +#define TS_WIRE_MODE_BIT BIT(1)
> +
> +/*
> + * Touch screen mount angles w.r.t LCD panel left side top corner
> + * TS (x_min,y_min) placed at LCD (x_min,y_min) rotation angle is 0
> + * TS (x_min,y_max) placed at LCD (x_min,y_min) rotation angle is 90
> + * TS (x_max,y_max) placed at LCD (x_min,y_min) rotation angle is 180
> + * TS (x_max,y_min) placed at LCD (x_min,y_min) rotation angle is 270
> + */
> +enum ts_rotation_angles {
> + TS_ROTATION_0,
> + TS_ROTATION_90,
> + TS_ROTATION_180,
> + TS_ROTATION_270,
> +};
> +
> +struct tsc_param {
> + /* Each step is 1024 us. Valid 1-256 */
> + u32 scanning_period;
> +
> + /* Each step is 512 us. Valid 0-255 */
> + u32 debounce_timeout;
> +
> + /*
> + * The settling duration (in ms) is the amount of time the tsc
> + * waits to allow the voltage to settle after turning on the
> + * drivers in detection mode. Valid values: 0-11
> + * 0 = 0.008 ms
> + * 1 = 0.01 ms
> + * 2 = 0.02 ms
> + * 3 = 0.04 ms
> + * 4 = 0.08 ms
> + * 5 = 0.16 ms
> + * 6 = 0.32 ms
> + * 7 = 0.64 ms
> + * 8 = 1.28 ms
> + * 9 = 2.56 ms
> + * 10 = 5.12 ms
> + * 11 = 10.24 ms
> + */
> + u32 settling_timeout;
> +
> + /* touch timeout in sample counts */
> + u32 touch_timeout;
> +
> + /*
> + * Number of data samples which are averaged before a final data point
> + * is placed into the FIFO
> + */
> + u32 average_data;
> +
> + /* FIFO threshold */
> + u32 fifo_threshold;
> +};
> +
> +struct iproc_ts_priv {
> + struct platform_device *pdev;
> + struct input_dev *idev;
> +
> + void __iomem *regs;
> + struct clk *tsc_clk;
> +
> + int pen_status;
> + int ts_rotation;
> + struct tsc_param cfg_params;
> +};
> +
> +/*
> + * Set default values the same as hardware reset values
> + * except for fifo_threshold with is set to 1.
> + */
> +static struct tsc_param default_config = {
> + .scanning_period = 0x5, /* 1 to 256 */
> + .debounce_timeout = 0x28, /* 0 to 255 */
> + .settling_timeout = 0x7, /* 0 to 11 */
> + .touch_timeout = 0xa, /* 0 to 255 */
> + .average_data = 5, /* entry 5 = 32 pts */
> + .fifo_threshold = 1, /* 0 to 31 */
> +};
> +
> +static void ts_reg_dump(struct iproc_ts_priv *priv)
> +{
> + struct device *dev = &priv->pdev->dev;
> +
> + dev_dbg(dev, "regCtl1 = 0x%08x\n",
> + readl(priv->regs + REGCTL1));
> + dev_dbg(dev, "regCtl2 = 0x%08x\n",
> + readl(priv->regs + REGCTL2));
> + dev_dbg(dev, "interrupt_Thres = 0x%08x\n",
> + readl(priv->regs + INTERRUPT_THRES));
> + dev_dbg(dev, "interrupt_Mask = 0x%08x\n",
> + readl(priv->regs + INTERRUPT_MASK));
> + dev_dbg(dev, "interrupt_Status = 0x%08x\n",
> + readl(priv->regs + INTERRUPT_STATUS));
> + dev_dbg(dev, "controller_Status = 0x%08x\n",
> + readl(priv->regs + CONTROLLER_STATUS));
> + dev_dbg(dev, "FIFO_Data = 0x%08x\n",
> + readl(priv->regs + FIFO_DATA));
> + dev_dbg(dev, "analog_Control = 0x%08x\n",
> + readl(priv->regs + ANALOG_CONTROL));
> + dev_dbg(dev, "aux_Data = 0x%08x\n",
> + readl(priv->regs + AUX_DATA));
> + dev_dbg(dev, "debounce_Cntr_Stat = 0x%08x\n",
> + readl(priv->regs + DEBOUNCE_CNTR_STAT));
> + dev_dbg(dev, "scan_Cntr_Stat = 0x%08x\n",
> + readl(priv->regs + SCAN_CNTR_STAT));
> + dev_dbg(dev, "rem_Cntr_Stat = 0x%08x\n",
> + readl(priv->regs + REM_CNTR_STAT));
> + dev_dbg(dev, "settling_Timer_Stat = 0x%08x\n",
> + readl(priv->regs + SETTLING_TIMER_STAT));
> + dev_dbg(dev, "spare_Reg = 0x%08x\n",
> + readl(priv->regs + SPARE_REG));
> + dev_dbg(dev, "soft_Bypass_Control = 0x%08x\n",
> + readl(priv->regs + SOFT_BYPASS_CONTROL));
> + dev_dbg(dev, "soft_Bypass_Data = 0x%08x\n",
> + readl(priv->regs + SOFT_BYPASS_DATA));
> +}
> +
> +static irqreturn_t iproc_touchscreen_interrupt(int irq, void *data)
> +{
> + struct platform_device *pdev = (struct platform_device *)data;

No need to cast here.

> + struct iproc_ts_priv *priv;
> + u32 intr_status = 0;
> + u32 raw_coordinate = 0;
> + u16 x = 0;
> + u16 y = 0;

Why do you need all these initialized? It just hides potential problems
when variable is used without assigning proper value.

> + int i;
> +
> + priv = (struct iproc_ts_priv *)platform_get_drvdata(pdev);

No need to cast here.

> +
> + intr_status = readl(priv->regs + INTERRUPT_STATUS);
> + intr_status &= (TS_PEN_INTR_MASK | TS_FIFO_INTR_MASK);
> + if (intr_status == 0)
> + return IRQ_NONE;
> +
> + /* Clear all interrupt status bits, write-1-clear */
> + writel(intr_status, priv->regs + INTERRUPT_STATUS);
> +
> + /* Pen up/down */
> + if (intr_status & TS_PEN_INTR_MASK) {
> + if (readl(priv->regs + CONTROLLER_STATUS) & TS_PEN_DOWN) {
> + priv->pen_status = PEN_DOWN_STATUS;
> + input_report_key(priv->idev, BTN_TOUCH,
> + priv->pen_status);

We do not do input_sync() here... But what happens if we do not get
TS_FIFO_INTR_MASK in the status? Won't we "lose" the sync?

I'd prefer you parsed the hardware state fully and then emitted all
needed events based on the state.

> + } else {
> + priv->pen_status = PEN_UP_STATUS;
> + input_report_key(priv->idev, BTN_TOUCH,
> + priv->pen_status);
> + input_sync(priv->idev);

Same here. Can we be reporting coordinates from FIFO even after pen is
up?

> + }
> +
> + dev_dbg(&priv->pdev->dev,
> + "pen up-down (%d)\n", priv->pen_status);
> + }
> +
> + /* coordinates in FIFO exceed the theshold */
> + if (intr_status & TS_FIFO_INTR_MASK) {
> + for (i = 0; i < priv->cfg_params.fifo_threshold; i++) {
> + raw_coordinate = readl(priv->regs + FIFO_DATA);
> + if (raw_coordinate == INVALID_COORD)
> + continue;
> +
> + /*
> + * The x and y coordinate are 16 bits each
> + * with the x in the lower 16 bits and y in the
> + * upper 16 bits.
> + */
> + x = (raw_coordinate >> X_COORD_SHIFT) &
> + FIFO_DATA_X_Y_MASK;
> + y = (raw_coordinate >> Y_COORD_SHIFT) &
> + FIFO_DATA_X_Y_MASK;
> +
> + /* We only want to retain the 12 msb of the 16 */
> + x = (x >> 4) & 0x0FFF;
> + y = (y >> 4) & 0x0FFF;
> +
> + /* adjust x y according to lcd tsc mount angle */
> + if (priv->ts_rotation == TS_ROTATION_90) {
> + y = Y_MAX - y;
> + } else if (priv->ts_rotation == TS_ROTATION_180) {
> + x = X_MAX - x;
> + y = Y_MAX - y;
> + } else if (priv->ts_rotation == TS_ROTATION_270) {
> + x = X_MAX - x;
> + }
> +
> + input_report_abs(priv->idev, ABS_X, x);
> + input_report_abs(priv->idev, ABS_Y, y);
> + input_sync(priv->idev);
> +
> + dev_dbg(&priv->pdev->dev, "xy (0x%x 0x%x)\n", x, y);
> + }
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int iproc_ts_start(struct input_dev *idev)
> +{
> + u32 val;
> + int ret;
> + struct iproc_ts_priv *priv;
> +
> + priv = input_get_drvdata(idev);
> + if (priv == NULL)
> + return -ENODEV;

How can it be NULL? It is fully controlled by this driver and it should
set it up before registering the input device.

> +
> + /* Enable clock */
> + ret = clk_prepare_enable(priv->tsc_clk);
> + if (ret) {
> + dev_err(&priv->pdev->dev, "%s clk_prepare_enable failed %d\n",
> + __func__, ret);
> + return ret;
> + }
> +
> + /*
> + * Interrupt is generated when:
> + * FIFO reaches the int_th value, and pen event(up/down)
> + */
> + val = TS_PEN_INTR_MASK | TS_FIFO_INTR_MASK;
> + writel(val, priv->regs + INTERRUPT_MASK);
> +
> + writel(priv->cfg_params.fifo_threshold, priv->regs + INTERRUPT_THRES);
> +
> + /* Initialize control reg1 */
> + val = 0;
> + val |= (priv->cfg_params.scanning_period << SCANNING_PERIOD_SHIFT);

Please drop extra parentheses here and elsewhere.

> + val |= (priv->cfg_params.debounce_timeout << DEBOUNCE_TIMEOUT_SHIFT);
> + val |= (priv->cfg_params.settling_timeout << SETTLING_TIMEOUT_SHIFT);
> + val |= (priv->cfg_params.touch_timeout << TOUCH_TIMEOUT_SHIFT);
> + writel(val, priv->regs + REGCTL1);
> +
> + /* Try to clear all interrupt status */
> + val = readl(priv->regs + INTERRUPT_STATUS);
> + val |= (TS_FIFO_INTR_MASK | TS_PEN_INTR_MASK);
> + writel(val, priv->regs + INTERRUPT_STATUS);
> +
> + /* Initialize control reg2 */
> + val = readl(priv->regs + REGCTL2);
> + val |= (TS_CONTROLLER_EN_BIT | TS_WIRE_MODE_BIT);
> +
> + val &= ~(TS_CONTROLLER_AVGDATA_MASK);
> + val |= (priv->cfg_params.average_data << TS_CONTROLLER_AVGDATA_SHIFT);
> +
> + val &= ~(TS_CONTROLLER_PWR_LDO | /* PWR up LDO */
> + TS_CONTROLLER_PWR_ADC | /* PWR up ADC */
> + TS_CONTROLLER_PWR_BGP | /* PWR up BGP */
> + TS_CONTROLLER_PWR_TS); /* PWR up TS */
> +
> + writel(val, priv->regs + REGCTL2);
> +
> + ts_reg_dump(priv);
> +
> + return 0;
> +}
> +
> +static void iproc_ts_stop(struct input_dev *dev)
> +{
> + u32 val;
> + struct iproc_ts_priv *priv;
> +
> + priv = input_get_drvdata(dev);
> + if (priv == NULL)
> + return;

Again, no need to check it here.

> +
> + writel(0, priv->regs + INTERRUPT_MASK); /* Disable all interrupts */
> +
> + /* Only power down touch screen controller */
> + val = readl(priv->regs + REGCTL2);
> + val |= TS_CONTROLLER_PWR_TS;
> + writel(val, priv->regs + REGCTL2);
> +
> + clk_disable(priv->tsc_clk);
> +}
> +
> +static int get_tsc_config(struct device_node *np, struct iproc_ts_priv *priv)
> +{
> + u32 val;
> + struct device *dev = &priv->pdev->dev;
> +
> + priv->cfg_params = default_config;
> +
> + if (of_property_read_u32(np, "scanning_period", &val) >= 0) {
> + if (val < 1 || val > 256) {
> + dev_err(dev, "scanning_period must be [1-256]\n");
> + return -EINVAL;
> + }
> + priv->cfg_params.scanning_period = val;
> + }
> +
> + if (of_property_read_u32(np, "debounce_timeout", &val) >= 0) {
> + if (val < 0 || val > 255) {
> + dev_err(dev, "debounce_timeout must be [0-255]\n");
> + return -EINVAL;
> + }
> + priv->cfg_params.debounce_timeout = val;
> + }
> +
> + if (of_property_read_u32(np, "settling_timeout", &val) >= 0) {
> + if (val < 0 || val > 11) {
> + dev_err(dev, "settling_timeout must be [0-11]\n");
> + return -EINVAL;
> + }
> + priv->cfg_params.settling_timeout = val;
> + }
> +
> + if (of_property_read_u32(np, "touch_timeout", &val) >= 0) {
> + if (val < 0 || val > 255) {
> + dev_err(dev, "touch_timeout must be [0-255]\n");
> + return -EINVAL;
> + }
> + priv->cfg_params.touch_timeout = val;
> + }
> +
> + if (of_property_read_u32(np, "average_data", &val) >= 0) {
> + if (val < 0 || val > 8) {
> + dev_err(dev, "average_data must be [0-8]\n");
> + return -EINVAL;
> + }
> + priv->cfg_params.average_data = val;
> + }
> +
> + if (of_property_read_u32(np, "fifo_threshold", &val) >= 0) {
> + if (val < 0 || val > 31) {
> + dev_err(dev, "fifo_threshold must be [0-31]\n");
> + return -EINVAL;
> + }
> + priv->cfg_params.fifo_threshold = val;
> + }
> +
> + priv->ts_rotation = TS_ROTATION_0;
> + if (of_property_read_u32(np, "ts-rotation", &val) >= 0) {
> + priv->ts_rotation = val;
> + dev_dbg(dev, "ts rotation [%d] degrees\n",
> + 90 * priv->ts_rotation);
> + }
> +
> + return 0;
> +}
> +
> +static int iproc_ts_probe(struct platform_device *pdev)
> +{
> + struct iproc_ts_priv *priv;
> + struct input_dev *idev;
> + struct resource *res;
> + int ret;
> + int irq;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + /* touchscreen controller memory mapped regs */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + priv->regs = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(priv->regs)) {
> + dev_err(&pdev->dev, "unable to map I/O memory\n");
> + return PTR_ERR(priv->regs);
> + }
> +
> + priv->tsc_clk = devm_clk_get(&pdev->dev, "tsc_clk");
> + if (IS_ERR(priv->tsc_clk)) {
> + dev_err(&pdev->dev,
> + "%s Failed getting clock tsc_clk\n", __func__);
> + return PTR_ERR(priv->tsc_clk);
> + }
> +
> + ret = get_tsc_config(pdev->dev.of_node, priv);
> + if (ret != 0) {
> + dev_err(&pdev->dev, "%s get_tsc_config failed\n", __func__);
> + return ret;
> + }
> +
> + idev = devm_input_allocate_device(&pdev->dev);
> + if (!idev) {
> + dev_err(&pdev->dev,
> + "%s Allocate input device failed\n", __func__);
> + return -ENOMEM;
> + }
> +
> + priv->idev = idev;
> + priv->pdev = pdev;
> +
> + priv->pen_status = PEN_UP_STATUS;
> +
> + /* Set input device info */
> + idev->name = IPROC_TS_NAME;
> + idev->dev.parent = &pdev->dev;
> +
> + idev->id.bustype = BUS_HOST;
> + idev->id.vendor = SERIO_UNKNOWN;
> + idev->id.product = 0;
> + idev->id.version = 0;
> +
> + idev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> + set_bit(BTN_TOUCH, idev->keybit);
> +
> + input_set_abs_params(idev, ABS_X, X_MIN, X_MAX, 0, 0);
> + input_set_abs_params(idev, ABS_Y, Y_MIN, Y_MAX, 0, 0);
> +
> + idev->open = iproc_ts_start;
> + idev->close = iproc_ts_stop;
> +
> + input_set_drvdata(idev, priv);
> + platform_set_drvdata(pdev, priv);
> +
> + /* get interrupt */
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "%s platform_get_irq failed\n", __func__);
> + return irq;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, irq,
> + iproc_touchscreen_interrupt,
> + IRQF_SHARED, IPROC_TS_NAME, pdev);
> + if (ret)
> + return ret;
> +
> + ret = input_register_device(priv->idev);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "%s register input device failed %d\n", __func__, ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id iproc_ts_of_match[] = {
> + {.compatible = "brcm,iproc-touchscreen", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, iproc_ts_of_match);
> +
> +static struct platform_driver iproc_ts_driver = {
> + .probe = iproc_ts_probe,
> + .driver = {
> + .name = IPROC_TS_NAME,
> + .of_match_table = of_match_ptr(iproc_ts_of_match),
> + },
> +};
> +
> +module_platform_driver(iproc_ts_driver);
> +
> +MODULE_DESCRIPTION("IPROC Touchscreen driver");
> +MODULE_AUTHOR("Broadcom");
> +MODULE_LICENSE("GPL v2");
> --
> 1.7.9.5
>

Thanks.

--
Dmitry

2015-02-27 01:02:32

by Jonathan Richardson

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] Input: touchscreen-iproc: Add Broadcom iProc touchscreen driver

Hi Dmitry,

Thanks. I'll go through his patch and make the appropriate changes to
our driver.

Jon

On 15-02-24 03:18 PM, Dmitry Torokhov wrote:
> Jonathan,
>
>
> On Wed, Feb 11, 2015 at 10:45:34AM -0800, Jonathan Richardson wrote:
>> Pinging maintainers... Am I ok to go ahead with the current rotation
>> implementation? I haven't heard anything further. Any feedback on naming
>> conventions from DT people?
>>
>
> I believe we should go with touchscreen-inverted-x,
> touchscreen-inverted-y and touchscreen-swapped-x-y properties since
> rotation can't really describe all permutations of potential
> connections.
>
> I'll be taking Hans de Goede's patch adding this new property to the
> bindings (adjusting the name slightly).
>
> Thanks.
>
>> Thanks!
>>
>> On 15-01-15 11:51 AM, Jonathan Richardson wrote:
>>> Hi Dmitry,
>>>
>>> On 15-01-14 10:07 PM, Dmitry Torokhov wrote:
>>>> On Wed, Jan 14, 2015 at 09:44:39PM -0800, Scott Branden wrote:
>>>>> On 15-01-14 05:02 PM, Dmitry Torokhov wrote:
>>>>>> Hi Jonathan,
>>>>>>
>>>>>> On Fri, Dec 19, 2014 at 02:17:49PM -0800, Jonathan Richardson wrote:
>>>>>>> + if (of_property_read_u32(np, "scanning_period", &val) >= 0) {
>>>>>>> + if (val < 1 || val > 256) {
>>>>>>> + dev_err(dev, "scanning_period must be [1-256]\n");
>>>>>>> + return -EINVAL;
>>>>>>> + }
>>>>>>> + priv->cfg_params.scanning_period = val;
>>>>>>> + }
>>>>>>> +
>>>>>>> + if (of_property_read_u32(np, "debounce_timeout", &val) >= 0) {
>>>>>>> + if (val < 0 || val > 255) {
>>>>>>> + dev_err(dev, "debounce_timeout must be [0-255]\n");
>>>>>>> + return -EINVAL;
>>>>>>> + }
>>>>>>> + priv->cfg_params.debounce_timeout = val;
>>>>>>> + }
>>>>>>> +
>>>>>>> + if (of_property_read_u32(np, "settling_timeout", &val) >= 0) {
>>>>>>> + if (val < 0 || val > 11) {
>>>>>>> + dev_err(dev, "settling_timeout must be [0-11]\n");
>>>>>>> + return -EINVAL;
>>>>>>> + }
>>>>>>> + priv->cfg_params.settling_timeout = val;
>>>>>>> + }
>>>>>>> +
>>>>>>> + if (of_property_read_u32(np, "touch_timeout", &val) >= 0) {
>>>>>>> + if (val < 0 || val > 255) {
>>>>>>> + dev_err(dev, "touch_timeout must be [0-255]\n");
>>>>>>> + return -EINVAL;
>>>>>>> + }
>>>>>>> + priv->cfg_params.touch_timeout = val;
>>>>>>> + }
>>>>>>> +
>>>>>>> + if (of_property_read_u32(np, "average_data", &val) >= 0) {
>>>>>>> + if (val < 0 || val > 8) {
>>>>>>> + dev_err(dev, "average_data must be [0-8]\n");
>>>>>>> + return -EINVAL;
>>>>>>> + }
>>>>>>> + priv->cfg_params.average_data = val;
>>>>>>> + }
>>>>>>> +
>>>>>>> + if (of_property_read_u32(np, "fifo_threshold", &val) >= 0) {
>>>>>>> + if (val < 0 || val > 31) {
>>>>>>> + dev_err(dev, "fifo_threshold must be [0-31]\n");
>>>>>>> + return -EINVAL;
>>>>>>> + }
>>>>>>> + priv->cfg_params.fifo_threshold = val;
>>>>>>> + }
>>>>>>
>>>>>> I think these are dveice specific and thus should have "brcm," prefix.
>>>>> I'm confused as to why we need the brcm prefix? Other device tree
>>>>> bindings we have for other drivers do not need such prefix.
>>>>
>>>> Properties that are not standard on the system (reg, interrupts,
>>>> clkocks, etc) or subsystem level customarily carry the vendor prefix so
>>>> that they do not clash with newly added global or subsystem properties.
>>>>
>>>>> Is this
>>>>> convention documented somewhere?
>>>>
>>>> Not sure. I glanced through Documentation/devicetree and do not see it
>>>> spelled out. Device tree overlords, what say you?
>>>
>>> Let me know. I haven't seen this before either. I will change the
>>> entries to use dashes though instead of underscores but will wait until
>>> these other issues are decided on before sending out another patch.
>>>
>>>>
>>>>>>
>>>>>>> +
>>>>>>> + priv->ts_rotation = TS_ROTATION_0;
>>>>>>> + if (of_property_read_u32(np, "ts-rotation", &val) >= 0) {
>>>>>>> + priv->ts_rotation = val;
>>>>>>> + dev_dbg(dev, "ts rotation [%d] degrees\n",
>>>>>>> + 90 * priv->ts_rotation);
>>>>>>> + }
>>>>>>
>>>>>> This I am not quite sure about - if we want rotation or swap+invert. You
>>>>>> are CCed on another email (tsc2007) that talks about need of generic
>>>>>> touchscreen transforms in input core/of bindings.
>>>>> Does such generic binding exist today? If not, I would like to go
>>>>> with this implementation and update to the new binding if/when it
>>>>> exists?
>>>>
>>>> Not yet but there several people interested. I think we have enough time
>>>> till 3.20 to hash it out properly.
>>>
>>> I think the rotation is simpler personally. Everyone would understand
>>> rotation refers to how it's oriented but I'm not sure everyone would
>>> immediately know how it is wired. Let me know what is decided and I'll
>>> make any changes required.
>>>
>>> Thanks,
>>> Jon
>>>
>>>
>>>>
>>>> Thanks.
>>>>
>>>
>>
>