From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754480AbbCBTOZ (ORCPT ); Mon, 2 Mar 2015 14:14:25 -0500 Received: from mail-gw3-out.broadcom.com ([216.31.210.64]:25047 "EHLO mail-gw3-out.broadcom.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753590AbbCBTOW (ORCPT ); Mon, 2 Mar 2015 14:14:22 -0500 X-IronPort-AV: E=Sophos;i="5.09,676,1418112000"; d="scan'208";a="58219634" Message-ID: <54F4B64D.8080503@broadcom.com> Date: Mon, 2 Mar 2015 11:13:17 -0800 From: Jonathan Richardson User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: Dmitry Torokhov CC: Grant Likely , Rob Herring , Ray Jui , , , , , , "Joe Perches" Subject: Re: [PATCH v2 1/2] Input: touchscreen-iproc: Add Broadcom iProc touchscreen driver References: <1419027470-7969-1-git-send-email-jonathar@broadcom.com> <1419027470-7969-2-git-send-email-jonathar@broadcom.com> <20150224232954.GB39158@dtor-ws> In-Reply-To: <20150224232954.GB39158@dtor-ws> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Dmitry, Thanks for the review. I'll fix everything as suggested. One comment below. New patchset coming once I can test the latest DT bindings in Hans de Goede's patch. Thanks, Jon On 15-02-24 03:29 PM, Dmitry Torokhov wrote: > 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 >> Reviewed-by: Scott Branden >> Tested-by: Scott Branden >> Signed-off-by: Jonathan Richardson >> --- >> 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 >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#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. I'll change this to one input_sync() call for all events. > >> + } 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? The h/w doesn't seem to. I couldn't see it generate a pen down without fifo data nor fifo data with pen up. But probably better not to assume it will never. One call to input_sync() as suggested above should fix the problem. > >> + } >> + >> + 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. > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jonathan Richardson Subject: Re: [PATCH v2 1/2] Input: touchscreen-iproc: Add Broadcom iProc touchscreen driver Date: Mon, 2 Mar 2015 11:13:17 -0800 Message-ID: <54F4B64D.8080503@broadcom.com> References: <1419027470-7969-1-git-send-email-jonathar@broadcom.com> <1419027470-7969-2-git-send-email-jonathar@broadcom.com> <20150224232954.GB39158@dtor-ws> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20150224232954.GB39158@dtor-ws> Sender: linux-kernel-owner@vger.kernel.org To: Dmitry Torokhov Cc: Grant Likely , Rob Herring , Ray Jui , linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, linux-arm-kernel@lists.infradead.org, bcm-kernel-feedback-list@broadcom.com, devicetree@vger.kernel.org, Joe Perches List-Id: devicetree@vger.kernel.org Hi Dmitry, Thanks for the review. I'll fix everything as suggested. One comment below. New patchset coming once I can test the latest DT bindings in Hans de Goede's patch. Thanks, Jon On 15-02-24 03:29 PM, Dmitry Torokhov wrote: > 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 >> Reviewed-by: Scott Branden >> Tested-by: Scott Branden >> Signed-off-by: Jonathan Richardson >> --- >> 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 >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#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. I'll change this to one input_sync() call for all events. > >> + } 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? The h/w doesn't seem to. I couldn't see it generate a pen down without fifo data nor fifo data with pen up. But probably better not to assume it will never. One call to input_sync() as suggested above should fix the problem. > >> + } >> + >> + 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. > From mboxrd@z Thu Jan 1 00:00:00 1970 From: jonathar@broadcom.com (Jonathan Richardson) Date: Mon, 2 Mar 2015 11:13:17 -0800 Subject: [PATCH v2 1/2] Input: touchscreen-iproc: Add Broadcom iProc touchscreen driver In-Reply-To: <20150224232954.GB39158@dtor-ws> References: <1419027470-7969-1-git-send-email-jonathar@broadcom.com> <1419027470-7969-2-git-send-email-jonathar@broadcom.com> <20150224232954.GB39158@dtor-ws> Message-ID: <54F4B64D.8080503@broadcom.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Dmitry, Thanks for the review. I'll fix everything as suggested. One comment below. New patchset coming once I can test the latest DT bindings in Hans de Goede's patch. Thanks, Jon On 15-02-24 03:29 PM, Dmitry Torokhov wrote: > 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 >> Reviewed-by: Scott Branden >> Tested-by: Scott Branden >> Signed-off-by: Jonathan Richardson >> --- >> 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 >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#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. I'll change this to one input_sync() call for all events. > >> + } 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? The h/w doesn't seem to. I couldn't see it generate a pen down without fifo data nor fifo data with pen up. But probably better not to assume it will never. One call to input_sync() as suggested above should fix the problem. > >> + } >> + >> + 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. >