[PATCH v2 0/6] Add support for STM32 ADC

[PATCH v2 0/6] Add support for STM32 ADC

[Fabrice Gasnier]

This series adds support for STM32F4 ADC into IIO framework.
STM32F4 ADC is a 12-bit successive approximation analog-to-digital
converter with multiplexed input channels. Conversions can
be performed in single, continuous, scan or discontinuous mode.
Conversions can be launched in software or using hardware triggers.

This driver has been developed and tested on STM32F429 eval board.
It consist of a MFD core driver, to manage common resources shared
between up to 3 ADC instances.

Changes in v2:
- Replace single driver model by MFD approach, to handle up to 3 ADCs
  as separate devices. Each ADC device then registers a unique IIO
  device.
- Make driver as simple as possible for the first instance, to ease
  review. For now, I dropped complexity by removing injected support,
  triggered buffer mode, dmas.
- Removed abstraction layer (indirection routines, ops) as only stm32f4
  is supported.

Fabrice Gasnier (6):
  Documentation: dt-bindings: Document STM32 ADC DT bindings
  mfd: stm32-adc: Add support for stm32 ADC
  iio: adc: Add support for STM32 ADC
  ARM: configs: stm32: enable ADC driver
  ARM: dts: stm32f429: Add adc support
  ARM: dts: stm32f429: enable adc on eval board

 .../devicetree/bindings/iio/adc/st,stm32-adc.txt   |  85 ++++
 arch/arm/boot/dts/stm32429i-eval.dts               |  25 +
 arch/arm/boot/dts/stm32f429.dtsi                   |  49 ++
 arch/arm/configs/stm32_defconfig                   |   3 +
 drivers/iio/adc/Kconfig                            |  10 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/stm32-adc.c                        | 525 +++++++++++++++++++++
 drivers/mfd/Kconfig                                |  14 +
 drivers/mfd/Makefile                               |   1 +
 drivers/mfd/stm32-adc-core.c                       | 301 ++++++++++++
 include/linux/mfd/stm32-adc-core.h                 |  52 ++
 11 files changed, 1066 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
 create mode 100644 drivers/iio/adc/stm32-adc.c
 create mode 100644 drivers/mfd/stm32-adc-core.c
 create mode 100644 include/linux/mfd/stm32-adc-core.h

-- 
1.9.1

[PATCH v2 1/6] Documentation: dt-bindings: Document STM32 ADC DT bindings

[Fabrice Gasnier]

This patch adds documentation of device tree bindings for the STM32 ADC.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 .../devicetree/bindings/iio/adc/st,stm32-adc.txt   | 85 ++++++++++++++++++++++
 1 file changed, 85 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt

diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
new file mode 100644
index 0000000..8b20c23
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
@@ -0,0 +1,85 @@
+STMicroelectronics STM32 ADC device driver
+
+STM32 ADC is a successive approximation analog-to-digital converter.
+It has several multiplexed input channels. Conversions can be performed
+in single, continuous, scan or discontinuous mode. Result of the ADC is
+stored in a left-aligned or right-aligned 32-bit data register.
+Conversions can be launched in software or using hardware triggers.
+
+The analog watchdog feature allows the application to detect if the input
+voltage goes beyond the user-defined, higher or lower thresholds.
+
+Each STM32 ADC block can have up to 3 ADC instances.
+
+Each instance supports two contexts to manage conversions, each one has its
+own configurable sequence and trigger:
+- regular conversion can be done in sequence, running in background
+- injected conversions have higher priority, and so have the ability to
+  interrupt regular conversion sequence (either triggered in SW or HW).
+  Regular sequence is resumed, in case it has been interrupted.
+
+Contents of a stm32 adc root node:
+-----------------------------------
+Required properties:
+- compatible: Should be "st,stm32f4-adc-core".
+- reg: Offset and length of the ADC block register set.
+- interrupts: Must contain the interrupt for ADC block.
+- clocks: Clock for the analog circuitry (common to all ADCs).
+- clock-names: Must be "adc".
+- interrupt-controller: Identifies the controller node as interrupt-parent
+- vref-supply: Phandle to the vref input analog reference voltage.
+- #interrupt-cells = <1>;
+- #address-cells = <1>;
+- #size-cells = <0>;
+
+Optional properties:
+- A pinctrl state named "default" for each ADC channel may be defined to set
+  inX ADC pins in mode of operation for analog input on external pin.
+
+Contents of a stm32 adc child node:
+-----------------------------------
+An ADC block node should contain at least one subnode, representing an
+ADC instance available on the machine.
+
+Required properties:
+- compatible: Should be "st,stm32f4-adc".
+- reg: Offset of ADC instance in ADC block (e.g. may be 0x0, 0x100, 0x200).
+- st,adc-channels: List of single-ended channels muxed for this ADC.
+  It can have up to 16 channels, numbered from 0 to 15 (resp. for in0..in15).
+- interrupt-parent: Phandle to the parent interrupt controller.
+- interrupts: IRQ Line for the ADC (e.g. may be 0 for adc@0, 1 for adc@100 or
+  2 for adc@200).
+- #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
+  Documentation/devicetree/bindings/iio/iio-bindings.txt
+
+Optional properties:
+- clocks: Input clock private to this ADC instance.
+
+Example:
+	adc: adc@40012000 {
+		compatible = "st,stm32f4-adc-core";
+		reg = <0x40012000 0x400>;
+		interrupts = <18>;
+		clocks = <&rcc 0 168>;
+		clock-names = "adc";
+		vref-supply = <&reg_vref>;
+		interrupt-controller;
+		pinctrl-names = "default";
+		pinctrl-0 = <&adc3_in8_pin>;
+
+		#interrupt-cells = <1>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		adc@0 {
+			compatible = "st,stm32f4-adc";
+			#io-channel-cells = <1>;
+			reg = <0x0>;
+			clocks = <&rcc 0 168>;
+			interrupt-parent = <&adc>;
+			interrupts = <0>;
+			st,adc-channels = <8>;
+		};
+		...
+		other adc child nodes follow...
+	};
-- 
1.9.1

[PATCH v2 2/6] mfd: stm32-adc: Add support for stm32 ADC

[Fabrice Gasnier]

Add core driver for STMicroelectronics STM32 ADC (Analog to Digital
Converter). STM32 ADC can be composed of up to 3 ADCs with shared
resources like clock prescaler, common interrupt line and analog
reference voltage.
This core driver basically manages shared resources.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/mfd/Kconfig                |  14 ++
 drivers/mfd/Makefile               |   1 +
 drivers/mfd/stm32-adc-core.c       | 301 +++++++++++++++++++++++++++++++++++++
 include/linux/mfd/stm32-adc-core.h |  52 +++++++
 4 files changed, 368 insertions(+)
 create mode 100644 drivers/mfd/stm32-adc-core.c
 create mode 100644 include/linux/mfd/stm32-adc-core.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index c6df644..2580cee 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1152,6 +1152,20 @@ config MFD_PALMAS
 	  If you say yes here you get support for the Palmas
 	  series of PMIC chips from Texas Instruments.
 
+config MFD_STM32_ADC
+	tristate "STMicroelectronics STM32 adc"
+	depends on ARCH_STM32 || COMPILE_TEST
+	depends on OF
+	select MFD_CORE
+	select REGULATOR
+	select REGULATOR_FIXED_VOLTAGE
+	help
+	  Select this option to enable the core driver for STMicroelectronics
+	  STM32 analog-to-digital converter (ADC).
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called stm32-adc-core.
+
 config TPS6105X
 	tristate "TI TPS61050/61052 Boost Converters"
 	depends on I2C
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 9834e66..4571506 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -185,6 +185,7 @@ obj-$(CONFIG_MFD_INTEL_LPSS_PCI)	+= intel-lpss-pci.o
 obj-$(CONFIG_MFD_INTEL_LPSS_ACPI)	+= intel-lpss-acpi.o
 obj-$(CONFIG_MFD_INTEL_MSIC)	+= intel_msic.o
 obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
+obj-$(CONFIG_MFD_STM32_ADC) 	+= stm32-adc-core.o
 obj-$(CONFIG_MFD_VIPERBOARD)    += viperboard.o
 obj-$(CONFIG_MFD_RC5T583)	+= rc5t583.o rc5t583-irq.o
 obj-$(CONFIG_MFD_RK808)		+= rk808.o
diff --git a/drivers/mfd/stm32-adc-core.c b/drivers/mfd/stm32-adc-core.c
new file mode 100644
index 0000000..bcf52fb
--- /dev/null
+++ b/drivers/mfd/stm32-adc-core.c
@@ -0,0 +1,301 @@
+/*
+ * This file is part of STM32 ADC driver
+ *
+ * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
+ * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
+ *
+ * Inspired from: fsl-imx25-tsadc
+ *
+ * License type: GPLv2
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdesc.h>
+#include <linux/irqdomain.h>
+#include <linux/mfd/stm32-adc-core.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+/* STM32F4 - common registers for all ADC instances: 1, 2 & 3 */
+#define STM32F4_ADC_CSR			(STM32_ADCX_COMN_OFFSET + 0x00)
+#define STM32F4_ADC_CCR			(STM32_ADCX_COMN_OFFSET + 0x04)
+
+/* STM32F4_ADC_CSR - bit fields */
+#define STM32F4_EOC3			BIT(17)
+#define STM32F4_EOC2			BIT(9)
+#define STM32F4_EOC1			BIT(1)
+
+/* STM32F4_ADC_CCR - bit fields */
+#define STM32F4_ADC_ADCPRE_SHIFT	16
+#define STM32F4_ADC_ADCPRE_MASK		GENMASK(17, 16)
+
+/* STM32 F4 maximum analog clock rate (from datasheet) */
+#define STM32F4_ADC_MAX_CLK_RATE	36000000
+
+/**
+ * struct stm32_adc_priv - stm32 ADC core private data
+ * @irq:		irq for ADC block
+ * @domain:		irq domain reference
+ * @aclk:		clock reference for the analog circuitry
+ * @vref:		regulator reference
+ * @common:		common data for all ADC instances
+ */
+struct stm32_adc_priv {
+	int				irq;
+	struct irq_domain		*domain;
+	struct clk			*aclk;
+	struct regulator		*vref;
+	struct stm32_adc_common		common;
+};
+
+static struct stm32_adc_priv *to_stm32_adc_priv(struct stm32_adc_common *com)
+{
+	return container_of(com, struct stm32_adc_priv, common);
+}
+
+/* STM32F4 ADC internal common clock prescaler division ratios */
+static int stm32f4_pclk_div[] = {2, 4, 6, 8};
+
+/**
+ * stm32f4_adc_clk_sel() - Select stm32f4 ADC common clock prescaler
+ * @priv: stm32 ADC core private data
+ * Select clock prescaler used for analog conversions, before using ADC.
+ */
+static int stm32f4_adc_clk_sel(struct platform_device *pdev,
+			       struct stm32_adc_priv *priv)
+{
+	unsigned long rate;
+	u32 val;
+	int i;
+
+	rate = clk_get_rate(priv->aclk);
+	for (i = 0; i < ARRAY_SIZE(stm32f4_pclk_div); i++) {
+		if ((rate / stm32f4_pclk_div[i]) <= STM32F4_ADC_MAX_CLK_RATE)
+			break;
+	}
+	if (i >= ARRAY_SIZE(stm32f4_pclk_div))
+		return -EINVAL;
+
+	val = readl_relaxed(priv->common.base + STM32F4_ADC_CCR);
+	val &= ~STM32F4_ADC_ADCPRE_MASK;
+	val |= i << STM32F4_ADC_ADCPRE_SHIFT;
+	writel_relaxed(val, priv->common.base + STM32F4_ADC_CCR);
+
+	dev_dbg(&pdev->dev, "Using analog clock source at %ld kHz\n",
+		rate / (stm32f4_pclk_div[i] * 1000));
+
+	return 0;
+}
+
+/* ADC common interrupt for all instances */
+static void stm32_adc_irq_handler(struct irq_desc *desc)
+{
+	struct stm32_adc_priv *priv = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	u32 status;
+
+	chained_irq_enter(chip, desc);
+	status = readl_relaxed(priv->common.base + STM32F4_ADC_CSR);
+
+	if (status & STM32F4_EOC1)
+		generic_handle_irq(irq_find_mapping(priv->domain, 0));
+
+	if (status & STM32F4_EOC2)
+		generic_handle_irq(irq_find_mapping(priv->domain, 1));
+
+	if (status & STM32F4_EOC3)
+		generic_handle_irq(irq_find_mapping(priv->domain, 2));
+
+	chained_irq_exit(chip, desc);
+};
+
+static int stm32_adc_domain_map(struct irq_domain *d, unsigned int irq,
+				irq_hw_number_t hwirq)
+{
+	irq_set_chip_data(irq, d->host_data);
+	irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_level_irq);
+
+	return 0;
+}
+
+static void stm32_adc_domain_unmap(struct irq_domain *d, unsigned int irq)
+{
+	irq_set_chip_and_handler(irq, NULL, NULL);
+	irq_set_chip_data(irq, NULL);
+}
+
+static const struct irq_domain_ops stm32_adc_domain_ops = {
+	.map = stm32_adc_domain_map,
+	.unmap  = stm32_adc_domain_unmap,
+	.xlate = irq_domain_xlate_onecell,
+};
+
+static int stm32_adc_irq_probe(struct platform_device *pdev,
+			       struct stm32_adc_priv *priv)
+{
+	struct device_node *np = pdev->dev.of_node;
+
+	priv->irq = platform_get_irq(pdev, 0);
+	if (priv->irq < 0) {
+		dev_err(&pdev->dev, "failed to get irq\n");
+		return priv->irq;
+	}
+
+	priv->domain = irq_domain_add_simple(np, STM32_ADC_MAX_ADCS, 0,
+					     &stm32_adc_domain_ops,
+					     priv);
+	if (!priv->domain) {
+		dev_err(&pdev->dev, "Failed to add irq domain\n");
+		return -ENOMEM;
+	}
+
+	irq_set_chained_handler(priv->irq, stm32_adc_irq_handler);
+	irq_set_handler_data(priv->irq, priv);
+
+	return 0;
+}
+
+static void stm32_adc_irq_remove(struct platform_device *pdev,
+				 struct stm32_adc_priv *priv)
+{
+	int hwirq;
+
+	for (hwirq = 0; hwirq < STM32_ADC_MAX_ADCS; hwirq++)
+		irq_dispose_mapping(irq_find_mapping(priv->domain, hwirq));
+	irq_domain_remove(priv->domain);
+	irq_set_chained_handler(priv->irq, NULL);
+}
+
+static int stm32_adc_probe(struct platform_device *pdev)
+{
+	struct stm32_adc_priv *priv;
+	struct device_node *np = pdev->dev.of_node;
+	struct resource *res;
+	int ret;
+
+	if (!pdev->dev.of_node)
+		return -ENODEV;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->common.base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(priv->common.base))
+		return PTR_ERR(priv->common.base);
+
+	priv->vref = devm_regulator_get(&pdev->dev, "vref");
+	if (IS_ERR(priv->vref)) {
+		ret = PTR_ERR(priv->vref);
+		dev_err(&pdev->dev, "vref get failed, %d\n", ret);
+		return ret;
+	}
+
+	ret = regulator_enable(priv->vref);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "vref enable failed\n");
+		return ret;
+	}
+
+	ret = regulator_get_voltage(priv->vref);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "vref get voltage failed, %d\n", ret);
+		goto err_regulator_disable;
+	}
+	priv->common.vref_mv = ret / 1000;
+	dev_dbg(&pdev->dev, "vref+=%dmV\n", priv->common.vref_mv);
+
+	priv->aclk = devm_clk_get(&pdev->dev, "adc");
+	if (IS_ERR(priv->aclk)) {
+		ret = PTR_ERR(priv->aclk);
+		dev_err(&pdev->dev, "Can't get 'adc' clock\n");
+		goto err_regulator_disable;
+	}
+
+	ret = clk_prepare_enable(priv->aclk);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "adc clk enable failed\n");
+		goto err_regulator_disable;
+	}
+
+	ret = stm32f4_adc_clk_sel(pdev, priv);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "adc clk selection failed\n");
+		goto err_clk_disable;
+	}
+
+	ret = stm32_adc_irq_probe(pdev, priv);
+	if (ret < 0)
+		goto err_clk_disable;
+
+	platform_set_drvdata(pdev, &priv->common);
+
+	ret = of_platform_populate(np, NULL, NULL, &pdev->dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to populate DT children\n");
+		goto err_irq_remove;
+	}
+
+	return 0;
+
+err_irq_remove:
+	stm32_adc_irq_remove(pdev, priv);
+
+err_clk_disable:
+	clk_disable_unprepare(priv->aclk);
+
+err_regulator_disable:
+	regulator_disable(priv->vref);
+
+	return ret;
+}
+
+static int stm32_adc_remove(struct platform_device *pdev)
+{
+	struct stm32_adc_common *common = platform_get_drvdata(pdev);
+	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
+
+	of_platform_depopulate(&pdev->dev);
+	stm32_adc_irq_remove(pdev, priv);
+	clk_disable_unprepare(priv->aclk);
+	regulator_disable(priv->vref);
+
+	return 0;
+}
+
+static const struct of_device_id stm32_adc_of_match[] = {
+	{ .compatible = "st,stm32f4-adc-core" },
+};
+MODULE_DEVICE_TABLE(of, stm32_adc_of_match);
+
+static struct platform_driver stm32_adc_driver = {
+	.probe = stm32_adc_probe,
+	.remove = stm32_adc_remove,
+	.driver = {
+		.name = "stm32-adc-core",
+		.of_match_table = stm32_adc_of_match,
+	},
+};
+module_platform_driver(stm32_adc_driver);
+
+MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@st.com>");
+MODULE_DESCRIPTION("STMicroelectronics STM32 ADC MFD driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:stm32-adc-core");
diff --git a/include/linux/mfd/stm32-adc-core.h b/include/linux/mfd/stm32-adc-core.h
new file mode 100644
index 0000000..081fa5f
--- /dev/null
+++ b/include/linux/mfd/stm32-adc-core.h
@@ -0,0 +1,52 @@
+/*
+ * This file is part of STM32 ADC driver
+ *
+ * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
+ * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
+ *
+ * License type: GPLv2
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __STM32_ADC_H
+#define __STM32_ADC_H
+
+/*
+ * STM32 - ADC global register map
+ * ________________________________________________________
+ * | Offset |                 Register                    |
+ * --------------------------------------------------------
+ * | 0x000  |                Master ADC1                  |
+ * --------------------------------------------------------
+ * | 0x100  |                Slave ADC2                   |
+ * --------------------------------------------------------
+ * | 0x200  |                Slave ADC3                   |
+ * --------------------------------------------------------
+ * | 0x300  |         Master & Slave common regs          |
+ * --------------------------------------------------------
+ */
+#define STM32_ADC_MAX_ADCS		3
+#define STM32_ADCX_COMN_OFFSET		0x300
+
+/**
+ * struct stm32_adc_common - stm32 ADC driver common data (for all instances)
+ * @base:		control registers base cpu addr
+ * @vref_mv:		vref voltage (mv)
+ */
+struct stm32_adc_common {
+	void __iomem			*base;
+	int				vref_mv;
+};
+
+#endif
-- 
1.9.1

[PATCH v2 3/6] iio: adc: Add support for STM32 ADC

[Fabrice Gasnier]

This patch adds support for STMicroelectronics STM32 MCU's analog to
digital converter.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/iio/adc/Kconfig     |  10 +
 drivers/iio/adc/Makefile    |   1 +
 drivers/iio/adc/stm32-adc.c | 525 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 536 insertions(+)
 create mode 100644 drivers/iio/adc/stm32-adc.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 7edcf32..61ba674 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -419,6 +419,16 @@ config ROCKCHIP_SARADC
 	  To compile this driver as a module, choose M here: the
 	  module will be called rockchip_saradc.
 
+config STM32_ADC
+	tristate "STMicroelectronics STM32 adc"
+	depends on MFD_STM32_ADC
+	help
+	  Say yes here to build support for STMicroelectronics stm32 Analog
+	  to Digital Converter (ADC).
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called stm32-adc.
+
 config STX104
 	tristate "Apex Embedded Systems STX104 driver"
 	depends on X86 && ISA_BUS_API
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 7a40c04..df7a221 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -41,6 +41,7 @@ obj-$(CONFIG_QCOM_SPMI_IADC) += qcom-spmi-iadc.o
 obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
 obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
 obj-$(CONFIG_STX104) += stx104.o
+obj-$(CONFIG_STM32_ADC) += stm32-adc.o
 obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
 obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
 obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
new file mode 100644
index 0000000..2be5fee
--- /dev/null
+++ b/drivers/iio/adc/stm32-adc.c
@@ -0,0 +1,525 @@
+/*
+ * This file is part of STM32 ADC driver
+ *
+ * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
+ * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
+ *
+ * License type: GPLv2
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/iio/iio.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/mfd/stm32-adc-core.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+
+/* STM32F4 - Registers for each ADC instance */
+#define STM32F4_ADCX_SR			0x00
+#define STM32F4_ADCX_CR1		0x04
+#define STM32F4_ADCX_CR2		0x08
+#define STM32F4_ADCX_SMPR1		0x0C
+#define STM32F4_ADCX_SMPR2		0x10
+#define STM32F4_ADCX_HTR		0x24
+#define STM32F4_ADCX_LTR		0x28
+#define STM32F4_ADCX_SQR1		0x2C
+#define STM32F4_ADCX_SQR2		0x30
+#define STM32F4_ADCX_SQR3		0x34
+#define STM32F4_ADCX_JSQR		0x38
+#define STM32F4_ADCX_JDR1		0x3C
+#define STM32F4_ADCX_JDR2		0x40
+#define STM32F4_ADCX_JDR3		0x44
+#define STM32F4_ADCX_JDR4		0x48
+#define STM32F4_ADCX_DR			0x4C
+
+/* STM32F4_ADCX_SR - bit fields */
+#define STM32F4_OVR			BIT(5)
+#define STM32F4_STRT			BIT(4)
+#define STM32F4_EOC			BIT(1)
+
+/* STM32F4_ADCX_CR1 - bit fields */
+#define STM32F4_OVRIE			BIT(26)
+#define STM32F4_SCAN			BIT(8)
+#define STM32F4_EOCIE			BIT(5)
+
+/* STM32F4_ADCX_CR2 - bit fields */
+#define STM32F4_SWSTART			BIT(30)
+#define STM32F4_EXTEN_MASK		GENMASK(29, 28)
+#define STM32F4_EOCS			BIT(10)
+#define STM32F4_ADON			BIT(0)
+
+/* STM32F4_ADCX_SQR1 - bit fields */
+#define STM32F4_L_SHIFT			20
+#define STM32F4_L_MASK			GENMASK(23, 20)
+
+/* STM32F4_ADCX_SQR3 - bit fields */
+#define STM32F4_SQ1_SHIFT		0
+#define STM32F4_SQ1_MASK		GENMASK(4, 0)
+
+#define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
+#define STM32_ADC_TIMEOUT_US		100000
+#define STM32_ADC_TIMEOUT	(msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
+
+/**
+ * struct stm32_adc - private data of each ADC IIO instance
+ * @common:		reference to ADC block common data
+ * @offset:		ADC instance register offset in ADC block
+ * @completion:		end of single conversion completion
+ * @buffer:		data buffer
+ * @clk:		optional adc clock, for this adc instance
+ * @irq:		interrupt for this adc instance
+ * @lock:		spinlock
+ */
+struct stm32_adc {
+	struct stm32_adc_common	*common;
+	u32			offset;
+	struct completion	completion;
+	u16			*buffer;
+	struct clk		*clk;
+	int			irq;
+	spinlock_t		lock;		/* interrupt lock */
+};
+
+/**
+ * struct stm32_adc_chan_spec - specification of stm32 adc channel
+ * @type:	IIO channel type
+ * @channel:	channel number (single ended)
+ * @name:	channel name (single ended)
+ */
+struct stm32_adc_chan_spec {
+	enum iio_chan_type	type;
+	int			channel;
+	const char		*name;
+};
+
+/* Input definitions common for all STM32F4 instances */
+static const struct stm32_adc_chan_spec stm32f4_adc123_channels[] = {
+	{ IIO_VOLTAGE, 0, "in0" },
+	{ IIO_VOLTAGE, 1, "in1" },
+	{ IIO_VOLTAGE, 2, "in2" },
+	{ IIO_VOLTAGE, 3, "in3" },
+	{ IIO_VOLTAGE, 4, "in4" },
+	{ IIO_VOLTAGE, 5, "in5" },
+	{ IIO_VOLTAGE, 6, "in6" },
+	{ IIO_VOLTAGE, 7, "in7" },
+	{ IIO_VOLTAGE, 8, "in8" },
+	{ IIO_VOLTAGE, 9, "in9" },
+	{ IIO_VOLTAGE, 10, "in10" },
+	{ IIO_VOLTAGE, 11, "in11" },
+	{ IIO_VOLTAGE, 12, "in12" },
+	{ IIO_VOLTAGE, 13, "in13" },
+	{ IIO_VOLTAGE, 14, "in14" },
+	{ IIO_VOLTAGE, 15, "in15" },
+};
+
+/**
+ * STM32 ADC registers access routines
+ * @adc: stm32 adc instance
+ * @reg: reg offset in adc instance
+ *
+ * Note: All instances share same base, with 0x0, 0x100 or 0x200 offset resp.
+ * for adc1, adc2 and adc3.
+ */
+static u32 stm32_adc_readl(struct stm32_adc *adc, u32 reg)
+{
+	return readl_relaxed(adc->common->base + adc->offset + reg);
+}
+
+static u32 stm32_adc_readw(struct stm32_adc *adc, u32 reg)
+{
+	return readw_relaxed(adc->common->base + adc->offset + reg);
+}
+
+static void stm32_adc_writel(struct stm32_adc *adc, u32 reg, u32 val)
+{
+	writel_relaxed(val, adc->common->base + adc->offset + reg);
+}
+
+static void stm32_adc_set_bits(struct stm32_adc *adc, u32 reg, u32 bits)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&adc->lock, flags);
+	stm32_adc_writel(adc, reg, stm32_adc_readl(adc, reg) | bits);
+	spin_unlock_irqrestore(&adc->lock, flags);
+}
+
+static void stm32_adc_clr_bits(struct stm32_adc *adc, u32 reg, u32 bits)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&adc->lock, flags);
+	stm32_adc_writel(adc, reg, stm32_adc_readl(adc, reg) & ~bits);
+	spin_unlock_irqrestore(&adc->lock, flags);
+}
+
+/**
+ * stm32_adc_conv_irq_enable() - Enable end of conversion interrupt
+ * @adc: stm32 adc instance
+ */
+static void stm32_adc_conv_irq_enable(struct stm32_adc *adc)
+{
+	stm32_adc_set_bits(adc, STM32F4_ADCX_CR1, STM32F4_EOCIE);
+};
+
+/**
+ * stm32_adc_conv_irq_disable() - Disable end of conversion interrupt
+ * @adc: stm32 adc instance
+ */
+static void stm32_adc_conv_irq_disable(struct stm32_adc *adc)
+{
+	stm32_adc_clr_bits(adc, STM32F4_ADCX_CR1, STM32F4_EOCIE);
+}
+
+/**
+ * stm32_adc_start_conv() - Start conversions for regular channels.
+ * @adc: stm32 adc instance
+ */
+static void stm32_adc_start_conv(struct stm32_adc *adc)
+{
+	stm32_adc_set_bits(adc, STM32F4_ADCX_CR1, STM32F4_SCAN);
+	stm32_adc_set_bits(adc, STM32F4_ADCX_CR2, STM32F4_EOCS | STM32F4_ADON);
+
+	/* Wait for Power-up time (tSTAB from datasheet) */
+	usleep_range(2, 3);
+
+	/* Software start ? (e.g. trigger detection disabled ?) */
+	if (!(stm32_adc_readl(adc, STM32F4_ADCX_CR2) & STM32F4_EXTEN_MASK))
+		stm32_adc_set_bits(adc, STM32F4_ADCX_CR2, STM32F4_SWSTART);
+}
+
+static void stm32_adc_stop_conv(struct stm32_adc *adc)
+{
+	stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_EXTEN_MASK);
+	stm32_adc_clr_bits(adc, STM32F4_ADCX_SR, STM32F4_STRT);
+
+	stm32_adc_clr_bits(adc, STM32F4_ADCX_CR1, STM32F4_SCAN);
+	stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_ADON);
+}
+
+/**
+ * stm32_adc_single_conv() - Performs a single conversion
+ * @indio_dev: IIO device
+ * @chan: IIO channel
+ * @res: conversion result
+ *
+ * The function performs a single conversion on a given channel:
+ * - Program sequencer with one channel (e.g. in SQ1 with len = 1)
+ * - Use SW trigger
+ * - Start conversion, then wait for interrupt completion.
+ */
+static int stm32_adc_single_conv(struct iio_dev *indio_dev,
+				 const struct iio_chan_spec *chan,
+				 int *res)
+{
+	struct stm32_adc *adc = iio_priv(indio_dev);
+	long timeout;
+	u32 val;
+	u16 result;
+	int ret;
+
+	reinit_completion(&adc->completion);
+
+	adc->buffer = &result;
+
+	/* Program chan number in regular sequence */
+	val = stm32_adc_readl(adc, STM32F4_ADCX_SQR3);
+	val &= ~STM32F4_SQ1_MASK;
+	val |= chan->channel << STM32F4_SQ1_SHIFT;
+	stm32_adc_writel(adc, STM32F4_ADCX_SQR3, val);
+
+	/* Set regular sequence len (0 for 1 conversion) */
+	stm32_adc_clr_bits(adc, STM32F4_ADCX_SQR1, STM32F4_L_MASK);
+
+	/* Trigger detection disabled (conversion can be launched in SW) */
+	stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_EXTEN_MASK);
+
+	stm32_adc_conv_irq_enable(adc);
+
+	stm32_adc_start_conv(adc);
+
+	timeout = wait_for_completion_interruptible_timeout(
+					&adc->completion, STM32_ADC_TIMEOUT);
+	if (timeout == 0) {
+		dev_warn(&indio_dev->dev, "Conversion timed out!\n");
+		ret = -ETIMEDOUT;
+	} else if (timeout < 0) {
+		dev_warn(&indio_dev->dev, "Interrupted conversion!\n");
+		ret = -EINTR;
+	} else {
+		*res = result;
+		ret = IIO_VAL_INT;
+	}
+
+	stm32_adc_stop_conv(adc);
+
+	stm32_adc_conv_irq_disable(adc);
+
+	return ret;
+}
+
+static int stm32_adc_read_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int *val, int *val2, long mask)
+{
+	struct stm32_adc *adc = iio_priv(indio_dev);
+	int ret = -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = iio_device_claim_direct_mode(indio_dev);
+		if (ret)
+			return ret;
+		if (chan->type == IIO_VOLTAGE)
+			ret = stm32_adc_single_conv(indio_dev, chan, val);
+		else
+			ret = -EINVAL;
+		iio_device_release_direct_mode(indio_dev);
+		break;
+	case IIO_CHAN_INFO_SCALE:
+		*val = adc->common->vref_mv;
+		*val2 = chan->scan_type.realbits;
+		ret = IIO_VAL_FRACTIONAL_LOG2;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static irqreturn_t stm32_adc_isr(int irq, void *data)
+{
+	struct stm32_adc *adc = data;
+	u32 status = stm32_adc_readl(adc, STM32F4_ADCX_SR);
+	irqreturn_t ret = IRQ_NONE;
+
+	if (status & STM32F4_EOC) {
+		*adc->buffer = stm32_adc_readw(adc, STM32F4_ADCX_DR);
+		complete(&adc->completion);
+		ret = IRQ_HANDLED;
+	}
+
+	return ret;
+}
+
+static int stm32_adc_of_xlate(struct iio_dev *indio_dev,
+			      const struct of_phandle_args *iiospec)
+{
+	int i;
+
+	for (i = 0; i < indio_dev->num_channels; i++)
+		if (indio_dev->channels[i].channel == iiospec->args[0])
+			return i;
+
+	return -EINVAL;
+}
+
+/**
+ * stm32_adc_debugfs_reg_access - read or write register value
+ *
+ * To read a value from an ADC register:
+ *   echo [ADC reg offset] > direct_reg_access
+ *   cat direct_reg_access
+ *
+ * To write a value in a ADC register:
+ *   echo [ADC_reg_offset] [value] > direct_reg_access
+ */
+static int stm32_adc_debugfs_reg_access(struct iio_dev *indio_dev,
+					unsigned reg, unsigned writeval,
+					unsigned *readval)
+{
+	struct stm32_adc *adc = iio_priv(indio_dev);
+
+	if (!readval)
+		stm32_adc_writel(adc, reg, writeval);
+	else
+		*readval = stm32_adc_readl(adc, reg);
+
+	return 0;
+}
+
+static const struct iio_info stm32_adc_iio_info = {
+	.read_raw = stm32_adc_read_raw,
+	.debugfs_reg_access = stm32_adc_debugfs_reg_access,
+	.of_xlate = stm32_adc_of_xlate,
+	.driver_module = THIS_MODULE,
+};
+
+static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
+				    struct iio_chan_spec *chan,
+				    const struct stm32_adc_chan_spec *channel,
+				    int scan_index)
+{
+	chan->type = channel->type;
+	chan->channel = channel->channel;
+	chan->datasheet_name = channel->name;
+	chan->extend_name = channel->name;
+	chan->scan_index = scan_index;
+	chan->indexed = 1;
+	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
+	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE);
+	chan->scan_type.sign = 'u';
+	chan->scan_type.realbits = 12;
+	chan->scan_type.storagebits = 16;
+}
+
+static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
+{
+	struct device_node *node = indio_dev->dev.of_node;
+	struct property *prop;
+	const __be32 *cur;
+	struct iio_chan_spec *channels;
+	int scan_index = 0, num_channels;
+	u32 val;
+
+	num_channels = of_property_count_u32_elems(node, "st,adc-channels");
+	if (num_channels < 0 ||
+	    num_channels >= ARRAY_SIZE(stm32f4_adc123_channels)) {
+		dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");
+		return num_channels < 0 ? num_channels : -EINVAL;
+	}
+
+	channels = devm_kcalloc(&indio_dev->dev, num_channels,
+				sizeof(struct iio_chan_spec), GFP_KERNEL);
+	if (!channels)
+		return -ENOMEM;
+
+	of_property_for_each_u32(node, "st,adc-channels", prop, cur, val) {
+		if (val >= ARRAY_SIZE(stm32f4_adc123_channels)) {
+			dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
+			return -EINVAL;
+		}
+		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
+					&stm32f4_adc123_channels[val],
+					scan_index);
+		scan_index++;
+	}
+
+	indio_dev->num_channels = scan_index;
+	indio_dev->channels = channels;
+
+	return 0;
+}
+
+static int stm32_adc_probe(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev;
+	struct stm32_adc *adc;
+	int ret;
+
+	if (!pdev->dev.of_node)
+		return -ENODEV;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*adc));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	adc = iio_priv(indio_dev);
+	adc->common = dev_get_drvdata(pdev->dev.parent);
+	spin_lock_init(&adc->lock);
+	init_completion(&adc->completion);
+
+	indio_dev->name = dev_name(&pdev->dev);
+	indio_dev->dev.parent = &pdev->dev;
+	indio_dev->dev.of_node = pdev->dev.of_node;
+	indio_dev->info = &stm32_adc_iio_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	platform_set_drvdata(pdev, adc);
+
+	ret = of_property_read_u32(pdev->dev.of_node, "reg", &adc->offset);
+	if (ret != 0) {
+		dev_err(&pdev->dev, "missing reg property\n");
+		return -EINVAL;
+	}
+
+	adc->irq = platform_get_irq(pdev, 0);
+	if (adc->irq < 0) {
+		dev_err(&pdev->dev, "failed to get irq\n");
+		return adc->irq;
+	}
+
+	ret = devm_request_irq(&pdev->dev, adc->irq, stm32_adc_isr,
+			       0, pdev->name, adc);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to request IRQ\n");
+		return ret;
+	}
+
+	adc->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(adc->clk)) {
+		adc->clk = NULL;
+		dev_dbg(&pdev->dev, "No child clk found\n");
+	} else {
+		ret = clk_prepare_enable(adc->clk);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "clk enable failed\n");
+			return ret;
+		}
+	}
+
+	ret = stm32_adc_chan_of_init(indio_dev);
+	if (ret < 0)
+		goto err_clk_disable;
+
+	ret = devm_iio_device_register(&pdev->dev, indio_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "iio dev register failed\n");
+		goto err_clk_disable;
+	}
+
+	return 0;
+
+err_clk_disable:
+	if (adc->clk)
+		clk_disable_unprepare(adc->clk);
+
+	return ret;
+}
+
+static int stm32_adc_remove(struct platform_device *pdev)
+{
+	struct stm32_adc *adc = platform_get_drvdata(pdev);
+
+	if (adc->clk)
+		clk_disable_unprepare(adc->clk);
+
+	return 0;
+}
+
+static const struct of_device_id stm32_adc_of_match[] = {
+	{ .compatible = "st,stm32f4-adc" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, stm32_adc_of_match);
+
+static struct platform_driver stm32_adc_driver = {
+	.probe = stm32_adc_probe,
+	.remove = stm32_adc_remove,
+	.driver = {
+		.name = "stm32-adc",
+		.of_match_table = stm32_adc_of_match,
+	},
+};
+module_platform_driver(stm32_adc_driver);
+
+MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@st.com>");
+MODULE_DESCRIPTION("STMicroelectronics STM32 ADC IIO driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:stm32-adc");
-- 
1.9.1

[PATCH v2 4/6] ARM: configs: stm32: enable ADC driver

[Fabrice Gasnier]

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 arch/arm/configs/stm32_defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig
index 1e5ec2a..e012570 100644
--- a/arch/arm/configs/stm32_defconfig
+++ b/arch/arm/configs/stm32_defconfig
@@ -49,6 +49,7 @@ CONFIG_SERIAL_STM32=y
 CONFIG_SERIAL_STM32_CONSOLE=y
 # CONFIG_HW_RANDOM is not set
 # CONFIG_HWMON is not set
+CONFIG_MFD_STM32_ADC=y
 # CONFIG_USB_SUPPORT is not set
 CONFIG_NEW_LEDS=y
 CONFIG_LEDS_CLASS=y
@@ -57,6 +58,8 @@ CONFIG_LEDS_TRIGGERS=y
 CONFIG_LEDS_TRIGGER_HEARTBEAT=y
 CONFIG_DMADEVICES=y
 CONFIG_STM32_DMA=y
+CONFIG_IIO=y
+CONFIG_STM32_ADC=y
 # CONFIG_FILE_LOCKING is not set
 # CONFIG_DNOTIFY is not set
 # CONFIG_INOTIFY_USER is not set
-- 
1.9.1

[PATCH v2 5/6] ARM: dts: stm32f429: Add adc support

[Fabrice Gasnier]

Add adc support & pinctrl analog phandle (adc3_in8) to stm32f429.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 arch/arm/boot/dts/stm32f429.dtsi | 49 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index 336ee4f..f198132 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -172,6 +172,49 @@
 			status = "disabled";
 		};
 
+		adc: adc@40012000 {
+			compatible = "st,stm32f4-adc-core";
+			reg = <0x40012000 0x400>;
+			interrupts = <18>;
+			clocks = <&rcc 0 168>;
+			clock-names = "adc";
+			interrupt-controller;
+			#interrupt-cells = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+
+			adc1: adc@0 {
+				compatible = "st,stm32f4-adc";
+				#io-channel-cells = <1>;
+				reg = <0x0>;
+				clocks = <&rcc 0 168>;
+				interrupt-parent = <&adc>;
+				interrupts = <0>;
+				status = "disabled";
+			};
+
+			adc2: adc@100 {
+				compatible = "st,stm32f4-adc";
+				#io-channel-cells = <1>;
+				reg = <0x100>;
+				clocks = <&rcc 0 169>;
+				interrupt-parent = <&adc>;
+				interrupts = <1>;
+				status = "disabled";
+			};
+
+			adc3: adc@200 {
+				compatible = "st,stm32f4-adc";
+				#io-channel-cells = <1>;
+				reg = <0x200>;
+				clocks = <&rcc 0 170>;
+				interrupt-parent = <&adc>;
+				interrupts = <2>;
+				status = "disabled";
+			};
+		};
+
 		syscfg: system-config@40013800 {
 			compatible = "syscon";
 			reg = <0x40013800 0x400>;
@@ -332,6 +375,12 @@
 					slew-rate = <2>;
 				};
 			};
+
+			adc3_in8_pin: adc@200 {
+				pins {
+					pinmux = <STM32F429_PF10_FUNC_ANALOG>;
+				};
+			};
 		};
 
 		rcc: rcc@40023810 {
-- 
1.9.1

[PATCH v2 6/6] ARM: dts: stm32f429: enable adc on eval board

[Fabrice Gasnier]

Enable analog to digital converter on stm32f429i-eval board.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 arch/arm/boot/dts/stm32429i-eval.dts | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm/boot/dts/stm32429i-eval.dts b/arch/arm/boot/dts/stm32429i-eval.dts
index 6bfc595..c144735 100644
--- a/arch/arm/boot/dts/stm32429i-eval.dts
+++ b/arch/arm/boot/dts/stm32429i-eval.dts
@@ -65,6 +65,20 @@
 		serial0 = &usart1;
 	};
 
+	regulators {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		reg_vref: regulator@0 {
+			compatible = "regulator-fixed";
+			reg = <0>;
+			regulator-name = "vref";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+		};
+	};
+
 	leds {
 		compatible = "gpio-leds";
 		green {
@@ -123,3 +137,14 @@
 	pinctrl-names = "default";
 	status = "okay";
 };
+
+&adc {
+	pinctrl-names = "default";
+	pinctrl-0 = <&adc3_in8_pin>;
+	vref-supply = <&reg_vref>;
+	status = "okay";
+	adc3: adc@200 {
+		st,adc-channels = <8>;
+		status = "okay";
+	};
+};
-- 
1.9.1

Re: [PATCH v2 2/6] mfd: stm32-adc: Add support for stm32 ADC

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2170 bytes --]

Hi Fabrice,

[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on v4.9-rc4 next-20161110]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Fabrice-Gasnier/Add-support-for-STM32-ADC/20161111-011922
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: s390-allmodconfig (attached as .config)
compiler: s390x-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=s390 

All errors (new ones prefixed by >>):

   drivers/mfd/stm32-adc-core: struct of_device_id is 200 bytes.  The last of 1 is:
   0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x73 0x74 0x2c 0x73 0x74 0x6d 0x33 0x32 0x66 0x34 0x2d 0x61 0x64 0x63 0x2d 0x63 0x6f 0x72 0x65 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 
>> FATAL: drivers/mfd/stm32-adc-core: struct of_device_id is not terminated with a NULL entry!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 43228 bytes --]

Re: [PATCH v2 2/6] mfd: stm32-adc: Add support for stm32 ADC

[Lee Jones]

On Fri, 11 Nov 2016, kbuild test robot wrote:

> Hi Fabrice,
> 
> [auto build test ERROR on ljones-mfd/for-mfd-next]
> [also build test ERROR on v4.9-rc4 next-20161110]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Fabrice-Gasnier/Add-support-for-STM32-ADC/20161111-011922
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
> config: s390-allmodconfig (attached as .config)
> compiler: s390x-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=s390 
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/mfd/stm32-adc-core: struct of_device_id is 200 bytes.  The last of 1 is:
>    0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x73 0x74 0x2c 0x73 0x74 0x6d 0x33 0x32 0x66 0x34 0x2d 0x61 0x64 0x63 0x2d 0x63 0x6f 0x72 0x65 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 
> >> FATAL: drivers/mfd/stm32-adc-core: struct of_device_id is not terminated with a NULL entry!
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Please fix this and re-submit.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 2/6] mfd: stm32-adc: Add support for stm32 ADC

[Jonathan Cameron]

On 10/11/16 16:18, Fabrice Gasnier wrote:
> Add core driver for STMicroelectronics STM32 ADC (Analog to Digital
> Converter). STM32 ADC can be composed of up to 3 ADCs with shared
> resources like clock prescaler, common interrupt line and analog
> reference voltage.
> This core driver basically manages shared resources.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Looks good to me (other than the build issue obviously ;)

The fun bit will be trying to keep the whole thing this clean as you
add the more 'interesting' functionality.  *fingers crossed*

Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/mfd/Kconfig                |  14 ++
>  drivers/mfd/Makefile               |   1 +
>  drivers/mfd/stm32-adc-core.c       | 301 +++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/stm32-adc-core.h |  52 +++++++
>  4 files changed, 368 insertions(+)
>  create mode 100644 drivers/mfd/stm32-adc-core.c
>  create mode 100644 include/linux/mfd/stm32-adc-core.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index c6df644..2580cee 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1152,6 +1152,20 @@ config MFD_PALMAS
>  	  If you say yes here you get support for the Palmas
>  	  series of PMIC chips from Texas Instruments.
>  
> +config MFD_STM32_ADC
> +	tristate "STMicroelectronics STM32 adc"
> +	depends on ARCH_STM32 || COMPILE_TEST
> +	depends on OF
> +	select MFD_CORE
> +	select REGULATOR
> +	select REGULATOR_FIXED_VOLTAGE
> +	help
> +	  Select this option to enable the core driver for STMicroelectronics
> +	  STM32 analog-to-digital converter (ADC).
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called stm32-adc-core.
> +
>  config TPS6105X
>  	tristate "TI TPS61050/61052 Boost Converters"
>  	depends on I2C
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 9834e66..4571506 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -185,6 +185,7 @@ obj-$(CONFIG_MFD_INTEL_LPSS_PCI)	+= intel-lpss-pci.o
>  obj-$(CONFIG_MFD_INTEL_LPSS_ACPI)	+= intel-lpss-acpi.o
>  obj-$(CONFIG_MFD_INTEL_MSIC)	+= intel_msic.o
>  obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
> +obj-$(CONFIG_MFD_STM32_ADC) 	+= stm32-adc-core.o
>  obj-$(CONFIG_MFD_VIPERBOARD)    += viperboard.o
>  obj-$(CONFIG_MFD_RC5T583)	+= rc5t583.o rc5t583-irq.o
>  obj-$(CONFIG_MFD_RK808)		+= rk808.o
> diff --git a/drivers/mfd/stm32-adc-core.c b/drivers/mfd/stm32-adc-core.c
> new file mode 100644
> index 0000000..bcf52fb
> --- /dev/null
> +++ b/drivers/mfd/stm32-adc-core.c
> @@ -0,0 +1,301 @@
> +/*
> + * This file is part of STM32 ADC driver
> + *
> + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
> + * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
> + *
> + * Inspired from: fsl-imx25-tsadc
> + *
> + * License type: GPLv2
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> + * or FITNESS FOR A PARTICULAR PURPOSE.
> + * See the GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdesc.h>
> +#include <linux/irqdomain.h>
> +#include <linux/mfd/stm32-adc-core.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +
> +/* STM32F4 - common registers for all ADC instances: 1, 2 & 3 */
> +#define STM32F4_ADC_CSR			(STM32_ADCX_COMN_OFFSET + 0x00)
> +#define STM32F4_ADC_CCR			(STM32_ADCX_COMN_OFFSET + 0x04)
> +
> +/* STM32F4_ADC_CSR - bit fields */
> +#define STM32F4_EOC3			BIT(17)
> +#define STM32F4_EOC2			BIT(9)
> +#define STM32F4_EOC1			BIT(1)
> +
> +/* STM32F4_ADC_CCR - bit fields */
> +#define STM32F4_ADC_ADCPRE_SHIFT	16
> +#define STM32F4_ADC_ADCPRE_MASK		GENMASK(17, 16)
> +
> +/* STM32 F4 maximum analog clock rate (from datasheet) */
> +#define STM32F4_ADC_MAX_CLK_RATE	36000000
> +
> +/**
> + * struct stm32_adc_priv - stm32 ADC core private data
> + * @irq:		irq for ADC block
> + * @domain:		irq domain reference
> + * @aclk:		clock reference for the analog circuitry
> + * @vref:		regulator reference
> + * @common:		common data for all ADC instances
> + */
> +struct stm32_adc_priv {
> +	int				irq;
> +	struct irq_domain		*domain;
> +	struct clk			*aclk;
> +	struct regulator		*vref;
> +	struct stm32_adc_common		common;
> +};
> +
> +static struct stm32_adc_priv *to_stm32_adc_priv(struct stm32_adc_common *com)
> +{
> +	return container_of(com, struct stm32_adc_priv, common);
> +}
> +
> +/* STM32F4 ADC internal common clock prescaler division ratios */
> +static int stm32f4_pclk_div[] = {2, 4, 6, 8};
> +
> +/**
> + * stm32f4_adc_clk_sel() - Select stm32f4 ADC common clock prescaler
> + * @priv: stm32 ADC core private data
> + * Select clock prescaler used for analog conversions, before using ADC.
> + */
> +static int stm32f4_adc_clk_sel(struct platform_device *pdev,
> +			       struct stm32_adc_priv *priv)
> +{
> +	unsigned long rate;
> +	u32 val;
> +	int i;
> +
> +	rate = clk_get_rate(priv->aclk);
> +	for (i = 0; i < ARRAY_SIZE(stm32f4_pclk_div); i++) {
> +		if ((rate / stm32f4_pclk_div[i]) <= STM32F4_ADC_MAX_CLK_RATE)
> +			break;
> +	}
> +	if (i >= ARRAY_SIZE(stm32f4_pclk_div))
> +		return -EINVAL;
> +
> +	val = readl_relaxed(priv->common.base + STM32F4_ADC_CCR);
> +	val &= ~STM32F4_ADC_ADCPRE_MASK;
> +	val |= i << STM32F4_ADC_ADCPRE_SHIFT;
> +	writel_relaxed(val, priv->common.base + STM32F4_ADC_CCR);
> +
> +	dev_dbg(&pdev->dev, "Using analog clock source at %ld kHz\n",
> +		rate / (stm32f4_pclk_div[i] * 1000));
> +
> +	return 0;
> +}
> +
> +/* ADC common interrupt for all instances */
> +static void stm32_adc_irq_handler(struct irq_desc *desc)
> +{
> +	struct stm32_adc_priv *priv = irq_desc_get_handler_data(desc);
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	u32 status;
> +
> +	chained_irq_enter(chip, desc);
> +	status = readl_relaxed(priv->common.base + STM32F4_ADC_CSR);
> +
> +	if (status & STM32F4_EOC1)
> +		generic_handle_irq(irq_find_mapping(priv->domain, 0));
> +
> +	if (status & STM32F4_EOC2)
> +		generic_handle_irq(irq_find_mapping(priv->domain, 1));
> +
> +	if (status & STM32F4_EOC3)
> +		generic_handle_irq(irq_find_mapping(priv->domain, 2));
> +
> +	chained_irq_exit(chip, desc);
> +};
> +
> +static int stm32_adc_domain_map(struct irq_domain *d, unsigned int irq,
> +				irq_hw_number_t hwirq)
> +{
> +	irq_set_chip_data(irq, d->host_data);
> +	irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_level_irq);
> +
> +	return 0;
> +}
> +
> +static void stm32_adc_domain_unmap(struct irq_domain *d, unsigned int irq)
> +{
> +	irq_set_chip_and_handler(irq, NULL, NULL);
> +	irq_set_chip_data(irq, NULL);
> +}
> +
> +static const struct irq_domain_ops stm32_adc_domain_ops = {
> +	.map = stm32_adc_domain_map,
> +	.unmap  = stm32_adc_domain_unmap,
> +	.xlate = irq_domain_xlate_onecell,
> +};
> +
> +static int stm32_adc_irq_probe(struct platform_device *pdev,
> +			       struct stm32_adc_priv *priv)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +
> +	priv->irq = platform_get_irq(pdev, 0);
> +	if (priv->irq < 0) {
> +		dev_err(&pdev->dev, "failed to get irq\n");
> +		return priv->irq;
> +	}
> +
> +	priv->domain = irq_domain_add_simple(np, STM32_ADC_MAX_ADCS, 0,
> +					     &stm32_adc_domain_ops,
> +					     priv);
> +	if (!priv->domain) {
> +		dev_err(&pdev->dev, "Failed to add irq domain\n");
> +		return -ENOMEM;
> +	}
> +
> +	irq_set_chained_handler(priv->irq, stm32_adc_irq_handler);
> +	irq_set_handler_data(priv->irq, priv);
> +
> +	return 0;
> +}
> +
> +static void stm32_adc_irq_remove(struct platform_device *pdev,
> +				 struct stm32_adc_priv *priv)
> +{
> +	int hwirq;
> +
> +	for (hwirq = 0; hwirq < STM32_ADC_MAX_ADCS; hwirq++)
> +		irq_dispose_mapping(irq_find_mapping(priv->domain, hwirq));
> +	irq_domain_remove(priv->domain);
> +	irq_set_chained_handler(priv->irq, NULL);
> +}
> +
> +static int stm32_adc_probe(struct platform_device *pdev)
> +{
> +	struct stm32_adc_priv *priv;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct resource *res;
> +	int ret;
> +
> +	if (!pdev->dev.of_node)
> +		return -ENODEV;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	priv->common.base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(priv->common.base))
> +		return PTR_ERR(priv->common.base);
> +
> +	priv->vref = devm_regulator_get(&pdev->dev, "vref");
> +	if (IS_ERR(priv->vref)) {
> +		ret = PTR_ERR(priv->vref);
> +		dev_err(&pdev->dev, "vref get failed, %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regulator_enable(priv->vref);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "vref enable failed\n");
> +		return ret;
> +	}
> +
> +	ret = regulator_get_voltage(priv->vref);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "vref get voltage failed, %d\n", ret);
> +		goto err_regulator_disable;
> +	}
> +	priv->common.vref_mv = ret / 1000;
> +	dev_dbg(&pdev->dev, "vref+=%dmV\n", priv->common.vref_mv);
> +
> +	priv->aclk = devm_clk_get(&pdev->dev, "adc");
> +	if (IS_ERR(priv->aclk)) {
> +		ret = PTR_ERR(priv->aclk);
> +		dev_err(&pdev->dev, "Can't get 'adc' clock\n");
> +		goto err_regulator_disable;
> +	}
> +
> +	ret = clk_prepare_enable(priv->aclk);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "adc clk enable failed\n");
> +		goto err_regulator_disable;
> +	}
> +
> +	ret = stm32f4_adc_clk_sel(pdev, priv);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "adc clk selection failed\n");
> +		goto err_clk_disable;
> +	}
> +
> +	ret = stm32_adc_irq_probe(pdev, priv);
> +	if (ret < 0)
> +		goto err_clk_disable;
> +
> +	platform_set_drvdata(pdev, &priv->common);
> +
> +	ret = of_platform_populate(np, NULL, NULL, &pdev->dev);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to populate DT children\n");
> +		goto err_irq_remove;
> +	}
> +
> +	return 0;
> +
> +err_irq_remove:
> +	stm32_adc_irq_remove(pdev, priv);
> +
> +err_clk_disable:
> +	clk_disable_unprepare(priv->aclk);
> +
> +err_regulator_disable:
> +	regulator_disable(priv->vref);
> +
> +	return ret;
> +}
> +
> +static int stm32_adc_remove(struct platform_device *pdev)
> +{
> +	struct stm32_adc_common *common = platform_get_drvdata(pdev);
> +	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
> +
> +	of_platform_depopulate(&pdev->dev);
> +	stm32_adc_irq_remove(pdev, priv);
> +	clk_disable_unprepare(priv->aclk);
> +	regulator_disable(priv->vref);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id stm32_adc_of_match[] = {
> +	{ .compatible = "st,stm32f4-adc-core" },
> +};
> +MODULE_DEVICE_TABLE(of, stm32_adc_of_match);
> +
> +static struct platform_driver stm32_adc_driver = {
> +	.probe = stm32_adc_probe,
> +	.remove = stm32_adc_remove,
> +	.driver = {
> +		.name = "stm32-adc-core",
> +		.of_match_table = stm32_adc_of_match,
> +	},
> +};
> +module_platform_driver(stm32_adc_driver);
> +
> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@st.com>");
> +MODULE_DESCRIPTION("STMicroelectronics STM32 ADC MFD driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:stm32-adc-core");
> diff --git a/include/linux/mfd/stm32-adc-core.h b/include/linux/mfd/stm32-adc-core.h
> new file mode 100644
> index 0000000..081fa5f
> --- /dev/null
> +++ b/include/linux/mfd/stm32-adc-core.h
> @@ -0,0 +1,52 @@
> +/*
> + * This file is part of STM32 ADC driver
> + *
> + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
> + * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
> + *
> + * License type: GPLv2
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> + * or FITNESS FOR A PARTICULAR PURPOSE.
> + * See the GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __STM32_ADC_H
> +#define __STM32_ADC_H
> +
> +/*
> + * STM32 - ADC global register map
> + * ________________________________________________________
> + * | Offset |                 Register                    |
> + * --------------------------------------------------------
> + * | 0x000  |                Master ADC1                  |
> + * --------------------------------------------------------
> + * | 0x100  |                Slave ADC2                   |
> + * --------------------------------------------------------
> + * | 0x200  |                Slave ADC3                   |
> + * --------------------------------------------------------
> + * | 0x300  |         Master & Slave common regs          |
> + * --------------------------------------------------------
> + */
> +#define STM32_ADC_MAX_ADCS		3
> +#define STM32_ADCX_COMN_OFFSET		0x300
> +
> +/**
> + * struct stm32_adc_common - stm32 ADC driver common data (for all instances)
> + * @base:		control registers base cpu addr
> + * @vref_mv:		vref voltage (mv)
> + */
> +struct stm32_adc_common {
> +	void __iomem			*base;
> +	int				vref_mv;
> +};
> +
> +#endif
> 

Re: [PATCH v2 3/6] iio: adc: Add support for STM32 ADC

[Jonathan Cameron]

On 10/11/16 16:18, Fabrice Gasnier wrote:
> This patch adds support for STMicroelectronics STM32 MCU's analog to
> digital converter.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Nice and clean - few minor things inline.

Jonathan
> ---
>  drivers/iio/adc/Kconfig     |  10 +
>  drivers/iio/adc/Makefile    |   1 +
>  drivers/iio/adc/stm32-adc.c | 525 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 536 insertions(+)
>  create mode 100644 drivers/iio/adc/stm32-adc.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 7edcf32..61ba674 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -419,6 +419,16 @@ config ROCKCHIP_SARADC
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called rockchip_saradc.
>  
> +config STM32_ADC
> +	tristate "STMicroelectronics STM32 adc"
> +	depends on MFD_STM32_ADC
> +	help
> +	  Say yes here to build support for STMicroelectronics stm32 Analog
> +	  to Digital Converter (ADC).
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called stm32-adc.
> +
>  config STX104
>  	tristate "Apex Embedded Systems STX104 driver"
>  	depends on X86 && ISA_BUS_API
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 7a40c04..df7a221 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -41,6 +41,7 @@ obj-$(CONFIG_QCOM_SPMI_IADC) += qcom-spmi-iadc.o
>  obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
>  obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
>  obj-$(CONFIG_STX104) += stx104.o
> +obj-$(CONFIG_STM32_ADC) += stm32-adc.o
>  obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>  obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
>  obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> new file mode 100644
> index 0000000..2be5fee
> --- /dev/null
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -0,0 +1,525 @@
> +/*
> + * This file is part of STM32 ADC driver
> + *
> + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
> + * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
> + *
> + * License type: GPLv2
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> + * or FITNESS FOR A PARTICULAR PURPOSE.
> + * See the GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/iio/iio.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/mfd/stm32-adc-core.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +
> +/* STM32F4 - Registers for each ADC instance */
Not really sure the X adds anything in these, but doesn't do much harm
I guess ;)
> +#define STM32F4_ADCX_SR			0x00
> +#define STM32F4_ADCX_CR1		0x04
> +#define STM32F4_ADCX_CR2		0x08
> +#define STM32F4_ADCX_SMPR1		0x0C
> +#define STM32F4_ADCX_SMPR2		0x10
> +#define STM32F4_ADCX_HTR		0x24
> +#define STM32F4_ADCX_LTR		0x28
> +#define STM32F4_ADCX_SQR1		0x2C
> +#define STM32F4_ADCX_SQR2		0x30
> +#define STM32F4_ADCX_SQR3		0x34
> +#define STM32F4_ADCX_JSQR		0x38
> +#define STM32F4_ADCX_JDR1		0x3C
> +#define STM32F4_ADCX_JDR2		0x40
> +#define STM32F4_ADCX_JDR3		0x44
> +#define STM32F4_ADCX_JDR4		0x48
> +#define STM32F4_ADCX_DR			0x4C
> +
> +/* STM32F4_ADCX_SR - bit fields */
> +#define STM32F4_OVR			BIT(5)
> +#define STM32F4_STRT			BIT(4)
> +#define STM32F4_EOC			BIT(1)
> +
> +/* STM32F4_ADCX_CR1 - bit fields */
> +#define STM32F4_OVRIE			BIT(26)
> +#define STM32F4_SCAN			BIT(8)
> +#define STM32F4_EOCIE			BIT(5)
> +
> +/* STM32F4_ADCX_CR2 - bit fields */
> +#define STM32F4_SWSTART			BIT(30)
> +#define STM32F4_EXTEN_MASK		GENMASK(29, 28)
> +#define STM32F4_EOCS			BIT(10)
> +#define STM32F4_ADON			BIT(0)
> +
> +/* STM32F4_ADCX_SQR1 - bit fields */
> +#define STM32F4_L_SHIFT			20
> +#define STM32F4_L_MASK			GENMASK(23, 20)
> +
> +/* STM32F4_ADCX_SQR3 - bit fields */
> +#define STM32F4_SQ1_SHIFT		0
> +#define STM32F4_SQ1_MASK		GENMASK(4, 0)
> +
> +#define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
> +#define STM32_ADC_TIMEOUT_US		100000
> +#define STM32_ADC_TIMEOUT	(msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
> +
> +/**
> + * struct stm32_adc - private data of each ADC IIO instance
> + * @common:		reference to ADC block common data
> + * @offset:		ADC instance register offset in ADC block
> + * @completion:		end of single conversion completion
> + * @buffer:		data buffer
> + * @clk:		optional adc clock, for this adc instance
> + * @irq:		interrupt for this adc instance
> + * @lock:		spinlock
> + */
> +struct stm32_adc {
> +	struct stm32_adc_common	*common;
> +	u32			offset;
> +	struct completion	completion;
> +	u16			*buffer;
> +	struct clk		*clk;
> +	int			irq;
> +	spinlock_t		lock;		/* interrupt lock */
> +};
> +
> +/**
> + * struct stm32_adc_chan_spec - specification of stm32 adc channel
> + * @type:	IIO channel type
> + * @channel:	channel number (single ended)
> + * @name:	channel name (single ended)
> + */
> +struct stm32_adc_chan_spec {
> +	enum iio_chan_type	type;
> +	int			channel;
> +	const char		*name;
> +};
> +
> +/* Input definitions common for all STM32F4 instances */
> +static const struct stm32_adc_chan_spec stm32f4_adc123_channels[] = {
> +	{ IIO_VOLTAGE, 0, "in0" },
> +	{ IIO_VOLTAGE, 1, "in1" },
> +	{ IIO_VOLTAGE, 2, "in2" },
> +	{ IIO_VOLTAGE, 3, "in3" },
> +	{ IIO_VOLTAGE, 4, "in4" },
> +	{ IIO_VOLTAGE, 5, "in5" },
> +	{ IIO_VOLTAGE, 6, "in6" },
> +	{ IIO_VOLTAGE, 7, "in7" },
> +	{ IIO_VOLTAGE, 8, "in8" },
> +	{ IIO_VOLTAGE, 9, "in9" },
> +	{ IIO_VOLTAGE, 10, "in10" },
> +	{ IIO_VOLTAGE, 11, "in11" },
> +	{ IIO_VOLTAGE, 12, "in12" },
> +	{ IIO_VOLTAGE, 13, "in13" },
> +	{ IIO_VOLTAGE, 14, "in14" },
> +	{ IIO_VOLTAGE, 15, "in15" },
> +};
> +
> +/**
> + * STM32 ADC registers access routines
> + * @adc: stm32 adc instance
> + * @reg: reg offset in adc instance
> + *
> + * Note: All instances share same base, with 0x0, 0x100 or 0x200 offset resp.
> + * for adc1, adc2 and adc3.
> + */
> +static u32 stm32_adc_readl(struct stm32_adc *adc, u32 reg)
> +{
> +	return readl_relaxed(adc->common->base + adc->offset + reg);
> +}
> +
> +static u32 stm32_adc_readw(struct stm32_adc *adc, u32 reg)
> +{
> +	return readw_relaxed(adc->common->base + adc->offset + reg);
> +}
> +
> +static void stm32_adc_writel(struct stm32_adc *adc, u32 reg, u32 val)
> +{
> +	writel_relaxed(val, adc->common->base + adc->offset + reg);
> +}
> +
> +static void stm32_adc_set_bits(struct stm32_adc *adc, u32 reg, u32 bits)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&adc->lock, flags);
> +	stm32_adc_writel(adc, reg, stm32_adc_readl(adc, reg) | bits);
> +	spin_unlock_irqrestore(&adc->lock, flags);
> +}
> +
> +static void stm32_adc_clr_bits(struct stm32_adc *adc, u32 reg, u32 bits)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&adc->lock, flags);
> +	stm32_adc_writel(adc, reg, stm32_adc_readl(adc, reg) & ~bits);
> +	spin_unlock_irqrestore(&adc->lock, flags);
> +}
> +
> +/**
> + * stm32_adc_conv_irq_enable() - Enable end of conversion interrupt
> + * @adc: stm32 adc instance
> + */
> +static void stm32_adc_conv_irq_enable(struct stm32_adc *adc)
> +{
> +	stm32_adc_set_bits(adc, STM32F4_ADCX_CR1, STM32F4_EOCIE);
> +};
> +
> +/**
> + * stm32_adc_conv_irq_disable() - Disable end of conversion interrupt
> + * @adc: stm32 adc instance
> + */
> +static void stm32_adc_conv_irq_disable(struct stm32_adc *adc)
> +{
> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_CR1, STM32F4_EOCIE);
> +}
> +
> +/**
> + * stm32_adc_start_conv() - Start conversions for regular channels.
> + * @adc: stm32 adc instance
> + */
> +static void stm32_adc_start_conv(struct stm32_adc *adc)
> +{
> +	stm32_adc_set_bits(adc, STM32F4_ADCX_CR1, STM32F4_SCAN);
> +	stm32_adc_set_bits(adc, STM32F4_ADCX_CR2, STM32F4_EOCS | STM32F4_ADON);
> +
> +	/* Wait for Power-up time (tSTAB from datasheet) */
> +	usleep_range(2, 3);
> +
> +	/* Software start ? (e.g. trigger detection disabled ?) */
> +	if (!(stm32_adc_readl(adc, STM32F4_ADCX_CR2) & STM32F4_EXTEN_MASK))
> +		stm32_adc_set_bits(adc, STM32F4_ADCX_CR2, STM32F4_SWSTART);
> +}
> +
> +static void stm32_adc_stop_conv(struct stm32_adc *adc)
> +{
> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_EXTEN_MASK);
> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_SR, STM32F4_STRT);
> +
> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_CR1, STM32F4_SCAN);
> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_ADON);
> +}
> +
> +/**
> + * stm32_adc_single_conv() - Performs a single conversion
> + * @indio_dev: IIO device
> + * @chan: IIO channel
> + * @res: conversion result
> + *
> + * The function performs a single conversion on a given channel:
> + * - Program sequencer with one channel (e.g. in SQ1 with len = 1)
> + * - Use SW trigger
> + * - Start conversion, then wait for interrupt completion.
> + */
> +static int stm32_adc_single_conv(struct iio_dev *indio_dev,
> +				 const struct iio_chan_spec *chan,
> +				 int *res)
> +{
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +	long timeout;
> +	u32 val;
> +	u16 result;
> +	int ret;
> +
> +	reinit_completion(&adc->completion);
> +
> +	adc->buffer = &result;
> +
> +	/* Program chan number in regular sequence */
> +	val = stm32_adc_readl(adc, STM32F4_ADCX_SQR3);
> +	val &= ~STM32F4_SQ1_MASK;
> +	val |= chan->channel << STM32F4_SQ1_SHIFT;
> +	stm32_adc_writel(adc, STM32F4_ADCX_SQR3, val);
> +
> +	/* Set regular sequence len (0 for 1 conversion) */
> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_SQR1, STM32F4_L_MASK);
> +
> +	/* Trigger detection disabled (conversion can be launched in SW) */
> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_EXTEN_MASK);
> +
> +	stm32_adc_conv_irq_enable(adc);
> +
> +	stm32_adc_start_conv(adc);
> +
> +	timeout = wait_for_completion_interruptible_timeout(
> +					&adc->completion, STM32_ADC_TIMEOUT);
> +	if (timeout == 0) {
> +		dev_warn(&indio_dev->dev, "Conversion timed out!\n");
> +		ret = -ETIMEDOUT;
> +	} else if (timeout < 0) {
> +		dev_warn(&indio_dev->dev, "Interrupted conversion!\n");
> +		ret = -EINTR;
> +	} else {
> +		*res = result;
> +		ret = IIO_VAL_INT;
> +	}
> +
> +	stm32_adc_stop_conv(adc);
> +
> +	stm32_adc_conv_irq_disable(adc);
> +
> +	return ret;
> +}
> +
> +static int stm32_adc_read_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      int *val, int *val2, long mask)
> +{
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +	int ret = -EINVAL;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = iio_device_claim_direct_mode(indio_dev);
> +		if (ret)
> +			return ret;
> +		if (chan->type == IIO_VOLTAGE)
> +			ret = stm32_adc_single_conv(indio_dev, chan, val);
> +		else
> +			ret = -EINVAL;
> +		iio_device_release_direct_mode(indio_dev);
return directly here.  Basically always preferred to return directly if
there is not cleanup to be done.
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = adc->common->vref_mv;
> +		*val2 = chan->scan_type.realbits;
> +		ret = IIO_VAL_FRACTIONAL_LOG2;
return directly here.
> +		break;
> +	default:
return -EINVAL here.
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static irqreturn_t stm32_adc_isr(int irq, void *data)
> +{
> +	struct stm32_adc *adc = data;
> +	u32 status = stm32_adc_readl(adc, STM32F4_ADCX_SR);
> +	irqreturn_t ret = IRQ_NONE;
> +
> +	if (status & STM32F4_EOC) {
> +		*adc->buffer = stm32_adc_readw(adc, STM32F4_ADCX_DR);
> +		complete(&adc->completion);
> +		ret = IRQ_HANDLED;
Slightly tidier to return IRQ_HANDLED here and directly return
IRQ_NONE below.
> +	}
> +
> +	return ret;
> +}
> +
> +static int stm32_adc_of_xlate(struct iio_dev *indio_dev,
> +			      const struct of_phandle_args *iiospec)
> +{
> +	int i;
> +
> +	for (i = 0; i < indio_dev->num_channels; i++)
> +		if (indio_dev->channels[i].channel == iiospec->args[0])
> +			return i;
> +
> +	return -EINVAL;
> +}
> +
> +/**
> + * stm32_adc_debugfs_reg_access - read or write register value
> + *
> + * To read a value from an ADC register:
> + *   echo [ADC reg offset] > direct_reg_access
> + *   cat direct_reg_access
> + *
> + * To write a value in a ADC register:
> + *   echo [ADC_reg_offset] [value] > direct_reg_access
> + */
> +static int stm32_adc_debugfs_reg_access(struct iio_dev *indio_dev,
> +					unsigned reg, unsigned writeval,
> +					unsigned *readval)
> +{
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +
> +	if (!readval)
> +		stm32_adc_writel(adc, reg, writeval);
> +	else
> +		*readval = stm32_adc_readl(adc, reg);
> +
> +	return 0;
> +}
> +
> +static const struct iio_info stm32_adc_iio_info = {
> +	.read_raw = stm32_adc_read_raw,
> +	.debugfs_reg_access = stm32_adc_debugfs_reg_access,
> +	.of_xlate = stm32_adc_of_xlate,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
> +				    struct iio_chan_spec *chan,
> +				    const struct stm32_adc_chan_spec *channel,
> +				    int scan_index)
> +{
> +	chan->type = channel->type;
> +	chan->channel = channel->channel;
> +	chan->datasheet_name = channel->name;
> +	chan->extend_name = channel->name;
Don't set extend_name. That name doesn't add sufficient information to
make it worth adding custom ABI to the userspace interface.


> +	chan->scan_index = scan_index;
> +	chan->indexed = 1;
> +	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> +	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE);
> +	chan->scan_type.sign = 'u';
> +	chan->scan_type.realbits = 12;
> +	chan->scan_type.storagebits = 16;
> +}
> +
> +static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
> +{
> +	struct device_node *node = indio_dev->dev.of_node;
> +	struct property *prop;
> +	const __be32 *cur;
> +	struct iio_chan_spec *channels;
> +	int scan_index = 0, num_channels;
> +	u32 val;
> +
> +	num_channels = of_property_count_u32_elems(node, "st,adc-channels");
> +	if (num_channels < 0 ||
> +	    num_channels >= ARRAY_SIZE(stm32f4_adc123_channels)) {
> +		dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");
> +		return num_channels < 0 ? num_channels : -EINVAL;
> +	}
> +
> +	channels = devm_kcalloc(&indio_dev->dev, num_channels,
> +				sizeof(struct iio_chan_spec), GFP_KERNEL);
> +	if (!channels)
> +		return -ENOMEM;
> +
> +	of_property_for_each_u32(node, "st,adc-channels", prop, cur, val) {
> +		if (val >= ARRAY_SIZE(stm32f4_adc123_channels)) {
> +			dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
> +			return -EINVAL;
> +		}
> +		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
> +					&stm32f4_adc123_channels[val],
> +					scan_index);
> +		scan_index++;
> +	}
> +
> +	indio_dev->num_channels = scan_index;
> +	indio_dev->channels = channels;
> +
> +	return 0;
> +}
> +
> +static int stm32_adc_probe(struct platform_device *pdev)
> +{
> +	struct iio_dev *indio_dev;
> +	struct stm32_adc *adc;
> +	int ret;
> +
> +	if (!pdev->dev.of_node)
> +		return -ENODEV;
> +
> +	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*adc));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	adc = iio_priv(indio_dev);
> +	adc->common = dev_get_drvdata(pdev->dev.parent);
> +	spin_lock_init(&adc->lock);
> +	init_completion(&adc->completion);
> +
> +	indio_dev->name = dev_name(&pdev->dev);
> +	indio_dev->dev.parent = &pdev->dev;
> +	indio_dev->dev.of_node = pdev->dev.of_node;
> +	indio_dev->info = &stm32_adc_iio_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	platform_set_drvdata(pdev, adc);
> +
> +	ret = of_property_read_u32(pdev->dev.of_node, "reg", &adc->offset);
> +	if (ret != 0) {
> +		dev_err(&pdev->dev, "missing reg property\n");
> +		return -EINVAL;
> +	}
> +
> +	adc->irq = platform_get_irq(pdev, 0);
> +	if (adc->irq < 0) {
> +		dev_err(&pdev->dev, "failed to get irq\n");
> +		return adc->irq;
> +	}
> +
> +	ret = devm_request_irq(&pdev->dev, adc->irq, stm32_adc_isr,
> +			       0, pdev->name, adc);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to request IRQ\n");
> +		return ret;
> +	}
> +
> +	adc->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(adc->clk)) {
Could it concievably be deferred?  Would be happier if this explicitly
checked for -ENODEV or whatever gets returned when not clock has
been specified.
> +		adc->clk = NULL;
> +		dev_dbg(&pdev->dev, "No child clk found\n");
> +	} else {
> +		ret = clk_prepare_enable(adc->clk);
> +		if (ret < 0) {
> +			dev_err(&pdev->dev, "clk enable failed\n");
> +			return ret;
> +		}
> +	}
> +
> +	ret = stm32_adc_chan_of_init(indio_dev);
> +	if (ret < 0)
> +		goto err_clk_disable;
> +
> +	ret = devm_iio_device_register(&pdev->dev, indio_dev);

This use of devm registration is going to cause a race in the remove.
The userspace interface will not be removed until after the remove
function has run.  That disables the clock thus leaving us a window
where we could try and access the device with no clock enabled.

Basic rule of thumb is that use of devm must not effect the ordering
of unrolling what you do in probe when it comes to remove.
(which more or less means that you can't use devm_iio_device_register
unless you have no remove at all).
> +	if (ret) {
> +		dev_err(&pdev->dev, "iio dev register failed\n");
> +		goto err_clk_disable;
> +	}
> +
> +	return 0;
> +
> +err_clk_disable:
> +	if (adc->clk)
> +		clk_disable_unprepare(adc->clk);
> +
> +	return ret;
> +}
> +
> +static int stm32_adc_remove(struct platform_device *pdev)
> +{
> +	struct stm32_adc *adc = platform_get_drvdata(pdev);
> +
> +	if (adc->clk)
> +		clk_disable_unprepare(adc->clk);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id stm32_adc_of_match[] = {
> +	{ .compatible = "st,stm32f4-adc" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, stm32_adc_of_match);
> +
> +static struct platform_driver stm32_adc_driver = {
> +	.probe = stm32_adc_probe,
> +	.remove = stm32_adc_remove,
> +	.driver = {
> +		.name = "stm32-adc",
> +		.of_match_table = stm32_adc_of_match,
> +	},
> +};
> +module_platform_driver(stm32_adc_driver);
> +
> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@st.com>");
> +MODULE_DESCRIPTION("STMicroelectronics STM32 ADC IIO driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:stm32-adc");
> 

Re: [PATCH v2 1/6] Documentation: dt-bindings: Document STM32 ADC DT bindings

[Jonathan Cameron]

On 10/11/16 16:18, Fabrice Gasnier wrote:
> This patch adds documentation of device tree bindings for the STM32 ADC.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> ---
>  .../devicetree/bindings/iio/adc/st,stm32-adc.txt   | 85 ++++++++++++++++++++++
>  1 file changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> new file mode 100644
> index 0000000..8b20c23
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> @@ -0,0 +1,85 @@
> +STMicroelectronics STM32 ADC device driver
> +
> +STM32 ADC is a successive approximation analog-to-digital converter.
> +It has several multiplexed input channels. Conversions can be performed
> +in single, continuous, scan or discontinuous mode. Result of the ADC is
> +stored in a left-aligned or right-aligned 32-bit data register.
> +Conversions can be launched in software or using hardware triggers.
> +
> +The analog watchdog feature allows the application to detect if the input
> +voltage goes beyond the user-defined, higher or lower thresholds.
> +
> +Each STM32 ADC block can have up to 3 ADC instances.
> +
> +Each instance supports two contexts to manage conversions, each one has its
> +own configurable sequence and trigger:
> +- regular conversion can be done in sequence, running in background
> +- injected conversions have higher priority, and so have the ability to
> +  interrupt regular conversion sequence (either triggered in SW or HW).
> +  Regular sequence is resumed, in case it has been interrupted.
> +
> +Contents of a stm32 adc root node:
> +-----------------------------------
> +Required properties:
> +- compatible: Should be "st,stm32f4-adc-core".
> +- reg: Offset and length of the ADC block register set.
> +- interrupts: Must contain the interrupt for ADC block.
> +- clocks: Clock for the analog circuitry (common to all ADCs).
> +- clock-names: Must be "adc".
> +- interrupt-controller: Identifies the controller node as interrupt-parent
> +- vref-supply: Phandle to the vref input analog reference voltage.
> +- #interrupt-cells = <1>;
> +- #address-cells = <1>;
> +- #size-cells = <0>;
> +
> +Optional properties:
> +- A pinctrl state named "default" for each ADC channel may be defined to set
> +  inX ADC pins in mode of operation for analog input on external pin.
> +
> +Contents of a stm32 adc child node:
> +-----------------------------------
> +An ADC block node should contain at least one subnode, representing an
> +ADC instance available on the machine.
> +
> +Required properties:
> +- compatible: Should be "st,stm32f4-adc".
> +- reg: Offset of ADC instance in ADC block (e.g. may be 0x0, 0x100, 0x200).
> +- st,adc-channels: List of single-ended channels muxed for this ADC.
> +  It can have up to 16 channels, numbered from 0 to 15 (resp. for in0..in15).
> +- interrupt-parent: Phandle to the parent interrupt controller.
> +- interrupts: IRQ Line for the ADC (e.g. may be 0 for adc@0, 1 for adc@100 or
> +  2 for adc@200).
> +- #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
> +  Documentation/devicetree/bindings/iio/iio-bindings.txt
> +
> +Optional properties:
> +- clocks: Input clock private to this ADC instance.
I'm a little surprised this is optional.  Perhaps some text here explaining why?
> +
> +Example:
> +	adc: adc@40012000 {
> +		compatible = "st,stm32f4-adc-core";
> +		reg = <0x40012000 0x400>;
> +		interrupts = <18>;
> +		clocks = <&rcc 0 168>;
> +		clock-names = "adc";
> +		vref-supply = <&reg_vref>;
> +		interrupt-controller;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&adc3_in8_pin>;
> +
> +		#interrupt-cells = <1>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		adc@0 {
> +			compatible = "st,stm32f4-adc";
> +			#io-channel-cells = <1>;
> +			reg = <0x0>;
> +			clocks = <&rcc 0 168>;
> +			interrupt-parent = <&adc>;
> +			interrupts = <0>;
> +			st,adc-channels = <8>;
> +		};
> +		...
> +		other adc child nodes follow...
> +	};
> 

Re: [PATCH v2 3/6] iio: adc: Add support for STM32 ADC

[Lars-Peter Clausen]

On 11/10/2016 05:18 PM, Fabrice Gasnier wrote:
[...]
> + static int stm32_adc_single_conv(struct iio_dev *indio_dev,
> +				 const struct iio_chan_spec *chan,
> +				 int *res)
> +{
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +	long timeout;
> +	u32 val;
> +	u16 result;
> +	int ret;
> +
> +	reinit_completion(&adc->completion);
> +
> +	adc->buffer = &result;
> +
> +	/* Program chan number in regular sequence */
> +	val = stm32_adc_readl(adc, STM32F4_ADCX_SQR3);
> +	val &= ~STM32F4_SQ1_MASK;
> +	val |= chan->channel << STM32F4_SQ1_SHIFT;
> +	stm32_adc_writel(adc, STM32F4_ADCX_SQR3, val);
> +
> +	/* Set regular sequence len (0 for 1 conversion) */
> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_SQR1, STM32F4_L_MASK);
> +
> +	/* Trigger detection disabled (conversion can be launched in SW) */
> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_EXTEN_MASK);
> +
> +	stm32_adc_conv_irq_enable(adc);
> +
> +	stm32_adc_start_conv(adc);
> +
> +	timeout = wait_for_completion_interruptible_timeout(
> +					&adc->completion, STM32_ADC_TIMEOUT);
> +	if (timeout == 0) {
> +		dev_warn(&indio_dev->dev, "Conversion timed out!\n");

This should be dev_dbg() at most. This out of band reporting is not
particular useful for applications as it is impossible to match the error to
the action that triggered it. And you also report the error through the
error code, so the applications knows what is going on.

> +		ret = -ETIMEDOUT;
> +	} else if (timeout < 0) {
> +		dev_warn(&indio_dev->dev, "Interrupted conversion!\n");
> +		ret = -EINTR;

This should just propagate the error returned by wait_for_completion...().
This will make sure that the right behavior occurs based on the SA_RESTART
policy.

> +	} else {
> +		*res = result;
> +		ret = IIO_VAL_INT;
> +	}
> +
> +	stm32_adc_stop_conv(adc);
> +
> +	stm32_adc_conv_irq_disable(adc);
> +
> +	return ret;

Re: [PATCH v2 2/6] mfd: stm32-adc: Add support for stm32 ADC

[Lee Jones]

On Sat, 12 Nov 2016, Jonathan Cameron wrote:

> On 10/11/16 16:18, Fabrice Gasnier wrote:
> > Add core driver for STMicroelectronics STM32 ADC (Analog to Digital
> > Converter). STM32 ADC can be composed of up to 3 ADCs with shared
> > resources like clock prescaler, common interrupt line and analog
> > reference voltage.
> > This core driver basically manages shared resources.
> > 
> > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Looks good to me (other than the build issue obviously ;)
> 
> The fun bit will be trying to keep the whole thing this clean as you
> add the more 'interesting' functionality.  *fingers crossed*
> 
> Acked-by: Jonathan Cameron <jic23@kernel.org>

There isn't anything MFD about this driver.

Please move it into IIO.

> > ---
> >  drivers/mfd/Kconfig                |  14 ++
> >  drivers/mfd/Makefile               |   1 +
> >  drivers/mfd/stm32-adc-core.c       | 301 +++++++++++++++++++++++++++++++++++++
> >  include/linux/mfd/stm32-adc-core.h |  52 +++++++
> >  4 files changed, 368 insertions(+)
> >  create mode 100644 drivers/mfd/stm32-adc-core.c
> >  create mode 100644 include/linux/mfd/stm32-adc-core.h
> > 
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index c6df644..2580cee 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -1152,6 +1152,20 @@ config MFD_PALMAS
> >  	  If you say yes here you get support for the Palmas
> >  	  series of PMIC chips from Texas Instruments.
> >  
> > +config MFD_STM32_ADC
> > +	tristate "STMicroelectronics STM32 adc"
> > +	depends on ARCH_STM32 || COMPILE_TEST
> > +	depends on OF
> > +	select MFD_CORE
> > +	select REGULATOR
> > +	select REGULATOR_FIXED_VOLTAGE
> > +	help
> > +	  Select this option to enable the core driver for STMicroelectronics
> > +	  STM32 analog-to-digital converter (ADC).
> > +
> > +	  This driver can also be built as a module.  If so, the module
> > +	  will be called stm32-adc-core.
> > +
> >  config TPS6105X
> >  	tristate "TI TPS61050/61052 Boost Converters"
> >  	depends on I2C
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > index 9834e66..4571506 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -185,6 +185,7 @@ obj-$(CONFIG_MFD_INTEL_LPSS_PCI)	+= intel-lpss-pci.o
> >  obj-$(CONFIG_MFD_INTEL_LPSS_ACPI)	+= intel-lpss-acpi.o
> >  obj-$(CONFIG_MFD_INTEL_MSIC)	+= intel_msic.o
> >  obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
> > +obj-$(CONFIG_MFD_STM32_ADC) 	+= stm32-adc-core.o
> >  obj-$(CONFIG_MFD_VIPERBOARD)    += viperboard.o
> >  obj-$(CONFIG_MFD_RC5T583)	+= rc5t583.o rc5t583-irq.o
> >  obj-$(CONFIG_MFD_RK808)		+= rk808.o
> > diff --git a/drivers/mfd/stm32-adc-core.c b/drivers/mfd/stm32-adc-core.c
> > new file mode 100644
> > index 0000000..bcf52fb
> > --- /dev/null
> > +++ b/drivers/mfd/stm32-adc-core.c
> > @@ -0,0 +1,301 @@
> > +/*
> > + * This file is part of STM32 ADC driver
> > + *
> > + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
> > + * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
> > + *
> > + * Inspired from: fsl-imx25-tsadc
> > + *
> > + * License type: GPLv2
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License version 2 as published by
> > + * the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> > + * or FITNESS FOR A PARTICULAR PURPOSE.
> > + * See the GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along with
> > + * this program. If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/irqchip/chained_irq.h>
> > +#include <linux/irqdesc.h>
> > +#include <linux/irqdomain.h>
> > +#include <linux/mfd/stm32-adc-core.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/slab.h>
> > +
> > +/* STM32F4 - common registers for all ADC instances: 1, 2 & 3 */
> > +#define STM32F4_ADC_CSR			(STM32_ADCX_COMN_OFFSET + 0x00)
> > +#define STM32F4_ADC_CCR			(STM32_ADCX_COMN_OFFSET + 0x04)
> > +
> > +/* STM32F4_ADC_CSR - bit fields */
> > +#define STM32F4_EOC3			BIT(17)
> > +#define STM32F4_EOC2			BIT(9)
> > +#define STM32F4_EOC1			BIT(1)
> > +
> > +/* STM32F4_ADC_CCR - bit fields */
> > +#define STM32F4_ADC_ADCPRE_SHIFT	16
> > +#define STM32F4_ADC_ADCPRE_MASK		GENMASK(17, 16)
> > +
> > +/* STM32 F4 maximum analog clock rate (from datasheet) */
> > +#define STM32F4_ADC_MAX_CLK_RATE	36000000
> > +
> > +/**
> > + * struct stm32_adc_priv - stm32 ADC core private data
> > + * @irq:		irq for ADC block
> > + * @domain:		irq domain reference
> > + * @aclk:		clock reference for the analog circuitry
> > + * @vref:		regulator reference
> > + * @common:		common data for all ADC instances
> > + */
> > +struct stm32_adc_priv {
> > +	int				irq;
> > +	struct irq_domain		*domain;
> > +	struct clk			*aclk;
> > +	struct regulator		*vref;
> > +	struct stm32_adc_common		common;
> > +};
> > +
> > +static struct stm32_adc_priv *to_stm32_adc_priv(struct stm32_adc_common *com)
> > +{
> > +	return container_of(com, struct stm32_adc_priv, common);
> > +}
> > +
> > +/* STM32F4 ADC internal common clock prescaler division ratios */
> > +static int stm32f4_pclk_div[] = {2, 4, 6, 8};
> > +
> > +/**
> > + * stm32f4_adc_clk_sel() - Select stm32f4 ADC common clock prescaler
> > + * @priv: stm32 ADC core private data
> > + * Select clock prescaler used for analog conversions, before using ADC.
> > + */
> > +static int stm32f4_adc_clk_sel(struct platform_device *pdev,
> > +			       struct stm32_adc_priv *priv)
> > +{
> > +	unsigned long rate;
> > +	u32 val;
> > +	int i;
> > +
> > +	rate = clk_get_rate(priv->aclk);
> > +	for (i = 0; i < ARRAY_SIZE(stm32f4_pclk_div); i++) {
> > +		if ((rate / stm32f4_pclk_div[i]) <= STM32F4_ADC_MAX_CLK_RATE)
> > +			break;
> > +	}
> > +	if (i >= ARRAY_SIZE(stm32f4_pclk_div))
> > +		return -EINVAL;
> > +
> > +	val = readl_relaxed(priv->common.base + STM32F4_ADC_CCR);
> > +	val &= ~STM32F4_ADC_ADCPRE_MASK;
> > +	val |= i << STM32F4_ADC_ADCPRE_SHIFT;
> > +	writel_relaxed(val, priv->common.base + STM32F4_ADC_CCR);
> > +
> > +	dev_dbg(&pdev->dev, "Using analog clock source at %ld kHz\n",
> > +		rate / (stm32f4_pclk_div[i] * 1000));
> > +
> > +	return 0;
> > +}
> > +
> > +/* ADC common interrupt for all instances */
> > +static void stm32_adc_irq_handler(struct irq_desc *desc)
> > +{
> > +	struct stm32_adc_priv *priv = irq_desc_get_handler_data(desc);
> > +	struct irq_chip *chip = irq_desc_get_chip(desc);
> > +	u32 status;
> > +
> > +	chained_irq_enter(chip, desc);
> > +	status = readl_relaxed(priv->common.base + STM32F4_ADC_CSR);
> > +
> > +	if (status & STM32F4_EOC1)
> > +		generic_handle_irq(irq_find_mapping(priv->domain, 0));
> > +
> > +	if (status & STM32F4_EOC2)
> > +		generic_handle_irq(irq_find_mapping(priv->domain, 1));
> > +
> > +	if (status & STM32F4_EOC3)
> > +		generic_handle_irq(irq_find_mapping(priv->domain, 2));
> > +
> > +	chained_irq_exit(chip, desc);
> > +};
> > +
> > +static int stm32_adc_domain_map(struct irq_domain *d, unsigned int irq,
> > +				irq_hw_number_t hwirq)
> > +{
> > +	irq_set_chip_data(irq, d->host_data);
> > +	irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_level_irq);
> > +
> > +	return 0;
> > +}
> > +
> > +static void stm32_adc_domain_unmap(struct irq_domain *d, unsigned int irq)
> > +{
> > +	irq_set_chip_and_handler(irq, NULL, NULL);
> > +	irq_set_chip_data(irq, NULL);
> > +}
> > +
> > +static const struct irq_domain_ops stm32_adc_domain_ops = {
> > +	.map = stm32_adc_domain_map,
> > +	.unmap  = stm32_adc_domain_unmap,
> > +	.xlate = irq_domain_xlate_onecell,
> > +};
> > +
> > +static int stm32_adc_irq_probe(struct platform_device *pdev,
> > +			       struct stm32_adc_priv *priv)
> > +{
> > +	struct device_node *np = pdev->dev.of_node;
> > +
> > +	priv->irq = platform_get_irq(pdev, 0);
> > +	if (priv->irq < 0) {
> > +		dev_err(&pdev->dev, "failed to get irq\n");
> > +		return priv->irq;
> > +	}
> > +
> > +	priv->domain = irq_domain_add_simple(np, STM32_ADC_MAX_ADCS, 0,
> > +					     &stm32_adc_domain_ops,
> > +					     priv);
> > +	if (!priv->domain) {
> > +		dev_err(&pdev->dev, "Failed to add irq domain\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	irq_set_chained_handler(priv->irq, stm32_adc_irq_handler);
> > +	irq_set_handler_data(priv->irq, priv);
> > +
> > +	return 0;
> > +}
> > +
> > +static void stm32_adc_irq_remove(struct platform_device *pdev,
> > +				 struct stm32_adc_priv *priv)
> > +{
> > +	int hwirq;
> > +
> > +	for (hwirq = 0; hwirq < STM32_ADC_MAX_ADCS; hwirq++)
> > +		irq_dispose_mapping(irq_find_mapping(priv->domain, hwirq));
> > +	irq_domain_remove(priv->domain);
> > +	irq_set_chained_handler(priv->irq, NULL);
> > +}
> > +
> > +static int stm32_adc_probe(struct platform_device *pdev)
> > +{
> > +	struct stm32_adc_priv *priv;
> > +	struct device_node *np = pdev->dev.of_node;
> > +	struct resource *res;
> > +	int ret;
> > +
> > +	if (!pdev->dev.of_node)
> > +		return -ENODEV;
> > +
> > +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	priv->common.base = devm_ioremap_resource(&pdev->dev, res);
> > +	if (IS_ERR(priv->common.base))
> > +		return PTR_ERR(priv->common.base);
> > +
> > +	priv->vref = devm_regulator_get(&pdev->dev, "vref");
> > +	if (IS_ERR(priv->vref)) {
> > +		ret = PTR_ERR(priv->vref);
> > +		dev_err(&pdev->dev, "vref get failed, %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	ret = regulator_enable(priv->vref);
> > +	if (ret < 0) {
> > +		dev_err(&pdev->dev, "vref enable failed\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = regulator_get_voltage(priv->vref);
> > +	if (ret < 0) {
> > +		dev_err(&pdev->dev, "vref get voltage failed, %d\n", ret);
> > +		goto err_regulator_disable;
> > +	}
> > +	priv->common.vref_mv = ret / 1000;
> > +	dev_dbg(&pdev->dev, "vref+=%dmV\n", priv->common.vref_mv);
> > +
> > +	priv->aclk = devm_clk_get(&pdev->dev, "adc");
> > +	if (IS_ERR(priv->aclk)) {
> > +		ret = PTR_ERR(priv->aclk);
> > +		dev_err(&pdev->dev, "Can't get 'adc' clock\n");
> > +		goto err_regulator_disable;
> > +	}
> > +
> > +	ret = clk_prepare_enable(priv->aclk);
> > +	if (ret < 0) {
> > +		dev_err(&pdev->dev, "adc clk enable failed\n");
> > +		goto err_regulator_disable;
> > +	}
> > +
> > +	ret = stm32f4_adc_clk_sel(pdev, priv);
> > +	if (ret < 0) {
> > +		dev_err(&pdev->dev, "adc clk selection failed\n");
> > +		goto err_clk_disable;
> > +	}
> > +
> > +	ret = stm32_adc_irq_probe(pdev, priv);
> > +	if (ret < 0)
> > +		goto err_clk_disable;
> > +
> > +	platform_set_drvdata(pdev, &priv->common);
> > +
> > +	ret = of_platform_populate(np, NULL, NULL, &pdev->dev);
> > +	if (ret < 0) {
> > +		dev_err(&pdev->dev, "failed to populate DT children\n");
> > +		goto err_irq_remove;
> > +	}
> > +
> > +	return 0;
> > +
> > +err_irq_remove:
> > +	stm32_adc_irq_remove(pdev, priv);
> > +
> > +err_clk_disable:
> > +	clk_disable_unprepare(priv->aclk);
> > +
> > +err_regulator_disable:
> > +	regulator_disable(priv->vref);
> > +
> > +	return ret;
> > +}
> > +
> > +static int stm32_adc_remove(struct platform_device *pdev)
> > +{
> > +	struct stm32_adc_common *common = platform_get_drvdata(pdev);
> > +	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
> > +
> > +	of_platform_depopulate(&pdev->dev);
> > +	stm32_adc_irq_remove(pdev, priv);
> > +	clk_disable_unprepare(priv->aclk);
> > +	regulator_disable(priv->vref);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id stm32_adc_of_match[] = {
> > +	{ .compatible = "st,stm32f4-adc-core" },
> > +};
> > +MODULE_DEVICE_TABLE(of, stm32_adc_of_match);
> > +
> > +static struct platform_driver stm32_adc_driver = {
> > +	.probe = stm32_adc_probe,
> > +	.remove = stm32_adc_remove,
> > +	.driver = {
> > +		.name = "stm32-adc-core",
> > +		.of_match_table = stm32_adc_of_match,
> > +	},
> > +};
> > +module_platform_driver(stm32_adc_driver);
> > +
> > +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@st.com>");
> > +MODULE_DESCRIPTION("STMicroelectronics STM32 ADC MFD driver");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_ALIAS("platform:stm32-adc-core");
> > diff --git a/include/linux/mfd/stm32-adc-core.h b/include/linux/mfd/stm32-adc-core.h
> > new file mode 100644
> > index 0000000..081fa5f
> > --- /dev/null
> > +++ b/include/linux/mfd/stm32-adc-core.h
> > @@ -0,0 +1,52 @@
> > +/*
> > + * This file is part of STM32 ADC driver
> > + *
> > + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
> > + * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
> > + *
> > + * License type: GPLv2
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License version 2 as published by
> > + * the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> > + * or FITNESS FOR A PARTICULAR PURPOSE.
> > + * See the GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along with
> > + * this program. If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#ifndef __STM32_ADC_H
> > +#define __STM32_ADC_H
> > +
> > +/*
> > + * STM32 - ADC global register map
> > + * ________________________________________________________
> > + * | Offset |                 Register                    |
> > + * --------------------------------------------------------
> > + * | 0x000  |                Master ADC1                  |
> > + * --------------------------------------------------------
> > + * | 0x100  |                Slave ADC2                   |
> > + * --------------------------------------------------------
> > + * | 0x200  |                Slave ADC3                   |
> > + * --------------------------------------------------------
> > + * | 0x300  |         Master & Slave common regs          |
> > + * --------------------------------------------------------
> > + */
> > +#define STM32_ADC_MAX_ADCS		3
> > +#define STM32_ADCX_COMN_OFFSET		0x300
> > +
> > +/**
> > + * struct stm32_adc_common - stm32 ADC driver common data (for all instances)
> > + * @base:		control registers base cpu addr
> > + * @vref_mv:		vref voltage (mv)
> > + */
> > +struct stm32_adc_common {
> > +	void __iomem			*base;
> > +	int				vref_mv;
> > +};
> > +
> > +#endif
> > 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 2/6] mfd: stm32-adc: Add support for stm32 ADC

[Fabrice Gasnier]

On 11/14/2016 05:47 PM, Lee Jones wrote:
> On Sat, 12 Nov 2016, Jonathan Cameron wrote:
>
>> On 10/11/16 16:18, Fabrice Gasnier wrote:
>>> Add core driver for STMicroelectronics STM32 ADC (Analog to Digital
>>> Converter). STM32 ADC can be composed of up to 3 ADCs with shared
>>> resources like clock prescaler, common interrupt line and analog
>>> reference voltage.
>>> This core driver basically manages shared resources.
>>>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>> Looks good to me (other than the build issue obviously ;)

Hi Jonathan,
Thanks for your review.
I'll fix build issue, sure ;-)

>>
>> The fun bit will be trying to keep the whole thing this clean as you
>> add the more 'interesting' functionality.  *fingers crossed*
Yes... But in the end, splitting shared resources in core driver makes 
it more simple.
Not sure there will be more complexity here.

>>
>> Acked-by: Jonathan Cameron <jic23@kernel.org>
> There isn't anything MFD about this driver.
>
> Please move it into IIO.

Hmm, there is no other sub sysbtem that may be used here, ADC driver 
belongs to IIO.
Also, of_platform_populate() is being used here. This can perfectly be 
called from within IIO.

Jonathan, can this "stm32-adc-core" driver be moved to, and live in 
drivers/iio/adc ?
(e.g. in addition to stm32-adc iio driver)
Is it ok for you ?

Please advise,
Best Regards,
Fabrice

>
>>> ---
>>>   drivers/mfd/Kconfig                |  14 ++
>>>   drivers/mfd/Makefile               |   1 +
>>>   drivers/mfd/stm32-adc-core.c       | 301 +++++++++++++++++++++++++++++++++++++
>>>   include/linux/mfd/stm32-adc-core.h |  52 +++++++
>>>   4 files changed, 368 insertions(+)
>>>   create mode 100644 drivers/mfd/stm32-adc-core.c
>>>   create mode 100644 include/linux/mfd/stm32-adc-core.h
>>>
>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>>> index c6df644..2580cee 100644
>>> --- a/drivers/mfd/Kconfig
>>> +++ b/drivers/mfd/Kconfig
>>> @@ -1152,6 +1152,20 @@ config MFD_PALMAS
>>>   	  If you say yes here you get support for the Palmas
>>>   	  series of PMIC chips from Texas Instruments.
>>>   
>>> +config MFD_STM32_ADC
>>> +	tristate "STMicroelectronics STM32 adc"
>>> +	depends on ARCH_STM32 || COMPILE_TEST
>>> +	depends on OF
>>> +	select MFD_CORE
>>> +	select REGULATOR
>>> +	select REGULATOR_FIXED_VOLTAGE
>>> +	help
>>> +	  Select this option to enable the core driver for STMicroelectronics
>>> +	  STM32 analog-to-digital converter (ADC).
>>> +
>>> +	  This driver can also be built as a module.  If so, the module
>>> +	  will be called stm32-adc-core.
>>> +
>>>   config TPS6105X
>>>   	tristate "TI TPS61050/61052 Boost Converters"
>>>   	depends on I2C
>>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>>> index 9834e66..4571506 100644
>>> --- a/drivers/mfd/Makefile
>>> +++ b/drivers/mfd/Makefile
>>> @@ -185,6 +185,7 @@ obj-$(CONFIG_MFD_INTEL_LPSS_PCI)	+= intel-lpss-pci.o
>>>   obj-$(CONFIG_MFD_INTEL_LPSS_ACPI)	+= intel-lpss-acpi.o
>>>   obj-$(CONFIG_MFD_INTEL_MSIC)	+= intel_msic.o
>>>   obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
>>> +obj-$(CONFIG_MFD_STM32_ADC) 	+= stm32-adc-core.o
>>>   obj-$(CONFIG_MFD_VIPERBOARD)    += viperboard.o
>>>   obj-$(CONFIG_MFD_RC5T583)	+= rc5t583.o rc5t583-irq.o
>>>   obj-$(CONFIG_MFD_RK808)		+= rk808.o
>>> diff --git a/drivers/mfd/stm32-adc-core.c b/drivers/mfd/stm32-adc-core.c
>>> new file mode 100644
>>> index 0000000..bcf52fb
>>> --- /dev/null
>>> +++ b/drivers/mfd/stm32-adc-core.c
>>> @@ -0,0 +1,301 @@
>>> +/*
>>> + * This file is part of STM32 ADC driver
>>> + *
>>> + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
>>> + * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
>>> + *
>>> + * Inspired from: fsl-imx25-tsadc
>>> + *
>>> + * License type: GPLv2
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms of the GNU General Public License version 2 as published by
>>> + * the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful, but
>>> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
>>> + * or FITNESS FOR A PARTICULAR PURPOSE.
>>> + * See the GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License along with
>>> + * this program. If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/irqchip/chained_irq.h>
>>> +#include <linux/irqdesc.h>
>>> +#include <linux/irqdomain.h>
>>> +#include <linux/mfd/stm32-adc-core.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/regulator/consumer.h>
>>> +#include <linux/slab.h>
>>> +
>>> +/* STM32F4 - common registers for all ADC instances: 1, 2 & 3 */
>>> +#define STM32F4_ADC_CSR			(STM32_ADCX_COMN_OFFSET + 0x00)
>>> +#define STM32F4_ADC_CCR			(STM32_ADCX_COMN_OFFSET + 0x04)
>>> +
>>> +/* STM32F4_ADC_CSR - bit fields */
>>> +#define STM32F4_EOC3			BIT(17)
>>> +#define STM32F4_EOC2			BIT(9)
>>> +#define STM32F4_EOC1			BIT(1)
>>> +
>>> +/* STM32F4_ADC_CCR - bit fields */
>>> +#define STM32F4_ADC_ADCPRE_SHIFT	16
>>> +#define STM32F4_ADC_ADCPRE_MASK		GENMASK(17, 16)
>>> +
>>> +/* STM32 F4 maximum analog clock rate (from datasheet) */
>>> +#define STM32F4_ADC_MAX_CLK_RATE	36000000
>>> +
>>> +/**
>>> + * struct stm32_adc_priv - stm32 ADC core private data
>>> + * @irq:		irq for ADC block
>>> + * @domain:		irq domain reference
>>> + * @aclk:		clock reference for the analog circuitry
>>> + * @vref:		regulator reference
>>> + * @common:		common data for all ADC instances
>>> + */
>>> +struct stm32_adc_priv {
>>> +	int				irq;
>>> +	struct irq_domain		*domain;
>>> +	struct clk			*aclk;
>>> +	struct regulator		*vref;
>>> +	struct stm32_adc_common		common;
>>> +};
>>> +
>>> +static struct stm32_adc_priv *to_stm32_adc_priv(struct stm32_adc_common *com)
>>> +{
>>> +	return container_of(com, struct stm32_adc_priv, common);
>>> +}
>>> +
>>> +/* STM32F4 ADC internal common clock prescaler division ratios */
>>> +static int stm32f4_pclk_div[] = {2, 4, 6, 8};
>>> +
>>> +/**
>>> + * stm32f4_adc_clk_sel() - Select stm32f4 ADC common clock prescaler
>>> + * @priv: stm32 ADC core private data
>>> + * Select clock prescaler used for analog conversions, before using ADC.
>>> + */
>>> +static int stm32f4_adc_clk_sel(struct platform_device *pdev,
>>> +			       struct stm32_adc_priv *priv)
>>> +{
>>> +	unsigned long rate;
>>> +	u32 val;
>>> +	int i;
>>> +
>>> +	rate = clk_get_rate(priv->aclk);
>>> +	for (i = 0; i < ARRAY_SIZE(stm32f4_pclk_div); i++) {
>>> +		if ((rate / stm32f4_pclk_div[i]) <= STM32F4_ADC_MAX_CLK_RATE)
>>> +			break;
>>> +	}
>>> +	if (i >= ARRAY_SIZE(stm32f4_pclk_div))
>>> +		return -EINVAL;
>>> +
>>> +	val = readl_relaxed(priv->common.base + STM32F4_ADC_CCR);
>>> +	val &= ~STM32F4_ADC_ADCPRE_MASK;
>>> +	val |= i << STM32F4_ADC_ADCPRE_SHIFT;
>>> +	writel_relaxed(val, priv->common.base + STM32F4_ADC_CCR);
>>> +
>>> +	dev_dbg(&pdev->dev, "Using analog clock source at %ld kHz\n",
>>> +		rate / (stm32f4_pclk_div[i] * 1000));
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +/* ADC common interrupt for all instances */
>>> +static void stm32_adc_irq_handler(struct irq_desc *desc)
>>> +{
>>> +	struct stm32_adc_priv *priv = irq_desc_get_handler_data(desc);
>>> +	struct irq_chip *chip = irq_desc_get_chip(desc);
>>> +	u32 status;
>>> +
>>> +	chained_irq_enter(chip, desc);
>>> +	status = readl_relaxed(priv->common.base + STM32F4_ADC_CSR);
>>> +
>>> +	if (status & STM32F4_EOC1)
>>> +		generic_handle_irq(irq_find_mapping(priv->domain, 0));
>>> +
>>> +	if (status & STM32F4_EOC2)
>>> +		generic_handle_irq(irq_find_mapping(priv->domain, 1));
>>> +
>>> +	if (status & STM32F4_EOC3)
>>> +		generic_handle_irq(irq_find_mapping(priv->domain, 2));
>>> +
>>> +	chained_irq_exit(chip, desc);
>>> +};
>>> +
>>> +static int stm32_adc_domain_map(struct irq_domain *d, unsigned int irq,
>>> +				irq_hw_number_t hwirq)
>>> +{
>>> +	irq_set_chip_data(irq, d->host_data);
>>> +	irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_level_irq);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static void stm32_adc_domain_unmap(struct irq_domain *d, unsigned int irq)
>>> +{
>>> +	irq_set_chip_and_handler(irq, NULL, NULL);
>>> +	irq_set_chip_data(irq, NULL);
>>> +}
>>> +
>>> +static const struct irq_domain_ops stm32_adc_domain_ops = {
>>> +	.map = stm32_adc_domain_map,
>>> +	.unmap  = stm32_adc_domain_unmap,
>>> +	.xlate = irq_domain_xlate_onecell,
>>> +};
>>> +
>>> +static int stm32_adc_irq_probe(struct platform_device *pdev,
>>> +			       struct stm32_adc_priv *priv)
>>> +{
>>> +	struct device_node *np = pdev->dev.of_node;
>>> +
>>> +	priv->irq = platform_get_irq(pdev, 0);
>>> +	if (priv->irq < 0) {
>>> +		dev_err(&pdev->dev, "failed to get irq\n");
>>> +		return priv->irq;
>>> +	}
>>> +
>>> +	priv->domain = irq_domain_add_simple(np, STM32_ADC_MAX_ADCS, 0,
>>> +					     &stm32_adc_domain_ops,
>>> +					     priv);
>>> +	if (!priv->domain) {
>>> +		dev_err(&pdev->dev, "Failed to add irq domain\n");
>>> +		return -ENOMEM;
>>> +	}
>>> +
>>> +	irq_set_chained_handler(priv->irq, stm32_adc_irq_handler);
>>> +	irq_set_handler_data(priv->irq, priv);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static void stm32_adc_irq_remove(struct platform_device *pdev,
>>> +				 struct stm32_adc_priv *priv)
>>> +{
>>> +	int hwirq;
>>> +
>>> +	for (hwirq = 0; hwirq < STM32_ADC_MAX_ADCS; hwirq++)
>>> +		irq_dispose_mapping(irq_find_mapping(priv->domain, hwirq));
>>> +	irq_domain_remove(priv->domain);
>>> +	irq_set_chained_handler(priv->irq, NULL);
>>> +}
>>> +
>>> +static int stm32_adc_probe(struct platform_device *pdev)
>>> +{
>>> +	struct stm32_adc_priv *priv;
>>> +	struct device_node *np = pdev->dev.of_node;
>>> +	struct resource *res;
>>> +	int ret;
>>> +
>>> +	if (!pdev->dev.of_node)
>>> +		return -ENODEV;
>>> +
>>> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>>> +	if (!priv)
>>> +		return -ENOMEM;
>>> +
>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +	priv->common.base = devm_ioremap_resource(&pdev->dev, res);
>>> +	if (IS_ERR(priv->common.base))
>>> +		return PTR_ERR(priv->common.base);
>>> +
>>> +	priv->vref = devm_regulator_get(&pdev->dev, "vref");
>>> +	if (IS_ERR(priv->vref)) {
>>> +		ret = PTR_ERR(priv->vref);
>>> +		dev_err(&pdev->dev, "vref get failed, %d\n", ret);
>>> +		return ret;
>>> +	}
>>> +
>>> +	ret = regulator_enable(priv->vref);
>>> +	if (ret < 0) {
>>> +		dev_err(&pdev->dev, "vref enable failed\n");
>>> +		return ret;
>>> +	}
>>> +
>>> +	ret = regulator_get_voltage(priv->vref);
>>> +	if (ret < 0) {
>>> +		dev_err(&pdev->dev, "vref get voltage failed, %d\n", ret);
>>> +		goto err_regulator_disable;
>>> +	}
>>> +	priv->common.vref_mv = ret / 1000;
>>> +	dev_dbg(&pdev->dev, "vref+=%dmV\n", priv->common.vref_mv);
>>> +
>>> +	priv->aclk = devm_clk_get(&pdev->dev, "adc");
>>> +	if (IS_ERR(priv->aclk)) {
>>> +		ret = PTR_ERR(priv->aclk);
>>> +		dev_err(&pdev->dev, "Can't get 'adc' clock\n");
>>> +		goto err_regulator_disable;
>>> +	}
>>> +
>>> +	ret = clk_prepare_enable(priv->aclk);
>>> +	if (ret < 0) {
>>> +		dev_err(&pdev->dev, "adc clk enable failed\n");
>>> +		goto err_regulator_disable;
>>> +	}
>>> +
>>> +	ret = stm32f4_adc_clk_sel(pdev, priv);
>>> +	if (ret < 0) {
>>> +		dev_err(&pdev->dev, "adc clk selection failed\n");
>>> +		goto err_clk_disable;
>>> +	}
>>> +
>>> +	ret = stm32_adc_irq_probe(pdev, priv);
>>> +	if (ret < 0)
>>> +		goto err_clk_disable;
>>> +
>>> +	platform_set_drvdata(pdev, &priv->common);
>>> +
>>> +	ret = of_platform_populate(np, NULL, NULL, &pdev->dev);
>>> +	if (ret < 0) {
>>> +		dev_err(&pdev->dev, "failed to populate DT children\n");
>>> +		goto err_irq_remove;
>>> +	}
>>> +
>>> +	return 0;
>>> +
>>> +err_irq_remove:
>>> +	stm32_adc_irq_remove(pdev, priv);
>>> +
>>> +err_clk_disable:
>>> +	clk_disable_unprepare(priv->aclk);
>>> +
>>> +err_regulator_disable:
>>> +	regulator_disable(priv->vref);
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static int stm32_adc_remove(struct platform_device *pdev)
>>> +{
>>> +	struct stm32_adc_common *common = platform_get_drvdata(pdev);
>>> +	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
>>> +
>>> +	of_platform_depopulate(&pdev->dev);
>>> +	stm32_adc_irq_remove(pdev, priv);
>>> +	clk_disable_unprepare(priv->aclk);
>>> +	regulator_disable(priv->vref);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static const struct of_device_id stm32_adc_of_match[] = {
>>> +	{ .compatible = "st,stm32f4-adc-core" },
>>> +};
>>> +MODULE_DEVICE_TABLE(of, stm32_adc_of_match);
>>> +
>>> +static struct platform_driver stm32_adc_driver = {
>>> +	.probe = stm32_adc_probe,
>>> +	.remove = stm32_adc_remove,
>>> +	.driver = {
>>> +		.name = "stm32-adc-core",
>>> +		.of_match_table = stm32_adc_of_match,
>>> +	},
>>> +};
>>> +module_platform_driver(stm32_adc_driver);
>>> +
>>> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@st.com>");
>>> +MODULE_DESCRIPTION("STMicroelectronics STM32 ADC MFD driver");
>>> +MODULE_LICENSE("GPL v2");
>>> +MODULE_ALIAS("platform:stm32-adc-core");
>>> diff --git a/include/linux/mfd/stm32-adc-core.h b/include/linux/mfd/stm32-adc-core.h
>>> new file mode 100644
>>> index 0000000..081fa5f
>>> --- /dev/null
>>> +++ b/include/linux/mfd/stm32-adc-core.h
>>> @@ -0,0 +1,52 @@
>>> +/*
>>> + * This file is part of STM32 ADC driver
>>> + *
>>> + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
>>> + * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
>>> + *
>>> + * License type: GPLv2
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms of the GNU General Public License version 2 as published by
>>> + * the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful, but
>>> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
>>> + * or FITNESS FOR A PARTICULAR PURPOSE.
>>> + * See the GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License along with
>>> + * this program. If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#ifndef __STM32_ADC_H
>>> +#define __STM32_ADC_H
>>> +
>>> +/*
>>> + * STM32 - ADC global register map
>>> + * ________________________________________________________
>>> + * | Offset |                 Register                    |
>>> + * --------------------------------------------------------
>>> + * | 0x000  |                Master ADC1                  |
>>> + * --------------------------------------------------------
>>> + * | 0x100  |                Slave ADC2                   |
>>> + * --------------------------------------------------------
>>> + * | 0x200  |                Slave ADC3                   |
>>> + * --------------------------------------------------------
>>> + * | 0x300  |         Master & Slave common regs          |
>>> + * --------------------------------------------------------
>>> + */
>>> +#define STM32_ADC_MAX_ADCS		3
>>> +#define STM32_ADCX_COMN_OFFSET		0x300
>>> +
>>> +/**
>>> + * struct stm32_adc_common - stm32 ADC driver common data (for all instances)
>>> + * @base:		control registers base cpu addr
>>> + * @vref_mv:		vref voltage (mv)
>>> + */
>>> +struct stm32_adc_common {
>>> +	void __iomem			*base;
>>> +	int				vref_mv;
>>> +};
>>> +
>>> +#endif
>>>

Re: [PATCH v2 2/6] mfd: stm32-adc: Add support for stm32 ADC

[Jonathan Cameron]

On 15 November 2016 10:47:52 GMT+00:00, Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
>On 11/14/2016 05:47 PM, Lee Jones wrote:
>> On Sat, 12 Nov 2016, Jonathan Cameron wrote:
>>
>>> On 10/11/16 16:18, Fabrice Gasnier wrote:
>>>> Add core driver for STMicroelectronics STM32 ADC (Analog to Digital
>>>> Converter). STM32 ADC can be composed of up to 3 ADCs with shared
>>>> resources like clock prescaler, common interrupt line and analog
>>>> reference voltage.
>>>> This core driver basically manages shared resources.
>>>>
>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>> Looks good to me (other than the build issue obviously ;)
>
>Hi Jonathan,
>Thanks for your review.
>I'll fix build issue, sure ;-)
>
>>>
>>> The fun bit will be trying to keep the whole thing this clean as you
>>> add the more 'interesting' functionality.  *fingers crossed*
>Yes... But in the end, splitting shared resources in core driver makes 
>it more simple.
>Not sure there will be more complexity here.
>
>>>
>>> Acked-by: Jonathan Cameron <jic23@kernel.org>
>> There isn't anything MFD about this driver.
>>
>> Please move it into IIO.
>
>Hmm, there is no other sub sysbtem that may be used here, ADC driver 
>belongs to IIO.
>Also, of_platform_populate() is being used here. This can perfectly be 
>called from within IIO.
>
>Jonathan, can this "stm32-adc-core" driver be moved to, and live in 
>drivers/iio/adc ?
>(e.g. in addition to stm32-adc iio driver)
>Is it ok for you ?
Yes. That's ideal. 
>
>Please advise,
>Best Regards,
>Fabrice
>
>>
>>>> ---
>>>>   drivers/mfd/Kconfig                |  14 ++
>>>>   drivers/mfd/Makefile               |   1 +
>>>>   drivers/mfd/stm32-adc-core.c       | 301
>+++++++++++++++++++++++++++++++++++++
>>>>   include/linux/mfd/stm32-adc-core.h |  52 +++++++
>>>>   4 files changed, 368 insertions(+)
>>>>   create mode 100644 drivers/mfd/stm32-adc-core.c
>>>>   create mode 100644 include/linux/mfd/stm32-adc-core.h
>>>>
>>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>>>> index c6df644..2580cee 100644
>>>> --- a/drivers/mfd/Kconfig
>>>> +++ b/drivers/mfd/Kconfig
>>>> @@ -1152,6 +1152,20 @@ config MFD_PALMAS
>>>>   	  If you say yes here you get support for the Palmas
>>>>   	  series of PMIC chips from Texas Instruments.
>>>>   
>>>> +config MFD_STM32_ADC
>>>> +	tristate "STMicroelectronics STM32 adc"
>>>> +	depends on ARCH_STM32 || COMPILE_TEST
>>>> +	depends on OF
>>>> +	select MFD_CORE
>>>> +	select REGULATOR
>>>> +	select REGULATOR_FIXED_VOLTAGE
>>>> +	help
>>>> +	  Select this option to enable the core driver for
>STMicroelectronics
>>>> +	  STM32 analog-to-digital converter (ADC).
>>>> +
>>>> +	  This driver can also be built as a module.  If so, the module
>>>> +	  will be called stm32-adc-core.
>>>> +
>>>>   config TPS6105X
>>>>   	tristate "TI TPS61050/61052 Boost Converters"
>>>>   	depends on I2C
>>>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>>>> index 9834e66..4571506 100644
>>>> --- a/drivers/mfd/Makefile
>>>> +++ b/drivers/mfd/Makefile
>>>> @@ -185,6 +185,7 @@ obj-$(CONFIG_MFD_INTEL_LPSS_PCI)	+=
>intel-lpss-pci.o
>>>>   obj-$(CONFIG_MFD_INTEL_LPSS_ACPI)	+= intel-lpss-acpi.o
>>>>   obj-$(CONFIG_MFD_INTEL_MSIC)	+= intel_msic.o
>>>>   obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
>>>> +obj-$(CONFIG_MFD_STM32_ADC) 	+= stm32-adc-core.o
>>>>   obj-$(CONFIG_MFD_VIPERBOARD)    += viperboard.o
>>>>   obj-$(CONFIG_MFD_RC5T583)	+= rc5t583.o rc5t583-irq.o
>>>>   obj-$(CONFIG_MFD_RK808)		+= rk808.o
>>>> diff --git a/drivers/mfd/stm32-adc-core.c
>b/drivers/mfd/stm32-adc-core.c
>>>> new file mode 100644
>>>> index 0000000..bcf52fb
>>>> --- /dev/null
>>>> +++ b/drivers/mfd/stm32-adc-core.c
>>>> @@ -0,0 +1,301 @@
>>>> +/*
>>>> + * This file is part of STM32 ADC driver
>>>> + *
>>>> + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
>>>> + * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
>>>> + *
>>>> + * Inspired from: fsl-imx25-tsadc
>>>> + *
>>>> + * License type: GPLv2
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or
>modify it
>>>> + * under the terms of the GNU General Public License version 2 as
>published by
>>>> + * the Free Software Foundation.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>but
>>>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>MERCHANTABILITY
>>>> + * or FITNESS FOR A PARTICULAR PURPOSE.
>>>> + * See the GNU General Public License for more details.
>>>> + *
>>>> + * You should have received a copy of the GNU General Public
>License along with
>>>> + * this program. If not, see <http://www.gnu.org/licenses/>.
>>>> + */
>>>> +
>>>> +#include <linux/clk.h>
>>>> +#include <linux/interrupt.h>
>>>> +#include <linux/irqchip/chained_irq.h>
>>>> +#include <linux/irqdesc.h>
>>>> +#include <linux/irqdomain.h>
>>>> +#include <linux/mfd/stm32-adc-core.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/of_device.h>
>>>> +#include <linux/regulator/consumer.h>
>>>> +#include <linux/slab.h>
>>>> +
>>>> +/* STM32F4 - common registers for all ADC instances: 1, 2 & 3 */
>>>> +#define STM32F4_ADC_CSR			(STM32_ADCX_COMN_OFFSET + 0x00)
>>>> +#define STM32F4_ADC_CCR			(STM32_ADCX_COMN_OFFSET + 0x04)
>>>> +
>>>> +/* STM32F4_ADC_CSR - bit fields */
>>>> +#define STM32F4_EOC3			BIT(17)
>>>> +#define STM32F4_EOC2			BIT(9)
>>>> +#define STM32F4_EOC1			BIT(1)
>>>> +
>>>> +/* STM32F4_ADC_CCR - bit fields */
>>>> +#define STM32F4_ADC_ADCPRE_SHIFT	16
>>>> +#define STM32F4_ADC_ADCPRE_MASK		GENMASK(17, 16)
>>>> +
>>>> +/* STM32 F4 maximum analog clock rate (from datasheet) */
>>>> +#define STM32F4_ADC_MAX_CLK_RATE	36000000
>>>> +
>>>> +/**
>>>> + * struct stm32_adc_priv - stm32 ADC core private data
>>>> + * @irq:		irq for ADC block
>>>> + * @domain:		irq domain reference
>>>> + * @aclk:		clock reference for the analog circuitry
>>>> + * @vref:		regulator reference
>>>> + * @common:		common data for all ADC instances
>>>> + */
>>>> +struct stm32_adc_priv {
>>>> +	int				irq;
>>>> +	struct irq_domain		*domain;
>>>> +	struct clk			*aclk;
>>>> +	struct regulator		*vref;
>>>> +	struct stm32_adc_common		common;
>>>> +};
>>>> +
>>>> +static struct stm32_adc_priv *to_stm32_adc_priv(struct
>stm32_adc_common *com)
>>>> +{
>>>> +	return container_of(com, struct stm32_adc_priv, common);
>>>> +}
>>>> +
>>>> +/* STM32F4 ADC internal common clock prescaler division ratios */
>>>> +static int stm32f4_pclk_div[] = {2, 4, 6, 8};
>>>> +
>>>> +/**
>>>> + * stm32f4_adc_clk_sel() - Select stm32f4 ADC common clock
>prescaler
>>>> + * @priv: stm32 ADC core private data
>>>> + * Select clock prescaler used for analog conversions, before
>using ADC.
>>>> + */
>>>> +static int stm32f4_adc_clk_sel(struct platform_device *pdev,
>>>> +			       struct stm32_adc_priv *priv)
>>>> +{
>>>> +	unsigned long rate;
>>>> +	u32 val;
>>>> +	int i;
>>>> +
>>>> +	rate = clk_get_rate(priv->aclk);
>>>> +	for (i = 0; i < ARRAY_SIZE(stm32f4_pclk_div); i++) {
>>>> +		if ((rate / stm32f4_pclk_div[i]) <= STM32F4_ADC_MAX_CLK_RATE)
>>>> +			break;
>>>> +	}
>>>> +	if (i >= ARRAY_SIZE(stm32f4_pclk_div))
>>>> +		return -EINVAL;
>>>> +
>>>> +	val = readl_relaxed(priv->common.base + STM32F4_ADC_CCR);
>>>> +	val &= ~STM32F4_ADC_ADCPRE_MASK;
>>>> +	val |= i << STM32F4_ADC_ADCPRE_SHIFT;
>>>> +	writel_relaxed(val, priv->common.base + STM32F4_ADC_CCR);
>>>> +
>>>> +	dev_dbg(&pdev->dev, "Using analog clock source at %ld kHz\n",
>>>> +		rate / (stm32f4_pclk_div[i] * 1000));
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +/* ADC common interrupt for all instances */
>>>> +static void stm32_adc_irq_handler(struct irq_desc *desc)
>>>> +{
>>>> +	struct stm32_adc_priv *priv = irq_desc_get_handler_data(desc);
>>>> +	struct irq_chip *chip = irq_desc_get_chip(desc);
>>>> +	u32 status;
>>>> +
>>>> +	chained_irq_enter(chip, desc);
>>>> +	status = readl_relaxed(priv->common.base + STM32F4_ADC_CSR);
>>>> +
>>>> +	if (status & STM32F4_EOC1)
>>>> +		generic_handle_irq(irq_find_mapping(priv->domain, 0));
>>>> +
>>>> +	if (status & STM32F4_EOC2)
>>>> +		generic_handle_irq(irq_find_mapping(priv->domain, 1));
>>>> +
>>>> +	if (status & STM32F4_EOC3)
>>>> +		generic_handle_irq(irq_find_mapping(priv->domain, 2));
>>>> +
>>>> +	chained_irq_exit(chip, desc);
>>>> +};
>>>> +
>>>> +static int stm32_adc_domain_map(struct irq_domain *d, unsigned int
>irq,
>>>> +				irq_hw_number_t hwirq)
>>>> +{
>>>> +	irq_set_chip_data(irq, d->host_data);
>>>> +	irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_level_irq);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static void stm32_adc_domain_unmap(struct irq_domain *d, unsigned
>int irq)
>>>> +{
>>>> +	irq_set_chip_and_handler(irq, NULL, NULL);
>>>> +	irq_set_chip_data(irq, NULL);
>>>> +}
>>>> +
>>>> +static const struct irq_domain_ops stm32_adc_domain_ops = {
>>>> +	.map = stm32_adc_domain_map,
>>>> +	.unmap  = stm32_adc_domain_unmap,
>>>> +	.xlate = irq_domain_xlate_onecell,
>>>> +};
>>>> +
>>>> +static int stm32_adc_irq_probe(struct platform_device *pdev,
>>>> +			       struct stm32_adc_priv *priv)
>>>> +{
>>>> +	struct device_node *np = pdev->dev.of_node;
>>>> +
>>>> +	priv->irq = platform_get_irq(pdev, 0);
>>>> +	if (priv->irq < 0) {
>>>> +		dev_err(&pdev->dev, "failed to get irq\n");
>>>> +		return priv->irq;
>>>> +	}
>>>> +
>>>> +	priv->domain = irq_domain_add_simple(np, STM32_ADC_MAX_ADCS, 0,
>>>> +					     &stm32_adc_domain_ops,
>>>> +					     priv);
>>>> +	if (!priv->domain) {
>>>> +		dev_err(&pdev->dev, "Failed to add irq domain\n");
>>>> +		return -ENOMEM;
>>>> +	}
>>>> +
>>>> +	irq_set_chained_handler(priv->irq, stm32_adc_irq_handler);
>>>> +	irq_set_handler_data(priv->irq, priv);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static void stm32_adc_irq_remove(struct platform_device *pdev,
>>>> +				 struct stm32_adc_priv *priv)
>>>> +{
>>>> +	int hwirq;
>>>> +
>>>> +	for (hwirq = 0; hwirq < STM32_ADC_MAX_ADCS; hwirq++)
>>>> +		irq_dispose_mapping(irq_find_mapping(priv->domain, hwirq));
>>>> +	irq_domain_remove(priv->domain);
>>>> +	irq_set_chained_handler(priv->irq, NULL);
>>>> +}
>>>> +
>>>> +static int stm32_adc_probe(struct platform_device *pdev)
>>>> +{
>>>> +	struct stm32_adc_priv *priv;
>>>> +	struct device_node *np = pdev->dev.of_node;
>>>> +	struct resource *res;
>>>> +	int ret;
>>>> +
>>>> +	if (!pdev->dev.of_node)
>>>> +		return -ENODEV;
>>>> +
>>>> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>>>> +	if (!priv)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> +	priv->common.base = devm_ioremap_resource(&pdev->dev, res);
>>>> +	if (IS_ERR(priv->common.base))
>>>> +		return PTR_ERR(priv->common.base);
>>>> +
>>>> +	priv->vref = devm_regulator_get(&pdev->dev, "vref");
>>>> +	if (IS_ERR(priv->vref)) {
>>>> +		ret = PTR_ERR(priv->vref);
>>>> +		dev_err(&pdev->dev, "vref get failed, %d\n", ret);
>>>> +		return ret;
>>>> +	}
>>>> +
>>>> +	ret = regulator_enable(priv->vref);
>>>> +	if (ret < 0) {
>>>> +		dev_err(&pdev->dev, "vref enable failed\n");
>>>> +		return ret;
>>>> +	}
>>>> +
>>>> +	ret = regulator_get_voltage(priv->vref);
>>>> +	if (ret < 0) {
>>>> +		dev_err(&pdev->dev, "vref get voltage failed, %d\n", ret);
>>>> +		goto err_regulator_disable;
>>>> +	}
>>>> +	priv->common.vref_mv = ret / 1000;
>>>> +	dev_dbg(&pdev->dev, "vref+=%dmV\n", priv->common.vref_mv);
>>>> +
>>>> +	priv->aclk = devm_clk_get(&pdev->dev, "adc");
>>>> +	if (IS_ERR(priv->aclk)) {
>>>> +		ret = PTR_ERR(priv->aclk);
>>>> +		dev_err(&pdev->dev, "Can't get 'adc' clock\n");
>>>> +		goto err_regulator_disable;
>>>> +	}
>>>> +
>>>> +	ret = clk_prepare_enable(priv->aclk);
>>>> +	if (ret < 0) {
>>>> +		dev_err(&pdev->dev, "adc clk enable failed\n");
>>>> +		goto err_regulator_disable;
>>>> +	}
>>>> +
>>>> +	ret = stm32f4_adc_clk_sel(pdev, priv);
>>>> +	if (ret < 0) {
>>>> +		dev_err(&pdev->dev, "adc clk selection failed\n");
>>>> +		goto err_clk_disable;
>>>> +	}
>>>> +
>>>> +	ret = stm32_adc_irq_probe(pdev, priv);
>>>> +	if (ret < 0)
>>>> +		goto err_clk_disable;
>>>> +
>>>> +	platform_set_drvdata(pdev, &priv->common);
>>>> +
>>>> +	ret = of_platform_populate(np, NULL, NULL, &pdev->dev);
>>>> +	if (ret < 0) {
>>>> +		dev_err(&pdev->dev, "failed to populate DT children\n");
>>>> +		goto err_irq_remove;
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +
>>>> +err_irq_remove:
>>>> +	stm32_adc_irq_remove(pdev, priv);
>>>> +
>>>> +err_clk_disable:
>>>> +	clk_disable_unprepare(priv->aclk);
>>>> +
>>>> +err_regulator_disable:
>>>> +	regulator_disable(priv->vref);
>>>> +
>>>> +	return ret;
>>>> +}
>>>> +
>>>> +static int stm32_adc_remove(struct platform_device *pdev)
>>>> +{
>>>> +	struct stm32_adc_common *common = platform_get_drvdata(pdev);
>>>> +	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
>>>> +
>>>> +	of_platform_depopulate(&pdev->dev);
>>>> +	stm32_adc_irq_remove(pdev, priv);
>>>> +	clk_disable_unprepare(priv->aclk);
>>>> +	regulator_disable(priv->vref);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static const struct of_device_id stm32_adc_of_match[] = {
>>>> +	{ .compatible = "st,stm32f4-adc-core" },
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, stm32_adc_of_match);
>>>> +
>>>> +static struct platform_driver stm32_adc_driver = {
>>>> +	.probe = stm32_adc_probe,
>>>> +	.remove = stm32_adc_remove,
>>>> +	.driver = {
>>>> +		.name = "stm32-adc-core",
>>>> +		.of_match_table = stm32_adc_of_match,
>>>> +	},
>>>> +};
>>>> +module_platform_driver(stm32_adc_driver);
>>>> +
>>>> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@st.com>");
>>>> +MODULE_DESCRIPTION("STMicroelectronics STM32 ADC MFD driver");
>>>> +MODULE_LICENSE("GPL v2");
>>>> +MODULE_ALIAS("platform:stm32-adc-core");
>>>> diff --git a/include/linux/mfd/stm32-adc-core.h
>b/include/linux/mfd/stm32-adc-core.h
>>>> new file mode 100644
>>>> index 0000000..081fa5f
>>>> --- /dev/null
>>>> +++ b/include/linux/mfd/stm32-adc-core.h
>>>> @@ -0,0 +1,52 @@
>>>> +/*
>>>> + * This file is part of STM32 ADC driver
>>>> + *
>>>> + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
>>>> + * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
>>>> + *
>>>> + * License type: GPLv2
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or
>modify it
>>>> + * under the terms of the GNU General Public License version 2 as
>published by
>>>> + * the Free Software Foundation.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>but
>>>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>MERCHANTABILITY
>>>> + * or FITNESS FOR A PARTICULAR PURPOSE.
>>>> + * See the GNU General Public License for more details.
>>>> + *
>>>> + * You should have received a copy of the GNU General Public
>License along with
>>>> + * this program. If not, see <http://www.gnu.org/licenses/>.
>>>> + */
>>>> +
>>>> +#ifndef __STM32_ADC_H
>>>> +#define __STM32_ADC_H
>>>> +
>>>> +/*
>>>> + * STM32 - ADC global register map
>>>> + * ________________________________________________________
>>>> + * | Offset |                 Register                    |
>>>> + * --------------------------------------------------------
>>>> + * | 0x000  |                Master ADC1                  |
>>>> + * --------------------------------------------------------
>>>> + * | 0x100  |                Slave ADC2                   |
>>>> + * --------------------------------------------------------
>>>> + * | 0x200  |                Slave ADC3                   |
>>>> + * --------------------------------------------------------
>>>> + * | 0x300  |         Master & Slave common regs          |
>>>> + * --------------------------------------------------------
>>>> + */
>>>> +#define STM32_ADC_MAX_ADCS		3
>>>> +#define STM32_ADCX_COMN_OFFSET		0x300
>>>> +
>>>> +/**
>>>> + * struct stm32_adc_common - stm32 ADC driver common data (for all
>instances)
>>>> + * @base:		control registers base cpu addr
>>>> + * @vref_mv:		vref voltage (mv)
>>>> + */
>>>> +struct stm32_adc_common {
>>>> +	void __iomem			*base;
>>>> +	int				vref_mv;
>>>> +};
>>>> +
>>>> +#endif
>>>>
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH v2 3/6] iio: adc: Add support for STM32 ADC

[Fabrice Gasnier]

On 11/12/2016 06:08 PM, Jonathan Cameron wrote:
> On 10/11/16 16:18, Fabrice Gasnier wrote:
>> This patch adds support for STMicroelectronics STM32 MCU's analog to
>> digital converter.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Nice and clean - few minor things inline.
>
> Jonathan
>> ---
>>   drivers/iio/adc/Kconfig     |  10 +
>>   drivers/iio/adc/Makefile    |   1 +
>>   drivers/iio/adc/stm32-adc.c | 525 ++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 536 insertions(+)
>>   create mode 100644 drivers/iio/adc/stm32-adc.c
>>
>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> index 7edcf32..61ba674 100644
>> --- a/drivers/iio/adc/Kconfig
>> +++ b/drivers/iio/adc/Kconfig
>> @@ -419,6 +419,16 @@ config ROCKCHIP_SARADC
>>   	  To compile this driver as a module, choose M here: the
>>   	  module will be called rockchip_saradc.
>>   
>> +config STM32_ADC
>> +	tristate "STMicroelectronics STM32 adc"
>> +	depends on MFD_STM32_ADC
>> +	help
>> +	  Say yes here to build support for STMicroelectronics stm32 Analog
>> +	  to Digital Converter (ADC).
>> +
>> +	  This driver can also be built as a module.  If so, the module
>> +	  will be called stm32-adc.
>> +
>>   config STX104
>>   	tristate "Apex Embedded Systems STX104 driver"
>>   	depends on X86 && ISA_BUS_API
>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>> index 7a40c04..df7a221 100644
>> --- a/drivers/iio/adc/Makefile
>> +++ b/drivers/iio/adc/Makefile
>> @@ -41,6 +41,7 @@ obj-$(CONFIG_QCOM_SPMI_IADC) += qcom-spmi-iadc.o
>>   obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
>>   obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
>>   obj-$(CONFIG_STX104) += stx104.o
>> +obj-$(CONFIG_STM32_ADC) += stm32-adc.o
>>   obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>>   obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
>>   obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
>> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
>> new file mode 100644
>> index 0000000..2be5fee
>> --- /dev/null
>> +++ b/drivers/iio/adc/stm32-adc.c
>> @@ -0,0 +1,525 @@
>> +/*
>> + * This file is part of STM32 ADC driver
>> + *
>> + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
>> + * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
>> + *
>> + * License type: GPLv2
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 as published by
>> + * the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
>> + * or FITNESS FOR A PARTICULAR PURPOSE.
>> + * See the GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/mfd/stm32-adc-core.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/of.h>
>> +
>> +/* STM32F4 - Registers for each ADC instance */
> Not really sure the X adds anything in these, but doesn't do much harm
> I guess ;)
You're right. I'll remove the X. I initially added that to remind this 
stands for ADC1, 2 or 3...
Moreover, removing it will make register name match with reference manual.
>> +#define STM32F4_ADCX_SR			0x00
>> +#define STM32F4_ADCX_CR1		0x04
>> +#define STM32F4_ADCX_CR2		0x08
>> +#define STM32F4_ADCX_SMPR1		0x0C
>> +#define STM32F4_ADCX_SMPR2		0x10
>> +#define STM32F4_ADCX_HTR		0x24
>> +#define STM32F4_ADCX_LTR		0x28
>> +#define STM32F4_ADCX_SQR1		0x2C
>> +#define STM32F4_ADCX_SQR2		0x30
>> +#define STM32F4_ADCX_SQR3		0x34
>> +#define STM32F4_ADCX_JSQR		0x38
>> +#define STM32F4_ADCX_JDR1		0x3C
>> +#define STM32F4_ADCX_JDR2		0x40
>> +#define STM32F4_ADCX_JDR3		0x44
>> +#define STM32F4_ADCX_JDR4		0x48
>> +#define STM32F4_ADCX_DR			0x4C
>> +
>> +/* STM32F4_ADCX_SR - bit fields */
>> +#define STM32F4_OVR			BIT(5)
>> +#define STM32F4_STRT			BIT(4)
>> +#define STM32F4_EOC			BIT(1)
>> +
>> +/* STM32F4_ADCX_CR1 - bit fields */
>> +#define STM32F4_OVRIE			BIT(26)
>> +#define STM32F4_SCAN			BIT(8)
>> +#define STM32F4_EOCIE			BIT(5)
>> +
>> +/* STM32F4_ADCX_CR2 - bit fields */
>> +#define STM32F4_SWSTART			BIT(30)
>> +#define STM32F4_EXTEN_MASK		GENMASK(29, 28)
>> +#define STM32F4_EOCS			BIT(10)
>> +#define STM32F4_ADON			BIT(0)
>> +
>> +/* STM32F4_ADCX_SQR1 - bit fields */
>> +#define STM32F4_L_SHIFT			20
>> +#define STM32F4_L_MASK			GENMASK(23, 20)
>> +
>> +/* STM32F4_ADCX_SQR3 - bit fields */
>> +#define STM32F4_SQ1_SHIFT		0
>> +#define STM32F4_SQ1_MASK		GENMASK(4, 0)
>> +
>> +#define STM32_ADC_MAX_SQ		16	/* SQ1..SQ16 */
>> +#define STM32_ADC_TIMEOUT_US		100000
>> +#define STM32_ADC_TIMEOUT	(msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
>> +
>> +/**
>> + * struct stm32_adc - private data of each ADC IIO instance
>> + * @common:		reference to ADC block common data
>> + * @offset:		ADC instance register offset in ADC block
>> + * @completion:		end of single conversion completion
>> + * @buffer:		data buffer
>> + * @clk:		optional adc clock, for this adc instance
>> + * @irq:		interrupt for this adc instance
>> + * @lock:		spinlock
>> + */
>> +struct stm32_adc {
>> +	struct stm32_adc_common	*common;
>> +	u32			offset;
>> +	struct completion	completion;
>> +	u16			*buffer;
>> +	struct clk		*clk;
>> +	int			irq;
>> +	spinlock_t		lock;		/* interrupt lock */
>> +};
>> +
>> +/**
>> + * struct stm32_adc_chan_spec - specification of stm32 adc channel
>> + * @type:	IIO channel type
>> + * @channel:	channel number (single ended)
>> + * @name:	channel name (single ended)
>> + */
>> +struct stm32_adc_chan_spec {
>> +	enum iio_chan_type	type;
>> +	int			channel;
>> +	const char		*name;
>> +};
>> +
>> +/* Input definitions common for all STM32F4 instances */
>> +static const struct stm32_adc_chan_spec stm32f4_adc123_channels[] = {
>> +	{ IIO_VOLTAGE, 0, "in0" },
>> +	{ IIO_VOLTAGE, 1, "in1" },
>> +	{ IIO_VOLTAGE, 2, "in2" },
>> +	{ IIO_VOLTAGE, 3, "in3" },
>> +	{ IIO_VOLTAGE, 4, "in4" },
>> +	{ IIO_VOLTAGE, 5, "in5" },
>> +	{ IIO_VOLTAGE, 6, "in6" },
>> +	{ IIO_VOLTAGE, 7, "in7" },
>> +	{ IIO_VOLTAGE, 8, "in8" },
>> +	{ IIO_VOLTAGE, 9, "in9" },
>> +	{ IIO_VOLTAGE, 10, "in10" },
>> +	{ IIO_VOLTAGE, 11, "in11" },
>> +	{ IIO_VOLTAGE, 12, "in12" },
>> +	{ IIO_VOLTAGE, 13, "in13" },
>> +	{ IIO_VOLTAGE, 14, "in14" },
>> +	{ IIO_VOLTAGE, 15, "in15" },
>> +};
>> +
>> +/**
>> + * STM32 ADC registers access routines
>> + * @adc: stm32 adc instance
>> + * @reg: reg offset in adc instance
>> + *
>> + * Note: All instances share same base, with 0x0, 0x100 or 0x200 offset resp.
>> + * for adc1, adc2 and adc3.
>> + */
>> +static u32 stm32_adc_readl(struct stm32_adc *adc, u32 reg)
>> +{
>> +	return readl_relaxed(adc->common->base + adc->offset + reg);
>> +}
>> +
>> +static u32 stm32_adc_readw(struct stm32_adc *adc, u32 reg)
>> +{
>> +	return readw_relaxed(adc->common->base + adc->offset + reg);
>> +}
>> +
>> +static void stm32_adc_writel(struct stm32_adc *adc, u32 reg, u32 val)
>> +{
>> +	writel_relaxed(val, adc->common->base + adc->offset + reg);
>> +}
>> +
>> +static void stm32_adc_set_bits(struct stm32_adc *adc, u32 reg, u32 bits)
>> +{
>> +	unsigned long flags;
>> +
>> +	spin_lock_irqsave(&adc->lock, flags);
>> +	stm32_adc_writel(adc, reg, stm32_adc_readl(adc, reg) | bits);
>> +	spin_unlock_irqrestore(&adc->lock, flags);
>> +}
>> +
>> +static void stm32_adc_clr_bits(struct stm32_adc *adc, u32 reg, u32 bits)
>> +{
>> +	unsigned long flags;
>> +
>> +	spin_lock_irqsave(&adc->lock, flags);
>> +	stm32_adc_writel(adc, reg, stm32_adc_readl(adc, reg) & ~bits);
>> +	spin_unlock_irqrestore(&adc->lock, flags);
>> +}
>> +
>> +/**
>> + * stm32_adc_conv_irq_enable() - Enable end of conversion interrupt
>> + * @adc: stm32 adc instance
>> + */
>> +static void stm32_adc_conv_irq_enable(struct stm32_adc *adc)
>> +{
>> +	stm32_adc_set_bits(adc, STM32F4_ADCX_CR1, STM32F4_EOCIE);
>> +};
>> +
>> +/**
>> + * stm32_adc_conv_irq_disable() - Disable end of conversion interrupt
>> + * @adc: stm32 adc instance
>> + */
>> +static void stm32_adc_conv_irq_disable(struct stm32_adc *adc)
>> +{
>> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_CR1, STM32F4_EOCIE);
>> +}
>> +
>> +/**
>> + * stm32_adc_start_conv() - Start conversions for regular channels.
>> + * @adc: stm32 adc instance
>> + */
>> +static void stm32_adc_start_conv(struct stm32_adc *adc)
>> +{
>> +	stm32_adc_set_bits(adc, STM32F4_ADCX_CR1, STM32F4_SCAN);
>> +	stm32_adc_set_bits(adc, STM32F4_ADCX_CR2, STM32F4_EOCS | STM32F4_ADON);
>> +
>> +	/* Wait for Power-up time (tSTAB from datasheet) */
>> +	usleep_range(2, 3);
>> +
>> +	/* Software start ? (e.g. trigger detection disabled ?) */
>> +	if (!(stm32_adc_readl(adc, STM32F4_ADCX_CR2) & STM32F4_EXTEN_MASK))
>> +		stm32_adc_set_bits(adc, STM32F4_ADCX_CR2, STM32F4_SWSTART);
>> +}
>> +
>> +static void stm32_adc_stop_conv(struct stm32_adc *adc)
>> +{
>> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_EXTEN_MASK);
>> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_SR, STM32F4_STRT);
>> +
>> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_CR1, STM32F4_SCAN);
>> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_ADON);
>> +}
>> +
>> +/**
>> + * stm32_adc_single_conv() - Performs a single conversion
>> + * @indio_dev: IIO device
>> + * @chan: IIO channel
>> + * @res: conversion result
>> + *
>> + * The function performs a single conversion on a given channel:
>> + * - Program sequencer with one channel (e.g. in SQ1 with len = 1)
>> + * - Use SW trigger
>> + * - Start conversion, then wait for interrupt completion.
>> + */
>> +static int stm32_adc_single_conv(struct iio_dev *indio_dev,
>> +				 const struct iio_chan_spec *chan,
>> +				 int *res)
>> +{
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +	long timeout;
>> +	u32 val;
>> +	u16 result;
>> +	int ret;
>> +
>> +	reinit_completion(&adc->completion);
>> +
>> +	adc->buffer = &result;
>> +
>> +	/* Program chan number in regular sequence */
>> +	val = stm32_adc_readl(adc, STM32F4_ADCX_SQR3);
>> +	val &= ~STM32F4_SQ1_MASK;
>> +	val |= chan->channel << STM32F4_SQ1_SHIFT;
>> +	stm32_adc_writel(adc, STM32F4_ADCX_SQR3, val);
>> +
>> +	/* Set regular sequence len (0 for 1 conversion) */
>> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_SQR1, STM32F4_L_MASK);
>> +
>> +	/* Trigger detection disabled (conversion can be launched in SW) */
>> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_EXTEN_MASK);
>> +
>> +	stm32_adc_conv_irq_enable(adc);
>> +
>> +	stm32_adc_start_conv(adc);
>> +
>> +	timeout = wait_for_completion_interruptible_timeout(
>> +					&adc->completion, STM32_ADC_TIMEOUT);
>> +	if (timeout == 0) {
>> +		dev_warn(&indio_dev->dev, "Conversion timed out!\n");
>> +		ret = -ETIMEDOUT;
>> +	} else if (timeout < 0) {
>> +		dev_warn(&indio_dev->dev, "Interrupted conversion!\n");
>> +		ret = -EINTR;
>> +	} else {
>> +		*res = result;
>> +		ret = IIO_VAL_INT;
>> +	}
>> +
>> +	stm32_adc_stop_conv(adc);
>> +
>> +	stm32_adc_conv_irq_disable(adc);
>> +
>> +	return ret;
>> +}
>> +
>> +static int stm32_adc_read_raw(struct iio_dev *indio_dev,
>> +			      struct iio_chan_spec const *chan,
>> +			      int *val, int *val2, long mask)
>> +{
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +	int ret = -EINVAL;
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_RAW:
>> +		ret = iio_device_claim_direct_mode(indio_dev);
>> +		if (ret)
>> +			return ret;
>> +		if (chan->type == IIO_VOLTAGE)
>> +			ret = stm32_adc_single_conv(indio_dev, chan, val);
>> +		else
>> +			ret = -EINVAL;
>> +		iio_device_release_direct_mode(indio_dev);
> return directly here.  Basically always preferred to return directly if
> there is not cleanup to be done.
I will fix it.
>> +		break;
>> +	case IIO_CHAN_INFO_SCALE:
>> +		*val = adc->common->vref_mv;
>> +		*val2 = chan->scan_type.realbits;
>> +		ret = IIO_VAL_FRACTIONAL_LOG2;
> return directly here.
I will fix it.
>> +		break;
>> +	default:
> return -EINVAL here.
I will fix it.
>> +		break;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static irqreturn_t stm32_adc_isr(int irq, void *data)
>> +{
>> +	struct stm32_adc *adc = data;
>> +	u32 status = stm32_adc_readl(adc, STM32F4_ADCX_SR);
>> +	irqreturn_t ret = IRQ_NONE;
>> +
>> +	if (status & STM32F4_EOC) {
>> +		*adc->buffer = stm32_adc_readw(adc, STM32F4_ADCX_DR);
>> +		complete(&adc->completion);
>> +		ret = IRQ_HANDLED;
> Slightly tidier to return IRQ_HANDLED here and directly return
> IRQ_NONE below.
I will fix it.
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int stm32_adc_of_xlate(struct iio_dev *indio_dev,
>> +			      const struct of_phandle_args *iiospec)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < indio_dev->num_channels; i++)
>> +		if (indio_dev->channels[i].channel == iiospec->args[0])
>> +			return i;
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +/**
>> + * stm32_adc_debugfs_reg_access - read or write register value
>> + *
>> + * To read a value from an ADC register:
>> + *   echo [ADC reg offset] > direct_reg_access
>> + *   cat direct_reg_access
>> + *
>> + * To write a value in a ADC register:
>> + *   echo [ADC_reg_offset] [value] > direct_reg_access
>> + */
>> +static int stm32_adc_debugfs_reg_access(struct iio_dev *indio_dev,
>> +					unsigned reg, unsigned writeval,
>> +					unsigned *readval)
>> +{
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +
>> +	if (!readval)
>> +		stm32_adc_writel(adc, reg, writeval);
>> +	else
>> +		*readval = stm32_adc_readl(adc, reg);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct iio_info stm32_adc_iio_info = {
>> +	.read_raw = stm32_adc_read_raw,
>> +	.debugfs_reg_access = stm32_adc_debugfs_reg_access,
>> +	.of_xlate = stm32_adc_of_xlate,
>> +	.driver_module = THIS_MODULE,
>> +};
>> +
>> +static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>> +				    struct iio_chan_spec *chan,
>> +				    const struct stm32_adc_chan_spec *channel,
>> +				    int scan_index)
>> +{
>> +	chan->type = channel->type;
>> +	chan->channel = channel->channel;
>> +	chan->datasheet_name = channel->name;
>> +	chan->extend_name = channel->name;
> Don't set extend_name. That name doesn't add sufficient information to
> make it worth adding custom ABI to the userspace interface.
I will fix it.
>
>
>> +	chan->scan_index = scan_index;
>> +	chan->indexed = 1;
>> +	chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
>> +	chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE);
>> +	chan->scan_type.sign = 'u';
>> +	chan->scan_type.realbits = 12;
>> +	chan->scan_type.storagebits = 16;
>> +}
>> +
>> +static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>> +{
>> +	struct device_node *node = indio_dev->dev.of_node;
>> +	struct property *prop;
>> +	const __be32 *cur;
>> +	struct iio_chan_spec *channels;
>> +	int scan_index = 0, num_channels;
>> +	u32 val;
>> +
>> +	num_channels = of_property_count_u32_elems(node, "st,adc-channels");
>> +	if (num_channels < 0 ||
>> +	    num_channels >= ARRAY_SIZE(stm32f4_adc123_channels)) {
>> +		dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");
>> +		return num_channels < 0 ? num_channels : -EINVAL;
>> +	}
>> +
>> +	channels = devm_kcalloc(&indio_dev->dev, num_channels,
>> +				sizeof(struct iio_chan_spec), GFP_KERNEL);
>> +	if (!channels)
>> +		return -ENOMEM;
>> +
>> +	of_property_for_each_u32(node, "st,adc-channels", prop, cur, val) {
>> +		if (val >= ARRAY_SIZE(stm32f4_adc123_channels)) {
>> +			dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
>> +			return -EINVAL;
>> +		}
>> +		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
>> +					&stm32f4_adc123_channels[val],
>> +					scan_index);
>> +		scan_index++;
>> +	}
>> +
>> +	indio_dev->num_channels = scan_index;
>> +	indio_dev->channels = channels;
>> +
>> +	return 0;
>> +}
>> +
>> +static int stm32_adc_probe(struct platform_device *pdev)
>> +{
>> +	struct iio_dev *indio_dev;
>> +	struct stm32_adc *adc;
>> +	int ret;
>> +
>> +	if (!pdev->dev.of_node)
>> +		return -ENODEV;
>> +
>> +	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*adc));
>> +	if (!indio_dev)
>> +		return -ENOMEM;
>> +
>> +	adc = iio_priv(indio_dev);
>> +	adc->common = dev_get_drvdata(pdev->dev.parent);
>> +	spin_lock_init(&adc->lock);
>> +	init_completion(&adc->completion);
>> +
>> +	indio_dev->name = dev_name(&pdev->dev);
>> +	indio_dev->dev.parent = &pdev->dev;
>> +	indio_dev->dev.of_node = pdev->dev.of_node;
>> +	indio_dev->info = &stm32_adc_iio_info;
>> +	indio_dev->modes = INDIO_DIRECT_MODE;
>> +
>> +	platform_set_drvdata(pdev, adc);
>> +
>> +	ret = of_property_read_u32(pdev->dev.of_node, "reg", &adc->offset);
>> +	if (ret != 0) {
>> +		dev_err(&pdev->dev, "missing reg property\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	adc->irq = platform_get_irq(pdev, 0);
>> +	if (adc->irq < 0) {
>> +		dev_err(&pdev->dev, "failed to get irq\n");
>> +		return adc->irq;
>> +	}
>> +
>> +	ret = devm_request_irq(&pdev->dev, adc->irq, stm32_adc_isr,
>> +			       0, pdev->name, adc);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "failed to request IRQ\n");
>> +		return ret;
>> +	}
>> +
>> +	adc->clk = devm_clk_get(&pdev->dev, NULL);
>> +	if (IS_ERR(adc->clk)) {
> Could it concievably be deferred?  Would be happier if this explicitly
> checked for -ENODEV or whatever gets returned when not clock has
> been specified.
I will fix it.
>> +		adc->clk = NULL;
>> +		dev_dbg(&pdev->dev, "No child clk found\n");
>> +	} else {
>> +		ret = clk_prepare_enable(adc->clk);
>> +		if (ret < 0) {
>> +			dev_err(&pdev->dev, "clk enable failed\n");
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	ret = stm32_adc_chan_of_init(indio_dev);
>> +	if (ret < 0)
>> +		goto err_clk_disable;
>> +
>> +	ret = devm_iio_device_register(&pdev->dev, indio_dev);
> This use of devm registration is going to cause a race in the remove.
> The userspace interface will not be removed until after the remove
> function has run.  That disables the clock thus leaving us a window
> where we could try and access the device with no clock enabled.
>
> Basic rule of thumb is that use of devm must not effect the ordering
> of unrolling what you do in probe when it comes to remove.
> (which more or less means that you can't use devm_iio_device_register
> unless you have no remove at all).
I will fix it.

Thanks,
Fabrice
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "iio dev register failed\n");
>> +		goto err_clk_disable;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_clk_disable:
>> +	if (adc->clk)
>> +		clk_disable_unprepare(adc->clk);
>> +
>> +	return ret;
>> +}
>> +
>> +static int stm32_adc_remove(struct platform_device *pdev)
>> +{
>> +	struct stm32_adc *adc = platform_get_drvdata(pdev);
>> +
>> +	if (adc->clk)
>> +		clk_disable_unprepare(adc->clk);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id stm32_adc_of_match[] = {
>> +	{ .compatible = "st,stm32f4-adc" },
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, stm32_adc_of_match);
>> +
>> +static struct platform_driver stm32_adc_driver = {
>> +	.probe = stm32_adc_probe,
>> +	.remove = stm32_adc_remove,
>> +	.driver = {
>> +		.name = "stm32-adc",
>> +		.of_match_table = stm32_adc_of_match,
>> +	},
>> +};
>> +module_platform_driver(stm32_adc_driver);
>> +
>> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@st.com>");
>> +MODULE_DESCRIPTION("STMicroelectronics STM32 ADC IIO driver");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_ALIAS("platform:stm32-adc");
>>

Re: [PATCH v2 3/6] iio: adc: Add support for STM32 ADC

[Fabrice Gasnier]

On 11/14/2016 01:11 PM, Lars-Peter Clausen wrote:
> On 11/10/2016 05:18 PM, Fabrice Gasnier wrote:
> [...]
>> + static int stm32_adc_single_conv(struct iio_dev *indio_dev,
>> +				 const struct iio_chan_spec *chan,
>> +				 int *res)
>> +{
>> +	struct stm32_adc *adc = iio_priv(indio_dev);
>> +	long timeout;
>> +	u32 val;
>> +	u16 result;
>> +	int ret;
>> +
>> +	reinit_completion(&adc->completion);
>> +
>> +	adc->buffer = &result;
>> +
>> +	/* Program chan number in regular sequence */
>> +	val = stm32_adc_readl(adc, STM32F4_ADCX_SQR3);
>> +	val &= ~STM32F4_SQ1_MASK;
>> +	val |= chan->channel << STM32F4_SQ1_SHIFT;
>> +	stm32_adc_writel(adc, STM32F4_ADCX_SQR3, val);
>> +
>> +	/* Set regular sequence len (0 for 1 conversion) */
>> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_SQR1, STM32F4_L_MASK);
>> +
>> +	/* Trigger detection disabled (conversion can be launched in SW) */
>> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_EXTEN_MASK);
>> +
>> +	stm32_adc_conv_irq_enable(adc);
>> +
>> +	stm32_adc_start_conv(adc);
>> +
>> +	timeout = wait_for_completion_interruptible_timeout(
>> +					&adc->completion, STM32_ADC_TIMEOUT);
>> +	if (timeout == 0) {
>> +		dev_warn(&indio_dev->dev, "Conversion timed out!\n");
> This should be dev_dbg() at most. This out of band reporting is not
> particular useful for applications as it is impossible to match the error to
> the action that triggered it. And you also report the error through the
> error code, so the applications knows what is going on.
>
>> +		ret = -ETIMEDOUT;
>> +	} else if (timeout < 0) {
>> +		dev_warn(&indio_dev->dev, "Interrupted conversion!\n");
>> +		ret = -EINTR;
> This should just propagate the error returned by wait_for_completion...().
> This will make sure that the right behavior occurs based on the SA_RESTART
> policy.
Hi Lars,

Thanks for reviewing.
I'll update this in next revision.

Regards,
Fabrice

>
>> +	} else {
>> +		*res = result;
>> +		ret = IIO_VAL_INT;
>> +	}
>> +
>> +	stm32_adc_stop_conv(adc);
>> +
>> +	stm32_adc_conv_irq_disable(adc);
>> +
>> +	return ret;