Skip to contentSkip to conversation

mfd: stmpe: Move ADC related defines to header of mfd

PATCH In review Philippe Schenker 6 participants 15 messages Nov 23, 2018 – Dec 10, 2018

Subsystems (8)
lore
Share & tools
lore
#0 lore raw

PATCH v3 1/4 mfd: stmpe: Move ADC related defines to header of mfd

Move defines that are ADC related to the header of the overlying mfd, so they can be used from multiple sub-devices.

---

Changes in v3:
- None

Changes in v2:
- This is a new added commit. Separate commit for moving the defines out of drivers/input/touchscreen/stmpe-ts.c to overlying mfd-device drivers/mfd/stmpe.c
- Pre-fix defines with STMPE_

 drivers/input/touchscreen/stmpe-ts.c | 34 +++++++++++-----------------
 include/linux/mfd/stmpe.h            | 11 +++++++++
 2 files changed, 24 insertions(+), 21 deletions(-)
drivers/input/touchscreen/stmpe-ts.c +13 -21
index 2a78e27b4495..c5d9006588a2 100644
@@ -49,17 +49,6 @@
 
 #define STMPE_IRQ_TOUCH_DET		0
 
-#define SAMPLE_TIME(x)			((x & 0xf) << 4)
-#define MOD_12B(x)			((x & 0x1) << 3)
-#define REF_SEL(x)			((x & 0x1) << 1)
-#define ADC_FREQ(x)			(x & 0x3)
-#define AVE_CTRL(x)			((x & 0x3) << 6)
-#define DET_DELAY(x)			((x & 0x7) << 3)
-#define SETTLING(x)			(x & 0x7)
-#define FRACTION_Z(x)			(x & 0x7)
-#define I_DRIVE(x)			(x & 0x1)
-#define OP_MODE(x)			((x & 0x7) << 1)
-
 #define STMPE_TS_NAME			"stmpe-ts"
 #define XY_MASK				0xfff
 
@@ -213,9 +202,10 @@ static int stmpe_init_hw(struct stmpe_touch *ts)
 		return ret;
 	}
 
-	adc_ctrl1 = SAMPLE_TIME(ts->sample_time) | MOD_12B(ts->mod_12b) |
-		REF_SEL(ts->ref_sel);
-	adc_ctrl1_mask = SAMPLE_TIME(0xff) | MOD_12B(0xff) | REF_SEL(0xff);
+	adc_ctrl1 = STMPE_SAMPLE_TIME(ts->sample_time) |
+		    STMPE_MOD_12B(ts->mod_12b) | STMPE_REF_SEL(ts->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,
 			adc_ctrl1_mask, adc_ctrl1);
@@ -225,15 +215,17 @@ static int stmpe_init_hw(struct stmpe_touch *ts)
 	}
 
 	ret = stmpe_set_bits(stmpe, STMPE_REG_ADC_CTRL2,
-			ADC_FREQ(0xff), ADC_FREQ(ts->adc_freq));
+			STMPE_ADC_FREQ(0xff), STMPE_ADC_FREQ(ts->adc_freq));
 	if (ret) {
 		dev_err(dev, "Could not setup ADC\n");
 		return ret;
 	}
 
-	tsc_cfg = AVE_CTRL(ts->ave_ctrl) | DET_DELAY(ts->touch_det_delay) |
-			SETTLING(ts->settling);
-	tsc_cfg_mask = AVE_CTRL(0xff) | DET_DELAY(0xff) | SETTLING(0xff);
+	tsc_cfg = STMPE_AVE_CTRL(ts->ave_ctrl) |
+		  STMPE_DET_DELAY(ts->touch_det_delay) |
+		  STMPE_SETTLING(ts->settling);
+	tsc_cfg_mask = STMPE_AVE_CTRL(0xff) | STMPE_DET_DELAY(0xff) |
+		       STMPE_SETTLING(0xff);
 
 	ret = stmpe_set_bits(stmpe, STMPE_REG_TSC_CFG, tsc_cfg_mask, tsc_cfg);
 	if (ret) {
@@ -242,14 +234,14 @@ static int stmpe_init_hw(struct stmpe_touch *ts)
 	}
 
 	ret = stmpe_set_bits(stmpe, STMPE_REG_TSC_FRACTION_Z,
-			FRACTION_Z(0xff), FRACTION_Z(ts->fraction_z));
+			STMPE_FRACTION_Z(0xff), STMPE_FRACTION_Z(ts->fraction_z));
 	if (ret) {
 		dev_err(dev, "Could not config touch\n");
 		return ret;
 	}
 
 	ret = stmpe_set_bits(stmpe, STMPE_REG_TSC_I_DRIVE,
-			I_DRIVE(0xff), I_DRIVE(ts->i_drive));
+			STMPE_I_DRIVE(0xff), STMPE_I_DRIVE(ts->i_drive));
 	if (ret) {
 		dev_err(dev, "Could not config touch\n");
 		return ret;
@@ -263,7 +255,7 @@ static int stmpe_init_hw(struct stmpe_touch *ts)
 	}
 
 	ret = stmpe_set_bits(stmpe, STMPE_REG_TSC_CTRL,
-			OP_MODE(0xff), OP_MODE(OP_MOD_XYZ));
+			STMPE_OP_MODE(0xff), STMPE_OP_MODE(OP_MOD_XYZ));
 	if (ret) {
 		dev_err(dev, "Could not set mode\n");
 		return ret;
include/linux/mfd/stmpe.h +11 -0
index 4a827af17e59..c0353f6431f9 100644
@@ -10,6 +10,17 @@
 
 #include <linux/mutex.h>
 
+#define STMPE_SAMPLE_TIME(x)	((x & 0xf) << 4)
+#define STMPE_MOD_12B(x)	((x & 0x1) << 3)
+#define STMPE_REF_SEL(x)	((x & 0x1) << 1)
+#define STMPE_ADC_FREQ(x)	(x & 0x3)
+#define STMPE_AVE_CTRL(x)	((x & 0x3) << 6)
+#define STMPE_DET_DELAY(x)	((x & 0x7) << 3)
+#define STMPE_SETTLING(x)	(x & 0x7)
+#define STMPE_FRACTION_Z(x)	(x & 0x7)
+#define STMPE_I_DRIVE(x)	(x & 0x1)
+#define STMPE_OP_MODE(x)	((x & 0x7) << 1)
+
 struct device;
 struct regulator;
 
Signature · 1 line
2.19.1
#1 lore raw

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

From: Stefan Agner <[email protected]>

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.

---

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
drivers/iio/adc/Kconfig +7 -0
index a52fea8749a9..224f2067494d 100644
@@ -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
drivers/iio/adc/Makefile +1 -0
index a6e6a0b659e2..cba889c30bf9 100644
@@ -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
drivers/iio/adc/stmpe-adc.c new file +326 -0

326 lines not on this page · raw message

drivers/input/touchscreen/stmpe-ts.c +11 -25
index c5d9006588a2..81aa313d6e5a 100644
@@ -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))
drivers/mfd/Kconfig +2 -1
index 8c5dfdce4326..bba159e8eaa4 100644
@@ -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
drivers/mfd/stmpe.c +81 -0

117 lines not on this page · raw message

include/linux/mfd/stmpe.h +9 -0
index c0353f6431f9..86dca9e9880a 100644
@@ -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);
Signature · 1 line
2.19.1
maintainer
#2 lore raw

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

Quoted 10 lines · On Fri, 23 Nov 2018 15:24:09 +0100 Philippe Schenker <[email protected]> wrote:

From: Stefan Agner <[email protected]>

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.

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

Quoted 125 lines

---

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
drivers/iio/adc/Kconfig +7 -0
index a52fea8749a9..224f2067494d 100644
@@ -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
drivers/iio/adc/Makefile +1 -0
index a6e6a0b659e2..cba889c30bf9 100644
@@ -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
drivers/iio/adc/stmpe-adc.c new file +62 -0
new file mode 100644
index 000000000000..bea3f3c27bb5
@@ -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 <[email protected]>
+ */
+
+#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,

drivers/iio/adc/stmpe-adc.c line 72

Quoted 10 lines
drivers/iio/adc/stmpe-adc.c new file +10 -0
+			  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);

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!.

drivers/iio/adc/stmpe-adc.c line 77

Quoted 5 lines
drivers/iio/adc/stmpe-adc.c new file +5 -0
+
+		info->channel = (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!

Quoted 88 lines
drivers/iio/adc/stmpe-adc.c new file +88 -0
+			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 */

drivers/iio/adc/stmpe-adc.c line 175

Quoted 10 lines
drivers/iio/adc/stmpe-adc.c new file +10 -0
+		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];

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.

Quoted 110 lines
drivers/iio/adc/stmpe-adc.c new file +110 -0
+	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;
+

drivers/iio/adc/stmpe-adc.c line 295

Quoted 10 lines
drivers/iio/adc/stmpe-adc.c new file +10 -0
+	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);

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.

Quoted 81 lines
drivers/iio/adc/stmpe-adc.c new file +31 -0
+
+	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 <[email protected]>");
+MODULE_DESCRIPTION("STMPEXXX ADC driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:stmpe-adc");
drivers/input/touchscreen/stmpe-ts.c +1 -16
index c5d9006588a2..81aa313d6e5a 100644
@@ -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 */

drivers/input/touchscreen/stmpe-ts.c static int stmpe_init_hw(struct stmpe_touch *ts) · line 192

Quoted 8 lines
drivers/input/touchscreen/stmpe-ts.c +3 -2
@@ -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);

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

Quoted 104 lines
drivers/input/touchscreen/stmpe-ts.c +7 -7
 	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))
drivers/mfd/Kconfig +2 -1
index 8c5dfdce4326..bba159e8eaa4 100644
@@ -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
drivers/mfd/stmpe.c +27 -0
index 566caca4efd8..35390d1c2e64 100644
@@ -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);

drivers/mfd/stmpe.c static int stmpe811_enable(struct stmpe *stmpe, unsigned int blocks, · line 554

Quoted 10 lines
drivers/mfd/stmpe.c +8 -0
 }
 
+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");

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.

Quoted 97 lines
drivers/mfd/stmpe.c +46 -0
+		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;
include/linux/mfd/stmpe.h +9 -0
index c0353f6431f9..86dca9e9880a 100644
@@ -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);
#3 lore raw

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

In reply to Philippe Schenker
Quoted 27 lines · Philippe Schenker

From: Stefan Agner <[email protected]>

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.

---

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.

Quoted 8 lines
 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.

Quoted 2 lines
 7 files changed, 437 insertions(+), 26 deletions(-)
 create mode 100644 drivers/iio/adc/stmpe-adc.c
Signature · 4 lines
Lee Jones [???]
Linaro Services Technical Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#4 lore raw

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

In reply to Philippe Schenker

From: Stefan Agner <[email protected]>

This adds the devicetree bindings for the 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
Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt new file +21 -0
new file mode 100644
index 000000000000..480e66422625
@@ -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 */
+	};
Documentation/devicetree/bindings/input/touchscreen/stmpe.txt +38 -22

74 lines not on this page · raw message

Documentation/devicetree/bindings/mfd/stmpe.txt +21 -7
index c797c05cd3c2..d4408a417193 100644
@@ -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:
 
Signature · 1 line
2.19.1
maintainer
#5 lore raw

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

Quoted 7 lines · On Fri, 23 Nov 2018 15:24:10 +0100 Philippe Schenker <[email protected]> wrote:

From: Stefan Agner <[email protected]>

This adds the devicetree bindings for the STMPE ADC.

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.

Quoted 33 lines

---

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
Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt new file +8 -0
new file mode 100644
index 000000000000..480e66422625
@@ -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

Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt line 18

Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt new file +10 -0
+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?

Quoted 87 lines
Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt new file +3 -0
+		compatible = "st,stmpe-adc";
+		st,norequest-mask = <0x0F>; /* dont use ADC CH3-0 */
+	};
Documentation/devicetree/bindings/input/touchscreen/stmpe.txt +38 -22
index 127baa31a77a..414586513a02 100644
@@ -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>;
Documentation/devicetree/bindings/mfd/stmpe.txt +0 -0
index c797c05cd3c2..d4408a417193 100644

Documentation/devicetree/bindings/mfd/stmpe.txt STMPE is an MFD device which may expose the following inbuilt devices: gpio, · line 8

Quoted 7 lines
Documentation/devicetree/bindings/mfd/stmpe.txt +2 -2
@@ -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.

Quoted 29 lines
Documentation/devicetree/bindings/mfd/stmpe.txt +19 -5
 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:
 
#6 lore raw

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

Quoted 7 lines · Jonathan Cameron
Quoted 4 lines · On Fri, 23 Nov 2018 15:24:10 +0100 Philippe Schenker <[email protected]> wrote:

From: Stefan Agner <[email protected]>

This adds the devicetree bindings for the STMPE ADC.

Quoted 7 lines
Quoted 3 lines

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 {' ?

Quoted 214 lines
Quoted 37 lines

---

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
Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt new file +8 -0
new file mode 100644
index 000000000000..480e66422625
@@ -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

Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt line 18

Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt new file +10 -0
+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?

Quoted 108 lines
Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt new file +3 -0
+		compatible = "st,stmpe-adc";
+		st,norequest-mask = <0x0F>; /* dont use ADC CH3-0 */
+	};
Documentation/devicetree/bindings/input/touchscreen/stmpe.txt +0 -1
index 127baa31a77a..414586513a02 100644
@@ -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

Documentation/devicetree/bindings/input/touchscreen/stmpe.txt +0 -5
-  -> 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

Documentation/devicetree/bindings/input/touchscreen/stmpe.txt +6 -1
-  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)

Documentation/devicetree/bindings/input/touchscreen/stmpe.txt +12 -0
+				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)

Documentation/devicetree/bindings/input/touchscreen/stmpe.txt +8 -0
+			  (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)

Documentation/devicetree/bindings/input/touchscreen/stmpe.txt +10 -0
+				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.

Documentation/devicetree/bindings/input/touchscreen/stmpe.txt +2 -4
+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>;
Documentation/devicetree/bindings/mfd/stmpe.txt +0 -1
index c797c05cd3c2..d4408a417193 100644
@@ -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                   :
Quoted 5 lines

"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.

Quoted 42 lines

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

Documentation/devicetree/bindings/mfd/stmpe.txt +4 -0
+
+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)

Documentation/devicetree/bindings/mfd/stmpe.txt +1 -0
+					1 -> 44 clocks		5 -> 96

clocks

Documentation/devicetree/bindings/mfd/stmpe.txt +1 -0
+					2 -> 56 clocks		6 -> 124

clocks

Documentation/devicetree/bindings/mfd/stmpe.txt +3 -0
+					3 -> 64 clocks
+ - st,mod-12b			: ADC Bit mode
+					0 -> 10bit ADC		1 -> 12bit

ADC
+ - st,ref-sel : ADC reference source

Documentation/devicetree/bindings/mfd/stmpe.txt +1 -0
+					0 -> internal		1 ->

external
+ - st,adc-freq : ADC Clock speed

Documentation/devicetree/bindings/mfd/stmpe.txt +1 -0
+					0 -> 1.625 MHz		2 || 3 ->

6.5 MHz

Documentation/devicetree/bindings/mfd/stmpe.txt +1 -0
+					1 -> 3.25 MHz
 
 Example:
maintainer
#7 lore raw

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

Quoted 16 lines · On Thu, 06 Dec 2018 16:49:33 +0100 Philippe Schenker <[email protected]> wrote:
Quoted 7 lines · Jonathan Cameron
Quoted 4 lines · On Fri, 23 Nov 2018 15:24:10 +0100 Philippe Schenker <[email protected]> wrote:

From: Stefan Agner <[email protected]>

This adds the devicetree bindings for the STMPE ADC.

Quoted 7 lines
Quoted 3 lines

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.

Quoted 7 lines

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

Quoted 216 lines
Quoted 215 lines
Quoted 38 lines

---

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
Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt new file +8 -0
new file mode 100644
index 000000000000..480e66422625
@@ -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

Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt line 18

Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt new file +10 -0
+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?

Quoted 108 lines
Documentation/devicetree/bindings/iio/adc/stmpe-adc.txt new file +3 -0
+		compatible = "st,stmpe-adc";
+		st,norequest-mask = <0x0F>; /* dont use ADC CH3-0 */
+	};
Documentation/devicetree/bindings/input/touchscreen/stmpe.txt +0 -1
index 127baa31a77a..414586513a02 100644
@@ -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

Documentation/devicetree/bindings/input/touchscreen/stmpe.txt +0 -5
-  -> 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

Documentation/devicetree/bindings/input/touchscreen/stmpe.txt +6 -1
-  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)

Documentation/devicetree/bindings/input/touchscreen/stmpe.txt +12 -0
+				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)

Documentation/devicetree/bindings/input/touchscreen/stmpe.txt +8 -0
+			  (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)

Documentation/devicetree/bindings/input/touchscreen/stmpe.txt +10 -0
+				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.

Documentation/devicetree/bindings/input/touchscreen/stmpe.txt +2 -4
+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>;
Documentation/devicetree/bindings/mfd/stmpe.txt +0 -1
index c797c05cd3c2..d4408a417193 100644
@@ -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                   :
Quoted 5 lines

"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.

Quoted 42 lines

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

Documentation/devicetree/bindings/mfd/stmpe.txt +4 -0
+
+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)

Documentation/devicetree/bindings/mfd/stmpe.txt +1 -0
+					1 -> 44 clocks		5 -> 96

clocks

Documentation/devicetree/bindings/mfd/stmpe.txt +1 -0
+					2 -> 56 clocks		6 -> 124

clocks

Documentation/devicetree/bindings/mfd/stmpe.txt +3 -0
+					3 -> 64 clocks
+ - st,mod-12b			: ADC Bit mode
+					0 -> 10bit ADC		1 -> 12bit

ADC
+ - st,ref-sel : ADC reference source

Documentation/devicetree/bindings/mfd/stmpe.txt +1 -0
+					0 -> internal		1 ->

external
+ - st,adc-freq : ADC Clock speed

Documentation/devicetree/bindings/mfd/stmpe.txt +1 -0
+					0 -> 1.625 MHz		2 || 3 ->

6.5 MHz

Documentation/devicetree/bindings/mfd/stmpe.txt +1 -0
+					1 -> 3.25 MHz
 
 Example:
#8 lore raw

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

In reply to Philippe Schenker
Quoted 25 lines · Philippe Schenker

From: Stefan Agner <[email protected]>

This adds the devicetree bindings for the 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

[...]

Documentation/devicetree/bindings/mfd/stmpe.txt STMPE is an MFD device which may expose the following inbuilt devices: gpio, · line 8

Quoted 12 lines
Documentation/devicetree/bindings/mfd/stmpe.txt +2 -2
index c797c05cd3c2..d4408a417193 100644
@@ -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.

Signature · 4 lines
Lee Jones [???]
Linaro Services Technical Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#9 lore raw

PATCH v3 4/4 ARM: dts: Add stmpe-adc driver to relevant devicetrees

In reply to Philippe Schenker

From: Philippe Schenker <[email protected]>

Activate the stmpe-adc driver as found on Apalis/Colibri iMX6/T30 modules

---

Changes in v3:
- None

Changes in v2:
- Put common ADC settings in mfd

 arch/arm/boot/dts/imx6qdl-apalis.dtsi  | 22 ++++++++++++++--------
 arch/arm/boot/dts/imx6qdl-colibri.dtsi | 23 +++++++++++++++--------
 arch/arm/boot/dts/tegra30-apalis.dtsi  | 22 ++++++++++++++--------
 arch/arm/boot/dts/tegra30-colibri.dtsi | 22 ++++++++++++++--------
 4 files changed, 57 insertions(+), 32 deletions(-)
arch/arm/boot/dts/imx6qdl-apalis.dtsi +14 -8
index 3dc99dd8dde1..8db476d8978d 100644
@@ -331,11 +331,18 @@
 		id = <0>;
 		blocks = <0x5>;
 		irq-trigger = <0x1>;
+		/* 3.25 MHz ADC clock speed */
+		st,adc-freq = <1>;
+		/* 12-bit ADC */
+		st,mod-12b = <1>;
+		/* internal ADC reference */
+		st,ref-sel = <0>;
+		/* ADC converstion time: 80 clocks */
+		st,sample-time = <4>;
+		/* forbid to use ADC channels 3-0 (touch) */
 
 		stmpe_touchscreen {
 			compatible = "st,stmpe-ts";
-			/* 3.25 MHz ADC clock speed */
-			st,adc-freq = <1>;
 			/* 8 sample average control */
 			st,ave-ctrl = <3>;
 			/* 7 length fractional part in z */
@@ -345,17 +352,16 @@
 			 * current limit value
 			 */
 			st,i-drive = <1>;
-			/* 12-bit ADC */
-			st,mod-12b = <1>;
-			/* internal ADC reference */
-			st,ref-sel = <0>;
-			/* ADC converstion time: 80 clocks */
-			st,sample-time = <4>;
 			/* 1 ms panel driver settling time */
 			st,settling = <3>;
 			/* 5 ms touch detect interrupt delay */
 			st,touch-det-delay = <5>;
 		};
+
+		stmpe_adc {
+			compatible = "st,stmpe-adc";
+			st,norequest-mask = <0x0F>;
+		};
 	};
 };
 
arch/arm/boot/dts/imx6qdl-colibri.dtsi +15 -8
index 87e15e7cb32b..2e303d79c7f8 100644
@@ -262,11 +262,18 @@
 		id = <0>;
 		blocks = <0x5>;
 		irq-trigger = <0x1>;
+		/* 3.25 MHz ADC clock speed */
+		st,adc-freq = <1>;
+		/* 12-bit ADC */
+		st,mod-12b = <1>;
+		/* internal ADC reference */
+		st,ref-sel = <0>;
+		/* ADC converstion time: 80 clocks */
+		st,sample-time = <4>;
+		/* forbid to use ADC channels 3-0 (touch) */
 
 		stmpe_touchscreen {
 			compatible = "st,stmpe-ts";
-			/* 3.25 MHz ADC clock speed */
-			st,adc-freq = <1>;
 			/* 8 sample average control */
 			st,ave-ctrl = <3>;
 			/* 7 length fractional part in z */
@@ -276,17 +283,17 @@
 			 * current limit value
 			 */
 			st,i-drive = <1>;
-			/* 12-bit ADC */
-			st,mod-12b = <1>;
-			/* internal ADC reference */
-			st,ref-sel = <0>;
-			/* ADC converstion time: 80 clocks */
-			st,sample-time = <4>;
 			/* 1 ms panel driver settling time */
 			st,settling = <3>;
 			/* 5 ms touch detect interrupt delay */
 			st,touch-det-delay = <5>;
 		};
+
+		stmpe_adc {
+			compatible = "st,stmpe-adc";
+			/* 3.25 MHz ADC clock speed */
+			st,norequest-mask = <0x0F>;
+		};
 	};
 };
 
arch/arm/boot/dts/tegra30-apalis.dtsi +14 -8
index 7f112f192fe9..850b0d13549a 100644
@@ -976,11 +976,18 @@
 			id = <0>;
 			blocks = <0x5>;
 			irq-trigger = <0x1>;
+			/* 3.25 MHz ADC clock speed */
+			st,adc-freq = <1>;
+			/* 12-bit ADC */
+			st,mod-12b = <1>;
+			/* internal ADC reference */
+			st,ref-sel = <0>;
+			/* ADC converstion time: 80 clocks */
+			st,sample-time = <4>;
+			/* forbid to use ADC channels 3-0 (touch) */
 
 			stmpe_touchscreen {
 				compatible = "st,stmpe-ts";
-				/* 3.25 MHz ADC clock speed */
-				st,adc-freq = <1>;
 				/* 8 sample average control */
 				st,ave-ctrl = <3>;
 				/* 7 length fractional part in z */
@@ -990,17 +997,16 @@
 				 * current limit value
 				 */
 				st,i-drive = <1>;
-				/* 12-bit ADC */
-				st,mod-12b = <1>;
-				/* internal ADC reference */
-				st,ref-sel = <0>;
-				/* ADC converstion time: 80 clocks */
-				st,sample-time = <4>;
 				/* 1 ms panel driver settling time */
 				st,settling = <3>;
 				/* 5 ms touch detect interrupt delay */
 				st,touch-det-delay = <5>;
 			};
+
+			stmpe_adc {
+				compatible = "st,stmpe-adc";
+				st,norequest-mask = <0x0F>;
+			};
 		};
 
 		/*
arch/arm/boot/dts/tegra30-colibri.dtsi +14 -8
index 35af03ca9e90..1f9198bb24ff 100644
@@ -845,11 +845,18 @@
 			id = <0>;
 			blocks = <0x5>;
 			irq-trigger = <0x1>;
+			/* 3.25 MHz ADC clock speed */
+			st,adc-freq = <1>;
+			/* 12-bit ADC */
+			st,mod-12b = <1>;
+			/* internal ADC reference */
+			st,ref-sel = <0>;
+			/* ADC converstion time: 80 clocks */
+			st,sample-time = <4>;
+			/* forbid to use ADC channels 3-0 (touch) */
 
 			stmpe_touchscreen {
 				compatible = "st,stmpe-ts";
-				/* 3.25 MHz ADC clock speed */
-				st,adc-freq = <1>;
 				/* 8 sample average control */
 				st,ave-ctrl = <3>;
 				/* 7 length fractional part in z */
@@ -859,17 +866,16 @@
 				 * current limit value
 				 */
 				st,i-drive = <1>;
-				/* 12-bit ADC */
-				st,mod-12b = <1>;
-				/* internal ADC reference */
-				st,ref-sel = <0>;
-				/* ADC converstion time: 80 clocks */
-				st,sample-time = <4>;
 				/* 1 ms panel driver settling time */
 				st,settling = <3>;
 				/* 5 ms touch detect interrupt delay */
 				st,touch-det-delay = <5>;
 			};
+
+			stmpe_adc {
+				compatible = "st,stmpe-adc";
+				st,norequest-mask = <0x0F>;
+			};
 		};
 
 		/*
Signature · 1 line
2.19.1
#10 lore raw

Re: [PATCH v3 4/4] ARM: dts: Add stmpe-adc driver to relevant devicetrees

Quoted 5 lines · Philippe Schenker

From: Philippe Schenker <[email protected]>

Activate the stmpe-adc driver as found on Apalis/Colibri iMX6/T30 modules

Hello Philippe,

Just a very minor comment.. device trees are not for drivers, but for HW description. It will be better to s/driver/DT node/ in both commit name and description.

#11 lore raw

Re: [PATCH v3 4/4] ARM: dts: Add stmpe-adc driver to relevant devicetrees

In reply to Philippe Schenker
Quoted 10 lines · Philippe Schenker

From: Philippe Schenker <[email protected]>

Activate the stmpe-adc driver as found on Apalis/Colibri iMX6/T30 modules

---

Changes in v3:
- None

Quoted 8 lines

Changes in v2:
- Put common ADC settings in mfd

 arch/arm/boot/dts/imx6qdl-apalis.dtsi  | 22 ++++++++++++++--------
 arch/arm/boot/dts/imx6qdl-colibri.dtsi | 23 +++++++++++++++--------
 arch/arm/boot/dts/tegra30-apalis.dtsi  | 22 ++++++++++++++--------
 arch/arm/boot/dts/tegra30-colibri.dtsi | 22 ++++++++++++++--------
 4 files changed, 57 insertions(+), 32 deletions(-)

You might want to split this up into two patches, one for i.MX and the other for Tegra so that the respective maintainers can apply them separately.

Also, where are patches 1-3?

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20181127/c0d2cd93/attachment.sig>

Acked-by on v3 1/4
#12 lore raw
In reply to Philippe Schenker
Quoted 9 lines · Philippe Schenker

Move defines that are ADC related to the header of the overlying mfd, so they can be used from multiple sub-devices.

---

Changes in v3:
- None

Quoted 7 lines

Changes in v2:
- This is a new added commit. Separate commit for moving the defines out of drivers/input/touchscreen/stmpe-ts.c to overlying mfd-device drivers/mfd/stmpe.c
- Pre-fix defines with STMPE_

 drivers/input/touchscreen/stmpe-ts.c | 34 +++++++++++-----------------
 include/linux/mfd/stmpe.h            | 11 +++++++++
Signature · 4 lines
Lee Jones [???]
Linaro Services Technical Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
maintainer Acked-by
#13 lore raw
Quoted 16 lines · Lee Jones
Quoted 14 lines · Philippe Schenker

Move defines that are ADC related to the header of the overlying mfd, so they can be used from multiple sub-devices.

---

Changes in v3:
- None

Changes in v2:
- This is a new added commit. Separate commit for moving the defines out of drivers/input/touchscreen/stmpe-ts.c to overlying mfd-device drivers/mfd/stmpe.c
- Pre-fix defines with STMPE_

Quoted 4 lines
Quoted 2 lines
 drivers/input/touchscreen/stmpe-ts.c | 34 +++++++++++-----------------
 include/linux/mfd/stmpe.h            | 11 +++++++++

The series seem to be heading towards MFD or IIO, so I assume this patch will be merged through one of these trees.

Thanks.

Signature · 1 line
Dmitry
#14 lore raw
Quoted 20 lines · Dmitry Torokhov
Quoted 19 lines · Lee Jones
Quoted 16 lines · Philippe Schenker

Move defines that are ADC related to the header of the overlying mfd, so they can be used from multiple sub-devices.

---

Changes in v3:
- None

Changes in v2:
- This is a new added commit. Separate commit for moving the defines out of drivers/input/touchscreen/stmpe-ts.c to overlying mfd-device drivers/mfd/stmpe.c
- Pre-fix defines with STMPE_

 drivers/input/touchscreen/stmpe-ts.c | 34 +++++++++++-----------------
 include/linux/mfd/stmpe.h            | 11 +++++++++
Quoted 6 lines
Quoted 1 line

The series seem to be heading towards MFD or IIO, so I assume this patch will be merged through one of these trees.

I'd be happy to take it.

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