[PATCH v3 0/5] Support for TSC/ADC MFD driver

[PATCH v3 0/5] Support for TSC/ADC MFD driver

[Patil, Rachna]

This patch set adds a MFD core driver which registers
touchscreen and ADC as its client drivers.
The existing touchscreen has been modified to work as
a MFD client driver and a new ADC driver has been added
in the IIO subsystem.

There are 8 analog input lines, which can be used as:
1. 8 general purpose ADC channels
2. 4 wire TS, with 4 general purpose ADC channels
3. 5 wire TS, with 3 general purpose ADC channels

This patch set has been tested on AM335x EVM.

These set of patches are based on top of Touchscreen patch
set submitted.
Subject: [PATCH 0/4] input: TSC: ti_tscadc: TI Touchscreen driver updates [1]

[1] http://www.spinics.net/lists/linux-input/msg22107.html

Changes in v2:
	Dropped one patch send in the last version after
	receiving internal comments. I have merged the changes
	into existing patches.
	Also added a new patch to support suspend/resume feature.
	Fixed review comments by Matthias Kaehlcke.

Changes in v3:
	Addressed review comments by Jonathan Cameron.
	Improved ADC driver with more readable labels,
	spaces and comments.

Patil, Rachna (5):
  input: TSC: ti_tscadc: Rename the existing touchscreen driver
  MFD: ti_tscadc: Add support for TI's TSC/ADC MFDevice
  input: TSC: ti_tsc: Convert TSC into a MFDevice
  IIO : ADC: tiadc: Add support of TI's ADC driver
  MFD: ti_tscadc: add suspend/resume functionality

 drivers/iio/adc/Kconfig                            |    7 +
 drivers/iio/adc/Makefile                           |    1 +
 drivers/iio/adc/ti_adc.c                           |  255 ++++++++++++++++
 drivers/input/touchscreen/Kconfig                  |    4 +-
 drivers/input/touchscreen/Makefile                 |    2 +-
 .../input/touchscreen/{ti_tscadc.c => ti_tsc.c}    |  310 ++++++--------------
 drivers/mfd/Kconfig                                |    9 +
 drivers/mfd/Makefile                               |    1 +
 drivers/mfd/ti_tscadc.c                            |  251 ++++++++++++++++
 include/linux/input/{ti_tscadc.h => ti_tsc.h}      |    4 +-
 include/linux/mfd/ti_tscadc.h                      |  151 ++++++++++
 include/linux/platform_data/ti_adc.h               |   14 +
 12 files changed, 781 insertions(+), 228 deletions(-)
 create mode 100644 drivers/iio/adc/ti_adc.c
 rename drivers/input/touchscreen/{ti_tscadc.c => ti_tsc.c} (55%)
 create mode 100644 drivers/mfd/ti_tscadc.c
 rename include/linux/input/{ti_tscadc.h => ti_tsc.h} (90%)
 create mode 100644 include/linux/mfd/ti_tscadc.h
 create mode 100644 include/linux/platform_data/ti_adc.h

[PATCH v3 1/5] input: TSC: ti_tscadc: Rename the existing touchscreen driver

[Patil, Rachna]

Make way for addition of MFD driver.
The existing touchsreen driver is a MFD client.
For better readability we rename the file to
indicate its functionality as only touchscreen.

Signed-off-by: Patil, Rachna <rachna@ti.com>
---
Changes in v2:
	Missed changing the name of touchscreen header file
	in the previous version.
	Adding the same.

Changes in v3:
	no changes.

 drivers/input/touchscreen/Kconfig                  |    4 ++--
 drivers/input/touchscreen/Makefile                 |    2 +-
 .../input/touchscreen/{ti_tscadc.c => ti_tsc.c}    |    2 +-
 include/linux/input/{ti_tscadc.h => ti_tsc.h}      |    4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)
 rename drivers/input/touchscreen/{ti_tscadc.c => ti_tsc.c} (99%)
 rename include/linux/input/{ti_tscadc.h => ti_tsc.h} (90%)

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 1ba232c..68ac802 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -529,7 +529,7 @@ config TOUCHSCREEN_TOUCHWIN
 	  To compile this driver as a module, choose M here: the
 	  module will be called touchwin.
 
-config TOUCHSCREEN_TI_TSCADC
+config TOUCHSCREEN_TI_TSC
 	tristate "TI Touchscreen Interface"
 	depends on ARCH_OMAP2PLUS
 	help
@@ -539,7 +539,7 @@ config TOUCHSCREEN_TI_TSCADC
 	  If unsure, say N.
 
 	  To compile this driver as a module, choose M here: the
-	  module will be called ti_tscadc.
+	  module will be called ti_tsc.
 
 config TOUCHSCREEN_ATMEL_TSADCC
 	tristate "Atmel Touchscreen Interface"
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 178eb12..d579427 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -52,7 +52,7 @@ obj-$(CONFIG_TOUCHSCREEN_PIXCIR)	+= pixcir_i2c_ts.o
 obj-$(CONFIG_TOUCHSCREEN_S3C2410)	+= s3c2410_ts.o
 obj-$(CONFIG_TOUCHSCREEN_ST1232)	+= st1232.o
 obj-$(CONFIG_TOUCHSCREEN_STMPE)		+= stmpe-ts.o
-obj-$(CONFIG_TOUCHSCREEN_TI_TSCADC)	+= ti_tscadc.o
+obj-$(CONFIG_TOUCHSCREEN_TI_TSC)	+= ti_tsc.o
 obj-$(CONFIG_TOUCHSCREEN_TNETV107X)	+= tnetv107x-ts.o
 obj-$(CONFIG_TOUCHSCREEN_TOUCHIT213)	+= touchit213.o
 obj-$(CONFIG_TOUCHSCREEN_TOUCHRIGHT)	+= touchright.o
diff --git a/drivers/input/touchscreen/ti_tscadc.c b/drivers/input/touchscreen/ti_tsc.c
similarity index 99%
rename from drivers/input/touchscreen/ti_tscadc.c
rename to drivers/input/touchscreen/ti_tsc.c
index ec0a442..8e94c00 100644
--- a/drivers/input/touchscreen/ti_tscadc.c
+++ b/drivers/input/touchscreen/ti_tsc.c
@@ -24,7 +24,7 @@
 #include <linux/clk.h>
 #include <linux/platform_device.h>
 #include <linux/io.h>
-#include <linux/input/ti_tscadc.h>
+#include <linux/input/ti_tsc.h>
 #include <linux/delay.h>
 
 #define REG_RAWIRQSTATUS	0x024
diff --git a/include/linux/input/ti_tscadc.h b/include/linux/input/ti_tsc.h
similarity index 90%
rename from include/linux/input/ti_tscadc.h
rename to include/linux/input/ti_tsc.h
index ad442a3..5da27d6 100644
--- a/include/linux/input/ti_tscadc.h
+++ b/include/linux/input/ti_tsc.h
@@ -1,5 +1,5 @@
-#ifndef __LINUX_TI_TSCADC_H
-#define __LINUX_TI_TSCADC_H
+#ifndef __LINUX_TI_TSC_H
+#define __LINUX_TI_TSC_H
 
 /**
  * struct tsc_data	Touchscreen wire configuration
-- 
1.7.0.4

[PATCH v3 2/5] MFD: ti_tscadc: Add support for TI's TSC/ADC MFDevice

[Patil, Rachna]

Add the mfd core driver which supports touchscreen
and ADC.
With this patch we are only adding infrastructure to
support the MFD clients.

Signed-off-by: Patil, Rachna <rachna@ti.com>
---
Changes in v2:
	Merged "[PATCH 5/5] MFD: ti_tscadc: Add check on number of i/p channels",
	patch submitted in previous version into this file.

Changes in v3:
	No changes

 drivers/mfd/Kconfig           |    9 ++
 drivers/mfd/Makefile          |    1 +
 drivers/mfd/ti_tscadc.c       |  193 +++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/ti_tscadc.h |  133 ++++++++++++++++++++++++++++
 4 files changed, 336 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mfd/ti_tscadc.c
 create mode 100644 include/linux/mfd/ti_tscadc.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index d1facef..81eb815 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -94,6 +94,15 @@ config MFD_TI_SSP
 	  To compile this driver as a module, choose M here: the
 	  module will be called ti-ssp.
 
+config MFD_TI_TSCADC
+	tristate "TI ADC / Touch Screen chip support"
+	depends on ARCH_OMAP2PLUS
+	help
+	  If you say yes here you get support for Texas Instruments series
+	  of Touch Screen /ADC chips.
+	  To compile this driver as a module, choose M here: the
+	  module will be called ti_tscadc.
+
 config HTC_EGPIO
 	bool "HTC EGPIO support"
 	depends on GENERIC_HARDIRQS && GPIOLIB && ARM
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 79dd22d..1537f74 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_HTC_I2CPLD)	+= htc-i2cpld.o
 obj-$(CONFIG_MFD_DAVINCI_VOICECODEC)	+= davinci_voicecodec.o
 obj-$(CONFIG_MFD_DM355EVM_MSP)	+= dm355evm_msp.o
 obj-$(CONFIG_MFD_TI_SSP)	+= ti-ssp.o
+obj-$(CONFIG_MFD_TI_TSCADC)	+= ti_tscadc.o
 
 obj-$(CONFIG_MFD_STA2X11)	+= sta2x11-mfd.o
 obj-$(CONFIG_MFD_STMPE)		+= stmpe.o
diff --git a/drivers/mfd/ti_tscadc.c b/drivers/mfd/ti_tscadc.c
new file mode 100644
index 0000000..ac4bf7b
--- /dev/null
+++ b/drivers/mfd/ti_tscadc.c
@@ -0,0 +1,193 @@
+/*
+ * TI Touch Screen / ADC MFD driver
+ *
+ * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * 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/slab.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/mfd/core.h>
+#include <linux/pm_runtime.h>
+#include <linux/mfd/ti_tscadc.h>
+
+static unsigned int tscadc_readl(struct ti_tscadc_dev *tsadc, unsigned int reg)
+{
+	return readl(tsadc->tscadc_base + reg);
+}
+
+static void tscadc_writel(struct ti_tscadc_dev *tsadc, unsigned int reg,
+					unsigned int val)
+{
+	writel(val, tsadc->tscadc_base + reg);
+}
+
+static void tscadc_idle_config(struct ti_tscadc_dev *config)
+{
+	unsigned int idleconfig;
+
+	idleconfig = STEPCONFIG_YNN | STEPCONFIG_INM_ADCREFM |
+			STEPCONFIG_INP_ADCREFM | STEPCONFIG_YPN;
+
+	tscadc_writel(config, REG_IDLECONFIG, idleconfig);
+}
+
+static	int __devinit ti_tscadc_probe(struct platform_device *pdev)
+{
+	struct ti_tscadc_dev	*tscadc;
+	struct resource		*res;
+	struct clk		*clk;
+	struct mfd_tscadc_board	*pdata = pdev->dev.platform_data;
+	int			irq;
+	int			err, ctrl;
+	int			clk_value, clock_rate;
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "Could not find platform data\n");
+		return -EINVAL;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "no memory resource defined.\n");
+		return -EINVAL;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "no irq ID is specified.\n");
+		return -EINVAL;
+	}
+
+	/* Allocate memory for device */
+	tscadc = kzalloc(sizeof(struct ti_tscadc_dev), GFP_KERNEL);
+	if (!tscadc) {
+		dev_err(&pdev->dev, "failed to allocate memory.\n");
+		return -ENOMEM;
+	}
+	tscadc->dev = &pdev->dev;
+	tscadc->irq = irq;
+
+	res = request_mem_region(res->start, resource_size(res), pdev->name);
+	if (!res) {
+		dev_err(&pdev->dev, "failed to reserve registers.\n");
+		err = -EBUSY;
+		goto err_free_mem;
+	}
+
+	tscadc->tscadc_base = ioremap(res->start, resource_size(res));
+	if (!tscadc->tscadc_base) {
+		dev_err(&pdev->dev, "failed to map registers.\n");
+		err = -ENOMEM;
+		goto err_release_mem_region;
+	}
+
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_get_sync(&pdev->dev);
+
+	/*
+	 * The TSC_ADC_Subsystem has 2 clock domains
+	 * OCP_CLK and ADC_CLK.
+	 * The ADC clock is expected to run at target of 3MHz,
+	 * and expected to capture 12-bit data at a rate of 200 KSPS.
+	 * The TSC_ADC_SS controller design assumes the OCP clock is
+	 * at least 6x faster than the ADC clock.
+	 */
+	clk = clk_get(&pdev->dev, "adc_tsc_fck");
+	if (IS_ERR(clk)) {
+		dev_err(&pdev->dev, "failed to get TSC fck\n");
+		err = PTR_ERR(clk);
+		goto err_disable_clk;
+	}
+	clock_rate = clk_get_rate(clk);
+	clk_put(clk);
+	clk_value = clock_rate / ADC_CLK;
+	if (clk_value < MAX_CLK_DIV) {
+		dev_err(&pdev->dev, "clock input less than min clock requirement\n");
+		err = -EINVAL;
+		goto err_disable_clk;
+	}
+	/* TSCADC_CLKDIV needs to be configured to the value minus 1 */
+	clk_value = clk_value - 1;
+	tscadc_writel(tscadc, REG_CLKDIV, clk_value);
+
+	/* Set the control register bits */
+	ctrl = CNTRLREG_STEPCONFIGWRT |
+			CNTRLREG_TSCENB |
+			CNTRLREG_STEPID |
+			CNTRLREG_4WIRE;
+	tscadc_writel(tscadc, REG_CTRL, ctrl);
+
+	/* Set register bits for Idle Config Mode */
+	tscadc_idle_config(tscadc);
+
+	/* Enable the TSC module enable bit */
+	ctrl = tscadc_readl(tscadc, REG_CTRL);
+	ctrl |= CNTRLREG_TSCSSENB;
+	tscadc_writel(tscadc, REG_CTRL, ctrl);
+
+	err = mfd_add_devices(&pdev->dev, pdev->id, tscadc->cells,
+			TSCADC_CELLS, NULL, 0);
+	if (err < 0)
+		goto err_disable_clk;
+
+	platform_set_drvdata(pdev, tscadc);
+	return 0;
+
+err_disable_clk:
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+	iounmap(tscadc->tscadc_base);
+err_release_mem_region:
+	release_mem_region(res->start, resource_size(res));
+	mfd_remove_devices(tscadc->dev);
+err_free_mem:
+	kfree(tscadc);
+	return err;
+}
+
+static int __devexit ti_tscadc_remove(struct platform_device *pdev)
+{
+	struct ti_tscadc_dev	*tscadc = platform_get_drvdata(pdev);
+	struct resource		*res;
+
+	tscadc_writel(tscadc, REG_SE, 0x00);
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	iounmap(tscadc->tscadc_base);
+	release_mem_region(res->start, resource_size(res));
+
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
+	mfd_remove_devices(tscadc->dev);
+	kfree(tscadc);
+	return 0;
+}
+
+static struct platform_driver ti_tscadc_driver = {
+	.driver = {
+		.name   = "ti_tscadc",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= ti_tscadc_probe,
+	.remove	= __devexit_p(ti_tscadc_remove),
+
+};
+
+module_platform_driver(ti_tscadc_driver);
+
+MODULE_DESCRIPTION("TI touchscreen / ADC MFD controller driver");
+MODULE_AUTHOR("Rachna Patil <rachna@ti.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/ti_tscadc.h b/include/linux/mfd/ti_tscadc.h
new file mode 100644
index 0000000..5bc2d61
--- /dev/null
+++ b/include/linux/mfd/ti_tscadc.h
@@ -0,0 +1,133 @@
+#ifndef __LINUX_TI_TSCADC_MFD_H
+#define __LINUX_TI_TSCADC_MFD_H
+
+/*
+ * TI Touch Screen / ADC MFD driver
+ *
+ * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * 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/mfd/core.h>
+
+#define REG_RAWIRQSTATUS	0x024
+#define REG_IRQSTATUS		0x028
+#define REG_IRQENABLE		0x02C
+#define REG_IRQCLR		0x030
+#define REG_IRQWAKEUP		0x034
+#define REG_CTRL		0x040
+#define REG_ADCFSM		0x044
+#define REG_CLKDIV		0x04C
+#define REG_SE			0x054
+#define REG_IDLECONFIG		0x058
+#define REG_CHARGECONFIG	0x05C
+#define REG_CHARGEDELAY		0x060
+#define REG_STEPCONFIG(n)	(0x64 + ((n - 1) * 8))
+#define REG_STEPDELAY(n)	(0x68 + ((n - 1) * 8))
+#define REG_FIFO0CNT		0xE4
+#define REG_FIFO0THR		0xE8
+#define REG_FIFO1CNT		0xF0
+#define REG_FIFO1THR		0xF4
+#define REG_FIFO0		0x100
+#define REG_FIFO1		0x200
+
+/*	Register Bitfields	*/
+/* Step Enable */
+#define STEPENB_MASK		(0x1FFFF << 0)
+#define STEPENB(val)		((val) << 0)
+#define STPENB_STEPENB		STEPENB(0x1FFFF)
+
+/* IRQ enable */
+#define IRQENB_HW_PEN		BIT(0)
+#define IRQENB_FIFO0THRES	BIT(2)
+#define IRQENB_FIFO1THRES	BIT(5)
+#define IRQENB_PENUP		BIT(9)
+
+/* Step Configuration */
+#define STEPCONFIG_MODE_MASK	(3 << 0)
+#define STEPCONFIG_MODE(val)	((val) << 0)
+#define STEPCONFIG_MODE_HWSYNC	STEPCONFIG_MODE(2)
+#define STEPCONFIG_AVG_MASK	(7 << 2)
+#define STEPCONFIG_AVG(val)	((val) << 2)
+#define STEPCONFIG_AVG_16	STEPCONFIG_AVG(4)
+#define STEPCONFIG_XPP		BIT(5)
+#define STEPCONFIG_XNN		BIT(6)
+#define STEPCONFIG_YPP		BIT(7)
+#define STEPCONFIG_YNN		BIT(8)
+#define STEPCONFIG_XNP		BIT(9)
+#define STEPCONFIG_YPN		BIT(10)
+#define STEPCONFIG_INM_MASK	(0xF << 15)
+#define STEPCONFIG_INM(val)	((val) << 15)
+#define STEPCONFIG_INM_ADCREFM	STEPCONFIG_INM(8)
+#define STEPCONFIG_INP_MASK	(0xF << 19)
+#define STEPCONFIG_INP(val)	((val) << 19)
+#define STEPCONFIG_INP_AN2	STEPCONFIG_INP(2)
+#define STEPCONFIG_INP_AN3	STEPCONFIG_INP(3)
+#define STEPCONFIG_INP_AN4	STEPCONFIG_INP(4)
+#define STEPCONFIG_INP_ADCREFM	STEPCONFIG_INP(8)
+#define STEPCONFIG_FIFO1	BIT(26)
+
+/* Delay register */
+#define STEPDELAY_OPEN_MASK	(0x3FFFF << 0)
+#define STEPDELAY_OPEN(val)	((val) << 0)
+#define STEPCONFIG_OPENDLY	STEPDELAY_OPEN(0x098)
+#define STEPDELAY_SAMPLE_MASK	(0xFF << 24)
+#define STEPDELAY_SAMPLE(val)	((val) << 24)
+#define STEPCONFIG_SAMPLEDLY	STEPDELAY_SAMPLE(0)
+
+/* Charge Config */
+#define STEPCHARGE_RFP_MASK	(7 << 12)
+#define STEPCHARGE_RFP(val)	((val) << 12)
+#define STEPCHARGE_RFP_XPUL	STEPCHARGE_RFP(1)
+#define STEPCHARGE_INM_MASK	(0xF << 15)
+#define STEPCHARGE_INM(val)	((val) << 15)
+#define STEPCHARGE_INM_AN1	STEPCHARGE_INM(1)
+#define STEPCHARGE_INP_MASK	(0xF << 19)
+#define STEPCHARGE_INP(val)	((val) << 19)
+#define STEPCHARGE_INP_AN1	STEPCHARGE_INP(1)
+#define STEPCHARGE_RFM_MASK	(3 << 23)
+#define STEPCHARGE_RFM(val)	((val) << 23)
+#define STEPCHARGE_RFM_XNUR	STEPCHARGE_RFM(1)
+
+/* Charge delay */
+#define CHARGEDLY_OPEN_MASK	(0x3FFFF << 0)
+#define CHARGEDLY_OPEN(val)	((val) << 0)
+#define CHARGEDLY_OPENDLY	CHARGEDLY_OPEN(1)
+
+/* Control register */
+#define CNTRLREG_TSCSSENB	BIT(0)
+#define CNTRLREG_STEPID		BIT(1)
+#define CNTRLREG_STEPCONFIGWRT	BIT(2)
+#define CNTRLREG_POWERDOWN	BIT(4)
+#define CNTRLREG_AFE_CTRL_MASK	(3 << 5)
+#define CNTRLREG_AFE_CTRL(val)	((val) << 5)
+#define CNTRLREG_4WIRE		CNTRLREG_AFE_CTRL(1)
+#define CNTRLREG_5WIRE		CNTRLREG_AFE_CTRL(2)
+#define CNTRLREG_8WIRE		CNTRLREG_AFE_CTRL(3)
+#define CNTRLREG_TSCENB		BIT(7)
+
+#define ADC_CLK			3000000
+#define	MAX_CLK_DIV		7
+
+#define TSCADC_CELLS		0
+
+struct mfd_tscadc_board {
+	struct tsc_data *tsc_init;
+};
+
+struct ti_tscadc_dev {
+	struct device *dev;
+	void __iomem *tscadc_base;
+	int irq;
+	struct mfd_cell cells[TSCADC_CELLS];
+};
+
+#endif
-- 
1.7.0.4

[PATCH v3 3/5] input: TSC: ti_tsc: Convert TSC into a MFDevice

[Patil, Rachna]

This patch converts touchscreen into a MFD client.
All the register definitions, clock initialization,
etc has been moved to MFD core driver.

Signed-off-by: Patil, Rachna <rachna@ti.com>
---
Changes in v2:
	No changes

Changes in v3:
	No changes

 drivers/input/touchscreen/ti_tsc.c |  277 +++++++-----------------------------
 drivers/mfd/ti_tscadc.c            |   11 ++
 include/linux/mfd/ti_tscadc.h      |   10 ++-
 3 files changed, 74 insertions(+), 224 deletions(-)

diff --git a/drivers/input/touchscreen/ti_tsc.c b/drivers/input/touchscreen/ti_tsc.c
index 8e94c00..ca8ce73 100644
--- a/drivers/input/touchscreen/ti_tsc.c
+++ b/drivers/input/touchscreen/ti_tsc.c
@@ -26,123 +26,31 @@
 #include <linux/io.h>
 #include <linux/input/ti_tsc.h>
 #include <linux/delay.h>
-
-#define REG_RAWIRQSTATUS	0x024
-#define REG_IRQSTATUS		0x028
-#define REG_IRQENABLE		0x02C
-#define REG_IRQWAKEUP		0x034
-#define REG_CTRL		0x040
-#define REG_ADCFSM		0x044
-#define REG_CLKDIV		0x04C
-#define REG_SE			0x054
-#define REG_IDLECONFIG		0x058
-#define REG_CHARGECONFIG	0x05C
-#define REG_CHARGEDELAY		0x060
-#define REG_STEPCONFIG(n)	(0x64 + ((n - 1) * 8))
-#define REG_STEPDELAY(n)	(0x68 + ((n - 1) * 8))
-#define REG_FIFO0CNT		0xE4
-#define REG_FIFO0THR		0xE8
-#define REG_FIFO1THR		0xF4
-#define REG_FIFO0		0x100
-#define REG_FIFO1		0x200
-
-/*	Register Bitfields	*/
-#define IRQWKUP_ENB		BIT(0)
-
-/* Step Enable */
-#define STEPENB_MASK		(0x1FFFF << 0)
-#define STEPENB(val)		(val << 0)
-#define STPENB_STEPENB		STEPENB(0x7FFF)
-
-/* IRQ enable */
-#define IRQENB_FIFO0THRES	BIT(2)
-#define IRQENB_FIFO1THRES	BIT(5)
-#define IRQENB_PENUP		BIT(9)
-
-/* Step Configuration */
-#define STEPCONFIG_MODE_MASK	(3 << 0)
-#define STEPCONFIG_MODE(val)	(val << 0)
-#define STEPCONFIG_MODE_HWSYNC	STEPCONFIG_MODE(2)
-#define STEPCONFIG_AVG_MASK	(7 << 2)
-#define STEPCONFIG_AVG(val)	(val << 2)
-#define STEPCONFIG_AVG_16	STEPCONFIG_AVG(4)
-#define STEPCONFIG_XPP		BIT(5)
-#define STEPCONFIG_XNN		BIT(6)
-#define STEPCONFIG_YPP		BIT(7)
-#define STEPCONFIG_YNN		BIT(8)
-#define STEPCONFIG_XNP		BIT(9)
-#define STEPCONFIG_YPN		BIT(10)
-#define STEPCONFIG_INM_MASK	(0xF << 15)
-#define STEPCONFIG_INM(val)	(val << 15)
-#define STEPCONFIG_INM_ADCREFM	STEPCONFIG_INM(8)
-#define STEPCONFIG_INP_MASK	(0xF << 19)
-#define STEPCONFIG_INP(val)	(val << 19)
-#define STEPCONFIG_INP_AN2	STEPCONFIG_INP(2)
-#define STEPCONFIG_INP_AN3	STEPCONFIG_INP(3)
-#define STEPCONFIG_INP_AN4	STEPCONFIG_INP(4)
-#define STEPCONFIG_INP_ADCREFM	STEPCONFIG_INP(8)
-#define STEPCONFIG_FIFO1	BIT(26)
-
-/* Delay register */
-#define STEPDELAY_OPEN_MASK	(0x3FFFF << 0)
-#define STEPDELAY_OPEN(val)	(val << 0)
-#define STEPCONFIG_OPENDLY	STEPDELAY_OPEN(0x098)
-
-/* Charge Config */
-#define STEPCHARGE_RFP_MASK	(7 << 12)
-#define STEPCHARGE_RFP(val)	(val << 12)
-#define STEPCHARGE_RFP_XPUL	STEPCHARGE_RFP(1)
-#define STEPCHARGE_INM_MASK	(0xF << 15)
-#define STEPCHARGE_INM(val)	(val << 15)
-#define STEPCHARGE_INM_AN1	STEPCHARGE_INM(1)
-#define STEPCHARGE_INP_MASK	(0xF << 19)
-#define STEPCHARGE_INP(val)	(val << 19)
-#define STEPCHARGE_INP_AN1	STEPCHARGE_INP(1)
-#define STEPCHARGE_RFM_MASK	(3 << 23)
-#define STEPCHARGE_RFM(val)	(val << 23)
-#define STEPCHARGE_RFM_XNUR	STEPCHARGE_RFM(1)
-
-/* Charge delay */
-#define CHARGEDLY_OPEN_MASK	(0x3FFFF << 0)
-#define CHARGEDLY_OPEN(val)	(val << 0)
-#define CHARGEDLY_OPENDLY	CHARGEDLY_OPEN(1)
-
-/* Control register */
-#define CNTRLREG_TSCSSENB	BIT(0)
-#define CNTRLREG_STEPID		BIT(1)
-#define CNTRLREG_STEPCONFIGWRT	BIT(2)
-#define CNTRLREG_AFE_CTRL_MASK	(3 << 5)
-#define CNTRLREG_AFE_CTRL(val)	(val << 5)
-#define CNTRLREG_4WIRE		CNTRLREG_AFE_CTRL(1)
-#define CNTRLREG_5WIRE		CNTRLREG_AFE_CTRL(2)
-#define CNTRLREG_8WIRE		CNTRLREG_AFE_CTRL(3)
-#define CNTRLREG_TSCENB		BIT(7)
+#include <linux/mfd/ti_tscadc.h>
 
 #define ADCFSM_STEPID		0x10
 #define SEQ_SETTLE		275
-#define ADC_CLK			3000000
 #define MAX_12BIT		((1 << 12) - 1)
 
 struct tscadc {
 	struct input_dev	*input;
-	struct clk		*tsc_ick;
-	void __iomem		*tsc_base;
 	unsigned int		irq;
 	unsigned int		wires;
 	unsigned int		x_plate_resistance;
 	bool			pen_down;
 	int			steps_to_configure;
+	struct ti_tscadc_dev	*mfd_tscadc;
 };
 
 static unsigned int tscadc_readl(struct tscadc *ts, unsigned int reg)
 {
-	return readl(ts->tsc_base + reg);
+	return readl(ts->mfd_tscadc->tscadc_base + reg);
 }
 
 static void tscadc_writel(struct tscadc *tsc, unsigned int reg,
 					unsigned int val)
 {
-	writel(val, tsc->tsc_base + reg);
+	writel(val, tsc->mfd_tscadc->tscadc_base + reg);
 }
 
 static void tscadc_step_config(struct tscadc *ts_dev)
@@ -219,17 +127,7 @@ static void tscadc_step_config(struct tscadc *ts_dev)
 	tscadc_writel(ts_dev, REG_STEPDELAY(total_steps + 2),
 			STEPCONFIG_OPENDLY);
 
-	tscadc_writel(ts_dev, REG_SE, STPENB_STEPENB);
-}
-
-static void tscadc_idle_config(struct tscadc *ts_config)
-{
-	unsigned int idleconfig;
-
-	idleconfig = STEPCONFIG_YNN |
-			STEPCONFIG_INM_ADCREFM |
-			STEPCONFIG_YPN | STEPCONFIG_INP_ADCREFM;
-	tscadc_writel(ts_config, REG_IDLECONFIG, idleconfig);
+	tscadc_writel(ts_dev, REG_SE, STPENB_STEPENB_TC);
 }
 
 static void tscadc_read_coordinates(struct tscadc *ts_dev,
@@ -239,7 +137,7 @@ static void tscadc_read_coordinates(struct tscadc *ts_dev,
 	unsigned int prev_val_x = ~0, prev_val_y = ~0;
 	unsigned int prev_diff_x = ~0, prev_diff_y = ~0;
 	unsigned int read, diff;
-	unsigned int i;
+	unsigned int i, channel;
 
 	/*
 	 * Delta filter is used to remove large variations in sampled
@@ -250,21 +148,32 @@ static void tscadc_read_coordinates(struct tscadc *ts_dev,
 	 * if true the value is reported to the sub system.
 	 */
 	for (i = 0; i < fifocount - 1; i++) {
-		read = tscadc_readl(ts_dev, REG_FIFO0) & 0xfff;
-		diff = abs(read - prev_val_x);
-		if (diff < prev_diff_x) {
-			prev_diff_x = diff;
-			*x = read;
+		read = tscadc_readl(ts_dev, REG_FIFO0);
+		channel = read & 0xf0000;
+		channel = channel >> 0x10;
+		if ((channel >= 0) && (channel < ts_dev->steps_to_configure)) {
+			read &= 0xfff;
+			diff = abs(read - prev_val_x);
+			if (diff < prev_diff_x) {
+				prev_diff_x = diff;
+				*x = read;
+			}
+			prev_val_x = read;
 		}
-		prev_val_x = read;
 
-		read = tscadc_readl(ts_dev, REG_FIFO1) & 0xfff;
-		diff = abs(read - prev_val_y);
-		if (diff < prev_diff_y) {
-			prev_diff_y = diff;
-			*y = read;
-		}
+		read = tscadc_readl(ts_dev, REG_FIFO1);
+		channel = read & 0xf0000;
+		channel = channel >> 0x10;
+		if ((channel >= ts_dev->steps_to_configure) &&
+			(channel < (2 * ts_dev->steps_to_configure - 1))) {
+			read &= 0xfff;
+			diff = abs(read - prev_val_y);
+			if (diff < prev_diff_y) {
+				prev_diff_y = diff;
+				*y = read;
+			}
 		prev_val_y = read;
+		}
 	}
 }
 
@@ -276,6 +185,8 @@ static irqreturn_t tscadc_irq(int irq, void *dev)
 	unsigned int x = 0, y = 0;
 	unsigned int z1, z2, z;
 	unsigned int fsm;
+	unsigned int fifo1count, fifo0count;
+	int i;
 
 	status = tscadc_readl(ts_dev, REG_IRQSTATUS);
 	if (status & IRQENB_FIFO0THRES) {
@@ -284,6 +195,14 @@ static irqreturn_t tscadc_irq(int irq, void *dev)
 		z1 = tscadc_readl(ts_dev, REG_FIFO0) & 0xfff;
 		z2 = tscadc_readl(ts_dev, REG_FIFO1) & 0xfff;
 
+		fifo1count = tscadc_readl(ts_dev, REG_FIFO1CNT);
+		for (i = 0; i < fifo1count; i++)
+			tscadc_readl(ts_dev, REG_FIFO1);
+
+		fifo0count = tscadc_readl(ts_dev, REG_FIFO0CNT);
+		for (i = 0; i < fifo0count; i++)
+			tscadc_readl(ts_dev, REG_FIFO0);
+
 		if (ts_dev->pen_down && z1 != 0 && z2 != 0) {
 			/*
 			 * Calculate pressure using formula
@@ -340,28 +259,15 @@ static irqreturn_t tscadc_irq(int irq, void *dev)
 
 static int __devinit tscadc_probe(struct platform_device *pdev)
 {
-	const struct tsc_data *pdata = pdev->dev.platform_data;
-	struct resource *res;
 	struct tscadc *ts_dev;
 	struct input_dev *input_dev;
-	struct clk *clk;
 	int err;
-	int clk_value, ctrl, irq;
+	struct ti_tscadc_dev *tscadc_dev = pdev->dev.platform_data;
+	struct mfd_tscadc_board	*pdata;
 
+	pdata = (struct mfd_tscadc_board *)tscadc_dev->dev->platform_data;
 	if (!pdata) {
-		dev_err(&pdev->dev, "missing platform data.\n");
-		return -EINVAL;
-	}
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		dev_err(&pdev->dev, "no memory resource defined.\n");
-		return -EINVAL;
-	}
-
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
-		dev_err(&pdev->dev, "no irq ID is specified.\n");
+		dev_err(tscadc_dev->dev, "Could not find platform data\n");
 		return -EINVAL;
 	}
 
@@ -374,85 +280,26 @@ static int __devinit tscadc_probe(struct platform_device *pdev)
 		goto err_free_mem;
 	}
 
+	tscadc_dev->tsc = ts_dev;
+	ts_dev->mfd_tscadc = tscadc_dev;
 	ts_dev->input = input_dev;
-	ts_dev->irq = irq;
-	ts_dev->wires = pdata->wires;
-	ts_dev->x_plate_resistance = pdata->x_plate_resistance;
-	ts_dev->steps_to_configure = pdata->steps_to_configure;
-
-	res = request_mem_region(res->start, resource_size(res), pdev->name);
-	if (!res) {
-		dev_err(&pdev->dev, "failed to reserve registers.\n");
-		err = -EBUSY;
-		goto err_free_mem;
-	}
-
-	ts_dev->tsc_base = ioremap(res->start, resource_size(res));
-	if (!ts_dev->tsc_base) {
-		dev_err(&pdev->dev, "failed to map registers.\n");
-		err = -ENOMEM;
-		goto err_release_mem_region;
-	}
+	ts_dev->irq = tscadc_dev->irq;
+	ts_dev->wires = pdata->tsc_init->wires;
+	ts_dev->x_plate_resistance = pdata->tsc_init->x_plate_resistance;
+	ts_dev->steps_to_configure = pdata->tsc_init->steps_to_configure;
 
 	err = request_irq(ts_dev->irq, tscadc_irq,
 			  0, pdev->dev.driver->name, ts_dev);
 	if (err) {
 		dev_err(&pdev->dev, "failed to allocate irq.\n");
-		goto err_unmap_regs;
-	}
-
-	ts_dev->tsc_ick = clk_get(&pdev->dev, "adc_tsc_ick");
-	if (IS_ERR(ts_dev->tsc_ick)) {
-		dev_err(&pdev->dev, "failed to get TSC ick\n");
-		goto err_free_irq;
-	}
-	clk_enable(ts_dev->tsc_ick);
-
-	clk = clk_get(&pdev->dev, "adc_tsc_fck");
-	if (IS_ERR(clk)) {
-		dev_err(&pdev->dev, "failed to get TSC fck\n");
-		err = PTR_ERR(clk);
-		goto err_disable_clk;
-	}
-
-	clk_value = clk_get_rate(clk) / ADC_CLK;
-	clk_put(clk);
-
-	if (clk_value < 7) {
-		dev_err(&pdev->dev, "clock input less than min clock requirement\n");
-		goto err_disable_clk;
-	}
-	/* CLKDIV needs to be configured to the value minus 1 */
-	tscadc_writel(ts_dev, REG_CLKDIV, clk_value - 1);
-
-	 /* Enable wake-up of the SoC using touchscreen */
-	tscadc_writel(ts_dev, REG_IRQWAKEUP, IRQWKUP_ENB);
-
-	ctrl = CNTRLREG_STEPCONFIGWRT |
-			CNTRLREG_TSCENB |
-			CNTRLREG_STEPID;
-	switch (ts_dev->wires) {
-	case 4:
-		ctrl |= CNTRLREG_4WIRE;
-		break;
-	case 5:
-		ctrl |= CNTRLREG_5WIRE;
-		break;
-	case 8:
-		ctrl |= CNTRLREG_8WIRE;
-		break;
+		goto err_free_mem;
 	}
-	tscadc_writel(ts_dev, REG_CTRL, ctrl);
 
-	tscadc_idle_config(ts_dev);
 	tscadc_writel(ts_dev, REG_IRQENABLE, IRQENB_FIFO0THRES);
 	tscadc_step_config(ts_dev);
 	tscadc_writel(ts_dev, REG_FIFO0THR, ts_dev->steps_to_configure);
 
-	ctrl |= CNTRLREG_TSCSSENB;
-	tscadc_writel(ts_dev, REG_CTRL, ctrl);
-
-	input_dev->name = "ti-tsc-adc";
+	input_dev->name = "ti-tsc";
 	input_dev->dev.parent = &pdev->dev;
 
 	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
@@ -465,20 +312,13 @@ static int __devinit tscadc_probe(struct platform_device *pdev)
 	/* register to the input system */
 	err = input_register_device(input_dev);
 	if (err)
-		goto err_disable_clk;
+		goto err_free_irq;
 
 	platform_set_drvdata(pdev, ts_dev);
 	return 0;
 
-err_disable_clk:
-	clk_disable(ts_dev->tsc_ick);
-	clk_put(ts_dev->tsc_ick);
 err_free_irq:
 	free_irq(ts_dev->irq, ts_dev);
-err_unmap_regs:
-	iounmap(ts_dev->tsc_base);
-err_release_mem_region:
-	release_mem_region(res->start, resource_size(res));
 err_free_mem:
 	input_free_device(input_dev);
 	kfree(ts_dev);
@@ -487,22 +327,13 @@ err_free_mem:
 
 static int __devexit tscadc_remove(struct platform_device *pdev)
 {
-	struct tscadc *ts_dev = platform_get_drvdata(pdev);
-	struct resource *res;
+	struct ti_tscadc_dev *tscadc_dev = pdev->dev.platform_data;
+	struct tscadc *ts_dev = tscadc_dev->tsc;
 
 	free_irq(ts_dev->irq, ts_dev);
-
 	input_unregister_device(ts_dev->input);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	iounmap(ts_dev->tsc_base);
-	release_mem_region(res->start, resource_size(res));
-
-	clk_disable(ts_dev->tsc_ick);
-	clk_put(ts_dev->tsc_ick);
-
 	kfree(ts_dev);
-
 	platform_set_drvdata(pdev, NULL);
 	return 0;
 }
diff --git a/drivers/mfd/ti_tscadc.c b/drivers/mfd/ti_tscadc.c
index ac4bf7b..f26e53b 100644
--- a/drivers/mfd/ti_tscadc.c
+++ b/drivers/mfd/ti_tscadc.c
@@ -22,6 +22,7 @@
 #include <linux/mfd/core.h>
 #include <linux/pm_runtime.h>
 #include <linux/mfd/ti_tscadc.h>
+#include <linux/input/ti_tsc.h>
 
 static unsigned int tscadc_readl(struct ti_tscadc_dev *tsadc, unsigned int reg)
 {
@@ -50,15 +51,19 @@ static	int __devinit ti_tscadc_probe(struct platform_device *pdev)
 	struct resource		*res;
 	struct clk		*clk;
 	struct mfd_tscadc_board	*pdata = pdev->dev.platform_data;
+	struct mfd_cell		*cell;
 	int			irq;
 	int			err, ctrl;
 	int			clk_value, clock_rate;
+	int			tsc_wires;
 
 	if (!pdata) {
 		dev_err(&pdev->dev, "Could not find platform data\n");
 		return -EINVAL;
 	}
 
+	tsc_wires = pdata->tsc_init->wires;
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(&pdev->dev, "no memory resource defined.\n");
@@ -138,6 +143,12 @@ static	int __devinit ti_tscadc_probe(struct platform_device *pdev)
 	ctrl |= CNTRLREG_TSCSSENB;
 	tscadc_writel(tscadc, REG_CTRL, ctrl);
 
+	/* TSC Cell */
+	cell = &tscadc->cells[TSC_CELL];
+	cell->name = "tsc";
+	cell->platform_data = tscadc;
+	cell->pdata_size = sizeof(*tscadc);
+
 	err = mfd_add_devices(&pdev->dev, pdev->id, tscadc->cells,
 			TSCADC_CELLS, NULL, 0);
 	if (err < 0)
diff --git a/include/linux/mfd/ti_tscadc.h b/include/linux/mfd/ti_tscadc.h
index 5bc2d61..d28a35c 100644
--- a/include/linux/mfd/ti_tscadc.h
+++ b/include/linux/mfd/ti_tscadc.h
@@ -44,6 +44,7 @@
 #define STEPENB_MASK		(0x1FFFF << 0)
 #define STEPENB(val)		((val) << 0)
 #define STPENB_STEPENB		STEPENB(0x1FFFF)
+#define STPENB_STEPENB_TC	STEPENB(0x1FFF)
 
 /* IRQ enable */
 #define IRQENB_HW_PEN		BIT(0)
@@ -117,7 +118,11 @@
 #define ADC_CLK			3000000
 #define	MAX_CLK_DIV		7
 
-#define TSCADC_CELLS		0
+#define TSCADC_CELLS		1
+
+enum tscadc_cells {
+	TSC_CELL,
+};
 
 struct mfd_tscadc_board {
 	struct tsc_data *tsc_init;
@@ -128,6 +133,9 @@ struct ti_tscadc_dev {
 	void __iomem *tscadc_base;
 	int irq;
 	struct mfd_cell cells[TSCADC_CELLS];
+
+	/* tsc device */
+	struct tscadc *tsc;
 };
 
 #endif
-- 
1.7.0.4

[PATCH v3 4/5] IIO : ADC: tiadc: Add support of TI's ADC driver

[Patil, Rachna]

This patch adds support for TI's ADC driver.
This is a multifunctional device.
Analog input lines are provided on which
voltage measurements can be carried out.
You can have upto 8 input lines.

Signed-off-by: Patil, Rachna <rachna@ti.com>
---
Changes in v2:
	Addressed review comments from Matthias Kaehlcke

Changes in v3:
	Addressed review comments from Jonathan Cameron.
	Added comments, new line appropriately.

 drivers/iio/adc/Kconfig              |    7 +
 drivers/iio/adc/Makefile             |    1 +
 drivers/iio/adc/ti_adc.c             |  223 ++++++++++++++++++++++++++++++++++
 drivers/mfd/ti_tscadc.c              |   18 +++-
 include/linux/mfd/ti_tscadc.h        |    9 ++-
 include/linux/platform_data/ti_adc.h |   14 ++
 6 files changed, 270 insertions(+), 2 deletions(-)
 create mode 100644 drivers/iio/adc/ti_adc.c
 create mode 100644 include/linux/platform_data/ti_adc.h

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 8a78b4f..ad32df8 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -22,4 +22,11 @@ config AT91_ADC
 	help
 	  Say yes here to build support for Atmel AT91 ADC.
 
+config TI_ADC
+	tristate "TI's ADC driver"
+	depends on ARCH_OMAP2PLUS
+	help
+	  Say yes here to build support for Texas Instruments ADC
+	  driver which is also a MFD client.
+
 endmenu
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 52eec25..a930cee 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -4,3 +4,4 @@
 
 obj-$(CONFIG_AD7266) += ad7266.o
 obj-$(CONFIG_AT91_ADC) += at91_adc.o
+obj-$(CONFIG_TI_ADC) += ti_adc.o
diff --git a/drivers/iio/adc/ti_adc.c b/drivers/iio/adc/ti_adc.c
new file mode 100644
index 0000000..56f8af2
--- /dev/null
+++ b/drivers/iio/adc/ti_adc.c
@@ -0,0 +1,223 @@
+/*
+ * TI ADC MFD driver
+ *
+ * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * 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/init.h>
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/iio/iio.h>
+
+#include <linux/mfd/ti_tscadc.h>
+#include <linux/platform_data/ti_adc.h>
+
+struct adc_device {
+	struct ti_tscadc_dev *mfd_tscadc;
+	struct iio_dev *idev;
+	int channels;
+};
+
+static unsigned int adc_readl(struct adc_device *adc, unsigned int reg)
+{
+	return readl(adc->mfd_tscadc->tscadc_base + reg);
+}
+
+static void adc_writel(struct adc_device *adc, unsigned int reg,
+					unsigned int val)
+{
+	writel(val, adc->mfd_tscadc->tscadc_base + reg);
+}
+
+static void adc_step_config(struct adc_device *adc_dev)
+{
+	unsigned int    stepconfig;
+	int i, channels = 0, steps;
+
+	/*
+	 * There are 16 configurable steps and 8 analog input
+	 * lines available which are shared between Touchscreen and ADC.
+	 *
+	 * Steps backwards i.e. from 16 towards 0 are used by ADC
+	 * depending on number of input lines needed.
+	 * Channel would represent which analog input
+	 * needs to be given to ADC to digitalize data.
+	 */
+
+	steps = TOTAL_STEPS - adc_dev->channels;
+	channels = TOTAL_CHANNELS - adc_dev->channels;
+
+	stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
+
+	for (i = (steps + 1); i <= TOTAL_STEPS; i++) {
+		adc_writel(adc_dev, REG_STEPCONFIG(i),
+				stepconfig | STEPCONFIG_INP(channels));
+		adc_writel(adc_dev, REG_STEPDELAY(i),
+				STEPCONFIG_OPENDLY);
+		channels++;
+	}
+	adc_writel(adc_dev, REG_SE, STPENB_STEPENB);
+}
+
+static int tiadc_channel_init(struct iio_dev *idev, int channels)
+{
+	struct iio_chan_spec *chan_array;
+	int i;
+
+	idev->num_channels = channels;
+	chan_array = kcalloc(idev->num_channels, sizeof(struct iio_chan_spec),
+					GFP_KERNEL);
+
+	if (chan_array == NULL)
+		return -ENOMEM;
+
+	for (i = 0; i < (idev->num_channels); i++) {
+		struct iio_chan_spec *chan = chan_array + i;
+		chan->type = IIO_VOLTAGE;
+		chan->indexed = 1;
+		chan->channel = i;
+		chan->info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT;
+	}
+
+	idev->channels = chan_array;
+
+	return idev->num_channels;
+}
+
+static void tiadc_channels_remove(struct iio_dev *idev)
+{
+	kfree(idev->channels);
+}
+
+static int tiadc_read_raw(struct iio_dev *idev,
+		struct iio_chan_spec const *chan,
+		int *val, int *val2, long mask)
+{
+	struct adc_device *adc_dev = iio_priv(idev);
+	int i;
+	unsigned int fifo1count, readx1;
+
+	/*
+	 * When the sub-system is first enabled,
+	 * the sequencer will always start with the
+	 * lowest step (1) and continue until step (16).
+	 * For ex: If we have enabled 4 ADC channels and
+	 * currently use only 1 out of them, the
+	 * sequencer still configures all the 4 steps,
+	 * leading to 3 unwanted data.
+	 * Hence we need to flush out this data.
+	 */
+
+	fifo1count = adc_readl(adc_dev, REG_FIFO1CNT);
+	for (i = 0; i < fifo1count; i++) {
+		readx1 = adc_readl(adc_dev, REG_FIFO1);
+		if (i == chan->channel)
+			*val = readx1 & 0xfff;
+	}
+	adc_writel(adc_dev, REG_SE, STPENB_STEPENB);
+
+	return IIO_VAL_INT;
+}
+
+static const struct iio_info tiadc_info = {
+	.read_raw = &tiadc_read_raw,
+};
+
+static int __devinit tiadc_probe(struct platform_device *pdev)
+{
+	struct iio_dev		*idev;
+	int			err;
+	struct adc_device	*adc_dev;
+	struct ti_tscadc_dev	*tscadc_dev = pdev->dev.platform_data;
+	struct mfd_tscadc_board	*pdata;
+
+	pdata = (struct mfd_tscadc_board *)tscadc_dev->dev->platform_data;
+	if (!pdata || !pdata->adc_init) {
+		dev_err(tscadc_dev->dev, "Could not find platform data\n");
+		return -EINVAL;
+	}
+
+	idev = iio_device_alloc(sizeof(struct adc_device));
+	if (idev == NULL) {
+		dev_err(&pdev->dev, "failed to allocate iio device.\n");
+		err = -ENOMEM;
+		goto err_ret;
+	}
+	adc_dev = iio_priv(idev);
+
+	tscadc_dev->adc = adc_dev;
+	adc_dev->mfd_tscadc = tscadc_dev;
+	adc_dev->idev = idev;
+	adc_dev->channels = pdata->adc_init->adc_channels;
+
+	idev->dev.parent = &pdev->dev;
+	idev->name = dev_name(&pdev->dev);
+	idev->modes = INDIO_DIRECT_MODE;
+	idev->info = &tiadc_info;
+
+	adc_step_config(adc_dev);
+
+	err = tiadc_channel_init(idev, adc_dev->channels);
+	if (err < 0)
+		goto err_free_device;
+
+	err = iio_device_register(idev);
+	if (err)
+		goto err_free_channels;
+
+	dev_info(&pdev->dev, "attached adc driver\n");
+	platform_set_drvdata(pdev, idev);
+
+	return 0;
+
+err_free_channels:
+	tiadc_channels_remove(idev);
+err_free_device:
+	iio_device_free(idev);
+err_ret:
+	return err;
+}
+
+static int __devexit tiadc_remove(struct platform_device *pdev)
+{
+	struct ti_tscadc_dev   *tscadc_dev = pdev->dev.platform_data;
+	struct adc_device	*adc_dev = tscadc_dev->adc;
+	struct iio_dev		*idev = adc_dev->idev;
+
+	iio_device_unregister(idev);
+	tiadc_channels_remove(idev);
+
+	tscadc_dev->adc = NULL;
+	iio_device_free(idev);
+
+	return 0;
+}
+
+static struct platform_driver tiadc_driver = {
+	.driver = {
+		.name   = "tiadc",
+		.owner = THIS_MODULE,
+	},
+	.probe	= tiadc_probe,
+	.remove	= __devexit_p(tiadc_remove),
+};
+
+module_platform_driver(tiadc_driver);
+
+MODULE_DESCRIPTION("TI ADC controller driver");
+MODULE_AUTHOR("Rachna Patil <rachna@ti.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/ti_tscadc.c b/drivers/mfd/ti_tscadc.c
index f26e53b..9dbd6d0 100644
--- a/drivers/mfd/ti_tscadc.c
+++ b/drivers/mfd/ti_tscadc.c
@@ -23,6 +23,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/mfd/ti_tscadc.h>
 #include <linux/input/ti_tsc.h>
+#include <linux/platform_data/ti_adc.h>
 
 static unsigned int tscadc_readl(struct ti_tscadc_dev *tsadc, unsigned int reg)
 {
@@ -55,14 +56,23 @@ static	int __devinit ti_tscadc_probe(struct platform_device *pdev)
 	int			irq;
 	int			err, ctrl;
 	int			clk_value, clock_rate;
-	int			tsc_wires;
+	int			tsc_wires, adc_channels = 0, total_channels;
 
 	if (!pdata) {
 		dev_err(&pdev->dev, "Could not find platform data\n");
 		return -EINVAL;
 	}
 
+	if (pdata->adc_init)
+		adc_channels = pdata->adc_init->adc_channels;
+
 	tsc_wires = pdata->tsc_init->wires;
+	total_channels = tsc_wires + adc_channels;
+
+	if (total_channels > 8) {
+		dev_err(&pdev->dev, "Number of i/p channels more than 8\n");
+		return -EINVAL;
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
@@ -149,6 +159,12 @@ static	int __devinit ti_tscadc_probe(struct platform_device *pdev)
 	cell->platform_data = tscadc;
 	cell->pdata_size = sizeof(*tscadc);
 
+	/* ADC Cell */
+	cell = &tscadc->cells[ADC_CELL];
+	cell->name = "tiadc";
+	cell->platform_data = tscadc;
+	cell->pdata_size = sizeof(*tscadc);
+
 	err = mfd_add_devices(&pdev->dev, pdev->id, tscadc->cells,
 			TSCADC_CELLS, NULL, 0);
 	if (err < 0)
diff --git a/include/linux/mfd/ti_tscadc.h b/include/linux/mfd/ti_tscadc.h
index d28a35c..67b7d5e 100644
--- a/include/linux/mfd/ti_tscadc.h
+++ b/include/linux/mfd/ti_tscadc.h
@@ -117,15 +117,19 @@
 
 #define ADC_CLK			3000000
 #define	MAX_CLK_DIV		7
+#define TOTAL_STEPS		16
+#define TOTAL_CHANNELS		8
 
-#define TSCADC_CELLS		1
+#define TSCADC_CELLS		2
 
 enum tscadc_cells {
 	TSC_CELL,
+	ADC_CELL,
 };
 
 struct mfd_tscadc_board {
 	struct tsc_data *tsc_init;
+	struct adc_data *adc_init;
 };
 
 struct ti_tscadc_dev {
@@ -136,6 +140,9 @@ struct ti_tscadc_dev {
 
 	/* tsc device */
 	struct tscadc *tsc;
+
+	/* adc device */
+	struct adc_device *adc;
 };
 
 #endif
diff --git a/include/linux/platform_data/ti_adc.h b/include/linux/platform_data/ti_adc.h
new file mode 100644
index 0000000..5a89f1d
--- /dev/null
+++ b/include/linux/platform_data/ti_adc.h
@@ -0,0 +1,14 @@
+#ifndef __LINUX_TI_ADC_H
+#define __LINUX_TI_ADC_H
+
+/**
+ * struct adc_data	ADC Input information
+ * @adc_channels:	Number of analog inputs
+ *			available for ADC.
+ */
+
+struct adc_data {
+	int adc_channels;
+};
+
+#endif
-- 
1.7.0.4

[PATCH v3 5/5] MFD: ti_tscadc: add suspend/resume functionality

[Patil, Rachna]

This patch adds support for suspend/resume of
TSC/ADC MFDevice.

Signed-off-by: Patil, Rachna <rachna@ti.com>
---
Changes in v2:
	Added this patch newly in this patch series.

Changes in v3:
	No changes.

 drivers/iio/adc/ti_adc.c           |   32 ++++++++++++++++++++++++++++++++
 drivers/input/touchscreen/ti_tsc.c |   33 +++++++++++++++++++++++++++++++++
 drivers/mfd/ti_tscadc.c            |   33 ++++++++++++++++++++++++++++++++-
 include/linux/mfd/ti_tscadc.h      |    3 +++
 4 files changed, 100 insertions(+), 1 deletions(-)

diff --git a/drivers/iio/adc/ti_adc.c b/drivers/iio/adc/ti_adc.c
index 56f8af2..69f19f0 100644
--- a/drivers/iio/adc/ti_adc.c
+++ b/drivers/iio/adc/ti_adc.c
@@ -207,6 +207,36 @@ static int __devexit tiadc_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int adc_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	struct ti_tscadc_dev   *tscadc_dev = pdev->dev.platform_data;
+	struct adc_device	*adc_dev = tscadc_dev->adc;
+	unsigned int idle;
+
+	if (!device_may_wakeup(tscadc_dev->dev)) {
+		idle = adc_readl(adc_dev, REG_CTRL);
+		idle &= ~(CNTRLREG_TSCSSENB);
+		adc_writel(adc_dev, REG_CTRL, (idle |
+				CNTRLREG_POWERDOWN));
+	}
+	return 0;
+}
+
+static int adc_resume(struct platform_device *pdev)
+{
+	struct ti_tscadc_dev   *tscadc_dev = pdev->dev.platform_data;
+	struct adc_device	*adc_dev = tscadc_dev->adc;
+	unsigned int restore;
+
+	/* Make sure ADC is powered up */
+	restore = adc_readl(adc_dev, REG_CTRL);
+	restore &= ~(CNTRLREG_POWERDOWN);
+	adc_writel(adc_dev, REG_CTRL, restore);
+
+	adc_step_config(adc_dev);
+	return 0;
+}
+
 static struct platform_driver tiadc_driver = {
 	.driver = {
 		.name   = "tiadc",
@@ -214,6 +244,8 @@ static struct platform_driver tiadc_driver = {
 	},
 	.probe	= tiadc_probe,
 	.remove	= __devexit_p(tiadc_remove),
+	.suspend = adc_suspend,
+	.resume = adc_resume,
 };
 
 module_platform_driver(tiadc_driver);
diff --git a/drivers/input/touchscreen/ti_tsc.c b/drivers/input/touchscreen/ti_tsc.c
index ca8ce73..f103e5f 100644
--- a/drivers/input/touchscreen/ti_tsc.c
+++ b/drivers/input/touchscreen/ti_tsc.c
@@ -338,6 +338,37 @@ static int __devexit tscadc_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int tsc_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	struct ti_tscadc_dev	*tscadc_dev = pdev->dev.platform_data;
+	struct tscadc		*ts_dev = tscadc_dev->tsc;
+	unsigned int idle;
+
+	if (device_may_wakeup(tscadc_dev->dev)) {
+		idle = tscadc_readl(ts_dev, REG_IRQENABLE);
+		tscadc_writel(ts_dev, REG_IRQENABLE,
+				(idle | IRQENB_HW_PEN));
+		tscadc_writel(ts_dev, REG_IRQWAKEUP, IRQWKUP_ENB);
+	}
+	return 0;
+}
+
+static int tsc_resume(struct platform_device *pdev)
+{
+	struct ti_tscadc_dev	*tscadc_dev = pdev->dev.platform_data;
+	struct tscadc		*ts_dev = tscadc_dev->tsc;
+
+	if (device_may_wakeup(tscadc_dev->dev)) {
+		tscadc_writel(ts_dev, REG_IRQWAKEUP,
+				0x00);
+		tscadc_writel(ts_dev, REG_IRQCLR, IRQENB_HW_PEN);
+	}
+	tscadc_step_config(ts_dev);
+	tscadc_writel(ts_dev, REG_FIFO0THR,
+			ts_dev->steps_to_configure);
+	return 0;
+}
+
 static struct platform_driver ti_tsc_driver = {
 	.probe	= tscadc_probe,
 	.remove	= __devexit_p(tscadc_remove),
@@ -345,6 +376,8 @@ static struct platform_driver ti_tsc_driver = {
 		.name   = "tsc",
 		.owner	= THIS_MODULE,
 	},
+	.suspend = tsc_suspend,
+	.resume = tsc_resume,
 };
 module_platform_driver(ti_tsc_driver);
 
diff --git a/drivers/mfd/ti_tscadc.c b/drivers/mfd/ti_tscadc.c
index 9dbd6d0..2c84aed 100644
--- a/drivers/mfd/ti_tscadc.c
+++ b/drivers/mfd/ti_tscadc.c
@@ -170,6 +170,7 @@ static	int __devinit ti_tscadc_probe(struct platform_device *pdev)
 	if (err < 0)
 		goto err_disable_clk;
 
+	device_init_wakeup(&pdev->dev, true);
 	platform_set_drvdata(pdev, tscadc);
 	return 0;
 
@@ -203,6 +204,35 @@ static int __devexit ti_tscadc_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int tscadc_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	struct ti_tscadc_dev	*tscadc_dev = platform_get_drvdata(pdev);
+
+	tscadc_writel(tscadc_dev, REG_SE, 0x00);
+	pm_runtime_put_sync(&pdev->dev);
+	return 0;
+}
+
+static int tscadc_resume(struct platform_device *pdev)
+{
+	struct ti_tscadc_dev	*tscadc_dev = platform_get_drvdata(pdev);
+	unsigned int restore, ctrl;
+
+	pm_runtime_get_sync(&pdev->dev);
+
+	/* context restore */
+	ctrl = CNTRLREG_STEPCONFIGWRT | CNTRLREG_TSCENB |
+			CNTRLREG_STEPID | CNTRLREG_4WIRE;
+	tscadc_writel(tscadc_dev, REG_CTRL, ctrl);
+	tscadc_idle_config(tscadc_dev);
+	tscadc_writel(tscadc_dev, REG_SE, STPENB_STEPENB);
+	restore = tscadc_readl(tscadc_dev, REG_CTRL);
+	tscadc_writel(tscadc_dev, REG_CTRL,
+			(restore | CNTRLREG_TSCSSENB));
+
+	return 0;
+}
+
 static struct platform_driver ti_tscadc_driver = {
 	.driver = {
 		.name   = "ti_tscadc",
@@ -210,7 +240,8 @@ static struct platform_driver ti_tscadc_driver = {
 	},
 	.probe	= ti_tscadc_probe,
 	.remove	= __devexit_p(ti_tscadc_remove),
-
+	.suspend = tscadc_suspend,
+	.resume = tscadc_resume,
 };
 
 module_platform_driver(ti_tscadc_driver);
diff --git a/include/linux/mfd/ti_tscadc.h b/include/linux/mfd/ti_tscadc.h
index 67b7d5e..f4763a0 100644
--- a/include/linux/mfd/ti_tscadc.h
+++ b/include/linux/mfd/ti_tscadc.h
@@ -40,6 +40,9 @@
 #define REG_FIFO1		0x200
 
 /*	Register Bitfields	*/
+/* IRQ wakeup enable */
+#define IRQWKUP_ENB		BIT(0)
+
 /* Step Enable */
 #define STEPENB_MASK		(0x1FFFF << 0)
 #define STEPENB(val)		((val) << 0)
-- 
1.7.0.4

Re: [PATCH v3 4/5] IIO : ADC: tiadc: Add support of TI's ADC driver

[Jonathan Cameron]

On 13/09/12 11:40, Patil, Rachna wrote:
> This patch adds support for TI's ADC driver.
> This is a multifunctional device.
> Analog input lines are provided on which
> voltage measurements can be carried out.
> You can have upto 8 input lines.
>
> Signed-off-by: Patil, Rachna <rachna@ti.com>

There's a little fuzz in applying this due to other drivers that have
gone in recently.

Actually this is going to be 'interesting' to merge. Dmitry, Samuel 
thoughts on who takes this one and how?  Maybe this is a case for a
'special' branch pulled into more than one tree?


One minor thing inline.  I have an aversion to dynamic allocation of
things that are then constant.

Also the module name is simply ti_adc. Does seem a little 'vague'
given the range of ADC's TI makes :)  Perhaps keep the reference
to the tsc in there?  Personally I'd have preferred the whole thing
being named after a particular part number (any one it support would
do) to avoid a clash in future with a new touch screen adc from TI.
Bit late for that though I guess ;)

Jonathan
> ---
> Changes in v2:
> 	Addressed review comments from Matthias Kaehlcke
>
> Changes in v3:
> 	Addressed review comments from Jonathan Cameron.
> 	Added comments, new line appropriately.
>
>   drivers/iio/adc/Kconfig              |    7 +
>   drivers/iio/adc/Makefile             |    1 +
>   drivers/iio/adc/ti_adc.c             |  223 ++++++++++++++++++++++++++++++++++
>   drivers/mfd/ti_tscadc.c              |   18 +++-
>   include/linux/mfd/ti_tscadc.h        |    9 ++-
>   include/linux/platform_data/ti_adc.h |   14 ++
>   6 files changed, 270 insertions(+), 2 deletions(-)
>   create mode 100644 drivers/iio/adc/ti_adc.c
>   create mode 100644 include/linux/platform_data/ti_adc.h
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 8a78b4f..ad32df8 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -22,4 +22,11 @@ config AT91_ADC
>   	help
>   	  Say yes here to build support for Atmel AT91 ADC.
>
> +config TI_ADC
> +	tristate "TI's ADC driver"
> +	depends on ARCH_OMAP2PLUS
> +	help
> +	  Say yes here to build support for Texas Instruments ADC
> +	  driver which is also a MFD client.
> +
>   endmenu
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 52eec25..a930cee 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -4,3 +4,4 @@
>
>   obj-$(CONFIG_AD7266) += ad7266.o
>   obj-$(CONFIG_AT91_ADC) += at91_adc.o
> +obj-$(CONFIG_TI_ADC) += ti_adc.o
> diff --git a/drivers/iio/adc/ti_adc.c b/drivers/iio/adc/ti_adc.c
> new file mode 100644
> index 0000000..56f8af2
> --- /dev/null
> +++ b/drivers/iio/adc/ti_adc.c
> @@ -0,0 +1,223 @@
> +/*
> + * TI ADC MFD driver
> + *
> + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
> + *
> + * 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/init.h>
> +#include <linux/kernel.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/iio/iio.h>
> +
> +#include <linux/mfd/ti_tscadc.h>
> +#include <linux/platform_data/ti_adc.h>
> +
> +struct adc_device {
> +	struct ti_tscadc_dev *mfd_tscadc;
> +	struct iio_dev *idev;
> +	int channels;
> +};
> +
> +static unsigned int adc_readl(struct adc_device *adc, unsigned int reg)
> +{
> +	return readl(adc->mfd_tscadc->tscadc_base + reg);
> +}
> +
> +static void adc_writel(struct adc_device *adc, unsigned int reg,
> +					unsigned int val)
> +{
> +	writel(val, adc->mfd_tscadc->tscadc_base + reg);
> +}
> +
> +static void adc_step_config(struct adc_device *adc_dev)
> +{
> +	unsigned int    stepconfig;
> +	int i, channels = 0, steps;
> +
> +	/*
> +	 * There are 16 configurable steps and 8 analog input
> +	 * lines available which are shared between Touchscreen and ADC.
> +	 *
> +	 * Steps backwards i.e. from 16 towards 0 are used by ADC
> +	 * depending on number of input lines needed.
> +	 * Channel would represent which analog input
> +	 * needs to be given to ADC to digitalize data.
> +	 */
> +
> +	steps = TOTAL_STEPS - adc_dev->channels;
> +	channels = TOTAL_CHANNELS - adc_dev->channels;
> +
> +	stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
> +
> +	for (i = (steps + 1); i <= TOTAL_STEPS; i++) {
> +		adc_writel(adc_dev, REG_STEPCONFIG(i),
> +				stepconfig | STEPCONFIG_INP(channels));
> +		adc_writel(adc_dev, REG_STEPDELAY(i),
> +				STEPCONFIG_OPENDLY);
> +		channels++;
> +	}
> +	adc_writel(adc_dev, REG_SE, STPENB_STEPENB);
> +}
> +
> +static int tiadc_channel_init(struct iio_dev *idev, int channels)
> +{
> +	struct iio_chan_spec *chan_array;
> +	int i;
> +
> +	idev->num_channels = channels;
> +	chan_array = kcalloc(idev->num_channels, sizeof(struct iio_chan_spec),
> +					GFP_KERNEL);
> +
> +	if (chan_array == NULL)
> +		return -ENOMEM;
> +
Minor point, but I'd be tempted to do this as a static const array and 
then just set num_channels appropriately.  Still it's such a simple 
driver that I'm not that fussed.
> +	for (i = 0; i < (idev->num_channels); i++) {
> +		struct iio_chan_spec *chan = chan_array + i;
> +		chan->type = IIO_VOLTAGE;
> +		chan->indexed = 1;
> +		chan->channel = i;
> +		chan->info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT;
> +	}
> +
> +	idev->channels = chan_array;
> +
> +	return idev->num_channels;
> +}
> +
> +static void tiadc_channels_remove(struct iio_dev *idev)
> +{
> +	kfree(idev->channels);
> +}
> +
> +static int tiadc_read_raw(struct iio_dev *idev,
> +		struct iio_chan_spec const *chan,
> +		int *val, int *val2, long mask)
> +{
> +	struct adc_device *adc_dev = iio_priv(idev);
> +	int i;
> +	unsigned int fifo1count, readx1;
> +
> +	/*
> +	 * When the sub-system is first enabled,
> +	 * the sequencer will always start with the
> +	 * lowest step (1) and continue until step (16).
> +	 * For ex: If we have enabled 4 ADC channels and
> +	 * currently use only 1 out of them, the
> +	 * sequencer still configures all the 4 steps,
> +	 * leading to 3 unwanted data.
> +	 * Hence we need to flush out this data.
> +	 */
> +
> +	fifo1count = adc_readl(adc_dev, REG_FIFO1CNT);
> +	for (i = 0; i < fifo1count; i++) {
> +		readx1 = adc_readl(adc_dev, REG_FIFO1);
> +		if (i == chan->channel)
> +			*val = readx1 & 0xfff;
> +	}
> +	adc_writel(adc_dev, REG_SE, STPENB_STEPENB);
> +
> +	return IIO_VAL_INT;
> +}
> +
> +static const struct iio_info tiadc_info = {
> +	.read_raw = &tiadc_read_raw,
> +};
> +
> +static int __devinit tiadc_probe(struct platform_device *pdev)
> +{
> +	struct iio_dev		*idev;
> +	int			err;
> +	struct adc_device	*adc_dev;
> +	struct ti_tscadc_dev	*tscadc_dev = pdev->dev.platform_data;
> +	struct mfd_tscadc_board	*pdata;
> +
> +	pdata = (struct mfd_tscadc_board *)tscadc_dev->dev->platform_data;
> +	if (!pdata || !pdata->adc_init) {
> +		dev_err(tscadc_dev->dev, "Could not find platform data\n");
> +		return -EINVAL;
> +	}
> +
> +	idev = iio_device_alloc(sizeof(struct adc_device));
> +	if (idev == NULL) {
> +		dev_err(&pdev->dev, "failed to allocate iio device.\n");
> +		err = -ENOMEM;
> +		goto err_ret;
> +	}
> +	adc_dev = iio_priv(idev);
> +
> +	tscadc_dev->adc = adc_dev;
> +	adc_dev->mfd_tscadc = tscadc_dev;
> +	adc_dev->idev = idev;
> +	adc_dev->channels = pdata->adc_init->adc_channels;
> +
> +	idev->dev.parent = &pdev->dev;
> +	idev->name = dev_name(&pdev->dev);
> +	idev->modes = INDIO_DIRECT_MODE;
> +	idev->info = &tiadc_info;
> +
> +	adc_step_config(adc_dev);
> +
> +	err = tiadc_channel_init(idev, adc_dev->channels);
> +	if (err < 0)
> +		goto err_free_device;
> +
> +	err = iio_device_register(idev);
> +	if (err)
> +		goto err_free_channels;
> +
> +	dev_info(&pdev->dev, "attached adc driver\n");
> +	platform_set_drvdata(pdev, idev);
> +
> +	return 0;
> +
> +err_free_channels:
> +	tiadc_channels_remove(idev);
> +err_free_device:
> +	iio_device_free(idev);
> +err_ret:
> +	return err;
> +}
> +
> +static int __devexit tiadc_remove(struct platform_device *pdev)
> +{
> +	struct ti_tscadc_dev   *tscadc_dev = pdev->dev.platform_data;
> +	struct adc_device	*adc_dev = tscadc_dev->adc;
> +	struct iio_dev		*idev = adc_dev->idev;
> +
> +	iio_device_unregister(idev);
> +	tiadc_channels_remove(idev);
> +
> +	tscadc_dev->adc = NULL;
> +	iio_device_free(idev);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver tiadc_driver = {
> +	.driver = {
> +		.name   = "tiadc",
> +		.owner = THIS_MODULE,
> +	},
> +	.probe	= tiadc_probe,
> +	.remove	= __devexit_p(tiadc_remove),
> +};
> +
> +module_platform_driver(tiadc_driver);
> +
> +MODULE_DESCRIPTION("TI ADC controller driver");
> +MODULE_AUTHOR("Rachna Patil <rachna@ti.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/mfd/ti_tscadc.c b/drivers/mfd/ti_tscadc.c
> index f26e53b..9dbd6d0 100644
> --- a/drivers/mfd/ti_tscadc.c
> +++ b/drivers/mfd/ti_tscadc.c
> @@ -23,6 +23,7 @@
>   #include <linux/pm_runtime.h>
>   #include <linux/mfd/ti_tscadc.h>
>   #include <linux/input/ti_tsc.h>
> +#include <linux/platform_data/ti_adc.h>
>
>   static unsigned int tscadc_readl(struct ti_tscadc_dev *tsadc, unsigned int reg)
>   {
> @@ -55,14 +56,23 @@ static	int __devinit ti_tscadc_probe(struct platform_device *pdev)
>   	int			irq;
>   	int			err, ctrl;
>   	int			clk_value, clock_rate;
> -	int			tsc_wires;
> +	int			tsc_wires, adc_channels = 0, total_channels;
>
>   	if (!pdata) {
>   		dev_err(&pdev->dev, "Could not find platform data\n");
>   		return -EINVAL;
>   	}
>
> +	if (pdata->adc_init)
> +		adc_channels = pdata->adc_init->adc_channels;
> +
>   	tsc_wires = pdata->tsc_init->wires;
> +	total_channels = tsc_wires + adc_channels;
> +
> +	if (total_channels > 8) {
> +		dev_err(&pdev->dev, "Number of i/p channels more than 8\n");
> +		return -EINVAL;
> +	}
>
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   	if (!res) {
> @@ -149,6 +159,12 @@ static	int __devinit ti_tscadc_probe(struct platform_device *pdev)
>   	cell->platform_data = tscadc;
>   	cell->pdata_size = sizeof(*tscadc);
>
> +	/* ADC Cell */
> +	cell = &tscadc->cells[ADC_CELL];
> +	cell->name = "tiadc";
> +	cell->platform_data = tscadc;
> +	cell->pdata_size = sizeof(*tscadc);
> +
>   	err = mfd_add_devices(&pdev->dev, pdev->id, tscadc->cells,
>   			TSCADC_CELLS, NULL, 0);
>   	if (err < 0)
> diff --git a/include/linux/mfd/ti_tscadc.h b/include/linux/mfd/ti_tscadc.h
> index d28a35c..67b7d5e 100644
> --- a/include/linux/mfd/ti_tscadc.h
> +++ b/include/linux/mfd/ti_tscadc.h
> @@ -117,15 +117,19 @@
>
>   #define ADC_CLK			3000000
>   #define	MAX_CLK_DIV		7
> +#define TOTAL_STEPS		16
> +#define TOTAL_CHANNELS		8
>
> -#define TSCADC_CELLS		1
> +#define TSCADC_CELLS		2
>
>   enum tscadc_cells {
>   	TSC_CELL,
> +	ADC_CELL,
>   };
>
>   struct mfd_tscadc_board {
>   	struct tsc_data *tsc_init;
> +	struct adc_data *adc_init;
>   };
>
>   struct ti_tscadc_dev {
> @@ -136,6 +140,9 @@ struct ti_tscadc_dev {
>
>   	/* tsc device */
>   	struct tscadc *tsc;
> +
> +	/* adc device */
> +	struct adc_device *adc;
>   };
>
>   #endif
> diff --git a/include/linux/platform_data/ti_adc.h b/include/linux/platform_data/ti_adc.h
> new file mode 100644
> index 0000000..5a89f1d
> --- /dev/null
> +++ b/include/linux/platform_data/ti_adc.h
> @@ -0,0 +1,14 @@
> +#ifndef __LINUX_TI_ADC_H
> +#define __LINUX_TI_ADC_H
> +
> +/**
> + * struct adc_data	ADC Input information
> + * @adc_channels:	Number of analog inputs
> + *			available for ADC.
> + */
> +
> +struct adc_data {
> +	int adc_channels;
> +};
> +
> +#endif
>

Re: [PATCH v3 4/5] IIO : ADC: tiadc: Add support of TI's ADC driver

[Lars-Peter Clausen]

On 09/13/2012 12:40 PM, Patil, Rachna wrote:
> This patch adds support for TI's ADC driver.
> This is a multifunctional device.
> Analog input lines are provided on which
> voltage measurements can be carried out.
> You can have upto 8 input lines.
> 

Hi,

couple of minor issues inline.

> Signed-off-by: Patil, Rachna <rachna@ti.com>
> ---
> Changes in v2:
> 	Addressed review comments from Matthias Kaehlcke
> 
> Changes in v3:
> 	Addressed review comments from Jonathan Cameron.
> 	Added comments, new line appropriately.
> 
>  drivers/iio/adc/Kconfig              |    7 +
>  drivers/iio/adc/Makefile             |    1 +
>  drivers/iio/adc/ti_adc.c             |  223 ++++++++++++++++++++++++++++++++++
>  drivers/mfd/ti_tscadc.c              |   18 +++-
>  include/linux/mfd/ti_tscadc.h        |    9 ++-
>  include/linux/platform_data/ti_adc.h |   14 ++
>  6 files changed, 270 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/iio/adc/ti_adc.c
>  create mode 100644 include/linux/platform_data/ti_adc.h
> 
[...]
> +
> +struct adc_device {

Not really an issue, but I'd use a consistent function/struct prefix.
Currently you use both "adc" and "tiadc"

> +	struct ti_tscadc_dev *mfd_tscadc;
> +	struct iio_dev *idev;

idev is used only once in the remove callback. But you can get a pointer to
it easily using platform_get_drvdata. So I'd remove it from the adc_device
struct.

> +	int channels;
> +};
> +
> +static unsigned int adc_readl(struct adc_device *adc, unsigned int reg)
> +{
> +	return readl(adc->mfd_tscadc->tscadc_base + reg);
> +}
> +
> +static void adc_writel(struct adc_device *adc, unsigned int reg,
> +					unsigned int val)
> +{
> +	writel(val, adc->mfd_tscadc->tscadc_base + reg);
> +}
> +
> +static void adc_step_config(struct adc_device *adc_dev)
> +{
> +	unsigned int    stepconfig;

extra whitespace

> +	int i, channels = 0, steps;
> +
> +	/*
> +	 * There are 16 configurable steps and 8 analog input
> +	 * lines available which are shared between Touchscreen and ADC.
> +	 *
> +	 * Steps backwards i.e. from 16 towards 0 are used by ADC
> +	 * depending on number of input lines needed.
> +	 * Channel would represent which analog input
> +	 * needs to be given to ADC to digitalize data.
> +	 */
> +
> +	steps = TOTAL_STEPS - adc_dev->channels;
> +	channels = TOTAL_CHANNELS - adc_dev->channels;
> +
> +	stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
> +
> +	for (i = (steps + 1); i <= TOTAL_STEPS; i++) {
> +		adc_writel(adc_dev, REG_STEPCONFIG(i),
> +				stepconfig | STEPCONFIG_INP(channels));
> +		adc_writel(adc_dev, REG_STEPDELAY(i),
> +				STEPCONFIG_OPENDLY);
> +		channels++;
> +	}
> +	adc_writel(adc_dev, REG_SE, STPENB_STEPENB);
> +}
> +
> [...]
> +
> +static const struct iio_info tiadc_info = {
> +	.read_raw = &tiadc_read_raw,

    .driver_module = THIS_MODULE,

> +};
> +
> +static int __devinit tiadc_probe(struct platform_device *pdev)
> +{
> +	struct iio_dev		*idev;

For consistency with other drivers please rename idev to indio_dev
throughout the driver.

> +	int			err;
> +	struct adc_device	*adc_dev;
> +	struct ti_tscadc_dev	*tscadc_dev = pdev->dev.platform_data;
> +	struct mfd_tscadc_board	*pdata;
> +
> +	pdata = (struct mfd_tscadc_board *)tscadc_dev->dev->platform_data;

The cast should not be necessary.

> +	if (!pdata || !pdata->adc_init) {
> +		dev_err(tscadc_dev->dev, "Could not find platform data\n");

I'd still use pdev->dev for the device parameter here. Seeing a message
printed by this driver for another device might be confusing.

> +		return -EINVAL;
> +	}
> +
> +	idev = iio_device_alloc(sizeof(struct adc_device));
> +	if (idev == NULL) {
> +		dev_err(&pdev->dev, "failed to allocate iio device.\n");
> +		err = -ENOMEM;
> +		goto err_ret;
> +	}
> +	adc_dev = iio_priv(idev);
> +
> +	tscadc_dev->adc = adc_dev;

This there any reason why you need to store a pointer to the adc struct in
the mfd struct? Is it going to be used outside of the adc driver? Currently
it is, as far as I can see, only used in the remove callback and
suspend/resume handlers. But there you can use iio_priv just as easily to
get the pointer to the adc device struct and it certainly will be also be
cleaner to do it that way.

> +	adc_dev->mfd_tscadc = tscadc_dev;
> +	adc_dev->idev = idev;
> +	adc_dev->channels = pdata->adc_init->adc_channels;
> +
> +	idev->dev.parent = &pdev->dev;
> +	idev->name = dev_name(&pdev->dev);
> +	idev->modes = INDIO_DIRECT_MODE;
> +	idev->info = &tiadc_info;
> +
> +	adc_step_config(adc_dev);
> +
> +	err = tiadc_channel_init(idev, adc_dev->channels);
> +	if (err < 0)
> +		goto err_free_device;
> +
> +	err = iio_device_register(idev);
> +	if (err)
> +		goto err_free_channels;
> +
> +	dev_info(&pdev->dev, "attached adc driver\n");

I'd remove that line, it's just noise.

> +	platform_set_drvdata(pdev, idev);
> +
> +	return 0;
> +
> +err_free_channels:
> +	tiadc_channels_remove(idev);
> +err_free_device:
> +	iio_device_free(idev);
> +err_ret:
> +	return err;
> +}
> +

[...]
> diff --git a/include/linux/platform_data/ti_adc.h b/include/linux/platform_data/ti_adc.h
> new file mode 100644
> index 0000000..5a89f1d
> --- /dev/null
> +++ b/include/linux/platform_data/ti_adc.h
> @@ -0,0 +1,14 @@
> +#ifndef __LINUX_TI_ADC_H
> +#define __LINUX_TI_ADC_H
> +
> +/**
> + * struct adc_data	ADC Input information
> + * @adc_channels:	Number of analog inputs
> + *			available for ADC.
> + */
> +
> +struct adc_data {

The struct name might be a bit to generic.

> +	int adc_channels;

It does not really matter, but considering that there hardly ever going to
be a negative number of channels I'd make this unsigned.

> +};
> +
> +#endif

Re: [PATCH v3 5/5] MFD: ti_tscadc: add suspend/resume functionality

[Lars-Peter Clausen]

On 09/13/2012 12:40 PM, Patil, Rachna wrote:
> This patch adds support for suspend/resume of
> TSC/ADC MFDevice.
> 
> Signed-off-by: Patil, Rachna <rachna@ti.com>
> ---
> Changes in v2:
> 	Added this patch newly in this patch series.
> 
> Changes in v3:
> 	No changes.
> 
>  drivers/iio/adc/ti_adc.c           |   32 ++++++++++++++++++++++++++++++++
>  drivers/input/touchscreen/ti_tsc.c |   33 +++++++++++++++++++++++++++++++++
>  drivers/mfd/ti_tscadc.c            |   33 ++++++++++++++++++++++++++++++++-
>  include/linux/mfd/ti_tscadc.h      |    3 +++
>  4 files changed, 100 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti_adc.c b/drivers/iio/adc/ti_adc.c
> index 56f8af2..69f19f0 100644
> --- a/drivers/iio/adc/ti_adc.c
> +++ b/drivers/iio/adc/ti_adc.c
> @@ -207,6 +207,36 @@ static int __devexit tiadc_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
[...]
>  static struct platform_driver tiadc_driver = {
>  	.driver = {
>  		.name   = "tiadc",
> @@ -214,6 +244,8 @@ static struct platform_driver tiadc_driver = {
>  	},
>  	.probe	= tiadc_probe,
>  	.remove	= __devexit_p(tiadc_remove),
> +	.suspend = adc_suspend,
> +	.resume = adc_resume,
>  };
>  


Using the suspend/resume callbacks is deprecated, please use dev_pm_ops.
Same comment applies to the other two drivers.

Re: [PATCH v3 4/5] IIO : ADC: tiadc: Add support of TI's ADC driver

[Dmitry Torokhov]

On Thu, Sep 13, 2012 at 01:13:30PM +0100, Jonathan Cameron wrote:
> On 13/09/12 11:40, Patil, Rachna wrote:
> >This patch adds support for TI's ADC driver.
> >This is a multifunctional device.
> >Analog input lines are provided on which
> >voltage measurements can be carried out.
> >You can have upto 8 input lines.
> >
> >Signed-off-by: Patil, Rachna <rachna@ti.com>
> 
> There's a little fuzz in applying this due to other drivers that have
> gone in recently.
> 
> Actually this is going to be 'interesting' to merge. Dmitry, Samuel
> thoughts on who takes this one and how?  Maybe this is a case for a
> 'special' branch pulled into more than one tree?

I'd be OK with it going through MFD tree.

-- 
Dmitry

RE: [PATCH v3 5/5] MFD: ti_tscadc: add suspend/resume functionality

[Patil, Rachna]

On Thu, Sep 13, 2012 at 18:31:35, Lars-Peter Clausen wrote:
> On 09/13/2012 12:40 PM, Patil, Rachna wrote:
> > This patch adds support for suspend/resume of TSC/ADC MFDevice.
> > 
> > Signed-off-by: Patil, Rachna <rachna@ti.com>
> > ---
> > Changes in v2:
> > 	Added this patch newly in this patch series.
> > 
> > Changes in v3:
> > 	No changes.
> > 
> >  drivers/iio/adc/ti_adc.c           |   32 ++++++++++++++++++++++++++++++++
> >  drivers/input/touchscreen/ti_tsc.c |   33 +++++++++++++++++++++++++++++++++
> >  drivers/mfd/ti_tscadc.c            |   33 ++++++++++++++++++++++++++++++++-
> >  include/linux/mfd/ti_tscadc.h      |    3 +++
> >  4 files changed, 100 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/ti_adc.c b/drivers/iio/adc/ti_adc.c index 
> > 56f8af2..69f19f0 100644
> > --- a/drivers/iio/adc/ti_adc.c
> > +++ b/drivers/iio/adc/ti_adc.c
> > @@ -207,6 +207,36 @@ static int __devexit tiadc_remove(struct platform_device *pdev)
> >  	return 0;
> >  }
> >  
> [...]
> >  static struct platform_driver tiadc_driver = {
> >  	.driver = {
> >  		.name   = "tiadc",
> > @@ -214,6 +244,8 @@ static struct platform_driver tiadc_driver = {
> >  	},
> >  	.probe	= tiadc_probe,
> >  	.remove	= __devexit_p(tiadc_remove),
> > +	.suspend = adc_suspend,
> > +	.resume = adc_resume,
> >  };
> >  
> 
> 
> Using the suspend/resume callbacks is deprecated, please use dev_pm_ops.
> Same comment applies to the other two drivers.

Ok. I will make changes in all the 3 driver to use dev_pm_ops instead of suspend/resume callbacks.

Regards,
Rachna

> 
> 
> 

RE: [PATCH v3 5/5] MFD: ti_tscadc: add suspend/resume functionality

[Venu Byravarasu]

> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
> owner@vger.kernel.org] On Behalf Of Patil, Rachna
> Sent: Friday, September 14, 2012 10:29 AM
> To: Lars-Peter Clausen
> Cc: linux-kernel@vger.kernel.org; linux-input@vger.kernel.org; linux-
> iio@vger.kernel.org; Samuel Ortiz; Dmitry Torokhov; Dmitry Torokhov;
> Jonathan Cameron
> Subject: RE: [PATCH v3 5/5] MFD: ti_tscadc: add suspend/resume
> functionality
> 
> On Thu, Sep 13, 2012 at 18:31:35, Lars-Peter Clausen wrote:
> > On 09/13/2012 12:40 PM, Patil, Rachna wrote:
> > > This patch adds support for suspend/resume of TSC/ADC MFDevice.
> > >
> > > Signed-off-by: Patil, Rachna <rachna@ti.com>
> > > ---
> > > Changes in v2:
> > > 	Added this patch newly in this patch series.
> > >
> > > Changes in v3:
> > > 	No changes.
> > >
> > >  drivers/iio/adc/ti_adc.c           |   32
> ++++++++++++++++++++++++++++++++
> > >  drivers/input/touchscreen/ti_tsc.c |   33
> +++++++++++++++++++++++++++++++++
> > >  drivers/mfd/ti_tscadc.c            |   33
> ++++++++++++++++++++++++++++++++-
> > >  include/linux/mfd/ti_tscadc.h      |    3 +++
> > >  4 files changed, 100 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/drivers/iio/adc/ti_adc.c b/drivers/iio/adc/ti_adc.c index
> > > 56f8af2..69f19f0 100644
> > > --- a/drivers/iio/adc/ti_adc.c
> > > +++ b/drivers/iio/adc/ti_adc.c
> > > @@ -207,6 +207,36 @@ static int __devexit tiadc_remove(struct
> platform_device *pdev)
> > >  	return 0;
> > >  }
> > >
> > [...]
> > >  static struct platform_driver tiadc_driver = {
> > >  	.driver = {
> > >  		.name   = "tiadc",
> > > @@ -214,6 +244,8 @@ static struct platform_driver tiadc_driver = {
> > >  	},
> > >  	.probe	= tiadc_probe,
> > >  	.remove	= __devexit_p(tiadc_remove),
> > > +	.suspend = adc_suspend,
> > > +	.resume = adc_resume,
> > >  };
> > >
> >
> >
> > Using the suspend/resume callbacks is deprecated, please use
> dev_pm_ops.
> > Same comment applies to the other two drivers.
> 
> Ok. I will make changes in all the 3 driver to use dev_pm_ops instead of
> suspend/resume callbacks.

Probably you might need to protect suspend/resumes with CONFIG_PM. 

> 
> Regards,
> Rachna
> 
> >
> >
> >
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v3 5/5] MFD: ti_tscadc: add suspend/resume functionality

[Patil, Rachna]

On Fri, Sep 14, 2012 at 10:39:20, Venu Byravarasu wrote:
> > -----Original Message-----
> > From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel- 
> > owner@vger.kernel.org] On Behalf Of Patil, Rachna
> > Sent: Friday, September 14, 2012 10:29 AM
> > To: Lars-Peter Clausen
> > Cc: linux-kernel@vger.kernel.org; linux-input@vger.kernel.org; linux- 
> > iio@vger.kernel.org; Samuel Ortiz; Dmitry Torokhov; Dmitry Torokhov; 
> > Jonathan Cameron
> > Subject: RE: [PATCH v3 5/5] MFD: ti_tscadc: add suspend/resume 
> > functionality
> > 
> > On Thu, Sep 13, 2012 at 18:31:35, Lars-Peter Clausen wrote:
> > > On 09/13/2012 12:40 PM, Patil, Rachna wrote:
> > > > This patch adds support for suspend/resume of TSC/ADC MFDevice.
> > > >
> > > > Signed-off-by: Patil, Rachna <rachna@ti.com>
> > > > ---
> > > > Changes in v2:
> > > > 	Added this patch newly in this patch series.
> > > >
> > > > Changes in v3:
> > > > 	No changes.
> > > >
> > > >  drivers/iio/adc/ti_adc.c           |   32
> > ++++++++++++++++++++++++++++++++
> > > >  drivers/input/touchscreen/ti_tsc.c |   33
> > +++++++++++++++++++++++++++++++++
> > > >  drivers/mfd/ti_tscadc.c            |   33
> > ++++++++++++++++++++++++++++++++-
> > > >  include/linux/mfd/ti_tscadc.h      |    3 +++
> > > >  4 files changed, 100 insertions(+), 1 deletions(-)
> > > >
> > > > diff --git a/drivers/iio/adc/ti_adc.c b/drivers/iio/adc/ti_adc.c 
> > > > index 56f8af2..69f19f0 100644
> > > > --- a/drivers/iio/adc/ti_adc.c
> > > > +++ b/drivers/iio/adc/ti_adc.c
> > > > @@ -207,6 +207,36 @@ static int __devexit tiadc_remove(struct
> > platform_device *pdev)
> > > >  	return 0;
> > > >  }
> > > >
> > > [...]
> > > >  static struct platform_driver tiadc_driver = {
> > > >  	.driver = {
> > > >  		.name   = "tiadc",
> > > > @@ -214,6 +244,8 @@ static struct platform_driver tiadc_driver = {
> > > >  	},
> > > >  	.probe	= tiadc_probe,
> > > >  	.remove	= __devexit_p(tiadc_remove),
> > > > +	.suspend = adc_suspend,
> > > > +	.resume = adc_resume,
> > > >  };
> > > >
> > >
> > >
> > > Using the suspend/resume callbacks is deprecated, please use
> > dev_pm_ops.
> > > Same comment applies to the other two drivers.
> > 
> > Ok. I will make changes in all the 3 driver to use dev_pm_ops instead 
> > of suspend/resume callbacks.
> 
> Probably you might need to protect suspend/resumes with CONFIG_PM. 

Yes, I will add that as well.

Regards,
Rachna

RE: [PATCH v3 4/5] IIO : ADC: tiadc: Add support of TI's ADC driver

[Patil, Rachna]

On Thu, Sep 13, 2012 at 17:43:30, Jonathan Cameron wrote:
> On 13/09/12 11:40, Patil, Rachna wrote:
> > This patch adds support for TI's ADC driver.
> > This is a multifunctional device.
> > Analog input lines are provided on which voltage measurements can be 
> > carried out.
> > You can have upto 8 input lines.
> >
> > Signed-off-by: Patil, Rachna <rachna@ti.com>
> 
> There's a little fuzz in applying this due to other drivers that have gone in recently.
> 
> Actually this is going to be 'interesting' to merge. Dmitry, Samuel thoughts on who takes this one and how?  Maybe this is a case for a 'special' branch pulled into more than one tree?
> 
> 
> One minor thing inline.  I have an aversion to dynamic allocation of
> things that are then constant.
> 
> Also the module name is simply ti_adc. Does seem a little 'vague'
> given the range of ADC's TI makes :)  Perhaps keep the reference
> to the tsc in there?  Personally I'd have preferred the whole thing
> being named after a particular part number (any one it support would
> do) to avoid a clash in future with a new touch screen adc from TI.
> Bit late for that though I guess ;)

Yes, true.
TI definitely might come up with more IP's of this type.
This IP(TSC / ADC) is present on AM335x. If necessary we can rename the driver to ti_am335x_XXX.

> 
> Jonathan
> > ---
> > Changes in v2:
> > 	Addressed review comments from Matthias Kaehlcke
> >
> > Changes in v3:
> > 	Addressed review comments from Jonathan Cameron.
> > 	Added comments, new line appropriately.
> >
> >   drivers/iio/adc/Kconfig              |    7 +
> >   drivers/iio/adc/Makefile             |    1 +
> >   drivers/iio/adc/ti_adc.c             |  223 ++++++++++++++++++++++++++++++++++
> >   drivers/mfd/ti_tscadc.c              |   18 +++-
> >   include/linux/mfd/ti_tscadc.h        |    9 ++-
> >   include/linux/platform_data/ti_adc.h |   14 ++
> >   6 files changed, 270 insertions(+), 2 deletions(-)
> >   create mode 100644 drivers/iio/adc/ti_adc.c
> >   create mode 100644 include/linux/platform_data/ti_adc.h
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 8a78b4f..ad32df8 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -22,4 +22,11 @@ config AT91_ADC
> >   	help
> >   	  Say yes here to build support for Atmel AT91 ADC.
> >
> > +config TI_ADC
> > +	tristate "TI's ADC driver"
> > +	depends on ARCH_OMAP2PLUS
> > +	help
> > +	  Say yes here to build support for Texas Instruments ADC
> > +	  driver which is also a MFD client.
> > +
> >   endmenu
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index 52eec25..a930cee 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -4,3 +4,4 @@
> >
> >   obj-$(CONFIG_AD7266) += ad7266.o
> >   obj-$(CONFIG_AT91_ADC) += at91_adc.o
> > +obj-$(CONFIG_TI_ADC) += ti_adc.o
> > diff --git a/drivers/iio/adc/ti_adc.c b/drivers/iio/adc/ti_adc.c
> > new file mode 100644
> > index 0000000..56f8af2
> > --- /dev/null
> > +++ b/drivers/iio/adc/ti_adc.c
> > @@ -0,0 +1,223 @@
> > +/*
> > + * TI ADC MFD driver
> > + *
> > + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
> > + *
> > + * 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/init.h>
> > +#include <linux/kernel.h>
> > +#include <linux/err.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/io.h>
> > +#include <linux/iio/iio.h>
> > +
> > +#include <linux/mfd/ti_tscadc.h>
> > +#include <linux/platform_data/ti_adc.h>
> > +
> > +struct adc_device {
> > +	struct ti_tscadc_dev *mfd_tscadc;
> > +	struct iio_dev *idev;
> > +	int channels;
> > +};
> > +
> > +static unsigned int adc_readl(struct adc_device *adc, unsigned int reg)
> > +{
> > +	return readl(adc->mfd_tscadc->tscadc_base + reg);
> > +}
> > +
> > +static void adc_writel(struct adc_device *adc, unsigned int reg,
> > +					unsigned int val)
> > +{
> > +	writel(val, adc->mfd_tscadc->tscadc_base + reg);
> > +}
> > +
> > +static void adc_step_config(struct adc_device *adc_dev)
> > +{
> > +	unsigned int    stepconfig;
> > +	int i, channels = 0, steps;
> > +
> > +	/*
> > +	 * There are 16 configurable steps and 8 analog input
> > +	 * lines available which are shared between Touchscreen and ADC.
> > +	 *
> > +	 * Steps backwards i.e. from 16 towards 0 are used by ADC
> > +	 * depending on number of input lines needed.
> > +	 * Channel would represent which analog input
> > +	 * needs to be given to ADC to digitalize data.
> > +	 */
> > +
> > +	steps = TOTAL_STEPS - adc_dev->channels;
> > +	channels = TOTAL_CHANNELS - adc_dev->channels;
> > +
> > +	stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
> > +
> > +	for (i = (steps + 1); i <= TOTAL_STEPS; i++) {
> > +		adc_writel(adc_dev, REG_STEPCONFIG(i),
> > +				stepconfig | STEPCONFIG_INP(channels));
> > +		adc_writel(adc_dev, REG_STEPDELAY(i),
> > +				STEPCONFIG_OPENDLY);
> > +		channels++;
> > +	}
> > +	adc_writel(adc_dev, REG_SE, STPENB_STEPENB);
> > +}
> > +
> > +static int tiadc_channel_init(struct iio_dev *idev, int channels)
> > +{
> > +	struct iio_chan_spec *chan_array;
> > +	int i;
> > +
> > +	idev->num_channels = channels;
> > +	chan_array = kcalloc(idev->num_channels, sizeof(struct iio_chan_spec),
> > +					GFP_KERNEL);
> > +
> > +	if (chan_array == NULL)
> > +		return -ENOMEM;
> > +
> Minor point, but I'd be tempted to do this as a static const array and 
> then just set num_channels appropriately.  Still it's such a simple 
> driver that I'm not that fussed.

I looked at some other drivers, they seem to be doing the same way.
I would like to go with the existing convention.

> > +	for (i = 0; i < (idev->num_channels); i++) {
> > +		struct iio_chan_spec *chan = chan_array + i;
> > +		chan->type = IIO_VOLTAGE;
> > +		chan->indexed = 1;
> > +		chan->channel = i;
> > +		chan->info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT;
> > +	}
> > +
> > +	idev->channels = chan_array;
> > +
> > +	return idev->num_channels;
> > +}
> > +

Regards,
Rachna

RE: [PATCH v3 4/5] IIO : ADC: tiadc: Add support of TI's ADC driver

[Patil, Rachna]

On Thu, Sep 13, 2012 at 18:21:19, Lars-Peter Clausen wrote:
> On 09/13/2012 12:40 PM, Patil, Rachna wrote:
> > This patch adds support for TI's ADC driver.
> > This is a multifunctional device.
> > Analog input lines are provided on which voltage measurements can be 
> > carried out.
> > You can have upto 8 input lines.
> > 
> 
> Hi,
> 
> couple of minor issues inline.

Hi,

Please find my comments inline.

> 
> > Signed-off-by: Patil, Rachna <rachna@ti.com>
> > ---
> > Changes in v2:
> > 	Addressed review comments from Matthias Kaehlcke
> > 
> > Changes in v3:
> > 	Addressed review comments from Jonathan Cameron.
> > 	Added comments, new line appropriately.
> > 
> >  drivers/iio/adc/Kconfig              |    7 +
> >  drivers/iio/adc/Makefile             |    1 +
> >  drivers/iio/adc/ti_adc.c             |  223 ++++++++++++++++++++++++++++++++++
> >  drivers/mfd/ti_tscadc.c              |   18 +++-
> >  include/linux/mfd/ti_tscadc.h        |    9 ++-
> >  include/linux/platform_data/ti_adc.h |   14 ++
> >  6 files changed, 270 insertions(+), 2 deletions(-)  create mode 
> > 100644 drivers/iio/adc/ti_adc.c  create mode 100644 
> > include/linux/platform_data/ti_adc.h
> > 
> [...]
> > +
> > +struct adc_device {
> 
> Not really an issue, but I'd use a consistent function/struct prefix.
> Currently you use both "adc" and "tiadc"

Ok. I will update this.

> 
> > +	struct ti_tscadc_dev *mfd_tscadc;
> > +	struct iio_dev *idev;
> 
> idev is used only once in the remove callback. But you can get a pointer to it easily using platform_get_drvdata. So I'd remove it from the adc_device struct.

Ok. I will remove this.

> 
> > +	int channels;
> > +};
> > +
> > +static unsigned int adc_readl(struct adc_device *adc, unsigned int 
> > +reg) {
> > +	return readl(adc->mfd_tscadc->tscadc_base + reg); }
> > +
> > +static void adc_writel(struct adc_device *adc, unsigned int reg,
> > +					unsigned int val)
> > +{
> > +	writel(val, adc->mfd_tscadc->tscadc_base + reg); }
> > +
> > +static void adc_step_config(struct adc_device *adc_dev) {
> > +	unsigned int    stepconfig;
> 
> extra whitespace

Ok. I will remove this.

> 
> > +	int i, channels = 0, steps;
> > +
> > +	/*
> > +	 * There are 16 configurable steps and 8 analog input
> > +	 * lines available which are shared between Touchscreen and ADC.
> > +	 *
> > +	 * Steps backwards i.e. from 16 towards 0 are used by ADC
> > +	 * depending on number of input lines needed.
> > +	 * Channel would represent which analog input
> > +	 * needs to be given to ADC to digitalize data.
> > +	 */
> > +
> > +	steps = TOTAL_STEPS - adc_dev->channels;
> > +	channels = TOTAL_CHANNELS - adc_dev->channels;
> > +
> > +	stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
> > +
> > +	for (i = (steps + 1); i <= TOTAL_STEPS; i++) {
> > +		adc_writel(adc_dev, REG_STEPCONFIG(i),
> > +				stepconfig | STEPCONFIG_INP(channels));
> > +		adc_writel(adc_dev, REG_STEPDELAY(i),
> > +				STEPCONFIG_OPENDLY);
> > +		channels++;
> > +	}
> > +	adc_writel(adc_dev, REG_SE, STPENB_STEPENB); }
> > +
> > [...]
> > +
> > +static const struct iio_info tiadc_info = {
> > +	.read_raw = &tiadc_read_raw,
> 
>     .driver_module = THIS_MODULE,
> 
> > +};
> > +
> > +static int __devinit tiadc_probe(struct platform_device *pdev) {
> > +	struct iio_dev		*idev;
> 
> For consistency with other drivers please rename idev to indio_dev throughout the driver.

Ok. I will rename this.

> 
> > +	int			err;
> > +	struct adc_device	*adc_dev;
> > +	struct ti_tscadc_dev	*tscadc_dev = pdev->dev.platform_data;
> > +	struct mfd_tscadc_board	*pdata;
> > +
> > +	pdata = (struct mfd_tscadc_board *)tscadc_dev->dev->platform_data;
> 
> The cast should not be necessary.

True,
I will correct this.

> 
> > +	if (!pdata || !pdata->adc_init) {
> > +		dev_err(tscadc_dev->dev, "Could not find platform data\n");
> 
> I'd still use pdev->dev for the device parameter here. Seeing a message printed by this driver for another device might be confusing.

Yes, I will correct this.

> 
> > +		return -EINVAL;
> > +	}
> > +
> > +	idev = iio_device_alloc(sizeof(struct adc_device));
> > +	if (idev == NULL) {
> > +		dev_err(&pdev->dev, "failed to allocate iio device.\n");
> > +		err = -ENOMEM;
> > +		goto err_ret;
> > +	}
> > +	adc_dev = iio_priv(idev);
> > +
> > +	tscadc_dev->adc = adc_dev;
> 
> This there any reason why you need to store a pointer to the adc struct in the mfd struct? Is it going to be used outside of the adc driver? Currently it is, as far as I can see, only used in the remove callback and suspend/resume handlers. But there you can use iio_priv just as easily to get the pointer to the adc device struct and it certainly will be also be cleaner to do it that way.

Yes, currently it is used only in suspend/resume.
I will drop this line.

> 
> > +	adc_dev->mfd_tscadc = tscadc_dev;
> > +	adc_dev->idev = idev;
> > +	adc_dev->channels = pdata->adc_init->adc_channels;
> > +
> > +	idev->dev.parent = &pdev->dev;
> > +	idev->name = dev_name(&pdev->dev);
> > +	idev->modes = INDIO_DIRECT_MODE;
> > +	idev->info = &tiadc_info;
> > +
> > +	adc_step_config(adc_dev);
> > +
> > +	err = tiadc_channel_init(idev, adc_dev->channels);
> > +	if (err < 0)
> > +		goto err_free_device;
> > +
> > +	err = iio_device_register(idev);
> > +	if (err)
> > +		goto err_free_channels;
> > +
> > +	dev_info(&pdev->dev, "attached adc driver\n");
> 
> I'd remove that line, it's just noise.

Ok. I will drop this.

> 
> > +	platform_set_drvdata(pdev, idev);
> > +
> > +	return 0;
> > +
> > +err_free_channels:
> > +	tiadc_channels_remove(idev);
> > +err_free_device:
> > +	iio_device_free(idev);
> > +err_ret:
> > +	return err;
> > +}
> > +
> 
> [...]
> > diff --git a/include/linux/platform_data/ti_adc.h 
> > b/include/linux/platform_data/ti_adc.h
> > new file mode 100644
> > index 0000000..5a89f1d
> > --- /dev/null
> > +++ b/include/linux/platform_data/ti_adc.h
> > @@ -0,0 +1,14 @@
> > +#ifndef __LINUX_TI_ADC_H
> > +#define __LINUX_TI_ADC_H
> > +
> > +/**
> > + * struct adc_data	ADC Input information
> > + * @adc_channels:	Number of analog inputs
> > + *			available for ADC.
> > + */
> > +
> > +struct adc_data {
> 
> The struct name might be a bit to generic.
> 
> > +	int adc_channels;
> 
> It does not really matter, but considering that there hardly ever going to be a negative number of channels I'd make this unsigned.

Ok. I will update this.

Regards,
Rachna

Re: [PATCH v3 4/5] IIO : ADC: tiadc: Add support of TI's ADC driver

[Jonathan Cameron]

On 14/09/12 07:00, Patil, Rachna wrote:
> On Thu, Sep 13, 2012 at 17:43:30, Jonathan Cameron wrote:
>> On 13/09/12 11:40, Patil, Rachna wrote:
>>> This patch adds support for TI's ADC driver.
>>> This is a multifunctional device.
>>> Analog input lines are provided on which voltage measurements can be
>>> carried out.
>>> You can have upto 8 input lines.
>>>
>>> Signed-off-by: Patil, Rachna <rachna@ti.com>
>>
>> There's a little fuzz in applying this due to other drivers that have gone in recently.
>>
>> Actually this is going to be 'interesting' to merge. Dmitry, Samuel thoughts on who takes this one and how?  Maybe this is a case for a 'special' branch pulled into more than one tree?
>>
>>
>> One minor thing inline.  I have an aversion to dynamic allocation of
>> things that are then constant.
>>
>> Also the module name is simply ti_adc. Does seem a little 'vague'
>> given the range of ADC's TI makes :)  Perhaps keep the reference
>> to the tsc in there?  Personally I'd have preferred the whole thing
>> being named after a particular part number (any one it support would
>> do) to avoid a clash in future with a new touch screen adc from TI.
>> Bit late for that though I guess ;)
>
> Yes, true.
> TI definitely might come up with more IP's of this type.
> This IP(TSC / ADC) is present on AM335x. If necessary we can rename the driver to ti_am335x_XXX.
I'd be in favour of doing this now rather than when the problem presents 
itself.  Anyone mind?
>
>>
>> Jonathan
>>> ---
>>> Changes in v2:
>>> 	Addressed review comments from Matthias Kaehlcke
>>>
>>> Changes in v3:
>>> 	Addressed review comments from Jonathan Cameron.
>>> 	Added comments, new line appropriately.
>>>
>>>    drivers/iio/adc/Kconfig              |    7 +
>>>    drivers/iio/adc/Makefile             |    1 +
>>>    drivers/iio/adc/ti_adc.c             |  223 ++++++++++++++++++++++++++++++++++
>>>    drivers/mfd/ti_tscadc.c              |   18 +++-
>>>    include/linux/mfd/ti_tscadc.h        |    9 ++-
>>>    include/linux/platform_data/ti_adc.h |   14 ++
>>>    6 files changed, 270 insertions(+), 2 deletions(-)
>>>    create mode 100644 drivers/iio/adc/ti_adc.c
>>>    create mode 100644 include/linux/platform_data/ti_adc.h
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 8a78b4f..ad32df8 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -22,4 +22,11 @@ config AT91_ADC
>>>    	help
>>>    	  Say yes here to build support for Atmel AT91 ADC.
>>>
>>> +config TI_ADC
>>> +	tristate "TI's ADC driver"
>>> +	depends on ARCH_OMAP2PLUS
>>> +	help
>>> +	  Say yes here to build support for Texas Instruments ADC
>>> +	  driver which is also a MFD client.
>>> +
>>>    endmenu
>>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>>> index 52eec25..a930cee 100644
>>> --- a/drivers/iio/adc/Makefile
>>> +++ b/drivers/iio/adc/Makefile
>>> @@ -4,3 +4,4 @@
>>>
>>>    obj-$(CONFIG_AD7266) += ad7266.o
>>>    obj-$(CONFIG_AT91_ADC) += at91_adc.o
>>> +obj-$(CONFIG_TI_ADC) += ti_adc.o
>>> diff --git a/drivers/iio/adc/ti_adc.c b/drivers/iio/adc/ti_adc.c
>>> new file mode 100644
>>> index 0000000..56f8af2
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/ti_adc.c
>>> @@ -0,0 +1,223 @@
>>> +/*
>>> + * TI ADC MFD driver
>>> + *
>>> + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
>>> + *
>>> + * 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/init.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/err.h>
>>> +#include <linux/module.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/io.h>
>>> +#include <linux/iio/iio.h>
>>> +
>>> +#include <linux/mfd/ti_tscadc.h>
>>> +#include <linux/platform_data/ti_adc.h>
>>> +
>>> +struct adc_device {
>>> +	struct ti_tscadc_dev *mfd_tscadc;
>>> +	struct iio_dev *idev;
>>> +	int channels;
>>> +};
>>> +
>>> +static unsigned int adc_readl(struct adc_device *adc, unsigned int reg)
>>> +{
>>> +	return readl(adc->mfd_tscadc->tscadc_base + reg);
>>> +}
>>> +
>>> +static void adc_writel(struct adc_device *adc, unsigned int reg,
>>> +					unsigned int val)
>>> +{
>>> +	writel(val, adc->mfd_tscadc->tscadc_base + reg);
>>> +}
>>> +
>>> +static void adc_step_config(struct adc_device *adc_dev)
>>> +{
>>> +	unsigned int    stepconfig;
>>> +	int i, channels = 0, steps;
>>> +
>>> +	/*
>>> +	 * There are 16 configurable steps and 8 analog input
>>> +	 * lines available which are shared between Touchscreen and ADC.
>>> +	 *
>>> +	 * Steps backwards i.e. from 16 towards 0 are used by ADC
>>> +	 * depending on number of input lines needed.
>>> +	 * Channel would represent which analog input
>>> +	 * needs to be given to ADC to digitalize data.
>>> +	 */
>>> +
>>> +	steps = TOTAL_STEPS - adc_dev->channels;
>>> +	channels = TOTAL_CHANNELS - adc_dev->channels;
>>> +
>>> +	stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
>>> +
>>> +	for (i = (steps + 1); i <= TOTAL_STEPS; i++) {
>>> +		adc_writel(adc_dev, REG_STEPCONFIG(i),
>>> +				stepconfig | STEPCONFIG_INP(channels));
>>> +		adc_writel(adc_dev, REG_STEPDELAY(i),
>>> +				STEPCONFIG_OPENDLY);
>>> +		channels++;
>>> +	}
>>> +	adc_writel(adc_dev, REG_SE, STPENB_STEPENB);
>>> +}
>>> +
>>> +static int tiadc_channel_init(struct iio_dev *idev, int channels)
>>> +{
>>> +	struct iio_chan_spec *chan_array;
>>> +	int i;
>>> +
>>> +	idev->num_channels = channels;
>>> +	chan_array = kcalloc(idev->num_channels, sizeof(struct iio_chan_spec),
>>> +					GFP_KERNEL);
>>> +
>>> +	if (chan_array == NULL)
>>> +		return -ENOMEM;
>>> +
>> Minor point, but I'd be tempted to do this as a static const array and
>> then just set num_channels appropriately.  Still it's such a simple
>> driver that I'm not that fussed.
>
> I looked at some other drivers, they seem to be doing the same way.
> I would like to go with the existing convention.
>
>>> +	for (i = 0; i < (idev->num_channels); i++) {
>>> +		struct iio_chan_spec *chan = chan_array + i;
>>> +		chan->type = IIO_VOLTAGE;
>>> +		chan->indexed = 1;
>>> +		chan->channel = i;
>>> +		chan->info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT;
>>> +	}
>>> +
>>> +	idev->channels = chan_array;
>>> +
>>> +	return idev->num_channels;
>>> +}
>>> +
>
> Regards,
> Rachna
>

RE: [PATCH v3 4/5] IIO : ADC: tiadc: Add support of TI's ADC driver

[Patil, Rachna]

On Fri, Sep 14, 2012 at 13:50:57, Jonathan Cameron wrote:
> On 14/09/12 07:00, Patil, Rachna wrote:
> > On Thu, Sep 13, 2012 at 17:43:30, Jonathan Cameron wrote:
> >> On 13/09/12 11:40, Patil, Rachna wrote:
> >>> This patch adds support for TI's ADC driver.
> >>> This is a multifunctional device.
> >>> Analog input lines are provided on which voltage measurements can be 
> >>> carried out.
> >>> You can have upto 8 input lines.
> >>>
> >>> Signed-off-by: Patil, Rachna <rachna@ti.com>
> >>
> >> There's a little fuzz in applying this due to other drivers that have gone in recently.
> >>
> >> Actually this is going to be 'interesting' to merge. Dmitry, Samuel thoughts on who takes this one and how?  Maybe this is a case for a 'special' branch pulled into more than one tree?
> >>
> >>
> >> One minor thing inline.  I have an aversion to dynamic allocation of 
> >> things that are then constant.
> >>
> >> Also the module name is simply ti_adc. Does seem a little 'vague'
> >> given the range of ADC's TI makes :)  Perhaps keep the reference to 
> >> the tsc in there?  Personally I'd have preferred the whole thing 
> >> being named after a particular part number (any one it support would
> >> do) to avoid a clash in future with a new touch screen adc from TI.
> >> Bit late for that though I guess ;)
> >
> > Yes, true.
> > TI definitely might come up with more IP's of this type.
> > This IP(TSC / ADC) is present on AM335x. If necessary we can rename the driver to ti_am335x_XXX.
> I'd be in favour of doing this now rather than when the problem presents itself.  Anyone mind?

I will also rename the touchscreen and the MFD driver to reflect the same.

<SNIP>

Regards,
Rachna