[PATCH v3 2/4] iio: adc: add STMPE ADC driver using IIO framework

[PATCH v3 2/4] iio: adc: add STMPE ADC driver using IIO framework

[Philippe Schenker]

From: Stefan Agner <stefan@agner.ch>

This adds an ADC driver for the STMPE device using the industrial
input/output interface. The driver supports raw reading of values.
The driver depends on the MFD STMPE driver. If the touchscreen
block is enabled too, only four of the 8 ADC channels are available.

Signed-off-by: Stefan Agner <stefan@agner.ch>
Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
---

Changes in v3:
 - Undo ADC-settings related code-deletions in stmpe-ts.c that the code is
   backwards-compatible to older devicetrees.

Changes in v2:
 - Code formatting
 - Move code to setup ADC to MFD device, as it is used by both drivers
   adc and touchscreen
 - Removed unused includes
 - Defined the macro STMPE_START_ONE_TEMP_CONV with other macros.
 - Added new macro that defines the channel of the temperature sensor.
   Took new name for STMPE_MAX_ADC->STMPE_ADC_LAST_NR and used it throughout
   the code for better readability.
 - Added mutex_unlock where missing.

 drivers/iio/adc/Kconfig              |   7 +
 drivers/iio/adc/Makefile             |   1 +
 drivers/iio/adc/stmpe-adc.c          | 326 +++++++++++++++++++++++++++
 drivers/input/touchscreen/stmpe-ts.c |  36 +--
 drivers/mfd/Kconfig                  |   3 +-
 drivers/mfd/stmpe.c                  |  81 +++++++
 include/linux/mfd/stmpe.h            |   9 +
 7 files changed, 437 insertions(+), 26 deletions(-)
 create mode 100644 drivers/iio/adc/stmpe-adc.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index a52fea8749a9..224f2067494d 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -734,6 +734,13 @@ config STM32_DFSDM_ADC
 	  This driver can also be built as a module.  If so, the module
 	  will be called stm32-dfsdm-adc.
 
+config STMPE_ADC
+	tristate "STMicroelectronics STMPE ADC driver"
+	depends on OF && MFD_STMPE
+	help
+	  Say yes here to build support for ST Microelectronics STMPE
+	  built-in ADC block (stmpe811).
+
 config STX104
 	tristate "Apex Embedded Systems STX104 driver"
 	depends on PC104 && X86
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index a6e6a0b659e2..cba889c30bf9 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o
 obj-$(CONFIG_STM32_ADC) += stm32-adc.o
 obj-$(CONFIG_STM32_DFSDM_CORE) += stm32-dfsdm-core.o
 obj-$(CONFIG_STM32_DFSDM_ADC) += stm32-dfsdm-adc.o
+obj-$(CONFIG_STMPE_ADC) += stmpe-adc.o
 obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
 obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
 obj-$(CONFIG_TI_ADC084S021) += ti-adc084s021.o
diff --git a/drivers/iio/adc/stmpe-adc.c b/drivers/iio/adc/stmpe-adc.c
new file mode 100644
index 000000000000..bea3f3c27bb5
--- /dev/null
+++ b/drivers/iio/adc/stmpe-adc.c
@@ -0,0 +1,326 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  STMicroelectronics STMPE811 IIO ADC Driver
+ *
+ *  4 channel, 10/12-bit ADC
+ *
+ *  Copyright (C) 2013-2018 Toradex AG <stefan.agner@toradex.com>
+ */
+
+#include <linux/completion.h>
+#include <linux/err.h>
+#include <linux/iio/iio.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/stmpe.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#define STMPE_REG_INT_STA		0x0B
+#define STMPE_REG_ADC_INT_EN		0x0E
+#define STMPE_REG_ADC_INT_STA		0x0F
+
+#define STMPE_REG_ADC_CTRL1		0x20
+#define STMPE_REG_ADC_CTRL2		0x21
+#define STMPE_REG_ADC_CAPT		0x22
+#define STMPE_REG_ADC_DATA_CH(channel)	(0x30 + 2 * (channel))
+
+#define STMPE_REG_TEMP_CTRL		0x60
+#define STMPE_TEMP_CTRL_ENABLE		BIT(0)
+#define STMPE_TEMP_CTRL_ACQ		BIT(1)
+#define STMPE_TEMP_CTRL_THRES_EN	BIT(3)
+#define STMPE_START_ONE_TEMP_CONV	(STMPE_TEMP_CTRL_ENABLE | \
+					STMPE_TEMP_CTRL_ACQ | \
+					STMPE_TEMP_CTRL_THRES_EN)
+#define STMPE_REG_TEMP_DATA		0x61
+#define STMPE_REG_TEMP_TH		0x63
+#define STMPE_ADC_LAST_NR		7
+#define STMPE_TEMP_CHANNEL		(STMPE_ADC_LAST_NR + 1)
+
+#define STMPE_ADC_CH(channel)		((1 << (channel)) & 0xff)
+
+#define STMPE_ADC_TIMEOUT		msecs_to_jiffies(1000)
+
+struct stmpe_adc {
+	struct stmpe *stmpe;
+	struct clk *clk;
+	struct device *dev;
+
+	/* We are allocating plus one for the temperature channel */
+	struct iio_chan_spec stmpe_adc_iio_channels[STMPE_ADC_LAST_NR + 2];
+
+	struct completion completion;
+
+	u8 channel;
+	u32 value;
+};
+
+static int stmpe_read_raw(struct iio_dev *indio_dev,
+			  struct iio_chan_spec const *chan,
+			  int *val,
+			  int *val2,
+			  long mask)
+{
+	struct stmpe_adc *info = iio_priv(indio_dev);
+	long ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_PROCESSED:
+
+		mutex_lock(&indio_dev->mlock);
+
+		info->channel = (u8)chan->channel;
+
+		switch (chan->type) {
+		case IIO_VOLTAGE:
+			if (info->channel > STMPE_ADC_LAST_NR) {
+				mutex_unlock(&indio_dev->mlock);
+				return -EINVAL;
+			}
+
+			stmpe_reg_write(info->stmpe, STMPE_REG_ADC_INT_EN,
+					STMPE_ADC_CH(info->channel));
+
+			stmpe_reg_write(info->stmpe, STMPE_REG_ADC_CAPT,
+					STMPE_ADC_CH(info->channel));
+
+			*val = info->value;
+			break;
+
+		case IIO_TEMP:
+			if (info->channel != STMPE_TEMP_CHANNEL) {
+				mutex_unlock(&indio_dev->mlock);
+				return -EINVAL;
+			}
+
+			stmpe_reg_write(info->stmpe, STMPE_REG_TEMP_CTRL,
+					STMPE_START_ONE_TEMP_CONV);
+			break;
+		default:
+			mutex_unlock(&indio_dev->mlock);
+			return -EINVAL;
+		}
+
+		ret = wait_for_completion_interruptible_timeout
+			(&info->completion, STMPE_ADC_TIMEOUT);
+
+		if (ret <= 0) {
+			mutex_unlock(&indio_dev->mlock);
+			if (ret == 0)
+				return -ETIMEDOUT;
+			else
+				return ret;
+		}
+
+		switch (chan->type) {
+		case IIO_VOLTAGE:
+			*val = info->value;
+			break;
+
+		case IIO_TEMP:
+			/*
+			 * absolute temp = +V3.3 * value /7.51 [K]
+			 * scale to [milli °C]
+			 */
+			*val = ((449960l * info->value) / 1024l) - 273150;
+			break;
+		default:
+			break;
+		}
+
+		mutex_unlock(&indio_dev->mlock);
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		*val = 3300;
+		*val2 = info->stmpe->mod_12b ? 12 : 10;
+		return IIO_VAL_FRACTIONAL_LOG2;
+
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static irqreturn_t stmpe_adc_isr(int irq, void *dev_id)
+{
+	struct stmpe_adc *info = (struct stmpe_adc *)dev_id;
+	u8 data[2];
+
+	if (info->channel > STMPE_TEMP_CHANNEL)
+		return IRQ_NONE;
+
+	if (info->channel <= STMPE_ADC_LAST_NR) {
+		int int_sta;
+
+		int_sta = stmpe_reg_read(info->stmpe, STMPE_REG_ADC_INT_STA);
+
+		/* Is the interrupt relevant */
+		if (!(int_sta & STMPE_ADC_CH(info->channel)))
+			return IRQ_NONE;
+
+		/* Read value */
+		stmpe_block_read(info->stmpe,
+			STMPE_REG_ADC_DATA_CH(info->channel), 2, data);
+
+		stmpe_reg_write(info->stmpe, STMPE_REG_ADC_INT_STA, int_sta);
+	} else if (info->channel == STMPE_TEMP_CHANNEL) {
+		/* Read value */
+		stmpe_block_read(info->stmpe, STMPE_REG_TEMP_DATA, 2, data);
+	}
+
+	info->value = ((u32)data[0] << 8) + data[1];
+	complete(&info->completion);
+
+	return IRQ_HANDLED;
+}
+
+static const struct iio_info stmpe_adc_iio_info = {
+	.read_raw = &stmpe_read_raw,
+};
+
+static void stmpe_adc_voltage_chan(struct iio_chan_spec *ics, int chan)
+{
+	ics->type = IIO_VOLTAGE;
+	ics->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
+	ics->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE);
+	ics->indexed = 1;
+	ics->channel = chan;
+}
+
+static void stmpe_adc_temp_chan(struct iio_chan_spec *ics, int chan)
+{
+	ics->type = IIO_TEMP;
+	ics->info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED);
+	ics->indexed = 1;
+	ics->channel = chan;
+}
+
+static int stmpe_adc_init_hw(struct stmpe_adc *adc)
+{
+	struct stmpe *stmpe = adc->stmpe;
+
+	/* use temp irq for each conversion completion */
+	stmpe_reg_write(stmpe, STMPE_REG_TEMP_TH, 0);
+	stmpe_reg_write(stmpe, STMPE_REG_TEMP_TH + 1, 0);
+
+	return 0;
+}
+
+static int stmpe_adc_probe(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev = NULL;
+	struct stmpe_adc *info = NULL;
+	struct device_node *np;
+	u32 norequest_mask = 0;
+	int irq_temp, irq_adc;
+	int num_chan = 0;
+	int i = 0;
+	int ret;
+
+	irq_adc = platform_get_irq_byname(pdev, "STMPE_ADC");
+	if (irq_adc < 0)
+		return irq_adc;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(struct stmpe_adc));
+	if (!indio_dev) {
+		dev_err(&pdev->dev, "failed allocating iio device\n");
+		return -ENOMEM;
+	}
+
+	info = iio_priv(indio_dev);
+
+	init_completion(&info->completion);
+	ret = devm_request_threaded_irq(&pdev->dev, irq_adc, NULL,
+					stmpe_adc_isr, IRQF_ONESHOT,
+					"stmpe-adc", info);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed requesting irq, irq = %d\n",
+				irq_adc);
+		return ret;
+	}
+
+	irq_temp = platform_get_irq_byname(pdev, "STMPE_TEMP_SENS");
+	if (irq_temp >= 0) {
+		ret = devm_request_threaded_irq(&pdev->dev, irq_temp, NULL,
+						stmpe_adc_isr, IRQF_ONESHOT,
+						"stmpe-adc", info);
+		if (ret < 0)
+			dev_warn(&pdev->dev, "failed requesting irq for"
+				 " temp sensor, irq = %d\n", irq_temp);
+	}
+
+	platform_set_drvdata(pdev, indio_dev);
+
+	indio_dev->name		= dev_name(&pdev->dev);
+	indio_dev->dev.parent	= &pdev->dev;
+	indio_dev->info		= &stmpe_adc_iio_info;
+	indio_dev->modes	= INDIO_DIRECT_MODE;
+
+	info->stmpe = dev_get_drvdata(pdev->dev.parent);
+
+	np = pdev->dev.of_node;
+
+	if (!np)
+		dev_err(&pdev->dev, "no device tree node found\n");
+
+	of_property_read_u32(np, "st,norequest-mask", &norequest_mask);
+
+	for_each_clear_bit(i, (unsigned long *) &norequest_mask,
+			   (STMPE_ADC_LAST_NR + 1)) {
+		stmpe_adc_voltage_chan(&info->stmpe_adc_iio_channels[num_chan], i);
+		num_chan++;
+	}
+	stmpe_adc_temp_chan(&info->stmpe_adc_iio_channels[num_chan], i);
+	num_chan++;
+	indio_dev->channels = info->stmpe_adc_iio_channels;
+	indio_dev->num_channels = num_chan;
+
+	ret = stmpe_adc_init_hw(info);
+	if (ret)
+		return ret;
+
+	return iio_device_register(indio_dev);
+}
+
+static int stmpe_adc_remove(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+	struct stmpe_adc *info = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	stmpe_disable(info->stmpe, STMPE_BLOCK_ADC);
+
+	return 0;
+}
+
+static int __maybe_unused stmpe_adc_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct stmpe_adc *info = iio_priv(indio_dev);
+
+	stmpe_adc_init_hw(info);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(stmpe_adc_pm_ops, NULL, stmpe_adc_resume);
+
+static struct platform_driver stmpe_adc_driver = {
+	.probe		= stmpe_adc_probe,
+	.remove		= stmpe_adc_remove,
+	.driver		= {
+		.name	= "stmpe-adc",
+		.pm	= &stmpe_adc_pm_ops,
+	},
+};
+
+module_platform_driver(stmpe_adc_driver);
+
+MODULE_AUTHOR("Stefan Agner <stefan.agner@toradex.com>");
+MODULE_DESCRIPTION("STMPEXXX ADC driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:stmpe-adc");
diff --git a/drivers/input/touchscreen/stmpe-ts.c b/drivers/input/touchscreen/stmpe-ts.c
index c5d9006588a2..81aa313d6e5a 100644
--- a/drivers/input/touchscreen/stmpe-ts.c
+++ b/drivers/input/touchscreen/stmpe-ts.c
@@ -30,8 +30,6 @@
  * with touchscreen controller
  */
 #define STMPE_REG_INT_STA		0x0B
-#define STMPE_REG_ADC_CTRL1		0x20
-#define STMPE_REG_ADC_CTRL2		0x21
 #define STMPE_REG_TSC_CTRL		0x40
 #define STMPE_REG_TSC_CFG		0x41
 #define STMPE_REG_FIFO_TH		0x4A
@@ -58,15 +56,6 @@
  * @idev: registered input device
  * @work: a work item used to scan the device
  * @dev: a pointer back to the MFD cell struct device*
- * @sample_time: ADC converstion time in number of clock.
- * (0 -> 36 clocks, 1 -> 44 clocks, 2 -> 56 clocks, 3 -> 64 clocks,
- * 4 -> 80 clocks, 5 -> 96 clocks, 6 -> 144 clocks),
- * recommended is 4.
- * @mod_12b: ADC Bit mode (0 -> 10bit ADC, 1 -> 12bit ADC)
- * @ref_sel: ADC reference source
- * (0 -> internal reference, 1 -> external reference)
- * @adc_freq: ADC Clock speed
- * (0 -> 1.625 MHz, 1 -> 3.25 MHz, 2 || 3 -> 6.5 MHz)
  * @ave_ctrl: Sample average control
  * (0 -> 1 sample, 1 -> 2 samples, 2 -> 4 samples, 3 -> 8 samples)
  * @touch_det_delay: Touch detect interrupt delay
@@ -88,10 +77,6 @@ struct stmpe_touch {
 	struct input_dev *idev;
 	struct delayed_work work;
 	struct device *dev;
-	u8 sample_time;
-	u8 mod_12b;
-	u8 ref_sel;
-	u8 adc_freq;
 	u8 ave_ctrl;
 	u8 touch_det_delay;
 	u8 settling;
@@ -176,7 +161,7 @@ static irqreturn_t stmpe_ts_handler(int irq, void *data)
 	input_report_key(ts->idev, BTN_TOUCH, 1);
 	input_sync(ts->idev);
 
-       /* flush the FIFO after we have read out our values. */
+	/* flush the FIFO after we have read out our values. */
 	__stmpe_reset_fifo(ts->stmpe);
 
 	/* reenable the tsc */
@@ -202,20 +187,21 @@ static int stmpe_init_hw(struct stmpe_touch *ts)
 		return ret;
 	}
 
-	adc_ctrl1 = STMPE_SAMPLE_TIME(ts->sample_time) |
-		    STMPE_MOD_12B(ts->mod_12b) | STMPE_REF_SEL(ts->ref_sel);
+	adc_ctrl1 = STMPE_SAMPLE_TIME(stmpe->sample_time) |
+		    STMPE_MOD_12B(stmpe->mod_12b) |
+		    STMPE_REF_SEL(stmpe->ref_sel);
 	adc_ctrl1_mask = STMPE_SAMPLE_TIME(0xff) | STMPE_MOD_12B(0xff) |
 			 STMPE_REF_SEL(0xff);
 
-	ret = stmpe_set_bits(stmpe, STMPE_REG_ADC_CTRL1,
+	ret = stmpe_set_bits(stmpe, STMPE811_REG_ADC_CTRL1,
 			adc_ctrl1_mask, adc_ctrl1);
 	if (ret) {
 		dev_err(dev, "Could not setup ADC\n");
 		return ret;
 	}
 
-	ret = stmpe_set_bits(stmpe, STMPE_REG_ADC_CTRL2,
-			STMPE_ADC_FREQ(0xff), STMPE_ADC_FREQ(ts->adc_freq));
+	ret = stmpe_set_bits(stmpe, STMPE811_REG_ADC_CTRL2,
+			STMPE_ADC_FREQ(0xff), STMPE_ADC_FREQ(stmpe->adc_freq));
 	if (ret) {
 		dev_err(dev, "Could not setup ADC\n");
 		return ret;
@@ -295,13 +281,13 @@ static void stmpe_ts_get_platform_info(struct platform_device *pdev,
 
 	if (np) {
 		if (!of_property_read_u32(np, "st,sample-time", &val))
-			ts->sample_time = val;
+			ts->stmpe->sample_time = val;
 		if (!of_property_read_u32(np, "st,mod-12b", &val))
-			ts->mod_12b = val;
+			ts->stmpe->mod_12b = val;
 		if (!of_property_read_u32(np, "st,ref-sel", &val))
-			ts->ref_sel = val;
+			ts->stmpe->ref_sel = val;
 		if (!of_property_read_u32(np, "st,adc-freq", &val))
-			ts->adc_freq = val;
+			ts->stmpe->adc_freq = val;
 		if (!of_property_read_u32(np, "st,ave-ctrl", &val))
 			ts->ave_ctrl = val;
 		if (!of_property_read_u32(np, "st,touch-det-delay", &val))
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 8c5dfdce4326..bba159e8eaa4 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1204,7 +1204,7 @@ config MFD_STMPE
 
 	  Currently supported devices are:
 
-		STMPE811: GPIO, Touchscreen
+		STMPE811: GPIO, Touchscreen, ADC
 		STMPE1601: GPIO, Keypad
 		STMPE1801: GPIO, Keypad
 		STMPE2401: GPIO, Keypad
@@ -1217,6 +1217,7 @@ config MFD_STMPE
 		GPIO: stmpe-gpio
 		Keypad: stmpe-keypad
 		Touchscreen: stmpe-ts
+		ADC: stmpe-adc
 
 menu "STMicroelectronics STMPE Interface Drivers"
 depends on MFD_STMPE
diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c
index 566caca4efd8..35390d1c2e64 100644
--- a/drivers/mfd/stmpe.c
+++ b/drivers/mfd/stmpe.c
@@ -463,6 +463,28 @@ static const struct mfd_cell stmpe_ts_cell = {
 	.num_resources	= ARRAY_SIZE(stmpe_ts_resources),
 };
 
+/*
+ * ADC (STMPE811)
+ */
+
+static struct resource stmpe_adc_resources[] = {
+	{
+		.name	= "STMPE_TEMP_SENS",
+		.flags	= IORESOURCE_IRQ,
+	},
+	{
+		.name	= "STMPE_ADC",
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
+static const struct mfd_cell stmpe_adc_cell = {
+	.name		= "stmpe-adc",
+	.of_compatible	= "st,stmpe-adc",
+	.resources	= stmpe_adc_resources,
+	.num_resources	= ARRAY_SIZE(stmpe_adc_resources),
+};
+
 /*
  * STMPE811 or STMPE610
  */
@@ -497,6 +519,11 @@ static struct stmpe_variant_block stmpe811_blocks[] = {
 		.irq	= STMPE811_IRQ_TOUCH_DET,
 		.block	= STMPE_BLOCK_TOUCHSCREEN,
 	},
+	{
+		.cell	= &stmpe_adc_cell,
+		.irq	= STMPE811_IRQ_TEMP_SENS,
+		.block	= STMPE_BLOCK_ADC,
+	},
 };
 
 static int stmpe811_enable(struct stmpe *stmpe, unsigned int blocks,
@@ -517,6 +544,44 @@ static int stmpe811_enable(struct stmpe *stmpe, unsigned int blocks,
 				enable ? 0 : mask);
 }
 
+static int stmpe811_init_adc(struct stmpe *stmpe)
+{
+	int ret;
+	u8 adc_ctrl1, adc_ctrl1_mask;
+
+	ret = stmpe_enable(stmpe, STMPE_BLOCK_ADC);
+	if (ret) {
+		dev_err(stmpe->dev, "Could not enable clock for ADC\n");
+		goto err_adc;
+	}
+
+	adc_ctrl1 = STMPE_SAMPLE_TIME(stmpe->sample_time) |
+		    STMPE_MOD_12B(stmpe->mod_12b) |
+		    STMPE_REF_SEL(stmpe->ref_sel);
+	adc_ctrl1_mask = STMPE_SAMPLE_TIME(0xff) | STMPE_MOD_12B(0xff) |
+			 STMPE_REF_SEL(0xff);
+
+	ret = stmpe_set_bits(stmpe, STMPE811_REG_ADC_CTRL1,
+			adc_ctrl1_mask, adc_ctrl1);
+	if (ret) {
+		dev_err(stmpe->dev, "Could not setup ADC\n");
+		goto err_adc;
+	}
+
+	ret = stmpe_set_bits(stmpe, STMPE811_REG_ADC_CTRL2,
+			STMPE_ADC_FREQ(0xff), STMPE_ADC_FREQ(stmpe->adc_freq));
+	if (ret) {
+		dev_err(stmpe->dev, "Could not setup ADC\n");
+		goto err_adc;
+	}
+
+	return 0;
+err_adc:
+	stmpe_disable(stmpe, STMPE_BLOCK_ADC);
+
+	return ret;
+}
+
 static int stmpe811_get_altfunc(struct stmpe *stmpe, enum stmpe_block block)
 {
 	/* 0 for touchscreen, 1 for GPIO */
@@ -1235,6 +1300,12 @@ static int stmpe_chip_init(struct stmpe *stmpe)
 			return ret;
 	}
 
+	if (id == STMPE811_ID) {
+		ret = stmpe811_init_adc(stmpe);
+		if (ret)
+			return ret;
+	}
+
 	return stmpe_reg_write(stmpe, stmpe->regs[STMPE_IDX_ICR_LSB], icr);
 }
 
@@ -1325,6 +1396,7 @@ int stmpe_probe(struct stmpe_client_info *ci, enum stmpe_partnum partnum)
 	struct device_node *np = ci->dev->of_node;
 	struct stmpe *stmpe;
 	int ret;
+	u32 val;
 
 	pdata = devm_kzalloc(ci->dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
@@ -1342,6 +1414,15 @@ int stmpe_probe(struct stmpe_client_info *ci, enum stmpe_partnum partnum)
 	mutex_init(&stmpe->irq_lock);
 	mutex_init(&stmpe->lock);
 
+	if (!of_property_read_u32(np, "st,sample-time", &val))
+		stmpe->sample_time = val;
+	if (!of_property_read_u32(np, "st,mod-12b", &val))
+		stmpe->mod_12b = val;
+	if (!of_property_read_u32(np, "st,ref-sel", &val))
+		stmpe->ref_sel = val;
+	if (!of_property_read_u32(np, "st,adc-freq", &val))
+		stmpe->adc_freq = val;
+
 	stmpe->dev = ci->dev;
 	stmpe->client = ci->client;
 	stmpe->pdata = pdata;
diff --git a/include/linux/mfd/stmpe.h b/include/linux/mfd/stmpe.h
index c0353f6431f9..86dca9e9880a 100644
--- a/include/linux/mfd/stmpe.h
+++ b/include/linux/mfd/stmpe.h
@@ -21,6 +21,9 @@
 #define STMPE_I_DRIVE(x)	(x & 0x1)
 #define STMPE_OP_MODE(x)	((x & 0x7) << 1)
 
+#define STMPE811_REG_ADC_CTRL1	0x20
+#define STMPE811_REG_ADC_CTRL2	0x21
+
 struct device;
 struct regulator;
 
@@ -134,6 +137,12 @@ struct stmpe {
 	u8 ier[2];
 	u8 oldier[2];
 	struct stmpe_platform_data *pdata;
+
+	/* For devices that use an ADC */
+	u8 sample_time;
+	u8 mod_12b;
+	u8 ref_sel;
+	u8 adc_freq;
 };
 
 extern int stmpe_reg_write(struct stmpe *stmpe, u8 reg, u8 data);
-- 
2.19.1

Re: [PATCH v3 2/4] iio: adc: add STMPE ADC driver using IIO framework

[Jonathan Cameron]

On Fri, 23 Nov 2018 15:24:09 +0100
Philippe Schenker <dev@pschenker.ch> wrote:

> From: Stefan Agner <stefan@agner.ch>
>=20
> This adds an ADC driver for the STMPE device using the industrial
> input/output interface. The driver supports raw reading of values.
> The driver depends on the MFD STMPE driver. If the touchscreen
> block is enabled too, only four of the 8 ADC channels are available.
>=20
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>

Given the dt related refactoring in here is mixed up somewhat with the
ADC driver, I would like to see that as a precursor patch.

Another area that I think needs a rethink is balance between
enable and disable.  The enable is in the mfd call, but the disable
in the iio driver remove which isn't right.  Not immediately
sure what we do about that, but it should be one or the other.

I'd previously missed your use of the IIO core mlock.  Please don't
use that in a driver directly.  It has very carefully defined
locking semantics which aren't even in use here.  A locally defined
lock with well documented scope is much better.  We 'used'
to abuse mlock for this and have spent years slowly unwinding that.

Thanks,

Jonathan

> ---
>=20
> Changes in v3:
>  - Undo ADC-settings related code-deletions in stmpe-ts.c that the code is
>    backwards-compatible to older devicetrees.
>=20
> Changes in v2:
>  - Code formatting
>  - Move code to setup ADC to MFD device, as it is used by both drivers
>    adc and touchscreen
>  - Removed unused includes
>  - Defined the macro STMPE_START_ONE_TEMP_CONV with other macros.
>  - Added new macro that defines the channel of the temperature sensor.
>    Took new name for STMPE_MAX_ADC->STMPE_ADC_LAST_NR and used it through=
out
>    the code for better readability.
>  - Added mutex_unlock where missing.
>=20
>  drivers/iio/adc/Kconfig              |   7 +
>  drivers/iio/adc/Makefile             |   1 +
>  drivers/iio/adc/stmpe-adc.c          | 326 +++++++++++++++++++++++++++
>  drivers/input/touchscreen/stmpe-ts.c |  36 +--
>  drivers/mfd/Kconfig                  |   3 +-
>  drivers/mfd/stmpe.c                  |  81 +++++++
>  include/linux/mfd/stmpe.h            |   9 +
>  7 files changed, 437 insertions(+), 26 deletions(-)
>  create mode 100644 drivers/iio/adc/stmpe-adc.c
>=20
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index a52fea8749a9..224f2067494d 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -734,6 +734,13 @@ config STM32_DFSDM_ADC
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called stm32-dfsdm-adc.
> =20
> +config STMPE_ADC
> +	tristate "STMicroelectronics STMPE ADC driver"
> +	depends on OF && MFD_STMPE
> +	help
> +	  Say yes here to build support for ST Microelectronics STMPE
> +	  built-in ADC block (stmpe811).
> +
>  config STX104
>  	tristate "Apex Embedded Systems STX104 driver"
>  	depends on PC104 && X86
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index a6e6a0b659e2..cba889c30bf9 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -69,6 +69,7 @@ obj-$(CONFIG_STM32_ADC_CORE) +=3D stm32-adc-core.o
>  obj-$(CONFIG_STM32_ADC) +=3D stm32-adc.o
>  obj-$(CONFIG_STM32_DFSDM_CORE) +=3D stm32-dfsdm-core.o
>  obj-$(CONFIG_STM32_DFSDM_ADC) +=3D stm32-dfsdm-adc.o
> +obj-$(CONFIG_STMPE_ADC) +=3D stmpe-adc.o
>  obj-$(CONFIG_TI_ADC081C) +=3D ti-adc081c.o
>  obj-$(CONFIG_TI_ADC0832) +=3D ti-adc0832.o
>  obj-$(CONFIG_TI_ADC084S021) +=3D ti-adc084s021.o
> diff --git a/drivers/iio/adc/stmpe-adc.c b/drivers/iio/adc/stmpe-adc.c
> new file mode 100644
> index 000000000000..bea3f3c27bb5
> --- /dev/null
> +++ b/drivers/iio/adc/stmpe-adc.c
> @@ -0,0 +1,326 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *  STMicroelectronics STMPE811 IIO ADC Driver
> + *
> + *  4 channel, 10/12-bit ADC
> + *
> + *  Copyright (C) 2013-2018 Toradex AG <stefan.agner@toradex.com>
> + */
> +
> +#include <linux/completion.h>
> +#include <linux/err.h>
> +#include <linux/iio/iio.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/stmpe.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +
> +#define STMPE_REG_INT_STA		0x0B
> +#define STMPE_REG_ADC_INT_EN		0x0E
> +#define STMPE_REG_ADC_INT_STA		0x0F
> +
> +#define STMPE_REG_ADC_CTRL1		0x20
> +#define STMPE_REG_ADC_CTRL2		0x21
> +#define STMPE_REG_ADC_CAPT		0x22
> +#define STMPE_REG_ADC_DATA_CH(channel)	(0x30 + 2 * (channel))
> +
> +#define STMPE_REG_TEMP_CTRL		0x60
> +#define STMPE_TEMP_CTRL_ENABLE		BIT(0)
> +#define STMPE_TEMP_CTRL_ACQ		BIT(1)
> +#define STMPE_TEMP_CTRL_THRES_EN	BIT(3)
> +#define STMPE_START_ONE_TEMP_CONV	(STMPE_TEMP_CTRL_ENABLE | \
> +					STMPE_TEMP_CTRL_ACQ | \
> +					STMPE_TEMP_CTRL_THRES_EN)
> +#define STMPE_REG_TEMP_DATA		0x61
> +#define STMPE_REG_TEMP_TH		0x63
> +#define STMPE_ADC_LAST_NR		7
> +#define STMPE_TEMP_CHANNEL		(STMPE_ADC_LAST_NR + 1)
> +
> +#define STMPE_ADC_CH(channel)		((1 << (channel)) & 0xff)
> +
> +#define STMPE_ADC_TIMEOUT		msecs_to_jiffies(1000)
> +
> +struct stmpe_adc {
> +	struct stmpe *stmpe;
> +	struct clk *clk;
> +	struct device *dev;
> +
> +	/* We are allocating plus one for the temperature channel */
> +	struct iio_chan_spec stmpe_adc_iio_channels[STMPE_ADC_LAST_NR + 2];
> +
> +	struct completion completion;
> +
> +	u8 channel;
> +	u32 value;
> +};
> +
> +static int stmpe_read_raw(struct iio_dev *indio_dev,
> +			  struct iio_chan_spec const *chan,
> +			  int *val,
> +			  int *val2,
> +			  long mask)
> +{
> +	struct stmpe_adc *info =3D iio_priv(indio_dev);
> +	long ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +	case IIO_CHAN_INFO_PROCESSED:
> +
> +		mutex_lock(&indio_dev->mlock);

That's not what this lock is for and I think we have almost
entirely removed it's use from drivers.  That's the internal IIO lock
used to protect against changing from polled to trigger driven operation.

Please add your own local lock for which the scope can be cleanly defined
and documented.  Sorry I missed this before. I only noticed now whilst
trying to sanity check the documentation for a lock I expected to find
in the private data structure that wasn't there!.

> +
> +		info->channel =3D (u8)chan->channel;
> +
> +		switch (chan->type) {
> +		case IIO_VOLTAGE:
There isn't a whole lot shared in the voltage raw / temp processed
paths.  It might be more readable to separate them out - perhaps
as a pair of utility functions rather than having the
multiple switch statements with common blocks inbetween.

stmpe_read_voltage / stmpe_read_temp?

A few lines will get repeated, but the flow will be more obvious
I think and the indenting considerably reduced which always
helps!


> +			if (info->channel > STMPE_ADC_LAST_NR) {
> +				mutex_unlock(&indio_dev->mlock);
> +				return -EINVAL;
> +			}
> +
> +			stmpe_reg_write(info->stmpe, STMPE_REG_ADC_INT_EN,
> +					STMPE_ADC_CH(info->channel));
> +
> +			stmpe_reg_write(info->stmpe, STMPE_REG_ADC_CAPT,
> +					STMPE_ADC_CH(info->channel));
> +
> +			*val =3D info->value;
> +			break;
> +
> +		case IIO_TEMP:
> +			if (info->channel !=3D STMPE_TEMP_CHANNEL) {
> +				mutex_unlock(&indio_dev->mlock);
> +				return -EINVAL;
> +			}
> +
> +			stmpe_reg_write(info->stmpe, STMPE_REG_TEMP_CTRL,
> +					STMPE_START_ONE_TEMP_CONV);
> +			break;
> +		default:
> +			mutex_unlock(&indio_dev->mlock);
> +			return -EINVAL;
> +		}
> +
> +		ret =3D wait_for_completion_interruptible_timeout
> +			(&info->completion, STMPE_ADC_TIMEOUT);
> +
> +		if (ret <=3D 0) {
> +			mutex_unlock(&indio_dev->mlock);
> +			if (ret =3D=3D 0)
> +				return -ETIMEDOUT;
> +			else
> +				return ret;
> +		}
> +
> +		switch (chan->type) {
> +		case IIO_VOLTAGE:
> +			*val =3D info->value;
> +			break;
> +
> +		case IIO_TEMP:
> +			/*
> +			 * absolute temp =3D +V3.3 * value /7.51 [K]
> +			 * scale to [milli =C2=B0C]
> +			 */
> +			*val =3D ((449960l * info->value) / 1024l) - 273150;
> +			break;
> +		default:
> +			break;
> +		}
> +
> +		mutex_unlock(&indio_dev->mlock);
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		*val =3D 3300;
> +		*val2 =3D info->stmpe->mod_12b ? 12 : 10;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static irqreturn_t stmpe_adc_isr(int irq, void *dev_id)
> +{
> +	struct stmpe_adc *info =3D (struct stmpe_adc *)dev_id;
> +	u8 data[2];
> +
> +	if (info->channel > STMPE_TEMP_CHANNEL)
> +		return IRQ_NONE;
> +
> +	if (info->channel <=3D STMPE_ADC_LAST_NR) {
> +		int int_sta;
> +
> +		int_sta =3D stmpe_reg_read(info->stmpe, STMPE_REG_ADC_INT_STA);
> +
> +		/* Is the interrupt relevant */
> +		if (!(int_sta & STMPE_ADC_CH(info->channel)))
> +			return IRQ_NONE;
> +
> +		/* Read value */
> +		stmpe_block_read(info->stmpe,
> +			STMPE_REG_ADC_DATA_CH(info->channel), 2, data);
> +
> +		stmpe_reg_write(info->stmpe, STMPE_REG_ADC_INT_STA, int_sta);
> +	} else if (info->channel =3D=3D STMPE_TEMP_CHANNEL) {
> +		/* Read value */
> +		stmpe_block_read(info->stmpe, STMPE_REG_TEMP_DATA, 2, data);
> +	}
> +
> +	info->value =3D ((u32)data[0] << 8) + data[1];

This is an endian conversion of aligned data.  Can we use an appropriate
endian to cpu function to do it as cleaner and quite possibly a noop.

> +	complete(&info->completion);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static const struct iio_info stmpe_adc_iio_info =3D {
> +	.read_raw =3D &stmpe_read_raw,
> +};
> +
> +static void stmpe_adc_voltage_chan(struct iio_chan_spec *ics, int chan)
> +{
> +	ics->type =3D IIO_VOLTAGE;
> +	ics->info_mask_separate =3D BIT(IIO_CHAN_INFO_RAW);
> +	ics->info_mask_shared_by_type =3D BIT(IIO_CHAN_INFO_SCALE);
> +	ics->indexed =3D 1;
> +	ics->channel =3D chan;
> +}
> +
> +static void stmpe_adc_temp_chan(struct iio_chan_spec *ics, int chan)
> +{
> +	ics->type =3D IIO_TEMP;
> +	ics->info_mask_separate =3D BIT(IIO_CHAN_INFO_PROCESSED);
> +	ics->indexed =3D 1;
> +	ics->channel =3D chan;
> +}
> +
> +static int stmpe_adc_init_hw(struct stmpe_adc *adc)
> +{
> +	struct stmpe *stmpe =3D adc->stmpe;
> +
> +	/* use temp irq for each conversion completion */
> +	stmpe_reg_write(stmpe, STMPE_REG_TEMP_TH, 0);
> +	stmpe_reg_write(stmpe, STMPE_REG_TEMP_TH + 1, 0);
> +
> +	return 0;
> +}
> +
> +static int stmpe_adc_probe(struct platform_device *pdev)
> +{
> +	struct iio_dev *indio_dev =3D NULL;
> +	struct stmpe_adc *info =3D NULL;
> +	struct device_node *np;
> +	u32 norequest_mask =3D 0;
> +	int irq_temp, irq_adc;
> +	int num_chan =3D 0;
> +	int i =3D 0;
> +	int ret;
> +
> +	irq_adc =3D platform_get_irq_byname(pdev, "STMPE_ADC");
> +	if (irq_adc < 0)
> +		return irq_adc;
> +
> +	indio_dev =3D devm_iio_device_alloc(&pdev->dev, sizeof(struct stmpe_adc=
));
> +	if (!indio_dev) {
> +		dev_err(&pdev->dev, "failed allocating iio device\n");
> +		return -ENOMEM;
> +	}
> +
> +	info =3D iio_priv(indio_dev);
> +
> +	init_completion(&info->completion);
> +	ret =3D devm_request_threaded_irq(&pdev->dev, irq_adc, NULL,
> +					stmpe_adc_isr, IRQF_ONESHOT,
> +					"stmpe-adc", info);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed requesting irq, irq =3D %d\n",
> +				irq_adc);
> +		return ret;
> +	}
> +
> +	irq_temp =3D platform_get_irq_byname(pdev, "STMPE_TEMP_SENS");
> +	if (irq_temp >=3D 0) {
> +		ret =3D devm_request_threaded_irq(&pdev->dev, irq_temp, NULL,
> +						stmpe_adc_isr, IRQF_ONESHOT,
> +						"stmpe-adc", info);
> +		if (ret < 0)
> +			dev_warn(&pdev->dev, "failed requesting irq for"
> +				 " temp sensor, irq =3D %d\n", irq_temp);
> +	}
> +
> +	platform_set_drvdata(pdev, indio_dev);
> +
> +	indio_dev->name		=3D dev_name(&pdev->dev);
> +	indio_dev->dev.parent	=3D &pdev->dev;
> +	indio_dev->info		=3D &stmpe_adc_iio_info;
> +	indio_dev->modes	=3D INDIO_DIRECT_MODE;
> +
> +	info->stmpe =3D dev_get_drvdata(pdev->dev.parent);
> +
> +	np =3D pdev->dev.of_node;
> +
> +	if (!np)
> +		dev_err(&pdev->dev, "no device tree node found\n");
> +
> +	of_property_read_u32(np, "st,norequest-mask", &norequest_mask);
> +
> +	for_each_clear_bit(i, (unsigned long *) &norequest_mask,
> +			   (STMPE_ADC_LAST_NR + 1)) {
> +		stmpe_adc_voltage_chan(&info->stmpe_adc_iio_channels[num_chan], i);
> +		num_chan++;
> +	}
> +	stmpe_adc_temp_chan(&info->stmpe_adc_iio_channels[num_chan], i);
> +	num_chan++;
> +	indio_dev->channels =3D info->stmpe_adc_iio_channels;
> +	indio_dev->num_channels =3D num_chan;
> +
> +	ret =3D stmpe_adc_init_hw(info);
> +	if (ret)
> +		return ret;
> +
> +	return iio_device_register(indio_dev);
> +}
> +
> +static int stmpe_adc_remove(struct platform_device *pdev)
> +{
> +	struct iio_dev *indio_dev =3D platform_get_drvdata(pdev);
> +	struct stmpe_adc *info =3D iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +	stmpe_disable(info->stmpe, STMPE_BLOCK_ADC);

Could we use a devm_add_action_or_reset call covering this so that
we can get rid of the remove function entirely?

Mind you, this is unwinding something being called from the mfd
I think rather than being called from the adc probe function.

We need to fix that balance by either calling the disable from
the ADC or moving the enable in here as necessary.

> +
> +	return 0;
> +}
> +
> +static int __maybe_unused stmpe_adc_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev =3D dev_get_drvdata(dev);
> +	struct stmpe_adc *info =3D iio_priv(indio_dev);
> +
> +	stmpe_adc_init_hw(info);
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(stmpe_adc_pm_ops, NULL, stmpe_adc_resume);
> +
> +static struct platform_driver stmpe_adc_driver =3D {
> +	.probe		=3D stmpe_adc_probe,
> +	.remove		=3D stmpe_adc_remove,
> +	.driver		=3D {
> +		.name	=3D "stmpe-adc",
> +		.pm	=3D &stmpe_adc_pm_ops,
> +	},
> +};
> +
> +module_platform_driver(stmpe_adc_driver);
> +
> +MODULE_AUTHOR("Stefan Agner <stefan.agner@toradex.com>");
> +MODULE_DESCRIPTION("STMPEXXX ADC driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:stmpe-adc");
> diff --git a/drivers/input/touchscreen/stmpe-ts.c b/drivers/input/touchsc=
reen/stmpe-ts.c
> index c5d9006588a2..81aa313d6e5a 100644
> --- a/drivers/input/touchscreen/stmpe-ts.c
> +++ b/drivers/input/touchscreen/stmpe-ts.c
> @@ -30,8 +30,6 @@
>   * with touchscreen controller
>   */
>  #define STMPE_REG_INT_STA		0x0B
> -#define STMPE_REG_ADC_CTRL1		0x20
> -#define STMPE_REG_ADC_CTRL2		0x21
>  #define STMPE_REG_TSC_CTRL		0x40
>  #define STMPE_REG_TSC_CFG		0x41
>  #define STMPE_REG_FIFO_TH		0x4A
> @@ -58,15 +56,6 @@
>   * @idev: registered input device
>   * @work: a work item used to scan the device
>   * @dev: a pointer back to the MFD cell struct device*
> - * @sample_time: ADC converstion time in number of clock.
> - * (0 -> 36 clocks, 1 -> 44 clocks, 2 -> 56 clocks, 3 -> 64 clocks,
> - * 4 -> 80 clocks, 5 -> 96 clocks, 6 -> 144 clocks),
> - * recommended is 4.
> - * @mod_12b: ADC Bit mode (0 -> 10bit ADC, 1 -> 12bit ADC)
> - * @ref_sel: ADC reference source
> - * (0 -> internal reference, 1 -> external reference)
> - * @adc_freq: ADC Clock speed
> - * (0 -> 1.625 MHz, 1 -> 3.25 MHz, 2 || 3 -> 6.5 MHz)
>   * @ave_ctrl: Sample average control
>   * (0 -> 1 sample, 1 -> 2 samples, 2 -> 4 samples, 3 -> 8 samples)
>   * @touch_det_delay: Touch detect interrupt delay
> @@ -88,10 +77,6 @@ struct stmpe_touch {
>  	struct input_dev *idev;
>  	struct delayed_work work;
>  	struct device *dev;
> -	u8 sample_time;
> -	u8 mod_12b;
> -	u8 ref_sel;
> -	u8 adc_freq;
>  	u8 ave_ctrl;
>  	u8 touch_det_delay;
>  	u8 settling;
> @@ -176,7 +161,7 @@ static irqreturn_t stmpe_ts_handler(int irq, void *da=
ta)
>  	input_report_key(ts->idev, BTN_TOUCH, 1);
>  	input_sync(ts->idev);
> =20
> -       /* flush the FIFO after we have read out our values. */
> +	/* flush the FIFO after we have read out our values. */
>  	__stmpe_reset_fifo(ts->stmpe);
> =20
>  	/* reenable the tsc */
> @@ -202,20 +187,21 @@ static int stmpe_init_hw(struct stmpe_touch *ts)
>  		return ret;
>  	}
> =20
> -	adc_ctrl1 =3D STMPE_SAMPLE_TIME(ts->sample_time) |
> -		    STMPE_MOD_12B(ts->mod_12b) | STMPE_REF_SEL(ts->ref_sel);
> +	adc_ctrl1 =3D STMPE_SAMPLE_TIME(stmpe->sample_time) |
> +		    STMPE_MOD_12B(stmpe->mod_12b) |
> +		    STMPE_REF_SEL(stmpe->ref_sel);

Could we split this refactor out 'ahead' of the ADC patch?

>  	adc_ctrl1_mask =3D STMPE_SAMPLE_TIME(0xff) | STMPE_MOD_12B(0xff) |
>  			 STMPE_REF_SEL(0xff);
> =20
> -	ret =3D stmpe_set_bits(stmpe, STMPE_REG_ADC_CTRL1,
> +	ret =3D stmpe_set_bits(stmpe, STMPE811_REG_ADC_CTRL1,
>  			adc_ctrl1_mask, adc_ctrl1);
>  	if (ret) {
>  		dev_err(dev, "Could not setup ADC\n");
>  		return ret;
>  	}
> =20
> -	ret =3D stmpe_set_bits(stmpe, STMPE_REG_ADC_CTRL2,
> -			STMPE_ADC_FREQ(0xff), STMPE_ADC_FREQ(ts->adc_freq));
> +	ret =3D stmpe_set_bits(stmpe, STMPE811_REG_ADC_CTRL2,
> +			STMPE_ADC_FREQ(0xff), STMPE_ADC_FREQ(stmpe->adc_freq));
>  	if (ret) {
>  		dev_err(dev, "Could not setup ADC\n");
>  		return ret;
> @@ -295,13 +281,13 @@ static void stmpe_ts_get_platform_info(struct platf=
orm_device *pdev,
> =20
>  	if (np) {
>  		if (!of_property_read_u32(np, "st,sample-time", &val))
> -			ts->sample_time =3D val;
> +			ts->stmpe->sample_time =3D val;
>  		if (!of_property_read_u32(np, "st,mod-12b", &val))
> -			ts->mod_12b =3D val;
> +			ts->stmpe->mod_12b =3D val;
>  		if (!of_property_read_u32(np, "st,ref-sel", &val))
> -			ts->ref_sel =3D val;
> +			ts->stmpe->ref_sel =3D val;
>  		if (!of_property_read_u32(np, "st,adc-freq", &val))
> -			ts->adc_freq =3D val;
> +			ts->stmpe->adc_freq =3D val;
>  		if (!of_property_read_u32(np, "st,ave-ctrl", &val))
>  			ts->ave_ctrl =3D val;
>  		if (!of_property_read_u32(np, "st,touch-det-delay", &val))
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 8c5dfdce4326..bba159e8eaa4 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1204,7 +1204,7 @@ config MFD_STMPE
> =20
>  	  Currently supported devices are:
> =20
> -		STMPE811: GPIO, Touchscreen
> +		STMPE811: GPIO, Touchscreen, ADC
>  		STMPE1601: GPIO, Keypad
>  		STMPE1801: GPIO, Keypad
>  		STMPE2401: GPIO, Keypad
> @@ -1217,6 +1217,7 @@ config MFD_STMPE
>  		GPIO: stmpe-gpio
>  		Keypad: stmpe-keypad
>  		Touchscreen: stmpe-ts
> +		ADC: stmpe-adc
> =20
>  menu "STMicroelectronics STMPE Interface Drivers"
>  depends on MFD_STMPE
> diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c
> index 566caca4efd8..35390d1c2e64 100644
> --- a/drivers/mfd/stmpe.c
> +++ b/drivers/mfd/stmpe.c
> @@ -463,6 +463,28 @@ static const struct mfd_cell stmpe_ts_cell =3D {
>  	.num_resources	=3D ARRAY_SIZE(stmpe_ts_resources),
>  };
> =20
> +/*
> + * ADC (STMPE811)
> + */
> +
> +static struct resource stmpe_adc_resources[] =3D {
> +	{
> +		.name	=3D "STMPE_TEMP_SENS",
> +		.flags	=3D IORESOURCE_IRQ,
> +	},
> +	{
> +		.name	=3D "STMPE_ADC",
> +		.flags	=3D IORESOURCE_IRQ,
> +	},
> +};
> +
> +static const struct mfd_cell stmpe_adc_cell =3D {
> +	.name		=3D "stmpe-adc",
> +	.of_compatible	=3D "st,stmpe-adc",
> +	.resources	=3D stmpe_adc_resources,
> +	.num_resources	=3D ARRAY_SIZE(stmpe_adc_resources),
> +};
> +
>  /*
>   * STMPE811 or STMPE610
>   */
> @@ -497,6 +519,11 @@ static struct stmpe_variant_block stmpe811_blocks[] =
=3D {
>  		.irq	=3D STMPE811_IRQ_TOUCH_DET,
>  		.block	=3D STMPE_BLOCK_TOUCHSCREEN,
>  	},
> +	{
> +		.cell	=3D &stmpe_adc_cell,
> +		.irq	=3D STMPE811_IRQ_TEMP_SENS,
> +		.block	=3D STMPE_BLOCK_ADC,
> +	},
>  };
> =20
>  static int stmpe811_enable(struct stmpe *stmpe, unsigned int blocks,
> @@ -517,6 +544,44 @@ static int stmpe811_enable(struct stmpe *stmpe, unsi=
gned int blocks,
>  				enable ? 0 : mask);
>  }
> =20
> +static int stmpe811_init_adc(struct stmpe *stmpe)
> +{
> +	int ret;
> +	u8 adc_ctrl1, adc_ctrl1_mask;
> +
> +	ret =3D stmpe_enable(stmpe, STMPE_BLOCK_ADC);
> +	if (ret) {
> +		dev_err(stmpe->dev, "Could not enable clock for ADC\n");

This is a little unusual flow wise. It would be expected that stmpe_enable
would leave no visible effects if it fails, so we shouldn't need
to disable it explicitly in the error path here.  Any requirement
for that should have been done inside stmpe_enable when it detects
the error.  There are corner cases in which it might fail undetectably
up, but we typically wouldn't handle those odd ones.

If there is a really good reason that I'm missing I'd like to see
a comment here saying why.

> +		goto err_adc;
> +	}
> +
> +	adc_ctrl1 =3D STMPE_SAMPLE_TIME(stmpe->sample_time) |
> +		    STMPE_MOD_12B(stmpe->mod_12b) |
> +		    STMPE_REF_SEL(stmpe->ref_sel);
> +	adc_ctrl1_mask =3D STMPE_SAMPLE_TIME(0xff) | STMPE_MOD_12B(0xff) |
> +			 STMPE_REF_SEL(0xff);
> +
> +	ret =3D stmpe_set_bits(stmpe, STMPE811_REG_ADC_CTRL1,
> +			adc_ctrl1_mask, adc_ctrl1);
> +	if (ret) {
> +		dev_err(stmpe->dev, "Could not setup ADC\n");
> +		goto err_adc;
> +	}
> +
> +	ret =3D stmpe_set_bits(stmpe, STMPE811_REG_ADC_CTRL2,
> +			STMPE_ADC_FREQ(0xff), STMPE_ADC_FREQ(stmpe->adc_freq));
> +	if (ret) {
> +		dev_err(stmpe->dev, "Could not setup ADC\n");
> +		goto err_adc;
> +	}
> +
> +	return 0;
> +err_adc:
> +	stmpe_disable(stmpe, STMPE_BLOCK_ADC);
> +
> +	return ret;
> +}
> +
>  static int stmpe811_get_altfunc(struct stmpe *stmpe, enum stmpe_block bl=
ock)
>  {
>  	/* 0 for touchscreen, 1 for GPIO */
> @@ -1235,6 +1300,12 @@ static int stmpe_chip_init(struct stmpe *stmpe)
>  			return ret;
>  	}
> =20
> +	if (id =3D=3D STMPE811_ID) {
> +		ret =3D stmpe811_init_adc(stmpe);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	return stmpe_reg_write(stmpe, stmpe->regs[STMPE_IDX_ICR_LSB], icr);
>  }
> =20
> @@ -1325,6 +1396,7 @@ int stmpe_probe(struct stmpe_client_info *ci, enum =
stmpe_partnum partnum)
>  	struct device_node *np =3D ci->dev->of_node;
>  	struct stmpe *stmpe;
>  	int ret;
> +	u32 val;
> =20
>  	pdata =3D devm_kzalloc(ci->dev, sizeof(*pdata), GFP_KERNEL);
>  	if (!pdata)
> @@ -1342,6 +1414,15 @@ int stmpe_probe(struct stmpe_client_info *ci, enum=
 stmpe_partnum partnum)
>  	mutex_init(&stmpe->irq_lock);
>  	mutex_init(&stmpe->lock);
> =20
> +	if (!of_property_read_u32(np, "st,sample-time", &val))
> +		stmpe->sample_time =3D val;
> +	if (!of_property_read_u32(np, "st,mod-12b", &val))
> +		stmpe->mod_12b =3D val;
> +	if (!of_property_read_u32(np, "st,ref-sel", &val))
> +		stmpe->ref_sel =3D val;
> +	if (!of_property_read_u32(np, "st,adc-freq", &val))
> +		stmpe->adc_freq =3D val;
> +
>  	stmpe->dev =3D ci->dev;
>  	stmpe->client =3D ci->client;
>  	stmpe->pdata =3D pdata;
> diff --git a/include/linux/mfd/stmpe.h b/include/linux/mfd/stmpe.h
> index c0353f6431f9..86dca9e9880a 100644
> --- a/include/linux/mfd/stmpe.h
> +++ b/include/linux/mfd/stmpe.h
> @@ -21,6 +21,9 @@
>  #define STMPE_I_DRIVE(x)	(x & 0x1)
>  #define STMPE_OP_MODE(x)	((x & 0x7) << 1)
> =20
> +#define STMPE811_REG_ADC_CTRL1	0x20
> +#define STMPE811_REG_ADC_CTRL2	0x21
> +
>  struct device;
>  struct regulator;
> =20
> @@ -134,6 +137,12 @@ struct stmpe {
>  	u8 ier[2];
>  	u8 oldier[2];
>  	struct stmpe_platform_data *pdata;
> +
> +	/* For devices that use an ADC */
> +	u8 sample_time;
> +	u8 mod_12b;
> +	u8 ref_sel;
> +	u8 adc_freq;
>  };
> =20
>  extern int stmpe_reg_write(struct stmpe *stmpe, u8 reg, u8 data);

Re: [PATCH v3 2/4] iio: adc: add STMPE ADC driver using IIO framework

[Lee Jones]

On Fri, 23 Nov 2018, Philippe Schenker wrote:

> From: Stefan Agner <stefan@agner.ch>
> 
> This adds an ADC driver for the STMPE device using the industrial
> input/output interface. The driver supports raw reading of values.
> The driver depends on the MFD STMPE driver. If the touchscreen
> block is enabled too, only four of the 8 ADC channels are available.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> ---
> 
> Changes in v3:
>  - Undo ADC-settings related code-deletions in stmpe-ts.c that the code is
>    backwards-compatible to older devicetrees.
> 
> Changes in v2:
>  - Code formatting
>  - Move code to setup ADC to MFD device, as it is used by both drivers
>    adc and touchscreen
>  - Removed unused includes
>  - Defined the macro STMPE_START_ONE_TEMP_CONV with other macros.
>  - Added new macro that defines the channel of the temperature sensor.
>    Took new name for STMPE_MAX_ADC->STMPE_ADC_LAST_NR and used it throughout
>    the code for better readability.
>  - Added mutex_unlock where missing.
> 
>  drivers/iio/adc/Kconfig              |   7 +
>  drivers/iio/adc/Makefile             |   1 +
>  drivers/iio/adc/stmpe-adc.c          | 326 +++++++++++++++++++++++++++
>  drivers/input/touchscreen/stmpe-ts.c |  36 +--

>  drivers/mfd/Kconfig                  |   3 +-
>  drivers/mfd/stmpe.c                  |  81 +++++++
>  include/linux/mfd/stmpe.h            |   9 +

The MFD changes should be contained in a separate patch.

I don't see anything which ties the changes together.

>  7 files changed, 437 insertions(+), 26 deletions(-)
>  create mode 100644 drivers/iio/adc/stmpe-adc.c

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v3 3/4] iio: adc: add STMPE ADC devicetree bindings

[Philippe Schenker]

From: Stefan Agner <stefan@agner.ch>

This adds the devicetree bindings for the STMPE ADC.

Signed-off-by: Stefan Agner <stefan@agner.ch>
Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
---

Changes in v3:
 - Reformatted documentation for touchscreen to use tabs and have a better
   overview of the settings.
 - Added note which adc-settings will take precedence.
 - changed typo in sample-time setting from 144 clocks to 124 clocks, as stated
   in the datasheet.

Changes in v2:
 - Moved the bindings for ADC to the overlying mfd.
 - Reformatted for better readability

 .../devicetree/bindings/iio/adc/stmpe-adc.txt | 21 +++++++
 .../bindings/input/touchscreen/stmpe.txt      | 60 ++++++++++++-------
 .../devicetree/bindings/mfd/stmpe.txt         | 28 ++++++---
 3 files changed, 80 insertions(+), 29 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt

diff --git a/Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt b/Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt
new file mode 100644
index 000000000000..480e66422625
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt
@@ -0,0 +1,21 @@
+STMPE ADC driver
+----------------
+
+Required properties:
+ - compatible: "st,stmpe-adc"
+
+Optional properties:
+Note that the ADC is shared with the STMPE touchscreen. ADC related settings
+have to be done in the mfd.
+- st,norequest-mask: bitmask specifying which ADC channels should _not_ be
+  requestable due to different usage (e.g. touch)
+
+Node name must be stmpe_adc and should be child node of stmpe node to
+which it belongs.
+
+Example:
+
+	stmpe_adc {
+		compatible = "st,stmpe-adc";
+		st,norequest-mask = <0x0F>; /* dont use ADC CH3-0 */
+	};
diff --git a/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
index 127baa31a77a..414586513a02 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
@@ -5,36 +5,52 @@ Required properties:
  - compatible: "st,stmpe-ts"
 
 Optional properties:
-- st,sample-time: ADC converstion time in number of clock.  (0 -> 36 clocks, 1 ->
-  44 clocks, 2 -> 56 clocks, 3 -> 64 clocks, 4 -> 80 clocks, 5 -> 96 clocks, 6
-  -> 144 clocks), recommended is 4.
-- st,mod-12b: ADC Bit mode (0 -> 10bit ADC, 1 -> 12bit ADC)
-- st,ref-sel: ADC reference source (0 -> internal reference, 1 -> external
-  reference)
-- st,adc-freq: ADC Clock speed (0 -> 1.625 MHz, 1 -> 3.25 MHz, 2 || 3 -> 6.5 MHz)
-- st,ave-ctrl: Sample average control (0 -> 1 sample, 1 -> 2 samples, 2 -> 4
-  samples, 3 -> 8 samples)
-- st,touch-det-delay: Touch detect interrupt delay (0 -> 10 us, 1 -> 50 us, 2 ->
-  100 us, 3 -> 500 us, 4-> 1 ms, 5 -> 5 ms, 6 -> 10 ms, 7 -> 50 ms) recommended
-  is 3
-- st,settling: Panel driver settling time (0 -> 10 us, 1 -> 100 us, 2 -> 500 us, 3
-  -> 1 ms, 4 -> 5 ms, 5 -> 10 ms, 6 for 50 ms, 7 -> 100 ms) recommended is 2
-- st,fraction-z: Length of the fractional part in z (fraction-z ([0..7]) = Count of
-  the fractional part) recommended is 7
-- st,i-drive: current limit value of the touchscreen drivers (0 -> 20 mA typical 35
-  mA max, 1 -> 50 mA typical 80 mA max)
+- st,ave-ctrl		: Sample average control
+				0 -> 1 sample
+				1 -> 2 samples
+				2 -> 4 samples
+				3 -> 8 samples
+- st,touch-det-delay	: Touch detect interrupt delay (recommended is 3)
+				0 -> 10 us		5 -> 5 ms
+				1 -> 50 us		6 -> 10 ms
+				2 -> 100 us		7 -> 50 ms
+				3 -> 500 us
+				4-> 1 ms
+- st,settling		: Panel driver settling time (recommended is 2)
+				0 -> 10 us		5 -> 10 ms
+				1 -> 100 us		6 for 50 ms
+				2 -> 500 us		7 -> 100 ms
+				3 -> 1 ms
+				4 -> 5 ms
+- st,fraction-z		: Length of the fractional part in z (recommended is 7)
+			  (fraction-z ([0..7]) = Count of the fractional part)
+- st,i-drive		: current limit value of the touchscreen drivers
+				0 -> 20 mA (typical 35mA max)
+				1 -> 50 mA (typical 80 mA max)
+
+Optional properties common with MFD (deprecated):
+ - st,sample-time	: ADC conversion time in number of clock.
+				0 -> 36 clocks		4 -> 80 clocks (recommended)
+				1 -> 44 clocks		5 -> 96 clocks
+				2 -> 56 clocks		6 -> 124 clocks
+				3 -> 64 clocks
+ - st,mod-12b		: ADC Bit mode
+				0 -> 10bit ADC		1 -> 12bit ADC
+ - st,ref-sel		: ADC reference source
+				0 -> internal		1 -> external
+ - st,adc-freq		: ADC Clock speed
+				0 -> 1.625 MHz		2 || 3 -> 6.5 MHz
+				1 -> 3.25 MHz
 
 Node name must be stmpe_touchscreen and should be child node of stmpe node to
 which it belongs.
 
+Note that common ADC settings of stmpe_touchscreen will take precedence.
+
 Example:
 
 	stmpe_touchscreen {
 		compatible = "st,stmpe-ts";
-		st,sample-time = <4>;
-		st,mod-12b = <1>;
-		st,ref-sel = <0>;
-		st,adc-freq = <1>;
 		st,ave-ctrl = <1>;
 		st,touch-det-delay = <2>;
 		st,settling = <2>;
diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt b/Documentation/devicetree/bindings/mfd/stmpe.txt
index c797c05cd3c2..d4408a417193 100644
--- a/Documentation/devicetree/bindings/mfd/stmpe.txt
+++ b/Documentation/devicetree/bindings/mfd/stmpe.txt
@@ -4,15 +4,29 @@ STMPE is an MFD device which may expose the following inbuilt devices: gpio,
 keypad, touchscreen, adc, pwm, rotator.
 
 Required properties:
- - compatible                   : "st,stmpe[610|801|811|1600|1601|2401|2403]"
- - reg                          : I2C/SPI address of the device
+ - compatible			: "st,stmpe[610|801|811|1600|1601|2401|2403]"
+ - reg				: I2C/SPI address of the device
 
 Optional properties:
- - interrupts                   : The interrupt outputs from the controller
- - interrupt-controller         : Marks the device node as an interrupt controller
- - wakeup-source                : Marks the input device as wakable
- - st,autosleep-timeout         : Valid entries (ms); 4, 16, 32, 64, 128, 256, 512 and 1024
- - irq-gpio                     : If present, which GPIO to use for event IRQ
+ - interrupts			: The interrupt outputs from the controller
+ - interrupt-controller		: Marks the device node as an interrupt controller
+ - wakeup-source		: Marks the input device as wakable
+ - st,autosleep-timeout		: Valid entries (ms); 4, 16, 32, 64, 128, 256, 512 and 1024
+ - irq-gpio			: If present, which GPIO to use for event IRQ
+
+Optional properties for devices with touch and ADC (STMPE811|STMPE610):
+ - st,sample-time		: ADC conversion time in number of clock.
+					0 -> 36 clocks		4 -> 80 clocks (recommended)
+					1 -> 44 clocks		5 -> 96 clocks
+					2 -> 56 clocks		6 -> 124 clocks
+					3 -> 64 clocks
+ - st,mod-12b			: ADC Bit mode
+					0 -> 10bit ADC		1 -> 12bit ADC
+ - st,ref-sel			: ADC reference source
+					0 -> internal		1 -> external
+ - st,adc-freq			: ADC Clock speed
+					0 -> 1.625 MHz		2 || 3 -> 6.5 MHz
+					1 -> 3.25 MHz
 
 Example:
 
-- 
2.19.1

Re: [PATCH v3 3/4] iio: adc: add STMPE ADC devicetree bindings

[Jonathan Cameron]

On Fri, 23 Nov 2018 15:24:10 +0100
Philippe Schenker <dev@pschenker.ch> wrote:

> From: Stefan Agner <stefan@agner.ch>
> 
> This adds the devicetree bindings for the STMPE ADC.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
Clearly this will need review from input and mfd.

I've suggested inline that you split the realignment out to a
separate patch for reviewability reasons.

> ---
> 
> Changes in v3:
>  - Reformatted documentation for touchscreen to use tabs and have a better
>    overview of the settings.
>  - Added note which adc-settings will take precedence.
>  - changed typo in sample-time setting from 144 clocks to 124 clocks, as stated
>    in the datasheet.
> 
> Changes in v2:
>  - Moved the bindings for ADC to the overlying mfd.
>  - Reformatted for better readability
> 
>  .../devicetree/bindings/iio/adc/stmpe-adc.txt | 21 +++++++
>  .../bindings/input/touchscreen/stmpe.txt      | 60 ++++++++++++-------
>  .../devicetree/bindings/mfd/stmpe.txt         | 28 ++++++---
>  3 files changed, 80 insertions(+), 29 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt b/Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt
> new file mode 100644
> index 000000000000..480e66422625
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt
> @@ -0,0 +1,21 @@
> +STMPE ADC driver
> +----------------
> +
> +Required properties:
> + - compatible: "st,stmpe-adc"
> +
> +Optional properties:
> +Note that the ADC is shared with the STMPE touchscreen. ADC related settings
> +have to be done in the mfd.
> +- st,norequest-mask: bitmask specifying which ADC channels should _not_ be
> +  requestable due to different usage (e.g. touch)
> +
> +Node name must be stmpe_adc and should be child node of stmpe node to
> +which it belongs.
> +
> +Example:
> +
> +	stmpe_adc {

Can we use adc { here to match standard naming?

> +		compatible = "st,stmpe-adc";
> +		st,norequest-mask = <0x0F>; /* dont use ADC CH3-0 */
> +	};
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
> index 127baa31a77a..414586513a02 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
> @@ -5,36 +5,52 @@ Required properties:
>   - compatible: "st,stmpe-ts"
>  
>  Optional properties:
> -- st,sample-time: ADC converstion time in number of clock.  (0 -> 36 clocks, 1 ->
> -  44 clocks, 2 -> 56 clocks, 3 -> 64 clocks, 4 -> 80 clocks, 5 -> 96 clocks, 6
> -  -> 144 clocks), recommended is 4.  
> -- st,mod-12b: ADC Bit mode (0 -> 10bit ADC, 1 -> 12bit ADC)
> -- st,ref-sel: ADC reference source (0 -> internal reference, 1 -> external
> -  reference)
> -- st,adc-freq: ADC Clock speed (0 -> 1.625 MHz, 1 -> 3.25 MHz, 2 || 3 -> 6.5 MHz)
> -- st,ave-ctrl: Sample average control (0 -> 1 sample, 1 -> 2 samples, 2 -> 4
> -  samples, 3 -> 8 samples)
> -- st,touch-det-delay: Touch detect interrupt delay (0 -> 10 us, 1 -> 50 us, 2 ->
> -  100 us, 3 -> 500 us, 4-> 1 ms, 5 -> 5 ms, 6 -> 10 ms, 7 -> 50 ms) recommended
> -  is 3
> -- st,settling: Panel driver settling time (0 -> 10 us, 1 -> 100 us, 2 -> 500 us, 3
> -  -> 1 ms, 4 -> 5 ms, 5 -> 10 ms, 6 for 50 ms, 7 -> 100 ms) recommended is 2  
> -- st,fraction-z: Length of the fractional part in z (fraction-z ([0..7]) = Count of
> -  the fractional part) recommended is 7
> -- st,i-drive: current limit value of the touchscreen drivers (0 -> 20 mA typical 35
> -  mA max, 1 -> 50 mA typical 80 mA max)
> +- st,ave-ctrl		: Sample average control
> +				0 -> 1 sample
> +				1 -> 2 samples
> +				2 -> 4 samples
> +				3 -> 8 samples
> +- st,touch-det-delay	: Touch detect interrupt delay (recommended is 3)
> +				0 -> 10 us		5 -> 5 ms
> +				1 -> 50 us		6 -> 10 ms
> +				2 -> 100 us		7 -> 50 ms
> +				3 -> 500 us
> +				4-> 1 ms
> +- st,settling		: Panel driver settling time (recommended is 2)
> +				0 -> 10 us		5 -> 10 ms
> +				1 -> 100 us		6 for 50 ms
> +				2 -> 500 us		7 -> 100 ms
> +				3 -> 1 ms
> +				4 -> 5 ms
> +- st,fraction-z		: Length of the fractional part in z (recommended is 7)
> +			  (fraction-z ([0..7]) = Count of the fractional part)
> +- st,i-drive		: current limit value of the touchscreen drivers
> +				0 -> 20 mA (typical 35mA max)
> +				1 -> 50 mA (typical 80 mA max)
> +
> +Optional properties common with MFD (deprecated):
> + - st,sample-time	: ADC conversion time in number of clock.
> +				0 -> 36 clocks		4 -> 80 clocks (recommended)
> +				1 -> 44 clocks		5 -> 96 clocks
> +				2 -> 56 clocks		6 -> 124 clocks
> +				3 -> 64 clocks
> + - st,mod-12b		: ADC Bit mode
> +				0 -> 10bit ADC		1 -> 12bit ADC
> + - st,ref-sel		: ADC reference source
> +				0 -> internal		1 -> external
> + - st,adc-freq		: ADC Clock speed
> +				0 -> 1.625 MHz		2 || 3 -> 6.5 MHz
> +				1 -> 3.25 MHz
>  
>  Node name must be stmpe_touchscreen and should be child node of stmpe node to
>  which it belongs.
>  
> +Note that common ADC settings of stmpe_touchscreen will take precedence.
> +
>  Example:
>  
>  	stmpe_touchscreen {
>  		compatible = "st,stmpe-ts";
> -		st,sample-time = <4>;
> -		st,mod-12b = <1>;
> -		st,ref-sel = <0>;
> -		st,adc-freq = <1>;
>  		st,ave-ctrl = <1>;
>  		st,touch-det-delay = <2>;
>  		st,settling = <2>;
> diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt b/Documentation/devicetree/bindings/mfd/stmpe.txt
> index c797c05cd3c2..d4408a417193 100644
> --- a/Documentation/devicetree/bindings/mfd/stmpe.txt
> +++ b/Documentation/devicetree/bindings/mfd/stmpe.txt
> @@ -4,15 +4,29 @@ STMPE is an MFD device which may expose the following inbuilt devices: gpio,
>  keypad, touchscreen, adc, pwm, rotator.
>  
>  Required properties:
> - - compatible                   : "st,stmpe[610|801|811|1600|1601|2401|2403]"
> - - reg                          : I2C/SPI address of the device
> + - compatible			: "st,stmpe[610|801|811|1600|1601|2401|2403]"
> + - reg				: I2C/SPI address of the device

Nothing wrong with correcting alignment, but it shouldn't be in the same patch
as a fundamental change lie this.  Just adds noise.

If that means you first have to introduce the new block missaligned, then
fix up the alignment in a follow up patch, then do that as we can then
effectively ignore the realignment as obviously correct and focus on
the real changes.

>  
>  Optional properties:
> - - interrupts                   : The interrupt outputs from the controller
> - - interrupt-controller         : Marks the device node as an interrupt controller
> - - wakeup-source                : Marks the input device as wakable
> - - st,autosleep-timeout         : Valid entries (ms); 4, 16, 32, 64, 128, 256, 512 and 1024
> - - irq-gpio                     : If present, which GPIO to use for event IRQ
> + - interrupts			: The interrupt outputs from the controller
> + - interrupt-controller		: Marks the device node as an interrupt controller
> + - wakeup-source		: Marks the input device as wakable
> + - st,autosleep-timeout		: Valid entries (ms); 4, 16, 32, 64, 128, 256, 512 and 1024
> + - irq-gpio			: If present, which GPIO to use for event IRQ
> +
> +Optional properties for devices with touch and ADC (STMPE811|STMPE610):
> + - st,sample-time		: ADC conversion time in number of clock.
> +					0 -> 36 clocks		4 -> 80 clocks (recommended)
> +					1 -> 44 clocks		5 -> 96 clocks
> +					2 -> 56 clocks		6 -> 124 clocks
> +					3 -> 64 clocks
> + - st,mod-12b			: ADC Bit mode
> +					0 -> 10bit ADC		1 -> 12bit ADC
> + - st,ref-sel			: ADC reference source
> +					0 -> internal		1 -> external
> + - st,adc-freq			: ADC Clock speed
> +					0 -> 1.625 MHz		2 || 3 -> 6.5 MHz
> +					1 -> 3.25 MHz
>  
>  Example:
>  

Re: [PATCH v3 3/4] iio: adc: add STMPE ADC devicetree bindings

[Philippe Schenker]

On Sun, 2018-11-25 at 10:04 +0000, Jonathan Cameron wrote:
> On Fri, 23 Nov 2018 15:24:10 +0100
> Philippe Schenker <dev@pschenker.ch> wrote:
> 
> > From: Stefan Agner <stefan@agner.ch>
> > 
> > This adds the devicetree bindings for the STMPE ADC.
> > 
> > Signed-off-by: Stefan Agner <stefan@agner.ch>
> > Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
> > Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> Clearly this will need review from input and mfd.
> 
> I've suggested inline that you split the realignment out to a
> separate patch for reviewability reasons.

Thank you again Jonathan for your feedback, and of course also all the others!

I will split it much more so everything will be much more readable in v4. You
suggested again, to use the naming 'adc {'. I know that this is standard naming,
but unfortunately, this naming is given by drivers/mfd/stmpe.c (line 1311). 
What do you suggest to do? break the naming scheme in mfd, or just use
'stmpe_adc {' ?

> 
> > ---
> > 
> > Changes in v3:
> >  - Reformatted documentation for touchscreen to use tabs and have a better
> >    overview of the settings.
> >  - Added note which adc-settings will take precedence.
> >  - changed typo in sample-time setting from 144 clocks to 124 clocks, as
> > stated
> >    in the datasheet.
> > 
> > Changes in v2:
> >  - Moved the bindings for ADC to the overlying mfd.
> >  - Reformatted for better readability
> > 
> >  .../devicetree/bindings/iio/adc/stmpe-adc.txt | 21 +++++++
> >  .../bindings/input/touchscreen/stmpe.txt      | 60 ++++++++++++-------
> >  .../devicetree/bindings/mfd/stmpe.txt         | 28 ++++++---
> >  3 files changed, 80 insertions(+), 29 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt
> > b/Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt
> > new file mode 100644
> > index 000000000000..480e66422625
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt
> > @@ -0,0 +1,21 @@
> > +STMPE ADC driver
> > +----------------
> > +
> > +Required properties:
> > + - compatible: "st,stmpe-adc"
> > +
> > +Optional properties:
> > +Note that the ADC is shared with the STMPE touchscreen. ADC related
> > settings
> > +have to be done in the mfd.
> > +- st,norequest-mask: bitmask specifying which ADC channels should _not_ be
> > +  requestable due to different usage (e.g. touch)
> > +
> > +Node name must be stmpe_adc and should be child node of stmpe node to
> > +which it belongs.
> > +
> > +Example:
> > +
> > +	stmpe_adc {
> 
> Can we use adc { here to match standard naming?
> 
> > +		compatible = "st,stmpe-adc";
> > +		st,norequest-mask = <0x0F>; /* dont use ADC CH3-0 */
> > +	};
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
> > b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
> > index 127baa31a77a..414586513a02 100644
> > --- a/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
> > @@ -5,36 +5,52 @@ Required properties:
> >   - compatible: "st,stmpe-ts"
> >  
> >  Optional properties:
> > -- st,sample-time: ADC converstion time in number of clock.  (0 -> 36
> > clocks, 1 ->
> > -  44 clocks, 2 -> 56 clocks, 3 -> 64 clocks, 4 -> 80 clocks, 5 -> 96
> > clocks, 6
> > -  -> 144 clocks), recommended is 4.  
> > -- st,mod-12b: ADC Bit mode (0 -> 10bit ADC, 1 -> 12bit ADC)
> > -- st,ref-sel: ADC reference source (0 -> internal reference, 1 -> external
> > -  reference)
> > -- st,adc-freq: ADC Clock speed (0 -> 1.625 MHz, 1 -> 3.25 MHz, 2 || 3 ->
> > 6.5 MHz)
> > -- st,ave-ctrl: Sample average control (0 -> 1 sample, 1 -> 2 samples, 2 ->
> > 4
> > -  samples, 3 -> 8 samples)
> > -- st,touch-det-delay: Touch detect interrupt delay (0 -> 10 us, 1 -> 50 us,
> > 2 ->
> > -  100 us, 3 -> 500 us, 4-> 1 ms, 5 -> 5 ms, 6 -> 10 ms, 7 -> 50 ms)
> > recommended
> > -  is 3
> > -- st,settling: Panel driver settling time (0 -> 10 us, 1 -> 100 us, 2 ->
> > 500 us, 3
> > -  -> 1 ms, 4 -> 5 ms, 5 -> 10 ms, 6 for 50 ms, 7 -> 100 ms) recommended is
> > 2  
> > -- st,fraction-z: Length of the fractional part in z (fraction-z ([0..7]) =
> > Count of
> > -  the fractional part) recommended is 7
> > -- st,i-drive: current limit value of the touchscreen drivers (0 -> 20 mA
> > typical 35
> > -  mA max, 1 -> 50 mA typical 80 mA max)
> > +- st,ave-ctrl		: Sample average control
> > +				0 -> 1 sample
> > +				1 -> 2 samples
> > +				2 -> 4 samples
> > +				3 -> 8 samples
> > +- st,touch-det-delay	: Touch detect interrupt delay (recommended is
> > 3)
> > +				0 -> 10 us		5 -> 5 ms
> > +				1 -> 50 us		6 -> 10 ms
> > +				2 -> 100 us		7 -> 50 ms
> > +				3 -> 500 us
> > +				4-> 1 ms
> > +- st,settling		: Panel driver settling time (recommended is 2)
> > +				0 -> 10 us		5 -> 10 ms
> > +				1 -> 100 us		6 for 50 ms
> > +				2 -> 500 us		7 -> 100 ms
> > +				3 -> 1 ms
> > +				4 -> 5 ms
> > +- st,fraction-z		: Length of the fractional part in z
> > (recommended is 7)
> > +			  (fraction-z ([0..7]) = Count of the fractional part)
> > +- st,i-drive		: current limit value of the touchscreen drivers
> > +				0 -> 20 mA (typical 35mA max)
> > +				1 -> 50 mA (typical 80 mA max)
> > +
> > +Optional properties common with MFD (deprecated):
> > + - st,sample-time	: ADC conversion time in number of clock.
> > +				0 -> 36 clocks		4 -> 80 clocks
> > (recommended)
> > +				1 -> 44 clocks		5 -> 96 clocks
> > +				2 -> 56 clocks		6 -> 124 clocks
> > +				3 -> 64 clocks
> > + - st,mod-12b		: ADC Bit mode
> > +				0 -> 10bit ADC		1 -> 12bit ADC
> > + - st,ref-sel		: ADC reference source
> > +				0 -> internal		1 -> external
> > + - st,adc-freq		: ADC Clock speed
> > +				0 -> 1.625 MHz		2 || 3 -> 6.5 MHz
> > +				1 -> 3.25 MHz
> >  
> >  Node name must be stmpe_touchscreen and should be child node of stmpe node
> > to
> >  which it belongs.
> >  
> > +Note that common ADC settings of stmpe_touchscreen will take precedence.
> > +
> >  Example:
> >  
> >  	stmpe_touchscreen {
> >  		compatible = "st,stmpe-ts";
> > -		st,sample-time = <4>;
> > -		st,mod-12b = <1>;
> > -		st,ref-sel = <0>;
> > -		st,adc-freq = <1>;
> >  		st,ave-ctrl = <1>;
> >  		st,touch-det-delay = <2>;
> >  		st,settling = <2>;
> > diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt
> > b/Documentation/devicetree/bindings/mfd/stmpe.txt
> > index c797c05cd3c2..d4408a417193 100644
> > --- a/Documentation/devicetree/bindings/mfd/stmpe.txt
> > +++ b/Documentation/devicetree/bindings/mfd/stmpe.txt
> > @@ -4,15 +4,29 @@ STMPE is an MFD device which may expose the following
> > inbuilt devices: gpio,
> >  keypad, touchscreen, adc, pwm, rotator.
> >  
> >  Required properties:
> > - - compatible                   :
> > "st,stmpe[610|801|811|1600|1601|2401|2403]"
> > - - reg                          : I2C/SPI address of the device
> > + - compatible			:
> > "st,stmpe[610|801|811|1600|1601|2401|2403]"
> > + - reg				: I2C/SPI address of the device
> 
> Nothing wrong with correcting alignment, but it shouldn't be in the same patch
> as a fundamental change lie this.  Just adds noise.
> 
> If that means you first have to introduce the new block missaligned, then
> fix up the alignment in a follow up patch, then do that as we can then
> effectively ignore the realignment as obviously correct and focus on
> the real changes.
> 
> >  
> >  Optional properties:
> > - - interrupts                   : The interrupt outputs from the controller
> > - - interrupt-controller         : Marks the device node as an interrupt
> > controller
> > - - wakeup-source                : Marks the input device as wakable
> > - - st,autosleep-timeout         : Valid entries (ms); 4, 16, 32, 64, 128,
> > 256, 512 and 1024
> > - - irq-gpio                     : If present, which GPIO to use for event
> > IRQ
> > + - interrupts			: The interrupt outputs from the
> > controller
> > + - interrupt-controller		: Marks the device node as an interrupt
> > controller
> > + - wakeup-source		: Marks the input device as wakable
> > + - st,autosleep-timeout		: Valid entries (ms); 4, 16, 32, 64,
> > 128, 256, 512 and 1024
> > + - irq-gpio			: If present, which GPIO to use for
> > event IRQ
> > +
> > +Optional properties for devices with touch and ADC (STMPE811|STMPE610):
> > + - st,sample-time		: ADC conversion time in number of clock.
> > +					0 -> 36 clocks		4 -> 80
> > clocks (recommended)
> > +					1 -> 44 clocks		5 -> 96
> > clocks
> > +					2 -> 56 clocks		6 -> 124
> > clocks
> > +					3 -> 64 clocks
> > + - st,mod-12b			: ADC Bit mode
> > +					0 -> 10bit ADC		1 -> 12bit
> > ADC
> > + - st,ref-sel			: ADC reference source
> > +					0 -> internal		1 ->
> > external
> > + - st,adc-freq			: ADC Clock speed
> > +					0 -> 1.625 MHz		2 || 3 ->
> > 6.5 MHz
> > +					1 -> 3.25 MHz
> >  
> >  Example:
> >  

Re: [PATCH v3 3/4] iio: adc: add STMPE ADC devicetree bindings

[Jonathan Cameron]

On Thu, 06 Dec 2018 16:49:33 +0100
Philippe Schenker <dev@pschenker.ch> wrote:

> On Sun, 2018-11-25 at 10:04 +0000, Jonathan Cameron wrote:
> > On Fri, 23 Nov 2018 15:24:10 +0100
> > Philippe Schenker <dev@pschenker.ch> wrote:
> >   
> > > From: Stefan Agner <stefan@agner.ch>
> > > 
> > > This adds the devicetree bindings for the STMPE ADC.
> > > 
> > > Signed-off-by: Stefan Agner <stefan@agner.ch>
> > > Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
> > > Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>  
> > Clearly this will need review from input and mfd.
> > 
> > I've suggested inline that you split the realignment out to a
> > separate patch for reviewability reasons.  
> 
> Thank you again Jonathan for your feedback, and of course also all the others!
> 
> I will split it much more so everything will be much more readable in v4. You
> suggested again, to use the naming 'adc {'. I know that this is standard naming,
> but unfortunately, this naming is given by drivers/mfd/stmpe.c (line 1311). 
> What do you suggest to do? break the naming scheme in mfd, or just use
> 'stmpe_adc {' ?
Leave it as it is, but add a note ideally to say that is the reason.

Jonathan
> 
> >   
> > > ---
> > > 
> > > Changes in v3:
> > >  - Reformatted documentation for touchscreen to use tabs and have a better
> > >    overview of the settings.
> > >  - Added note which adc-settings will take precedence.
> > >  - changed typo in sample-time setting from 144 clocks to 124 clocks, as
> > > stated
> > >    in the datasheet.
> > > 
> > > Changes in v2:
> > >  - Moved the bindings for ADC to the overlying mfd.
> > >  - Reformatted for better readability
> > > 
> > >  .../devicetree/bindings/iio/adc/stmpe-adc.txt | 21 +++++++
> > >  .../bindings/input/touchscreen/stmpe.txt      | 60 ++++++++++++-------
> > >  .../devicetree/bindings/mfd/stmpe.txt         | 28 ++++++---
> > >  3 files changed, 80 insertions(+), 29 deletions(-)
> > >  create mode 100644 Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt
> > > b/Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt
> > > new file mode 100644
> > > index 000000000000..480e66422625
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt
> > > @@ -0,0 +1,21 @@
> > > +STMPE ADC driver
> > > +----------------
> > > +
> > > +Required properties:
> > > + - compatible: "st,stmpe-adc"
> > > +
> > > +Optional properties:
> > > +Note that the ADC is shared with the STMPE touchscreen. ADC related
> > > settings
> > > +have to be done in the mfd.
> > > +- st,norequest-mask: bitmask specifying which ADC channels should _not_ be
> > > +  requestable due to different usage (e.g. touch)
> > > +
> > > +Node name must be stmpe_adc and should be child node of stmpe node to
> > > +which it belongs.
> > > +
> > > +Example:
> > > +
> > > +	stmpe_adc {  
> > 
> > Can we use adc { here to match standard naming?
> >   
> > > +		compatible = "st,stmpe-adc";
> > > +		st,norequest-mask = <0x0F>; /* dont use ADC CH3-0 */
> > > +	};
> > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
> > > b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
> > > index 127baa31a77a..414586513a02 100644
> > > --- a/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
> > > @@ -5,36 +5,52 @@ Required properties:
> > >   - compatible: "st,stmpe-ts"
> > >  
> > >  Optional properties:
> > > -- st,sample-time: ADC converstion time in number of clock.  (0 -> 36
> > > clocks, 1 ->
> > > -  44 clocks, 2 -> 56 clocks, 3 -> 64 clocks, 4 -> 80 clocks, 5 -> 96
> > > clocks, 6  
> > > -  -> 144 clocks), recommended is 4.    
> > > -- st,mod-12b: ADC Bit mode (0 -> 10bit ADC, 1 -> 12bit ADC)
> > > -- st,ref-sel: ADC reference source (0 -> internal reference, 1 -> external
> > > -  reference)
> > > -- st,adc-freq: ADC Clock speed (0 -> 1.625 MHz, 1 -> 3.25 MHz, 2 || 3 ->
> > > 6.5 MHz)
> > > -- st,ave-ctrl: Sample average control (0 -> 1 sample, 1 -> 2 samples, 2 ->
> > > 4
> > > -  samples, 3 -> 8 samples)
> > > -- st,touch-det-delay: Touch detect interrupt delay (0 -> 10 us, 1 -> 50 us,
> > > 2 ->
> > > -  100 us, 3 -> 500 us, 4-> 1 ms, 5 -> 5 ms, 6 -> 10 ms, 7 -> 50 ms)
> > > recommended
> > > -  is 3
> > > -- st,settling: Panel driver settling time (0 -> 10 us, 1 -> 100 us, 2 ->
> > > 500 us, 3  
> > > -  -> 1 ms, 4 -> 5 ms, 5 -> 10 ms, 6 for 50 ms, 7 -> 100 ms) recommended is  
> > > 2  
> > > -- st,fraction-z: Length of the fractional part in z (fraction-z ([0..7]) =
> > > Count of
> > > -  the fractional part) recommended is 7
> > > -- st,i-drive: current limit value of the touchscreen drivers (0 -> 20 mA
> > > typical 35
> > > -  mA max, 1 -> 50 mA typical 80 mA max)
> > > +- st,ave-ctrl		: Sample average control
> > > +				0 -> 1 sample
> > > +				1 -> 2 samples
> > > +				2 -> 4 samples
> > > +				3 -> 8 samples
> > > +- st,touch-det-delay	: Touch detect interrupt delay (recommended is
> > > 3)
> > > +				0 -> 10 us		5 -> 5 ms
> > > +				1 -> 50 us		6 -> 10 ms
> > > +				2 -> 100 us		7 -> 50 ms
> > > +				3 -> 500 us
> > > +				4-> 1 ms
> > > +- st,settling		: Panel driver settling time (recommended is 2)
> > > +				0 -> 10 us		5 -> 10 ms
> > > +				1 -> 100 us		6 for 50 ms
> > > +				2 -> 500 us		7 -> 100 ms
> > > +				3 -> 1 ms
> > > +				4 -> 5 ms
> > > +- st,fraction-z		: Length of the fractional part in z
> > > (recommended is 7)
> > > +			  (fraction-z ([0..7]) = Count of the fractional part)
> > > +- st,i-drive		: current limit value of the touchscreen drivers
> > > +				0 -> 20 mA (typical 35mA max)
> > > +				1 -> 50 mA (typical 80 mA max)
> > > +
> > > +Optional properties common with MFD (deprecated):
> > > + - st,sample-time	: ADC conversion time in number of clock.
> > > +				0 -> 36 clocks		4 -> 80 clocks
> > > (recommended)
> > > +				1 -> 44 clocks		5 -> 96 clocks
> > > +				2 -> 56 clocks		6 -> 124 clocks
> > > +				3 -> 64 clocks
> > > + - st,mod-12b		: ADC Bit mode
> > > +				0 -> 10bit ADC		1 -> 12bit ADC
> > > + - st,ref-sel		: ADC reference source
> > > +				0 -> internal		1 -> external
> > > + - st,adc-freq		: ADC Clock speed
> > > +				0 -> 1.625 MHz		2 || 3 -> 6.5 MHz
> > > +				1 -> 3.25 MHz
> > >  
> > >  Node name must be stmpe_touchscreen and should be child node of stmpe node
> > > to
> > >  which it belongs.
> > >  
> > > +Note that common ADC settings of stmpe_touchscreen will take precedence.
> > > +
> > >  Example:
> > >  
> > >  	stmpe_touchscreen {
> > >  		compatible = "st,stmpe-ts";
> > > -		st,sample-time = <4>;
> > > -		st,mod-12b = <1>;
> > > -		st,ref-sel = <0>;
> > > -		st,adc-freq = <1>;
> > >  		st,ave-ctrl = <1>;
> > >  		st,touch-det-delay = <2>;
> > >  		st,settling = <2>;
> > > diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt
> > > b/Documentation/devicetree/bindings/mfd/stmpe.txt
> > > index c797c05cd3c2..d4408a417193 100644
> > > --- a/Documentation/devicetree/bindings/mfd/stmpe.txt
> > > +++ b/Documentation/devicetree/bindings/mfd/stmpe.txt
> > > @@ -4,15 +4,29 @@ STMPE is an MFD device which may expose the following
> > > inbuilt devices: gpio,
> > >  keypad, touchscreen, adc, pwm, rotator.
> > >  
> > >  Required properties:
> > > - - compatible                   :
> > > "st,stmpe[610|801|811|1600|1601|2401|2403]"
> > > - - reg                          : I2C/SPI address of the device
> > > + - compatible			:
> > > "st,stmpe[610|801|811|1600|1601|2401|2403]"
> > > + - reg				: I2C/SPI address of the device  
> > 
> > Nothing wrong with correcting alignment, but it shouldn't be in the same patch
> > as a fundamental change lie this.  Just adds noise.
> > 
> > If that means you first have to introduce the new block missaligned, then
> > fix up the alignment in a follow up patch, then do that as we can then
> > effectively ignore the realignment as obviously correct and focus on
> > the real changes.
> >   
> > >  
> > >  Optional properties:
> > > - - interrupts                   : The interrupt outputs from the controller
> > > - - interrupt-controller         : Marks the device node as an interrupt
> > > controller
> > > - - wakeup-source                : Marks the input device as wakable
> > > - - st,autosleep-timeout         : Valid entries (ms); 4, 16, 32, 64, 128,
> > > 256, 512 and 1024
> > > - - irq-gpio                     : If present, which GPIO to use for event
> > > IRQ
> > > + - interrupts			: The interrupt outputs from the
> > > controller
> > > + - interrupt-controller		: Marks the device node as an interrupt
> > > controller
> > > + - wakeup-source		: Marks the input device as wakable
> > > + - st,autosleep-timeout		: Valid entries (ms); 4, 16, 32, 64,
> > > 128, 256, 512 and 1024
> > > + - irq-gpio			: If present, which GPIO to use for
> > > event IRQ
> > > +
> > > +Optional properties for devices with touch and ADC (STMPE811|STMPE610):
> > > + - st,sample-time		: ADC conversion time in number of clock.
> > > +					0 -> 36 clocks		4 -> 80
> > > clocks (recommended)
> > > +					1 -> 44 clocks		5 -> 96
> > > clocks
> > > +					2 -> 56 clocks		6 -> 124
> > > clocks
> > > +					3 -> 64 clocks
> > > + - st,mod-12b			: ADC Bit mode
> > > +					0 -> 10bit ADC		1 -> 12bit
> > > ADC
> > > + - st,ref-sel			: ADC reference source
> > > +					0 -> internal		1 ->
> > > external
> > > + - st,adc-freq			: ADC Clock speed
> > > +					0 -> 1.625 MHz		2 || 3 ->
> > > 6.5 MHz
> > > +					1 -> 3.25 MHz
> > >  
> > >  Example:
> > >    
> 

Re: [PATCH v3 3/4] iio: adc: add STMPE ADC devicetree bindings

[Lee Jones]

On Fri, 23 Nov 2018, Philippe Schenker wrote:

> From: Stefan Agner <stefan@agner.ch>
> 
> This adds the devicetree bindings for the STMPE ADC.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> ---
> 
> Changes in v3:
>  - Reformatted documentation for touchscreen to use tabs and have a better
>    overview of the settings.
>  - Added note which adc-settings will take precedence.
>  - changed typo in sample-time setting from 144 clocks to 124 clocks, as stated
>    in the datasheet.
> 
> Changes in v2:
>  - Moved the bindings for ADC to the overlying mfd.
>  - Reformatted for better readability
> 
>  .../devicetree/bindings/iio/adc/stmpe-adc.txt | 21 +++++++
>  .../bindings/input/touchscreen/stmpe.txt      | 60 ++++++++++++-------
>  .../devicetree/bindings/mfd/stmpe.txt         | 28 ++++++---
>  3 files changed, 80 insertions(+), 29 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt
[...]

> diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt b/Documentation/devicetree/bindings/mfd/stmpe.txt
> index c797c05cd3c2..d4408a417193 100644
> --- a/Documentation/devicetree/bindings/mfd/stmpe.txt
> +++ b/Documentation/devicetree/bindings/mfd/stmpe.txt
> @@ -4,15 +4,29 @@ STMPE is an MFD device which may expose the following inbuilt devices: gpio,
>  keypad, touchscreen, adc, pwm, rotator.
>  
>  Required properties:
> - - compatible                   : "st,stmpe[610|801|811|1600|1601|2401|2403]"
> - - reg                          : I2C/SPI address of the device
> + - compatible			: "st,stmpe[610|801|811|1600|1601|2401|2403]"
> + - reg				: I2C/SPI address of the device

Formatting changes should be completed in a separate patch.

Here they only serve to obscure the real changes you have made.

Please split the formatting changes out and re-submit.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog