imx25 adc and touchscreen driver

The documentation describes the bindings for the imx25 GCQ unit which is essentially a generic conversion queue using the imx25 ADC.

Signed-off-by Markus Pargmann <[email protected]>

---

Notes:
Changes in v6:
- Moved the raw_read code out to a different function to cleanup the code

- Changed bindings to use adc-refp and adc-refn. Also a bit of cleanup in the setup routine.
- Added defines for a complete list of references in the dt binding macros

Changes in v5:
- Fixed locking
- Removed module owner

 .../devicetree/bindings/iio/adc/fsl,imx25-gcq.txt  | 54 ++++++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/fsl,imx25-gcq.txt

Documentation/devicetree/bindings/iio/adc/fsl,imx25-gcq.txt new file +7 -0

new file mode 100644
index 000000000000..9f7c08990034
@@ -0,0 +1,54 @@
+Freescale i.MX25 ADC GCQ device
+
+This is a generic conversion queue device that can convert any of the
+analog inputs using the ADC unit of the i.MX25.
+
+Required properties:
+ - compatible: Should be "fsl,imx25-gcq".

Documentation/devicetree/bindings/iio/adc/fsl,imx25-gcq.txt new file +37 -0

+
+Sub-nodes:
+Optionally you can define subnodes which define the reference voltage
+for the analog inputs.
+
+Required properties for subnodes:
+ - reg: Should be the number of the analog input.
+     0: xp
+     1: yp
+     2: xn
+     3: yn
+     4: wiper
+     5: inaux0
+     6: inaux1
+     7: inaux2
+Optional properties for subnodes:
+ - fsl,adc-refp: specifies the positive reference input as defined in
+     <dt-bindings/iio/adc/fsl-imx25-gcq.h>
+ - fsl,adc-refn: specifies the negative reference input as defined in
+     <dt-bindings/iio/adc/fsl-imx25-gcq.h>
+
+Example:
+
+	adc: adc at 50030800 {
+		compatible = "fsl,imx25-gcq";
+		reg = <0x50030800 0x60>;
+		interrupt-parent = <&tscadc>;
+		interrupts = <1>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		inaux at 5 {
+			reg = <5>;
+			fsl,adc-refp = <MX25_ADC_REF_INT>;
+			fsl,adc-refn = <MX25_ADC_REF_NGND>;
+		};
+	};

This is the core driver for imx25 touchscreen/adc driver. The module has one shared ADC and two different conversion queues which use the ADC. The two queues are identical. Both can be used for general purpose ADC but one is meant to be used for touchscreens.

This driver is the core which manages the central components and registers of the TSC/ADC unit. It manages the IRQs and forwards them to the correct components.

drivers/mfd/fsl-imx25-tsadc.c new file +0 -0

new file mode 100644
index 000000000000..8e4013d57500
@@ -0,0 +1,167 @@

+#define MX25_ADCQ_CFG_REFP(sel) (sel << 7)

+#define MX25_ADCQ_CFG_IN(sel) (sel << 4)

Hi,

Quoted 9 lines · Markus Pargmann

This is the core driver for imx25 touchscreen/adc driver. The module has one shared ADC and two different conversion queues which use the ADC. The two queues are identical. Both can be used for general purpose ADC but one is meant to be used for touchscreens.

This driver is the core which manages the central components and registers of the TSC/ADC unit. It manages the IRQs and forwards them to the correct components.

[...]

Quoted 6 lines

drivers/mfd/fsl-imx25-tsadc.c new file +0 -0

new file mode 100644
index 000000000000..8e4013d57500
@@ -0,0 +1,167 @@

[...]

drivers/mfd/fsl-imx25-tsadc.c

Quoted 4 lines

drivers/mfd/fsl-imx25-tsadc.c new file +3 -0

+#define MX25_TGCR_POWERMODE_MASK	(3 << 8)
+#define MX25_TGCR_POWERMODE_SAVE	BIT(8)
+#define MX25_TGCR_POWERMODE_ON		(2 << 8)

This looks a bit weird and conceals the fact, that MX25_TGCR_POWERMODE_SAVE is in fact one of the possible settings of a two bit bitfield. For consistency I would write:
#define MX25_TGCR_POWERMODE_MASK (3 << 8)
#define MX25_TGCR_POWERMODE_SAVE (1 << 8)
#define MX25_TGCR_POWERMODE_ON (2 << 8)

[...]

drivers/mfd/fsl-imx25-tsadc.c

Quoted 4 lines

drivers/mfd/fsl-imx25-tsadc.c new file +3 -0

+#define MX25_ADCQ_CFG_YPLL_HIGH	0
+#define MX25_ADCQ_CFG_YPLL_OFF		BIT(12)
+#define MX25_ADCQ_CFG_YPLL_LOW		(3 << 12)

dto.

drivers/mfd/fsl-imx25-tsadc.c

Quoted 4 lines

drivers/mfd/fsl-imx25-tsadc.c new file +3 -0

+#define MX25_ADCQ_CFG_XNUR_HIGH	0
+#define MX25_ADCQ_CFG_XNUR_OFF		BIT(10)
+#define MX25_ADCQ_CFG_XNUR_LOW		(3 << 10)

dto.

drivers/mfd/fsl-imx25-tsadc.c

Quoted 3 lines

drivers/mfd/fsl-imx25-tsadc.c new file +2 -0

+#define MX25_ADCQ_CFG_XPUL_OFF		BIT(9)
+#define MX25_ADCQ_CFG_XPUL_HIGH	0

|#define MX25_ADCQ_CFG_XPUL_OFF (1 << 9)
|#define MX25_ADCQ_CFG_XPUL_HIGH (0 << 9)
would make it more clear, that these refer to the two states of the same bit.

Quoted 2 lines

+#define MX25_ADCQ_CFG_REFP(sel) (sel << 7)

missing () around macro argument

drivers/mfd/fsl-imx25-tsadc.c

Quoted 6 lines

drivers/mfd/fsl-imx25-tsadc.c new file +5 -0

+#define MX25_ADCQ_CFG_REFP_YP		0
+#define MX25_ADCQ_CFG_REFP_XP		(1 << 7)
+#define MX25_ADCQ_CFG_REFP_EXT		(2 << 7)
+#define MX25_ADCQ_CFG_REFP_INT		(3 << 7)
+#define MX25_ADCQ_CFG_REFP_MASK		(3 << 7)

see my previous comment.

Quoted 2 lines

+#define MX25_ADCQ_CFG_IN(sel) (sel << 4)

missing () around macro argument

drivers/mfd/fsl-imx25-tsadc.c

Quoted 5 lines

drivers/mfd/fsl-imx25-tsadc.c new file +4 -0

+#define MX25_ADCQ_CFG_IN_XP		0
+#define MX25_ADCQ_CFG_IN_YP		(1 << 4)
+#define MX25_ADCQ_CFG_IN_XN		(2 << 4)
+#define MX25_ADCQ_CFG_IN_YN		(3 << 4)

see my previous comment.

drivers/mfd/fsl-imx25-tsadc.c

Quoted 6 lines

drivers/mfd/fsl-imx25-tsadc.c new file +5 -0

+#define MX25_ADCQ_CFG_IN_WIPER		(4 << 4)
+#define MX25_ADCQ_CFG_IN_AUX0		(5 << 4)
+#define MX25_ADCQ_CFG_IN_AUX1		(6 << 4)
+#define MX25_ADCQ_CFG_IN_AUX2		(7 << 4)
+#define MX25_ADCQ_CFG_REFN(sel)		(sel << 2)

missing () around macro argument

drivers/mfd/fsl-imx25-tsadc.c

Quoted 6 lines

drivers/mfd/fsl-imx25-tsadc.c new file +5 -0

+#define MX25_ADCQ_CFG_REFN_XN		0
+#define MX25_ADCQ_CFG_REFN_YN		(1 << 2)
+#define MX25_ADCQ_CFG_REFN_NGND		(2 << 2)
+#define MX25_ADCQ_CFG_REFN_NGND2	(3 << 2)
+#define MX25_ADCQ_CFG_REFN_MASK		(3 << 2)

see my previous comment.

This is a conversion queue driver for the mx25 SoC. It uses the central ADC which is used by two seperate independent queues. This driver prepares different conversion configurations for each possible input. For a conversion it creates a conversionqueue of one item with the correct configuration for the chosen channel. It then executes the queue once and disables the conversion queue afterwards.

The reference voltages are configurable through devicetree subnodes, depending on the connections of the ADC inputs.

Signed-off-by Markus Pargmann <[email protected]>

Signed-off-by Denis Carikli <[email protected]>

Signed-off-by Markus Pargmann <[email protected]>

---
  drivers/iio/adc/Kconfig                     |   7 +
  drivers/iio/adc/Makefile                    |   1 +
  drivers/iio/adc/fsl-imx25-gcq.c             | 361 ++++++++++++++++++++++++++++
  include/dt-bindings/iio/adc/fsl-imx25-gcq.h |  18 ++
  4 files changed, 387 insertions(+)

create mode 100644 drivers/iio/adc/fsl-imx25-gcq.c create mode 100644 include/dt-bindings/iio/adc/fsl-imx25-gcq.h

drivers/iio/adc/fsl-imx25-gcq.c new file +46 -0

+static int mx25_gcq_probe(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev;
+	struct mx25_gcq_priv *priv;
+	struct mx25_tsadc *tsadc = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	void __iomem *mem;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	priv = iio_priv(indio_dev);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mem = devm_ioremap_resource(dev, res);
+	if (!mem)
+		return -ENOMEM;
+
+	priv->regs = devm_regmap_init_mmio(dev, mem, &mx25_gcq_regconfig);
+	if (IS_ERR(priv->regs)) {
+		dev_err(dev, "Failed to initialize regmap\n");
+		return PTR_ERR(priv->regs);
+	}
+
+	init_completion(&priv->completed);
+
+	/* Optional external regulator */
+	priv->ext_vref = devm_regulator_get(&pdev->dev, "vref");
+	if (!IS_ERR_OR_NULL(priv->ext_vref)) {
+		ret = regulator_enable(priv->ext_vref);
+		if (ret)
+			return ret;
+	}
+
+	ret = mx25_gcq_setup_cfgs(pdev, priv);
+	if (ret)
+		return ret;
+
+	priv->clk = tsadc->clk;
+	ret = clk_prepare_enable(priv->clk);
+	if (ret) {
+		dev_err(dev, "Failed to enable clock\n");
+		return ret;

drivers/iio/adc/fsl-imx25-gcq.c new file +12 -0

+	if (ret) {
+		dev_err(dev, "Failed requesting IRQ\n");
+		goto err_clk_unprepare;
+	}
+
+	indio_dev->dev.parent = &pdev->dev;
+	indio_dev->channels = mx25_gcq_channels;
+	indio_dev->num_channels = ARRAY_SIZE(mx25_gcq_channels);
+	indio_dev->info = &mx25_gcq_iio_info;
+
+	ret = iio_device_register(indio_dev);
+	if (ret) {

Hi,

Quoted 22 lines · Markus Pargmann

This is a conversion queue driver for the mx25 SoC. It uses the central ADC which is used by two seperate independent queues. This driver prepares different conversion configurations for each possible input. For a conversion it creates a conversionqueue of one item with the correct configuration for the chosen channel. It then executes the queue once and disables the conversion queue afterwards.

The reference voltages are configurable through devicetree subnodes, depending on the connections of the ADC inputs.

Signed-off-by Markus Pargmann <[email protected]>

Signed-off-by Denis Carikli <[email protected]>

Signed-off-by Markus Pargmann <[email protected]>

---
 drivers/iio/adc/Kconfig                     |   7 +
 drivers/iio/adc/Makefile                    |   1 +
 drivers/iio/adc/fsl-imx25-gcq.c             | 361 ++++++++++++++++++++++++++++
 include/dt-bindings/iio/adc/fsl-imx25-gcq.h |  18 ++
 4 files changed, 387 insertions(+)
 create mode 100644 drivers/iio/adc/fsl-imx25-gcq.c
 create mode 100644 include/dt-bindings/iio/adc/fsl-imx25-gcq.h

(...)

Quoted 50 lines

drivers/iio/adc/fsl-imx25-gcq.c new file +50 -0

+static int mx25_gcq_probe(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev;
+	struct mx25_gcq_priv *priv;
+	struct mx25_tsadc *tsadc = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	void __iomem *mem;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	priv = iio_priv(indio_dev);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mem = devm_ioremap_resource(dev, res);
+	if (!mem)
+		return -ENOMEM;
+
+	priv->regs = devm_regmap_init_mmio(dev, mem, &mx25_gcq_regconfig);
+	if (IS_ERR(priv->regs)) {
+		dev_err(dev, "Failed to initialize regmap\n");
+		return PTR_ERR(priv->regs);
+	}
+
+	init_completion(&priv->completed);
+
+	/* Optional external regulator */
+	priv->ext_vref = devm_regulator_get(&pdev->dev, "vref");
+	if (!IS_ERR_OR_NULL(priv->ext_vref)) {
+		ret = regulator_enable(priv->ext_vref);
+		if (ret)
+			return ret;
+	}
+
+	ret = mx25_gcq_setup_cfgs(pdev, priv);
+	if (ret)
+		return ret;
+
+	priv->clk = tsadc->clk;
+	ret = clk_prepare_enable(priv->clk);
+	if (ret) {
+		dev_err(dev, "Failed to enable clock\n");
+		return ret;
+	}
+
+	priv->irq = platform_get_irq(pdev, 0);
+	if (priv->irq <= 0) {

drivers/iio/adc/fsl-imx25-gcq.c line 304

Quoted 6 lines

drivers/iio/adc/fsl-imx25-gcq.c new file +6 -0

+		dev_err(dev, "Failed to get IRQ\n");
+		ret = priv->irq;
+		goto err_clk_unprepare;
+	}
+
+	ret = request_irq(priv->irq, mx25_gcq_irq, 0, pdev->name, priv);

Use of devm_* API

Hi,

Quoted 22 lines · Markus Pargmann

This is a conversion queue driver for the mx25 SoC. It uses the central ADC which is used by two seperate independent queues. This driver prepares different conversion configurations for each possible input. For a conversion it creates a conversionqueue of one item with the correct configuration for the chosen channel. It then executes the queue once and disables the conversion queue afterwards.

The reference voltages are configurable through devicetree subnodes, depending on the connections of the ADC inputs.

Signed-off-by Markus Pargmann <[email protected]>

Signed-off-by Denis Carikli <[email protected]>

Signed-off-by Markus Pargmann <[email protected]>

---
  drivers/iio/adc/Kconfig                     |   7 +
  drivers/iio/adc/Makefile                    |   1 +
  drivers/iio/adc/fsl-imx25-gcq.c             | 361 ++++++++++++++++++++++++++++
  include/dt-bindings/iio/adc/fsl-imx25-gcq.h |  18 ++
  4 files changed, 387 insertions(+)

create mode 100644 drivers/iio/adc/fsl-imx25-gcq.c create mode 100644 include/dt-bindings/iio/adc/fsl-imx25-gcq.h

(...)

Quoted 50 lines

drivers/iio/adc/fsl-imx25-gcq.c new file +50 -0

+static int mx25_gcq_probe(struct platform_device *pdev)
+{
+    struct iio_dev *indio_dev;
+    struct mx25_gcq_priv *priv;
+    struct mx25_tsadc *tsadc = dev_get_drvdata(pdev->dev.parent);
+    struct device *dev = &pdev->dev;
+    struct resource *res;
+    void __iomem *mem;
+    int ret;
+
+    indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
+    if (!indio_dev)
+        return -ENOMEM;
+
+    priv = iio_priv(indio_dev);
+
+    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+    mem = devm_ioremap_resource(dev, res);
+    if (!mem)
+        return -ENOMEM;
+
+    priv->regs = devm_regmap_init_mmio(dev, mem, &mx25_gcq_regconfig);
+    if (IS_ERR(priv->regs)) {
+        dev_err(dev, "Failed to initialize regmap\n");
+        return PTR_ERR(priv->regs);
+    }
+
+    init_completion(&priv->completed);
+
+    /* Optional external regulator */
+    priv->ext_vref = devm_regulator_get(&pdev->dev, "vref");
+    if (!IS_ERR_OR_NULL(priv->ext_vref)) {
+        ret = regulator_enable(priv->ext_vref);
+        if (ret)
+            return ret;
+    }
+
+    ret = mx25_gcq_setup_cfgs(pdev, priv);
+    if (ret)
+        return ret;
+
+    priv->clk = tsadc->clk;
+    ret = clk_prepare_enable(priv->clk);
+    if (ret) {
+        dev_err(dev, "Failed to enable clock\n");
+        return ret;
+    }
+
+    priv->irq = platform_get_irq(pdev, 0);
+    if (priv->irq <= 0) {

drivers/iio/adc/fsl-imx25-gcq.c line 304

Quoted 6 lines

drivers/iio/adc/fsl-imx25-gcq.c new file +6 -0

+        dev_err(dev, "Failed to get IRQ\n");
+        ret = priv->irq;
+        goto err_clk_unprepare;
+    }
+
+    ret = request_irq(priv->irq, mx25_gcq_irq, 0, pdev->name, priv);

Use of devm_* API

Quoted 13 lines

drivers/iio/adc/fsl-imx25-gcq.c new file +12 -0

+    if (ret) {
+        dev_err(dev, "Failed requesting IRQ\n");
+        goto err_clk_unprepare;
+    }
+
+    indio_dev->dev.parent = &pdev->dev;
+    indio_dev->channels = mx25_gcq_channels;
+    indio_dev->num_channels = ARRAY_SIZE(mx25_gcq_channels);
+    indio_dev->info = &mx25_gcq_iio_info;
+
+    ret = iio_device_register(indio_dev);
+    if (ret) {

drivers/iio/adc/fsl-imx25-gcq.c line 326

Quoted 10 lines

drivers/iio/adc/fsl-imx25-gcq.c new file +10 -0

+        dev_err(dev, "Failed to register iio device\n");
+        goto err_irq_free;
+    }
+
+    platform_set_drvdata(pdev, priv);
+
+    return 0;
+
+err_irq_free:
+    free_irq(priv->irq, (void *)priv);

This is not required if use devm_*

drivers/iio/adc/fsl-imx25-gcq.c line 338

Quoted 12 lines

drivers/iio/adc/fsl-imx25-gcq.c new file +12 -0

+err_clk_unprepare:
+    clk_disable_unprepare(priv->clk);
+    return ret;
+}
+
+static int mx25_gcq_remove(struct platform_device *pdev)
+{
+    struct mx25_gcq_priv *priv = platform_get_drvdata(pdev);
+    struct iio_dev *indio_dev = iio_priv_to_dev(pdev);
+
+    iio_device_unregister(indio_dev);
+    free_irq(priv->irq, priv);

Dto...

Quoted 1 line

This is a conversion queue driver for the mx25 SoC. It uses the central ADC which is used by two seperate independent queues. This driver prepares different conversion configurations for each possible input. For a conversion it creates a conversionqueue of one item with the correct configuration for the chosen channel. It then executes the queue once and disables the conversion queue afterwards.

The reference voltages are configurable through devicetree subnodes, depending on the connections of the ADC inputs.

Signed-off-by Markus Pargmann <[email protected]>

Signed-off-by Denis Carikli <[email protected]>

Signed-off-by Markus Pargmann <[email protected]>

---
 drivers/iio/adc/Kconfig                     |   7 +
 drivers/iio/adc/Makefile                    |   1 +
 drivers/iio/adc/fsl-imx25-gcq.c             | 361 ++++++++++++++++++++++++++++
 include/dt-bindings/iio/adc/fsl-imx25-gcq.h |  18 ++
 4 files changed, 387 insertions(+)
 create mode 100644 drivers/iio/adc/fsl-imx25-gcq.c
 create mode 100644 include/dt-bindings/iio/adc/fsl-imx25-gcq.h

drivers/iio/adc/Kconfig +7 -0

index 0f79e4725763..fccbb4bf44cc 100644
@@ -143,6 +143,13 @@ config EXYNOS_ADC
 	  of SoCs for drivers such as the touchscreen and hwmon to use to share
 	  this resource.
 
+config FSL_MX25_ADC
+	tristate "Freescale MX25 ADC driver"
+	depends on MFD_MX25_TSADC
+	help
+	  Generic Conversion Queue driver used for general purpose ADC in the
+	  MX25. This driver supports single measurements using the MX25 ADC.
+
 config LP8788_ADC
 	tristate "LP8788 ADC driver"
 	depends on MFD_LP8788

drivers/iio/adc/Makefile +1 -0

index 701fdb7c96aa..acab8d956371 100644
@@ -16,6 +16,7 @@ obj-$(CONFIG_AD799X) += ad799x.o
 obj-$(CONFIG_AT91_ADC) += at91_adc.o
 obj-$(CONFIG_AXP288_ADC) += axp288_adc.o
 obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
+obj-$(CONFIG_FSL_MX25_ADC) += fsl-imx25-gcq.o
 obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
 obj-$(CONFIG_MAX1027) += max1027.o
 obj-$(CONFIG_MAX1363) += max1363.o

drivers/iio/adc/fsl-imx25-gcq.c new file +59 -0

new file mode 100644
index 000000000000..398e40c0e4fd
@@ -0,0 +1,361 @@
+/*
+ * Copyright 2014 Markus Pargmann, Pengutronix <[email protected]>
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * This is the driver for the imx25 GCQ (Generic Conversion Queue)
+ * connected to the imx25 ADC.
+ */
+
+#include <dt-bindings/iio/adc/fsl-imx25-gcq.h>
+#include <linux/clk.h>
+#include <linux/iio/iio.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/imx25-tsadc.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+
+#define MX25_GCQ_TIMEOUT (msecs_to_jiffies(2000))
+
+enum mx25_gcq_cfgs {
+	MX25_CFG_XP = 0,
+	MX25_CFG_YP,
+	MX25_CFG_XN,
+	MX25_CFG_YN,
+	MX25_CFG_WIPER,
+	MX25_CFG_INAUX0,
+	MX25_CFG_INAUX1,
+	MX25_CFG_INAUX2,
+	MX25_NUM_CFGS,
+};
+
+struct mx25_gcq_priv {
+	struct regmap *regs;
+	struct completion completed;
+	unsigned int settling_time;
+	struct clk *clk;
+	int irq;
+	struct regulator *ext_vref;
+	u32 channel_vref_mv[MX25_NUM_CFGS];
+};
+
+#define MX25_CQG_CHAN(chan, id) {\
+	.type = IIO_VOLTAGE,\
+	.indexed = 1,\
+	.channel = chan,\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
+	.datasheet_name = id,\
+}
+
+static const struct iio_chan_spec mx25_gcq_channels[MX25_NUM_CFGS] = {

drivers/iio/adc/fsl-imx25-gcq.c new file +165 -0

+static void mx25_gcq_disable_eoq(struct mx25_gcq_priv *priv)
+{
+	regmap_update_bits(priv->regs, MX25_ADCQ_MR, MX25_ADCQ_MR_EOQ_IRQ,
+			   MX25_ADCQ_MR_EOQ_IRQ);
+}
+
+static void mx25_gcq_enable_eoq(struct mx25_gcq_priv *priv)
+{
+	regmap_update_bits(priv->regs, MX25_ADCQ_MR,
+			   MX25_ADCQ_MR_EOQ_IRQ, 0);
+}
+
+static irqreturn_t mx25_gcq_irq(int irq, void *data)
+{
+	struct mx25_gcq_priv *priv = data;
+	u32 stats;
+
+	regmap_read(priv->regs, MX25_ADCQ_SR, &stats);
+
+	if (stats & MX25_ADCQ_SR_EOQ) {
+		mx25_gcq_disable_eoq(priv);
+		complete(&priv->completed);
+	}
+
+	/* Disable conversion queue run */
+	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_FQS, 0);
+
+	/* Acknowledge all possible irqs */
+	regmap_write(priv->regs, MX25_ADCQ_SR, MX25_ADCQ_SR_FRR |
+		     MX25_ADCQ_SR_FUR | MX25_ADCQ_SR_FOR |
+		     MX25_ADCQ_SR_EOQ | MX25_ADCQ_SR_PD);
+
+	return IRQ_HANDLED;
+}
+
+static int mx25_gcq_get_raw_value(struct device *dev,
+				  struct iio_chan_spec const *chan,
+				  struct mx25_gcq_priv *priv,
+				  int *val)
+{
+	long timeout;
+	u32 data;
+
+	/* Setup the configuration we want to use */
+	regmap_write(priv->regs, MX25_ADCQ_ITEM_7_0,
+		     MX25_ADCQ_ITEM(0, chan->channel));
+
+	mx25_gcq_enable_eoq(priv);
+
+	/* Trigger queue for one run */
+	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_FQS,
+			   MX25_ADCQ_CR_FQS);
+
+	timeout = wait_for_completion_interruptible_timeout(
+		&priv->completed, MX25_GCQ_TIMEOUT);
+	if (timeout < 0) {
+		dev_err(dev,
+			"ADC wait for measurement failed\n");
+		return timeout;
+	} else if (timeout == 0) {
+		dev_err(dev, "ADC timed out\n");
+		return -ETIMEDOUT;
+	}
+
+	regmap_read(priv->regs, MX25_ADCQ_FIFO, &data);
+
+	*val = MX25_ADCQ_FIFO_DATA(data);
+
+	return IIO_VAL_INT;
+}
+
+static int mx25_gcq_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan, int *val,
+			     int *val2, long mask)
+{
+	struct mx25_gcq_priv *priv = iio_priv(indio_dev);
+	int ret = 0;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&indio_dev->mlock);
+		ret = mx25_gcq_get_raw_value(&indio_dev->dev, chan, priv, val);
+		mutex_unlock(&indio_dev->mlock);
+		return ret;
+
+	case IIO_CHAN_INFO_SCALE:
+		*val = priv->channel_vref_mv[chan->channel];
+		*val2 = 12;
+		return IIO_VAL_FRACTIONAL_LOG2;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info mx25_gcq_iio_info = {
+	.read_raw = mx25_gcq_read_raw,
+};
+
+static const struct regmap_config mx25_gcq_regconfig = {
+	.max_register = 0x5c,
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
+static int mx25_gcq_setup_cfgs(struct platform_device *pdev,
+			       struct mx25_gcq_priv *priv)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *child;
+	struct device *dev = &pdev->dev;
+	int ret, i;
+
+	/*
+	 * Setup all configurations registers with a default conversion
+	 * configuration for each input
+	 */
+	for (i = 0; i < MX25_NUM_CFGS; ++i)
+		regmap_write(priv->regs, MX25_ADCQ_CFG(i),
+			     MX25_ADCQ_CFG_YPLL_OFF |
+			     MX25_ADCQ_CFG_XNUR_OFF |
+			     MX25_ADCQ_CFG_XPUL_OFF |
+			     MX25_ADCQ_CFG_REFP_INT |
+			     MX25_ADCQ_CFG_IN(i) |
+			     MX25_ADCQ_CFG_REFN_NGND2);
+
+	for_each_child_of_node(np, child) {
+		u32 reg;
+		u32 refp = MX25_ADCQ_CFG_REFP_INT;
+		u32 refn = MX25_ADCQ_CFG_REFN_NGND2;
+
+		ret = of_property_read_u32(child, "reg", &reg);
+		if (ret) {
+			dev_err(dev, "Failed to get reg property\n");
+			return ret;
+		}
+
+		if (reg >= MX25_NUM_CFGS) {
+			dev_err(dev,
+				"reg value is greater than the number of available configuration registers\n");
+			return -EINVAL;
+		}
+
+		of_property_read_u32(child, "fsl,adc-refp", &refp);
+		of_property_read_u32(child, "fsl,adc-refn", &refn);
+
+		/*
+		 * Shift the read values to the correct positions within the
+		 * register.
+		 */
+		refp = MX25_ADCQ_CFG_REFP(refp);
+		refn = MX25_ADCQ_CFG_REFN(refn);
+
+		if ((refp & MX25_ADCQ_CFG_REFP_MASK) != refp) {
+			dev_err(dev, "Invalid fsl,adc-refp property value\n");
+			return -EINVAL;
+		}
+		if ((refn & MX25_ADCQ_CFG_REFN_MASK) != refn) {
+			dev_err(dev, "Invalid fsl,adc-refn property value");
+			return -EINVAL;
+		}
+
+		regmap_update_bits(priv->regs, MX25_ADCQ_CFG(reg),
+				   MX25_ADCQ_CFG_REFP_MASK |

drivers/iio/adc/fsl-imx25-gcq.c new file +27 -0

+
+	return 0;
+}
+
+static int mx25_gcq_probe(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev;
+	struct mx25_gcq_priv *priv;
+	struct mx25_tsadc *tsadc = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	void __iomem *mem;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	priv = iio_priv(indio_dev);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mem = devm_ioremap_resource(dev, res);
+	if (!mem)
+		return -ENOMEM;
+
+	priv->regs = devm_regmap_init_mmio(dev, mem, &mx25_gcq_regconfig);
+	if (IS_ERR(priv->regs)) {

drivers/iio/adc/fsl-imx25-gcq.c new file +30 -0

+		if (ret)
+			return ret;
+	}
+
+	ret = mx25_gcq_setup_cfgs(pdev, priv);
+	if (ret)
+		return ret;
+
+	priv->clk = tsadc->clk;
+	ret = clk_prepare_enable(priv->clk);
+	if (ret) {
+		dev_err(dev, "Failed to enable clock\n");
+		return ret;
+	}
+
+	priv->irq = platform_get_irq(pdev, 0);
+	if (priv->irq <= 0) {
+		dev_err(dev, "Failed to get IRQ\n");
+		ret = priv->irq;
+		goto err_clk_unprepare;
+	}
+
+	ret = request_irq(priv->irq, mx25_gcq_irq, 0, pdev->name, priv);
+	if (ret) {
+		dev_err(dev, "Failed requesting IRQ\n");
+		goto err_clk_unprepare;
+	}
+
+	indio_dev->dev.parent = &pdev->dev;
+	indio_dev->channels = mx25_gcq_channels;

drivers/iio/adc/fsl-imx25-gcq.c new file +40 -0

+
+	return 0;
+
+err_irq_free:
+	free_irq(priv->irq, (void *)priv);
+err_clk_unprepare:
+	clk_disable_unprepare(priv->clk);
+	return ret;
+}
+
+static int mx25_gcq_remove(struct platform_device *pdev)
+{
+	struct mx25_gcq_priv *priv = platform_get_drvdata(pdev);
+	struct iio_dev *indio_dev = iio_priv_to_dev(pdev);
+
+	iio_device_unregister(indio_dev);
+	free_irq(priv->irq, priv);
+	clk_disable_unprepare(priv->clk);
+
+	return 0;
+}
+
+static struct of_device_id mx25_gcq_ids[] = {
+	{ .compatible = "fsl,imx25-gcq", },
+	{ /* Sentinel */ }
+};
+
+static struct platform_driver mx25_gcq_driver = {
+	.driver		= {
+		.name	= "mx25-gcq",
+		.of_match_table = mx25_gcq_ids,
+	},
+	.probe		= mx25_gcq_probe,
+	.remove		= mx25_gcq_remove,
+};
+module_platform_driver(mx25_gcq_driver);
+
+MODULE_DESCRIPTION("ADC driver for Freescale mx25");
+MODULE_AUTHOR("Markus Pargmann <[email protected]>");
+MODULE_LICENSE("GPL v2");

include/dt-bindings/iio/adc/fsl-imx25-gcq.h new file +18 -0

new file mode 100644
index 000000000000..4f2f4a9baafe
@@ -0,0 +1,18 @@
+/*
+ * This header provides constants for configuring the I.MX25 ADC
+ */
+
+#ifndef _DT_BINDINGS_IIO_ADC_FS_IMX25_GCQ_H
+#define _DT_BINDINGS_IIO_ADC_FS_IMX25_GCQ_H
+
+#define MX25_ADC_REFP_YP	0 /* YP voltage reference */
+#define MX25_ADC_REFP_XP	1 /* XP voltage reference */
+#define MX25_ADC_REFP_INT	2 /* Internal voltage reference */
+#define MX25_ADC_REFP_EXT	3 /* External voltage reference */
+
+#define MX25_ADC_REFN_XN	0
+#define MX25_ADC_REFN_YN	1
+#define MX25_ADC_REFN_NGND	2
+#define MX25_ADC_REFN_NGND2	3
+
+#endif

Quoted 7 lines · Markus Pargmann

This is a conversion queue driver for the mx25 SoC. It uses the central ADC which is used by two seperate independent queues. This driver prepares different conversion configurations for each possible input. For a conversion it creates a conversionqueue of one item with the correct configuration for the chosen channel. It then executes the queue once and disables the conversion queue afterwards.

Quoted 6 lines

The reference voltages are configurable through devicetree subnodes, depending on the connections of the ADC inputs.

Signed-off-by Markus Pargmann <[email protected]>

Signed-off-by Denis Carikli <[email protected]>

Signed-off-by Markus Pargmann <[email protected]>

Some really small bits and bobs inline...
Only non trivial one is that I'd prefer that the need for a regulator in the event of an external reference being specified was explicitly enforced.

Quoted 108 lines

---
 drivers/iio/adc/Kconfig                     |   7 +
 drivers/iio/adc/Makefile                    |   1 +
 drivers/iio/adc/fsl-imx25-gcq.c             | 361 ++++++++++++++++++++++++++++
 include/dt-bindings/iio/adc/fsl-imx25-gcq.h |  18 ++
 4 files changed, 387 insertions(+)
 create mode 100644 drivers/iio/adc/fsl-imx25-gcq.c
 create mode 100644 include/dt-bindings/iio/adc/fsl-imx25-gcq.h

drivers/iio/adc/Kconfig +7 -0

index 0f79e4725763..fccbb4bf44cc 100644
@@ -143,6 +143,13 @@ config EXYNOS_ADC
 	  of SoCs for drivers such as the touchscreen and hwmon to use to share
 	  this resource.
 
+config FSL_MX25_ADC
+	tristate "Freescale MX25 ADC driver"
+	depends on MFD_MX25_TSADC
+	help
+	  Generic Conversion Queue driver used for general purpose ADC in the
+	  MX25. This driver supports single measurements using the MX25 ADC.
+
 config LP8788_ADC
 	tristate "LP8788 ADC driver"
 	depends on MFD_LP8788

drivers/iio/adc/Makefile +1 -0

index 701fdb7c96aa..acab8d956371 100644
@@ -16,6 +16,7 @@ obj-$(CONFIG_AD799X) += ad799x.o
 obj-$(CONFIG_AT91_ADC) += at91_adc.o
 obj-$(CONFIG_AXP288_ADC) += axp288_adc.o
 obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
+obj-$(CONFIG_FSL_MX25_ADC) += fsl-imx25-gcq.o
 obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
 obj-$(CONFIG_MAX1027) += max1027.o
 obj-$(CONFIG_MAX1363) += max1363.o

drivers/iio/adc/fsl-imx25-gcq.c new file +63 -0

new file mode 100644
index 000000000000..398e40c0e4fd
@@ -0,0 +1,361 @@
+/*
+ * Copyright 2014 Markus Pargmann, Pengutronix <[email protected]>
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * This is the driver for the imx25 GCQ (Generic Conversion Queue)
+ * connected to the imx25 ADC.
+ */
+
+#include <dt-bindings/iio/adc/fsl-imx25-gcq.h>
+#include <linux/clk.h>
+#include <linux/iio/iio.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/imx25-tsadc.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+
+#define MX25_GCQ_TIMEOUT (msecs_to_jiffies(2000))
+
+enum mx25_gcq_cfgs {
+	MX25_CFG_XP = 0,
+	MX25_CFG_YP,
+	MX25_CFG_XN,
+	MX25_CFG_YN,
+	MX25_CFG_WIPER,
+	MX25_CFG_INAUX0,
+	MX25_CFG_INAUX1,
+	MX25_CFG_INAUX2,
+	MX25_NUM_CFGS,
+};
+
+struct mx25_gcq_priv {
+	struct regmap *regs;
+	struct completion completed;
+	unsigned int settling_time;
+	struct clk *clk;
+	int irq;
+	struct regulator *ext_vref;
+	u32 channel_vref_mv[MX25_NUM_CFGS];
+};
+
+#define MX25_CQG_CHAN(chan, id) {\
+	.type = IIO_VOLTAGE,\
+	.indexed = 1,\
+	.channel = chan,\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
+	.datasheet_name = id,\
+}
+
+static const struct iio_chan_spec mx25_gcq_channels[MX25_NUM_CFGS] = {
+	MX25_CQG_CHAN(0, "xp"),
+	MX25_CQG_CHAN(1, "yp"),
+	MX25_CQG_CHAN(2, "xn"),
+	MX25_CQG_CHAN(3, "yn"),

drivers/iio/adc/fsl-imx25-gcq.c line 69

Quoted 6 lines

drivers/iio/adc/fsl-imx25-gcq.c new file +6 -0

+	MX25_CQG_CHAN(4, "wiper"),
+	MX25_CQG_CHAN(5, "inaux0"),
+	MX25_CQG_CHAN(6, "inaux1"),
+	MX25_CQG_CHAN(7, "inaux2"),
+};
+

I'd be tempted to not bother separating out the next two given they are only used in one place each..

Quoted 168 lines

drivers/iio/adc/fsl-imx25-gcq.c new file +168 -0

+static void mx25_gcq_disable_eoq(struct mx25_gcq_priv *priv)
+{
+	regmap_update_bits(priv->regs, MX25_ADCQ_MR, MX25_ADCQ_MR_EOQ_IRQ,
+			   MX25_ADCQ_MR_EOQ_IRQ);
+}
+
+static void mx25_gcq_enable_eoq(struct mx25_gcq_priv *priv)
+{
+	regmap_update_bits(priv->regs, MX25_ADCQ_MR,
+			   MX25_ADCQ_MR_EOQ_IRQ, 0);
+}
+
+static irqreturn_t mx25_gcq_irq(int irq, void *data)
+{
+	struct mx25_gcq_priv *priv = data;
+	u32 stats;
+
+	regmap_read(priv->regs, MX25_ADCQ_SR, &stats);
+
+	if (stats & MX25_ADCQ_SR_EOQ) {
+		mx25_gcq_disable_eoq(priv);
+		complete(&priv->completed);
+	}
+
+	/* Disable conversion queue run */
+	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_FQS, 0);
+
+	/* Acknowledge all possible irqs */
+	regmap_write(priv->regs, MX25_ADCQ_SR, MX25_ADCQ_SR_FRR |
+		     MX25_ADCQ_SR_FUR | MX25_ADCQ_SR_FOR |
+		     MX25_ADCQ_SR_EOQ | MX25_ADCQ_SR_PD);
+
+	return IRQ_HANDLED;
+}
+
+static int mx25_gcq_get_raw_value(struct device *dev,
+				  struct iio_chan_spec const *chan,
+				  struct mx25_gcq_priv *priv,
+				  int *val)
+{
+	long timeout;
+	u32 data;
+
+	/* Setup the configuration we want to use */
+	regmap_write(priv->regs, MX25_ADCQ_ITEM_7_0,
+		     MX25_ADCQ_ITEM(0, chan->channel));
+
+	mx25_gcq_enable_eoq(priv);
+
+	/* Trigger queue for one run */
+	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_FQS,
+			   MX25_ADCQ_CR_FQS);
+
+	timeout = wait_for_completion_interruptible_timeout(
+		&priv->completed, MX25_GCQ_TIMEOUT);
+	if (timeout < 0) {
+		dev_err(dev,
+			"ADC wait for measurement failed\n");
+		return timeout;
+	} else if (timeout == 0) {
+		dev_err(dev, "ADC timed out\n");
+		return -ETIMEDOUT;
+	}
+
+	regmap_read(priv->regs, MX25_ADCQ_FIFO, &data);
+
+	*val = MX25_ADCQ_FIFO_DATA(data);
+
+	return IIO_VAL_INT;
+}
+
+static int mx25_gcq_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan, int *val,
+			     int *val2, long mask)
+{
+	struct mx25_gcq_priv *priv = iio_priv(indio_dev);
+	int ret = 0;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&indio_dev->mlock);
+		ret = mx25_gcq_get_raw_value(&indio_dev->dev, chan, priv, val);
+		mutex_unlock(&indio_dev->mlock);
+		return ret;
+
+	case IIO_CHAN_INFO_SCALE:
+		*val = priv->channel_vref_mv[chan->channel];
+		*val2 = 12;
+		return IIO_VAL_FRACTIONAL_LOG2;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info mx25_gcq_iio_info = {
+	.read_raw = mx25_gcq_read_raw,
+};
+
+static const struct regmap_config mx25_gcq_regconfig = {
+	.max_register = 0x5c,
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
+static int mx25_gcq_setup_cfgs(struct platform_device *pdev,
+			       struct mx25_gcq_priv *priv)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *child;
+	struct device *dev = &pdev->dev;
+	int ret, i;
+
+	/*
+	 * Setup all configurations registers with a default conversion
+	 * configuration for each input
+	 */
+	for (i = 0; i < MX25_NUM_CFGS; ++i)
+		regmap_write(priv->regs, MX25_ADCQ_CFG(i),
+			     MX25_ADCQ_CFG_YPLL_OFF |
+			     MX25_ADCQ_CFG_XNUR_OFF |
+			     MX25_ADCQ_CFG_XPUL_OFF |
+			     MX25_ADCQ_CFG_REFP_INT |
+			     MX25_ADCQ_CFG_IN(i) |
+			     MX25_ADCQ_CFG_REFN_NGND2);
+
+	for_each_child_of_node(np, child) {
+		u32 reg;
+		u32 refp = MX25_ADCQ_CFG_REFP_INT;
+		u32 refn = MX25_ADCQ_CFG_REFN_NGND2;
+
+		ret = of_property_read_u32(child, "reg", &reg);
+		if (ret) {
+			dev_err(dev, "Failed to get reg property\n");
+			return ret;
+		}
+
+		if (reg >= MX25_NUM_CFGS) {
+			dev_err(dev,
+				"reg value is greater than the number of available configuration registers\n");
+			return -EINVAL;
+		}
+
+		of_property_read_u32(child, "fsl,adc-refp", &refp);
+		of_property_read_u32(child, "fsl,adc-refn", &refn);
+
+		/*
+		 * Shift the read values to the correct positions within the
+		 * register.
+		 */
+		refp = MX25_ADCQ_CFG_REFP(refp);
+		refn = MX25_ADCQ_CFG_REFN(refn);
+
+		if ((refp & MX25_ADCQ_CFG_REFP_MASK) != refp) {
+			dev_err(dev, "Invalid fsl,adc-refp property value\n");
+			return -EINVAL;
+		}
+		if ((refn & MX25_ADCQ_CFG_REFN_MASK) != refn) {
+			dev_err(dev, "Invalid fsl,adc-refn property value");
+			return -EINVAL;
+		}
+
+		regmap_update_bits(priv->regs, MX25_ADCQ_CFG(reg),
+				   MX25_ADCQ_CFG_REFP_MASK |
+				   MX25_ADCQ_CFG_REFN_MASK,
+				   refp | refn);
+	}

drivers/iio/adc/fsl-imx25-gcq.c line 244

Quoted 7 lines

drivers/iio/adc/fsl-imx25-gcq.c new file +7 -0

+	regmap_update_bits(priv->regs, MX25_ADCQ_CR,
+			   MX25_ADCQ_CR_FRST | MX25_ADCQ_CR_QRST,
+			   MX25_ADCQ_CR_FRST | MX25_ADCQ_CR_QRST);
+
+	regmap_write(priv->regs, MX25_ADCQ_CR,
+		     MX25_ADCQ_CR_PDMSK |
+		     MX25_ADCQ_CR_QSM_FQS);

Odd line breaking. I certainly wouldn't break the last two lines.

Quoted 27 lines

drivers/iio/adc/fsl-imx25-gcq.c new file +27 -0

+
+	return 0;
+}
+
+static int mx25_gcq_probe(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev;
+	struct mx25_gcq_priv *priv;
+	struct mx25_tsadc *tsadc = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	void __iomem *mem;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	priv = iio_priv(indio_dev);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mem = devm_ioremap_resource(dev, res);
+	if (!mem)
+		return -ENOMEM;
+
+	priv->regs = devm_regmap_init_mmio(dev, mem, &mx25_gcq_regconfig);
+	if (IS_ERR(priv->regs)) {

drivers/iio/adc/fsl-imx25-gcq.c line 281

Quoted 10 lines

drivers/iio/adc/fsl-imx25-gcq.c new file +10 -0

+		dev_err(dev, "Failed to initialize regmap\n");
+		return PTR_ERR(priv->regs);
+	}
+
+	init_completion(&priv->completed);
+
+	/* Optional external regulator */
+	priv->ext_vref = devm_regulator_get(&pdev->dev, "vref");
+	if (!IS_ERR_OR_NULL(priv->ext_vref)) {
+		ret = regulator_enable(priv->ext_vref);

This is documented in the bindings as being only optional in the sense that there may or may not be channels using it. I'd be inclined to have it as non optional, but only request it if in use.

If truely optional then should be using the get_optional form as otherwise IIRC the regulator framework will feed you a dummy regulator under some circumstances.

Quoted 35 lines

drivers/iio/adc/fsl-imx25-gcq.c new file +35 -0

+		if (ret)
+			return ret;
+	}
+
+	ret = mx25_gcq_setup_cfgs(pdev, priv);
+	if (ret)
+		return ret;
+
+	priv->clk = tsadc->clk;
+	ret = clk_prepare_enable(priv->clk);
+	if (ret) {
+		dev_err(dev, "Failed to enable clock\n");
+		return ret;
+	}
+
+	priv->irq = platform_get_irq(pdev, 0);
+	if (priv->irq <= 0) {
+		dev_err(dev, "Failed to get IRQ\n");
+		ret = priv->irq;
+		goto err_clk_unprepare;
+	}
+
+	ret = request_irq(priv->irq, mx25_gcq_irq, 0, pdev->name, priv);
+	if (ret) {
+		dev_err(dev, "Failed requesting IRQ\n");
+		goto err_clk_unprepare;
+	}
+
+	indio_dev->dev.parent = &pdev->dev;
+	indio_dev->channels = mx25_gcq_channels;
+	indio_dev->num_channels = ARRAY_SIZE(mx25_gcq_channels);
+	indio_dev->info = &mx25_gcq_iio_info;
+
+	ret = iio_device_register(indio_dev);
+	if (ret) {

drivers/iio/adc/fsl-imx25-gcq.c line 321

Quoted 5 lines

drivers/iio/adc/fsl-imx25-gcq.c new file +5 -0

+		dev_err(dev, "Failed to register iio device\n");
+		goto err_irq_free;
+	}
+
+	platform_set_drvdata(pdev, priv);

I'd normally prefer slightly if this used the indio_dev (just a case of logically what contains what...) Doesn't really matter though.

This is a driver for the imx25 ADC/TSC module. It controls the touchscreen conversion queue and creates a touchscreen input device. The driver currently only supports 4 wire touchscreens. The driver uses a simple conversion queue of precharge, touch detection, X measurement, Y measurement, precharge and another touch detection.

This driver uses the regmap from the parent to setup some touch specific settings in the core driver and setup a idle configuration with touch detection.

Signed-off-by Markus Pargmann <[email protected]>

Signed-off-by Denis Carikli <[email protected]>

Acked-by Dmitry Torokhov <[email protected]>

Signed-off-by Markus Pargmann <[email protected]>

---
  drivers/input/touchscreen/Kconfig         |   6 +
  drivers/input/touchscreen/Makefile        |   1 +
  drivers/input/touchscreen/fsl-imx25-tcq.c | 587 ++++++++++++++++++++++++++++++
  3 files changed, 594 insertions(+)

create mode 100644 drivers/input/touchscreen/fsl-imx25-tcq.c

drivers/input/touchscreen/fsl-imx25-tcq.c new file +7 -0

+	if (ret) {
+		dev_err(dev, "Failed requesting IRQ\n");
+		goto err_clk_unprepare;
+	}
+
+	ret = mx25_tcq_init(priv);
+	if (ret) {

Hi,

Quoted 21 lines · Markus Pargmann

This is a driver for the imx25 ADC/TSC module. It controls the touchscreen conversion queue and creates a touchscreen input device. The driver currently only supports 4 wire touchscreens. The driver uses a simple conversion queue of precharge, touch detection, X measurement, Y measurement, precharge and another touch detection.

This driver uses the regmap from the parent to setup some touch specific settings in the core driver and setup a idle configuration with touch detection.

Signed-off-by Markus Pargmann <[email protected]>

Signed-off-by Denis Carikli <[email protected]>

Acked-by Dmitry Torokhov <[email protected]>

Signed-off-by Markus Pargmann <[email protected]>

---
 drivers/input/touchscreen/Kconfig         |   6 +
 drivers/input/touchscreen/Makefile        |   1 +
 drivers/input/touchscreen/fsl-imx25-tcq.c | 587 ++++++++++++++++++++++++++++++
 3 files changed, 594 insertions(+)
 create mode 100644 drivers/input/touchscreen/fsl-imx25-tcq.c

(...)

drivers/input/touchscreen/fsl-imx25-tcq.c line 542

Quoted 2 lines

drivers/input/touchscreen/fsl-imx25-tcq.c new file +2 -0

+	ret = request_threaded_irq(priv->irq, mx25_tcq_irq, mx25_tcq_irq_thread,
+				   IRQF_ONESHOT, pdev->name, priv);

We can use devres API for request_thread_irq()...

Quoted 11 lines

drivers/input/touchscreen/fsl-imx25-tcq.c new file +11 -0

+	if (ret) {
+		dev_err(dev, "Failed requesting IRQ\n");
+		goto err_clk_unprepare;
+	}
+
+	ret = mx25_tcq_init(priv);
+	if (ret) {
+		dev_err(dev, "Failed to init tcq\n");
+		goto error_free_irq;
+	}
+

drivers/input/touchscreen/fsl-imx25-tcq.c line 559

Quoted 6 lines

drivers/input/touchscreen/fsl-imx25-tcq.c new file +6 -0

+	platform_set_drvdata(pdev, priv);
+
+	return 0;
+
+error_free_irq:
+	free_irq(priv->irq, priv);

This is not required if we use devres API

Quoted 42 lines · Varka Bhadram

Hi,

Quoted 21 lines · Markus Pargmann

This is a driver for the imx25 ADC/TSC module. It controls the touchscreen conversion queue and creates a touchscreen input device. The driver currently only supports 4 wire touchscreens. The driver uses a simple conversion queue of precharge, touch detection, X measurement, Y measurement, precharge and another touch detection.

This driver uses the regmap from the parent to setup some touch specific settings in the core driver and setup a idle configuration with touch detection.

Signed-off-by Markus Pargmann <[email protected]>

Signed-off-by Denis Carikli <[email protected]>

Acked-by Dmitry Torokhov <[email protected]>

Signed-off-by Markus Pargmann <[email protected]>

---
 drivers/input/touchscreen/Kconfig         |   6 +
 drivers/input/touchscreen/Makefile        |   1 +
 drivers/input/touchscreen/fsl-imx25-tcq.c | 587 ++++++++++++++++++++++++++++++
 3 files changed, 594 insertions(+)
 create mode 100644 drivers/input/touchscreen/fsl-imx25-tcq.c

(...)

drivers/input/touchscreen/fsl-imx25-tcq.c line 542

Quoted 2 lines

drivers/input/touchscreen/fsl-imx25-tcq.c new file +2 -0

+	ret = request_threaded_irq(priv->irq, mx25_tcq_irq, mx25_tcq_irq_thread,
+				   IRQF_ONESHOT, pdev->name, priv);

We can use devres API for request_thread_irq()...

Quoted 11 lines

drivers/input/touchscreen/fsl-imx25-tcq.c new file +11 -0

+	if (ret) {
+		dev_err(dev, "Failed requesting IRQ\n");
+		goto err_clk_unprepare;
+	}
+
+	ret = mx25_tcq_init(priv);
+	if (ret) {
+		dev_err(dev, "Failed to init tcq\n");
+		goto error_free_irq;
+	}
+

Quoted 8 lines

drivers/input/touchscreen/fsl-imx25-tcq.c line 559

Quoted 6 lines

drivers/input/touchscreen/fsl-imx25-tcq.c new file +6 -0

+	platform_set_drvdata(pdev, priv);
+
+	return 0;
+
+error_free_irq:
+	free_irq(priv->irq, priv);

This is not required if we use devres API

Yes it does - you do not really want to stop clocks in the middle of servicing interrupt.

Hi,

Quoted 52 lines · Dmitry Torokhov

Quoted 50 lines · Varka Bhadram

Hi,

Quoted 21 lines · Markus Pargmann

This is a driver for the imx25 ADC/TSC module. It controls the touchscreen conversion queue and creates a touchscreen input device. The driver currently only supports 4 wire touchscreens. The driver uses a simple conversion queue of precharge, touch detection, X measurement, Y measurement, precharge and another touch detection.

This driver uses the regmap from the parent to setup some touch specific settings in the core driver and setup a idle configuration with touch detection.

Signed-off-by Markus Pargmann <[email protected]>

Signed-off-by Denis Carikli <[email protected]>

Acked-by Dmitry Torokhov <[email protected]>

Signed-off-by Markus Pargmann <[email protected]>

---
 drivers/input/touchscreen/Kconfig         |   6 +
 drivers/input/touchscreen/Makefile        |   1 +
 drivers/input/touchscreen/fsl-imx25-tcq.c | 587 ++++++++++++++++++++++++++++++
 3 files changed, 594 insertions(+)
 create mode 100644 drivers/input/touchscreen/fsl-imx25-tcq.c

(...)

drivers/input/touchscreen/fsl-imx25-tcq.c line 542

Quoted 2 lines

drivers/input/touchscreen/fsl-imx25-tcq.c new file +2 -0

+	ret = request_threaded_irq(priv->irq, mx25_tcq_irq, mx25_tcq_irq_thread,
+				   IRQF_ONESHOT, pdev->name, priv);

We can use devres API for request_thread_irq()...

Quoted 7 lines

drivers/input/touchscreen/fsl-imx25-tcq.c new file +7 -0

+	if (ret) {
+		dev_err(dev, "Failed requesting IRQ\n");
+		goto err_clk_unprepare;
+	}
+
+	ret = mx25_tcq_init(priv);
+	if (ret) {

drivers/input/touchscreen/fsl-imx25-tcq.c line 559

Quoted 10 lines

drivers/input/touchscreen/fsl-imx25-tcq.c new file +10 -0

+		dev_err(dev, "Failed to init tcq\n");
+		goto error_free_irq;
+	}
+
+	platform_set_drvdata(pdev, priv);
+
+	return 0;
+
+error_free_irq:
+	free_irq(priv->irq, priv);

This is not required if we use devres API

Quoted 2 lines

Yes it does - you do not really want to stop clocks in the middle of servicing interrupt.

Thanks, I missed the clocks. I will not use devm here then.

Quoted 58 lines · Markus Pargmann

Hi,

Quoted 54 lines · Dmitry Torokhov

Quoted 42 lines · Varka Bhadram

Hi,

Quoted 21 lines · Markus Pargmann

This is a driver for the imx25 ADC/TSC module. It controls the touchscreen conversion queue and creates a touchscreen input device. The driver currently only supports 4 wire touchscreens. The driver uses a simple conversion queue of precharge, touch detection, X measurement, Y measurement, precharge and another touch detection.

This driver uses the regmap from the parent to setup some touch specific settings in the core driver and setup a idle configuration with touch detection.

Signed-off-by Markus Pargmann <[email protected]>

Signed-off-by Denis Carikli <[email protected]>

Acked-by Dmitry Torokhov <[email protected]>

Signed-off-by Markus Pargmann <[email protected]>

---
 drivers/input/touchscreen/Kconfig         |   6 +
 drivers/input/touchscreen/Makefile        |   1 +
 drivers/input/touchscreen/fsl-imx25-tcq.c | 587 ++++++++++++++++++++++++++++++
 3 files changed, 594 insertions(+)
 create mode 100644 drivers/input/touchscreen/fsl-imx25-tcq.c

(...)

drivers/input/touchscreen/fsl-imx25-tcq.c line 542

Quoted 2 lines

drivers/input/touchscreen/fsl-imx25-tcq.c new file +2 -0

+	ret = request_threaded_irq(priv->irq, mx25_tcq_irq, mx25_tcq_irq_thread,
+				   IRQF_ONESHOT, pdev->name, priv);

We can use devres API for request_thread_irq()...

Quoted 11 lines

drivers/input/touchscreen/fsl-imx25-tcq.c new file +11 -0

+	if (ret) {
+		dev_err(dev, "Failed requesting IRQ\n");
+		goto err_clk_unprepare;
+	}
+
+	ret = mx25_tcq_init(priv);
+	if (ret) {
+		dev_err(dev, "Failed to init tcq\n");
+		goto error_free_irq;
+	}
+

Quoted 8 lines

drivers/input/touchscreen/fsl-imx25-tcq.c line 559

Quoted 6 lines

drivers/input/touchscreen/fsl-imx25-tcq.c new file +6 -0

+	platform_set_drvdata(pdev, priv);
+
+	return 0;
+
+error_free_irq:
+	free_irq(priv->irq, priv);

This is not required if we use devres API

Yes it does - you do not really want to stop clocks in the middle of servicing interrupt.

Quoted 1 line

Thanks, I missed the clocks. I will not use devm here then.

Actually, you still can if you move clock enabling/disabling and mx25_tcq_init() into input_dev->open() and ->close() callbacks. Close will be called during input device un-registration which happens (given your current sequence) after freeing irq by devm.

By the way, I used my old @vmware address by accident. Can you please replace the original acked by with:

Acked-by Dmitry Torokhov <[email protected]>