[PATCH v5 0/3] *** iio:adc:imx: Add Freescale vf610 ADC driver support ***

[PATCH v5 0/3] *** iio:adc:imx: Add Freescale vf610 ADC driver support ***

[Fugang Duan]

The patch set is to enable Vybrid VF610 ADC driver.

Please see the Reference Manual of the ADC:
http://cache.freescale.com/files/32bit/doc/ref_manual/VYBRIDRM.pdf?fpsp=1&WT_TYPE=
Reference%20Manuals&WT_VENDOR=FREESCALE&WT_FILE_FORMAT=pdf&WT_ASSET=Documentation

*** Change Log***
* V5:
- Suggested by Jonathan to simply the driver tp supply generalized userspace ABIs.
- Do not use the devm_iio_device_register here as there have stuff to do in the remove.

* V4: (reviewed by:
        Shawn Guo <shawn.guo@linaro.org>
        Jonathan Cameron <jic23@kernel.org>
        Mark Rutland <mark.rutland@arm.com>
        Otavio Salvador <otavio@ossystems.com.br>
        Peter Meerwald <pmeerw@pmeerw.net>
        Lars-Peter Clausen <lars@metafoo.de>)
- Describe ADC refrence voltage with fixed regulator.
- Remove ADC configuration from DT, let them change at runtime.
- Add "VF610_" prefix for ADC register define.
- Update devicetree binding Documentation.

* V3: (review by Shawn Guo)
- DTS properties: use prefix "fsl," instead of "vf610".
- Drop DTS unnessary comments, and add "vref" optional propert comments.
- Modify driver format.
- Remove driver debug code.
- Remove the of_device_id .data init.
- Remove unnecessary device node check.
- Add clk_prepare_enable() return check.

* V2: (Simply review by Jonathan Cameron)
- Rename the driver after "vf610_adc".

* V1:
Feature:
- Enable Vybrid vf610 board ADC0 in dts file.
- ADC driver only support software trigger for the init version.
- ADC configuration support dts set.
Note: Since the ADC IP is used for Freescale Vybrid VF610, i.MX6SLX, i.MX7 serial sillicons.

Fugang Duan (3):
  ARM: dts: vf610-twr: Add ADC support
  iio:adc:imx: add Freescale Vybrid vf610 adc driver
  Documentation: add the binding file for Freescale vf610 ADC driver

 .../devicetree/bindings/iio/adc/vf610-adc.txt      |   22 +
 arch/arm/boot/dts/vf610-twr.dts                    |   21 +
 arch/arm/boot/dts/vf610.dtsi                       |   26 +
 drivers/iio/adc/Kconfig                            |    9 +
 drivers/iio/adc/Makefile                           |    1 +
 drivers/iio/adc/vf610_adc.c                        |  709 ++++++++++++++++++++
 6 files changed, 788 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
 create mode 100644 drivers/iio/adc/vf610_adc.c

-- 
1.7.2.rc3

[PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[Fugang Duan]

vf610 has two ADC controllers, and vf610-twr board ADC0_SE5 pin connect
to sliding rheostat for ADC test, other ADC pins connect to connectors for
future use.

Add support for ADC0_SE5.

CC: Shawn Guo <shawn.guo@linaro.org>
CC: Jonathan Cameron <jic23@kernel.org>
CC: Mark Rutland <mark.rutland@arm.com>
CC: Otavio Salvador <otavio@ossystems.com.br>
CC: Peter Meerwald <pmeerw@pmeerw.net>
CC: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
 arch/arm/boot/dts/vf610-twr.dts |   21 +++++++++++++++++++++
 arch/arm/boot/dts/vf610.dtsi    |   26 ++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
index c8047ca..d867be3 100644
--- a/arch/arm/boot/dts/vf610-twr.dts
+++ b/arch/arm/boot/dts/vf610-twr.dts
@@ -34,6 +34,27 @@
 		};
 	};
 
+	regulators {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		reg_vcc_3v3_mcu: regulator@0 {
+			compatible = "regulator-fixed";
+			reg = <0>;
+			regulator-name = "vcc_3v3_mcu";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+		};
+	};
+
+};
+
+&adc0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_adc0_ad5>;
+	vref-supply = <&reg_vcc_3v3_mcu>;
+	status = "okay";
 };
 
 &dspi0 {
diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
index d31ce1b..b5b21ea 100644
--- a/arch/arm/boot/dts/vf610.dtsi
+++ b/arch/arm/boot/dts/vf610.dtsi
@@ -152,6 +152,15 @@
 				clock-names = "pit";
 			};
 
+			adc0: adc@4003b000 {
+				compatible = "fsl,vf610-adc";
+				reg = <0x4003b000 0x1000>;
+				interrupts = <0 53 0x04>;
+				clocks = <&clks VF610_CLK_ADC0>;
+				clock-names = "adc";
+				status = "disabled";
+			};
+
 			wdog@4003e000 {
 				compatible = "fsl,vf610-wdt", "fsl,imx21-wdt";
 				reg = <0x4003e000 0x1000>;
@@ -178,6 +187,14 @@
 
 				/* functions and groups pins */
 
+				adc0 {
+					pinctrl_adc0_ad5: adc0_ad5 {
+						fsl,pins = <
+						VF610_PAD_PTC30__ADC0_SE5	0xa1
+						>;
+					};
+				};
+
 				dcu0 {
 					pinctrl_dcu0_1: dcu0grp_1 {
 						fsl,pins = <
@@ -450,6 +467,15 @@
 				status = "disabled";
 			};
 
+			adc1: adc@400bb000 {
+				compatible = "fsl,vf610-adc";
+				reg = <0x400bb000 0x1000>;
+				interrupts = <0 54 0x04>;
+				clocks = <&clks VF610_CLK_ADC1>;
+				clock-names = "adc";
+				status = "disabled";
+			};
+
 			fec0: ethernet@400d0000 {
 				compatible = "fsl,mvf600-fec";
 				reg = <0x400d0000 0x1000>;
-- 
1.7.2.rc3

[PATCH v5 2/3] iio:adc:imx: add Freescale Vybrid vf610 adc driver

[Fugang Duan]

Add Freescale Vybrid vf610 adc driver. The driver only support
ADC software trigger.

VF610 ADC device documentation is available at below reference manual
(chapter 37):
http://cache.freescale.com/files/32bit/doc/ref_manual/VYBRIDRM.pdf?
fpsp=1&WT_TYPE=Reference%20Manuals&WT_VENDOR=FREESCALE&WT_FILE_FORMAT
=pdf&WT_ASSET=Documentation

CC: Shawn Guo <shawn.guo@linaro.org>
CC: Jonathan Cameron <jic23@kernel.org>
CC: Mark Rutland <mark.rutland@arm.com>
CC: Otavio Salvador <otavio@ossystems.com.br>
CC: Peter Meerwald <pmeerw@pmeerw.net>
CC: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
 drivers/iio/adc/Kconfig     |    9 +
 drivers/iio/adc/Makefile    |    1 +
 drivers/iio/adc/vf610_adc.c |  709 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 719 insertions(+), 0 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 2209f28..3677165 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -197,6 +197,15 @@ config TWL6030_GPADC
 	  This driver can also be built as a module. If so, the module will be
 	  called twl6030-gpadc.
 
+config VF610_ADC
+	tristate "Freescale vf610 ADC driver"
+	help
+	  Say yes here to support for Vybrid board analog-to-digital converter.
+	  Since the IP is used for i.MX6SLX, the driver also support i.MX6SLX.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called vf610_adc.
+
 config VIPERBOARD_ADC
 	tristate "Viperboard ADC support"
 	depends on MFD_VIPERBOARD && USB
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index ba9a10a..6d96b0f 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -21,4 +21,5 @@ obj-$(CONFIG_NAU7802) += nau7802.o
 obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
 obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
 obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
+obj-$(CONFIG_VF610_ADC) += vf610_adc.o
 obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
new file mode 100644
index 0000000..84c7aac
--- /dev/null
+++ b/drivers/iio/adc/vf610_adc.c
@@ -0,0 +1,709 @@
+/*
+ * Freescale Vybrid vf610 ADC driver
+ *
+ * Copyright 2013 Freescale Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/regulator/consumer.h>
+#include <linux/of_platform.h>
+#include <linux/err.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/driver.h>
+
+/* This will be the driver name the kernel reports */
+#define DRIVER_NAME "vf610-adc"
+
+/* Vybrid/IMX ADC registers */
+#define VF610_REG_ADC_HC0		0x00
+#define VF610_REG_ADC_HC1		0x04
+#define VF610_REG_ADC_HS		0x08
+#define VF610_REG_ADC_R0		0x0c
+#define VF610_REG_ADC_R1		0x10
+#define VF610_REG_ADC_CFG		0x14
+#define VF610_REG_ADC_GC		0x18
+#define VF610_REG_ADC_GS		0x1c
+#define VF610_REG_ADC_CV		0x20
+#define VF610_REG_ADC_OFS		0x24
+#define VF610_REG_ADC_CAL		0x28
+#define VF610_REG_ADC_PCTL		0x30
+
+/* Configuration register field define */
+#define VF610_ADC_MODE_BIT8		0x00
+#define VF610_ADC_MODE_BIT10		0x04
+#define VF610_ADC_MODE_BIT12		0x08
+#define VF610_ADC_MODE_MASK		0x0c
+#define VF610_ADC_BUSCLK2_SEL		0x01
+#define VF610_ADC_ALTCLK_SEL		0x02
+#define VF610_ADC_ADACK_SEL		0x03
+#define VF610_ADC_ADCCLK_MASK		0x03
+#define VF610_ADC_CLK_DIV2		0x20
+#define VF610_ADC_CLK_DIV4		0x40
+#define VF610_ADC_CLK_DIV8		0x60
+#define VF610_ADC_CLK_MASK		0x60
+#define VF610_ADC_ADLSMP_LONG		0x10
+#define VF610_ADC_ADSTS_MASK		0x300
+#define VF610_ADC_ADLPC_EN		0x80
+#define VF610_ADC_ADHSC_EN		0x400
+#define VF610_ADC_REFSEL_VALT		0x100
+#define VF610_ADC_REFSEL_VBG		0x1000
+#define VF610_ADC_ADTRG_HARD		0x2000
+#define VF610_ADC_AVGS_8		0x4000
+#define VF610_ADC_AVGS_16		0x8000
+#define VF610_ADC_AVGS_32		0xC000
+#define VF610_ADC_AVGS_MASK		0xC000
+#define VF610_ADC_OVWREN		0x10000
+
+/* General control register field define */
+#define VF610_ADC_ADACKEN		0x1
+#define VF610_ADC_DMAEN			0x2
+#define VF610_ADC_ACREN			0x4
+#define VF610_ADC_ACFGT			0x8
+#define VF610_ADC_ACFE			0x10
+#define VF610_ADC_AVGEN			0x20
+#define VF610_ADC_ADCON			0x40
+#define VF610_ADC_CAL			0x80
+
+/* Other field define */
+#define VF610_ADC_ADCHC(x)		((x) & 0xF)
+#define VF610_ADC_AIEN			(0x1 << 7)
+#define VF610_ADC_CONV_DISABLE		0x1F
+#define VF610_ADC_HS_COCO0		0x1
+#define VF610_ADC_CALF			0x2
+#define VF610_ADC_TIMEOUT		msecs_to_jiffies(100)
+
+enum clk_sel {
+	VF610_ADCIOC_BUSCLK_SET,
+	VF610_ADCIOC_ALTCLK_SET,
+	VF610_ADCIOC_ADACK_SET,
+};
+
+enum vol_ref {
+	VF610_ADCIOC_VR_VREF_SET,
+	VF610_ADCIOC_VR_VALT_SET,
+	VF610_ADCIOC_VR_VBG_SET,
+};
+
+enum average_sel {
+	VF610_ADC_SAMPLE_1,
+	VF610_ADC_SAMPLE_4,
+	VF610_ADC_SAMPLE_8,
+	VF610_ADC_SAMPLE_16,
+	VF610_ADC_SAMPLE_32,
+};
+
+struct vf610_adc_feature {
+	enum clk_sel	clk_sel;
+	enum vol_ref	vol_ref;
+
+	int	clk_div;
+	int     sample_rate;
+	int	res_mode;
+
+	bool	lpm;
+	bool	calibration;
+	bool	ovwren;
+};
+
+struct vf610_adc {
+	struct device *dev;
+	void __iomem *regs;
+	struct clk *clk;
+
+	u32 vref_uv;
+	u32 value;
+	struct regulator *vref;
+	struct vf610_adc_feature adc_feature;
+
+	struct completion completion;
+};
+
+#define VF610_ADC_CHAN(_idx, _chan_type) {			\
+	.type = (_chan_type),					\
+	.indexed = 1,						\
+	.channel = (_idx),					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
+				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+}
+
+static const struct iio_chan_spec vf610_adc_iio_channels[] = {
+	VF610_ADC_CHAN(0, IIO_VOLTAGE),
+	VF610_ADC_CHAN(1, IIO_VOLTAGE),
+	VF610_ADC_CHAN(2, IIO_VOLTAGE),
+	VF610_ADC_CHAN(3, IIO_VOLTAGE),
+	VF610_ADC_CHAN(4, IIO_VOLTAGE),
+	VF610_ADC_CHAN(5, IIO_VOLTAGE),
+	VF610_ADC_CHAN(6, IIO_VOLTAGE),
+	VF610_ADC_CHAN(7, IIO_VOLTAGE),
+	VF610_ADC_CHAN(8, IIO_VOLTAGE),
+	VF610_ADC_CHAN(9, IIO_VOLTAGE),
+	VF610_ADC_CHAN(10, IIO_VOLTAGE),
+	VF610_ADC_CHAN(11, IIO_VOLTAGE),
+	VF610_ADC_CHAN(12, IIO_VOLTAGE),
+	VF610_ADC_CHAN(13, IIO_VOLTAGE),
+	VF610_ADC_CHAN(14, IIO_VOLTAGE),
+	VF610_ADC_CHAN(15, IIO_VOLTAGE),
+	/* sentinel */
+};
+
+/*
+ * ADC sample frequency, unit is ADCK cycles.
+ * ADC clk source is ipg clock, which is the same as bus clock.
+ *
+ * ADC conversion time = SFCAdder + AverageNum x (BCT + LSTAdder)
+ * SFCAdder: fixed to 6 ADCK cycles
+ * AverageNum: 1, 4, 8, 16, 32 samples for hardware average.
+ * BCT (Base Conversion Time): fixed to 25 ADCK cycles for 12 bit mode
+ * LSTAdder(Long Sample Time): fixed to 3 ADCK cycles
+ *
+ * By default, enable 12 bit resolution mode, clock source
+ * set to ipg clock, So get below frequency group:
+ */
+static const u32 vf610_sample_freq_avail[5] =
+{1941176, 559332, 286957, 145374, 73171};
+
+static inline void vf610_adc_cfg_init(struct vf610_adc *info)
+{
+	/* set default Configuration for ADC controller */
+	info->adc_feature.clk_sel = VF610_ADCIOC_BUSCLK_SET;
+	info->adc_feature.vol_ref = VF610_ADCIOC_VR_VREF_SET;
+
+	info->adc_feature.calibration = true;
+	info->adc_feature.ovwren = true;
+
+	info->adc_feature.clk_div = 1;
+	info->adc_feature.res_mode = 12;
+	info->adc_feature.sample_rate = 1;
+	info->adc_feature.lpm = true;
+}
+
+static void vf610_adc_cfg_post_set(struct vf610_adc *info)
+{
+	struct vf610_adc_feature *adc_feature = &info->adc_feature;
+	int cfg_data = 0;
+	int gc_data = 0;
+
+	switch (adc_feature->clk_sel) {
+	case VF610_ADCIOC_ALTCLK_SET:
+		cfg_data |= VF610_ADC_ALTCLK_SEL;
+		break;
+	case VF610_ADCIOC_ADACK_SET:
+		cfg_data |= VF610_ADC_ADACK_SEL;
+		break;
+	default:
+		break;
+	}
+
+	/* low power set for calibration */
+	cfg_data |= VF610_ADC_ADLPC_EN;
+
+	/* enable high speed for calibration */
+	cfg_data |= VF610_ADC_ADHSC_EN;
+
+	/* voltage reference */
+	switch (adc_feature->vol_ref) {
+	case VF610_ADCIOC_VR_VREF_SET:
+		break;
+	case VF610_ADCIOC_VR_VALT_SET:
+		cfg_data |= VF610_ADC_REFSEL_VALT;
+		break;
+	case VF610_ADCIOC_VR_VBG_SET:
+		cfg_data |= VF610_ADC_REFSEL_VBG;
+		break;
+	default:
+		dev_err(info->dev, "error voltage reference\n");
+	}
+
+	/* data overwrite enable */
+	if (adc_feature->ovwren)
+		cfg_data |= VF610_ADC_OVWREN;
+
+	writel(cfg_data, info->regs + VF610_REG_ADC_CFG);
+	writel(gc_data, info->regs + VF610_REG_ADC_GC);
+}
+
+static void vf610_adc_calibration(struct vf610_adc *info)
+{
+	int adc_gc, hc_cfg;
+	int timeout;
+
+	if (!info->adc_feature.calibration)
+		return;
+
+	/* enable calibration interrupt */
+	hc_cfg = VF610_ADC_AIEN | VF610_ADC_CONV_DISABLE;
+	writel(hc_cfg, info->regs + VF610_REG_ADC_HC0);
+
+	adc_gc = readl(info->regs + VF610_REG_ADC_GC);
+	writel(adc_gc | VF610_ADC_CAL, info->regs + VF610_REG_ADC_GC);
+
+	timeout = wait_for_completion_timeout
+			(&info->completion, VF610_ADC_TIMEOUT);
+	if (timeout == 0)
+		dev_err(info->dev, "Timeout for adc calibration\n");
+
+	adc_gc = readl(info->regs + VF610_REG_ADC_GS);
+	if (adc_gc & VF610_ADC_CALF)
+		dev_err(info->dev, "ADC calibration failed\n");
+
+	info->adc_feature.calibration = false;
+}
+
+static void vf610_adc_cfg_set(struct vf610_adc *info)
+{
+	struct vf610_adc_feature *adc_feature = &(info->adc_feature);
+	int cfg_data;
+
+	cfg_data = readl(info->regs + VF610_REG_ADC_CFG);
+
+	/* low power configuration */
+	cfg_data &= ~VF610_ADC_ADLPC_EN;
+	if (adc_feature->lpm)
+		cfg_data |= VF610_ADC_ADLPC_EN;
+
+	/* disable high speed */
+	cfg_data &= ~VF610_ADC_ADHSC_EN;
+
+	writel(cfg_data, info->regs + VF610_REG_ADC_CFG);
+}
+
+static void vf610_adc_sample_set(struct vf610_adc *info)
+{
+	struct vf610_adc_feature *adc_feature = &(info->adc_feature);
+	int cfg_data, gc_data;
+
+	cfg_data = readl(info->regs + VF610_REG_ADC_CFG);
+	gc_data = readl(info->regs + VF610_REG_ADC_GC);
+
+	/* resolution mode */
+	cfg_data &= ~VF610_ADC_MODE_MASK;
+	switch (adc_feature->res_mode) {
+	case 8:
+		cfg_data |= VF610_ADC_MODE_BIT8;
+		break;
+	case 10:
+		cfg_data |= VF610_ADC_MODE_BIT10;
+		break;
+	case 12:
+		cfg_data |= VF610_ADC_MODE_BIT12;
+		break;
+	default:
+		dev_err(info->dev, "error resolution mode\n");
+		break;
+	}
+
+	/* clock select and clock divider */
+	cfg_data &= ~(VF610_ADC_CLK_MASK | VF610_ADC_ADCCLK_MASK);
+	switch (adc_feature->clk_div) {
+	case 1:
+		break;
+	case 2:
+		cfg_data |= VF610_ADC_CLK_DIV2;
+		break;
+	case 4:
+		cfg_data |= VF610_ADC_CLK_DIV4;
+		break;
+	case 8:
+		cfg_data |= VF610_ADC_CLK_DIV8;
+		break;
+	case 16:
+		switch (adc_feature->clk_sel) {
+		case VF610_ADCIOC_BUSCLK_SET:
+			cfg_data |= VF610_ADC_BUSCLK2_SEL | VF610_ADC_CLK_DIV8;
+			break;
+		default:
+			dev_err(info->dev, "error clk divider\n");
+			break;
+		}
+		break;
+	}
+
+	/* Use the short sample mode */
+	cfg_data &= ~(VF610_ADC_ADLSMP_LONG | VF610_ADC_ADSTS_MASK);
+
+	/* update hardware average selection */
+	cfg_data &= ~VF610_ADC_AVGS_MASK;
+	gc_data &= ~VF610_ADC_AVGEN;
+	switch (adc_feature->sample_rate) {
+	case VF610_ADC_SAMPLE_1:
+		break;
+	case VF610_ADC_SAMPLE_4:
+		gc_data |= VF610_ADC_AVGEN;
+		break;
+	case VF610_ADC_SAMPLE_8:
+		gc_data |= VF610_ADC_AVGEN;
+		cfg_data |= VF610_ADC_AVGS_8;
+		break;
+	case VF610_ADC_SAMPLE_16:
+		gc_data |= VF610_ADC_AVGEN;
+		cfg_data |= VF610_ADC_AVGS_16;
+		break;
+	case VF610_ADC_SAMPLE_32:
+		gc_data |= VF610_ADC_AVGEN;
+		cfg_data |= VF610_ADC_AVGS_32;
+		break;
+	default:
+		dev_err(info->dev,
+			"error hardware sample average select\n");
+	}
+
+	writel(cfg_data, info->regs + VF610_REG_ADC_CFG);
+	writel(gc_data, info->regs + VF610_REG_ADC_GC);
+}
+
+static void vf610_adc_hw_init(struct vf610_adc *info)
+{
+	/* CFG: Feature set */
+	vf610_adc_cfg_post_set(info);
+	vf610_adc_sample_set(info);
+
+	/* adc calibration */
+	vf610_adc_calibration(info);
+
+	/* CFG: power and speed set */
+	vf610_adc_cfg_set(info);
+}
+
+static int vf610_adc_read_data(struct vf610_adc *info)
+{
+	int result;
+
+	result = readl(info->regs + VF610_REG_ADC_R0);
+
+	switch (info->adc_feature.res_mode) {
+	case 8:
+		result &= 0xFF;
+		break;
+	case 10:
+		result &= 0x3FF;
+		break;
+	case 12:
+		result &= 0xFFF;
+		break;
+	default:
+		break;
+	}
+
+	return result;
+}
+
+static irqreturn_t vf610_adc_isr(int irq, void *dev_id)
+{
+	struct vf610_adc *info = (struct vf610_adc *)dev_id;
+	int coco;
+
+	coco = readl(info->regs + VF610_REG_ADC_HS);
+	if (coco & VF610_ADC_HS_COCO0) {
+		info->value = vf610_adc_read_data(info);
+		complete(&info->completion);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("1941176, 559332, 286957, 145374, 73171");
+
+static struct attribute *vf610_attributes[] = {
+	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group vf610_attribute_group = {
+	.attrs = vf610_attributes,
+};
+
+static int vf610_read_raw(struct iio_dev *indio_dev,
+			struct iio_chan_spec const *chan,
+			int *val,
+			int *val2,
+			long mask)
+{
+	struct vf610_adc *info = iio_priv(indio_dev);
+	unsigned int hc_cfg;
+	unsigned long ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&indio_dev->mlock);
+		reinit_completion(&info->completion);
+
+		hc_cfg = VF610_ADC_ADCHC(chan->channel);
+		hc_cfg |= VF610_ADC_AIEN;
+		writel(hc_cfg, info->regs + VF610_REG_ADC_HC0);
+		ret = wait_for_completion_interruptible_timeout
+				(&info->completion, VF610_ADC_TIMEOUT);
+		if (ret == 0)
+			return -ETIMEDOUT;
+		if (ret < 0) {
+			mutex_unlock(&indio_dev->mlock);
+			return ret;
+		}
+
+		*val = info->value;
+		mutex_unlock(&indio_dev->mlock);
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		*val = info->vref_uv / 1000;
+		*val2 = info->adc_feature.res_mode;
+		return IIO_VAL_FRACTIONAL_LOG2;
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = vf610_sample_freq_avail[info->adc_feature.sample_rate];
+		*val2 = 0;
+		return IIO_VAL_INT;
+
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static int vf610_write_raw(struct iio_dev *indio_dev,
+			struct iio_chan_spec const *chan,
+			int val,
+			int val2,
+			long mask)
+{
+	struct vf610_adc *info = iio_priv(indio_dev);
+	int i;
+
+	switch (mask) {
+		case IIO_CHAN_INFO_SAMP_FREQ:
+			for (i = 0;
+				i < ARRAY_SIZE(vf610_sample_freq_avail);
+				i++)
+				if (val == vf610_sample_freq_avail[i]) {
+					info->adc_feature.sample_rate = i;
+					vf610_adc_sample_set(info);
+					return 0;
+				}
+			break;
+
+		default:
+			break;
+	}
+
+	return -EINVAL;
+}
+
+static int vf610_adc_reg_access(struct iio_dev *indio_dev,
+			unsigned reg, unsigned writeval,
+			unsigned *readval)
+{
+	struct vf610_adc *info = iio_priv(indio_dev);
+
+	if ((readval == NULL) ||
+		(!(reg % 4) || (reg > VF610_REG_ADC_PCTL)))
+		return -EINVAL;
+
+	*readval = readl(info->regs + reg);
+
+	return 0;
+}
+
+static const struct iio_info vf610_adc_iio_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = &vf610_read_raw,
+	.write_raw = &vf610_write_raw,
+	.debugfs_reg_access = &vf610_adc_reg_access,
+	.attrs = &vf610_attribute_group,
+};
+
+static const struct of_device_id vf610_adc_match[] = {
+	{ .compatible = "fsl,vf610-adc", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, vf610_adc_dt_ids);
+
+static int vf610_adc_probe(struct platform_device *pdev)
+{
+	struct vf610_adc *info;
+	struct iio_dev *indio_dev;
+	struct resource *mem;
+	int irq;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(struct vf610_adc));
+	if (!indio_dev) {
+		dev_err(&pdev->dev, "Failed allocating iio device\n");
+		return -ENOMEM;
+	}
+
+	info = iio_priv(indio_dev);
+	info->dev = &pdev->dev;
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	info->regs = devm_ioremap_resource(&pdev->dev, mem);
+	if (IS_ERR(info->regs))
+		return PTR_ERR(info->regs);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq <= 0) {
+		dev_err(&pdev->dev, "no irq resource?\n");
+		return -EINVAL;
+	}
+
+	ret = devm_request_irq(info->dev, irq,
+				vf610_adc_isr, 0,
+				dev_name(&pdev->dev), info);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed requesting irq, irq = %d\n", irq);
+		return ret;
+	}
+
+	info->clk = devm_clk_get(&pdev->dev, "adc");
+	if (IS_ERR(info->clk)) {
+		dev_err(&pdev->dev, "failed getting clock, err = %ld\n",
+						PTR_ERR(info->clk));
+		ret = PTR_ERR(info->clk);
+		return ret;
+	}
+
+	info->vref = devm_regulator_get(&pdev->dev, "vref");
+	if (IS_ERR(info->vref))
+		return PTR_ERR(info->vref);
+
+	ret = regulator_enable(info->vref);
+	if (ret)
+		return ret;
+
+	info->vref_uv = regulator_get_voltage(info->vref);
+
+	platform_set_drvdata(pdev, indio_dev);
+
+	init_completion(&info->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 = &vf610_adc_iio_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = vf610_adc_iio_channels;
+	indio_dev->num_channels = ARRAY_SIZE(vf610_adc_iio_channels);
+
+	ret = clk_prepare_enable(info->clk);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"Could not prepare or enable the clock.\n");
+		goto error_adc_clk_enable;
+	}
+
+	vf610_adc_cfg_init(info);
+	vf610_adc_hw_init(info);
+
+	ret = iio_device_register(indio_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Couldn't register the device.\n");
+		goto error_iio_device_register;
+	}
+
+	return 0;
+
+
+error_iio_device_register:
+	clk_disable_unprepare(info->clk);
+error_adc_clk_enable:
+	regulator_disable(info->vref);
+
+	return ret;
+}
+
+static int vf610_adc_remove(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+	struct vf610_adc *info = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	regulator_disable(info->vref);
+	clk_disable_unprepare(info->clk);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int vf610_adc_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct vf610_adc *info = iio_priv(indio_dev);
+	int hc_cfg;
+
+	/* ADC controller enters to stop mode */
+	hc_cfg = readl(info->regs + VF610_REG_ADC_HC0);
+	hc_cfg |= VF610_ADC_CONV_DISABLE;
+	writel(hc_cfg, info->regs + VF610_REG_ADC_HC0);
+
+	clk_disable_unprepare(info->clk);
+	regulator_disable(info->vref);
+
+	return 0;
+}
+
+static int vf610_adc_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct vf610_adc *info = iio_priv(indio_dev);
+	int ret;
+
+	ret = regulator_enable(info->vref);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(info->clk);
+	if (ret)
+		return ret;
+
+	vf610_adc_hw_init(info);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(vf610_adc_pm_ops,
+			vf610_adc_suspend,
+			vf610_adc_resume);
+
+static struct platform_driver vf610_adc_driver = {
+	.probe          = vf610_adc_probe,
+	.remove         = vf610_adc_remove,
+	.driver         = {
+		.name   = DRIVER_NAME,
+		.owner  = THIS_MODULE,
+		.of_match_table = vf610_adc_match,
+		.pm     = &vf610_adc_pm_ops,
+	},
+};
+
+module_platform_driver(vf610_adc_driver);
+
+MODULE_AUTHOR("Fugang Duan <B38611@freescale.com>");
+MODULE_DESCRIPTION("Freescale VF610 ADC driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.2.rc3

[PATCH v5 3/3] Documentation: add the binding file for Freescale vf610 ADC driver

[Fugang Duan]

The patch adds the binding file for Freescale vf610 ADC driver.

CC: Shawn Guo <shawn.guo@linaro.org>
CC: Jonathan Cameron <jic23@kernel.org>
CC: Mark Rutland <mark.rutland@arm.com>
CC: Otavio Salvador <otavio@ossystems.com.br>
CC: Peter Meerwald <pmeerw@pmeerw.net>
CC: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
 .../devicetree/bindings/iio/adc/vf610-adc.txt      |   22 ++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
new file mode 100644
index 0000000..dcebff1
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
@@ -0,0 +1,22 @@
+Freescale vf610 Analog to Digital Converter bindings
+
+The devicetree bindings are for the new ADC driver written for
+vf610/i.MX6slx and upward SoCs from Freescale.
+
+Required properties:
+- compatible: Should contain "fsl,vf610-adc"
+- reg: Offset and length of the register set for the device
+- interrupts: Should contain the interrupt for the device
+- clocks: The clock is needed by the ADC controller, ADC clock source is ipg clock.
+- clock-names: Must contain "adc", matching entry in the clocks property.
+- vref-supply: The regulator supply ADC refrence voltage.
+
+Example:
+adc0: adc@4003b000 {
+	compatible = "fsl,vf610-adc";
+	reg = <0x4003b000 0x1000>;
+	interrupts = <0 53 0x04>;
+	clocks = <&clks VF610_CLK_ADC0>;
+	clock-names = "adc";
+	vref-supply = <&reg_vcc_3v3_mcu>;
+};
-- 
1.7.2.rc3

Re: [PATCH v5 2/3] iio:adc:imx: add Freescale Vybrid vf610 adc driver

[Jonathan Cameron]

On 26/01/14 05:39, Fugang Duan wrote:
> Add Freescale Vybrid vf610 adc driver. The driver only support
> ADC software trigger.
>
> VF610 ADC device documentation is available at below reference manual
> (chapter 37):
> http://cache.freescale.com/files/32bit/doc/ref_manual/VYBRIDRM.pdf?
> fpsp=1&WT_TYPE=Reference%20Manuals&WT_VENDOR=FREESCALE&WT_FILE_FORMAT
> =pdf&WT_ASSET=Documentation
>
> CC: Shawn Guo <shawn.guo@linaro.org>
> CC: Jonathan Cameron <jic23@kernel.org>
> CC: Mark Rutland <mark.rutland@arm.com>
> CC: Otavio Salvador <otavio@ossystems.com.br>
> CC: Peter Meerwald <pmeerw@pmeerw.net>
> CC: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Fugang Duan <B38611@freescale.com>

I very much like the simplified approach you have taken for this initial
merge.  Much easier to start simple and build up to the more complex
functionality.

There was one missing mutex unlock in an error path (noted below). I have
fixed that and applied to the togreg branch of iio.git.   This will initially
go out as the testing branch for the autobuilders to play with it.

Also devm_irq requests always make me nervous, but I think this one is fine
so have left it alone

Jonathan

> ---
>   drivers/iio/adc/Kconfig     |    9 +
>   drivers/iio/adc/Makefile    |    1 +
>   drivers/iio/adc/vf610_adc.c |  709 +++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 719 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 2209f28..3677165 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -197,6 +197,15 @@ config TWL6030_GPADC
>   	  This driver can also be built as a module. If so, the module will be
>   	  called twl6030-gpadc.
>
> +config VF610_ADC
> +	tristate "Freescale vf610 ADC driver"
> +	help
> +	  Say yes here to support for Vybrid board analog-to-digital converter.
> +	  Since the IP is used for i.MX6SLX, the driver also support i.MX6SLX.
> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called vf610_adc.
> +
>   config VIPERBOARD_ADC
>   	tristate "Viperboard ADC support"
>   	depends on MFD_VIPERBOARD && USB
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index ba9a10a..6d96b0f 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -21,4 +21,5 @@ obj-$(CONFIG_NAU7802) += nau7802.o
>   obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>   obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
>   obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
> +obj-$(CONFIG_VF610_ADC) += vf610_adc.o
>   obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
> diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
> new file mode 100644
> index 0000000..84c7aac
> --- /dev/null
> +++ b/drivers/iio/adc/vf610_adc.c
> @@ -0,0 +1,709 @@
> +/*
> + * Freescale Vybrid vf610 ADC driver
> + *
> + * Copyright 2013 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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, write to the Free Software
> + *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/io.h>
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/of_platform.h>
> +#include <linux/err.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/driver.h>
> +
> +/* This will be the driver name the kernel reports */
> +#define DRIVER_NAME "vf610-adc"
> +
> +/* Vybrid/IMX ADC registers */
> +#define VF610_REG_ADC_HC0		0x00
> +#define VF610_REG_ADC_HC1		0x04
> +#define VF610_REG_ADC_HS		0x08
> +#define VF610_REG_ADC_R0		0x0c
> +#define VF610_REG_ADC_R1		0x10
> +#define VF610_REG_ADC_CFG		0x14
> +#define VF610_REG_ADC_GC		0x18
> +#define VF610_REG_ADC_GS		0x1c
> +#define VF610_REG_ADC_CV		0x20
> +#define VF610_REG_ADC_OFS		0x24
> +#define VF610_REG_ADC_CAL		0x28
> +#define VF610_REG_ADC_PCTL		0x30
> +
> +/* Configuration register field define */
> +#define VF610_ADC_MODE_BIT8		0x00
> +#define VF610_ADC_MODE_BIT10		0x04
> +#define VF610_ADC_MODE_BIT12		0x08
> +#define VF610_ADC_MODE_MASK		0x0c
> +#define VF610_ADC_BUSCLK2_SEL		0x01
> +#define VF610_ADC_ALTCLK_SEL		0x02
> +#define VF610_ADC_ADACK_SEL		0x03
> +#define VF610_ADC_ADCCLK_MASK		0x03
> +#define VF610_ADC_CLK_DIV2		0x20
> +#define VF610_ADC_CLK_DIV4		0x40
> +#define VF610_ADC_CLK_DIV8		0x60
> +#define VF610_ADC_CLK_MASK		0x60
> +#define VF610_ADC_ADLSMP_LONG		0x10
> +#define VF610_ADC_ADSTS_MASK		0x300
> +#define VF610_ADC_ADLPC_EN		0x80
> +#define VF610_ADC_ADHSC_EN		0x400
> +#define VF610_ADC_REFSEL_VALT		0x100
> +#define VF610_ADC_REFSEL_VBG		0x1000
> +#define VF610_ADC_ADTRG_HARD		0x2000
> +#define VF610_ADC_AVGS_8		0x4000
> +#define VF610_ADC_AVGS_16		0x8000
> +#define VF610_ADC_AVGS_32		0xC000
> +#define VF610_ADC_AVGS_MASK		0xC000
> +#define VF610_ADC_OVWREN		0x10000
> +
> +/* General control register field define */
> +#define VF610_ADC_ADACKEN		0x1
> +#define VF610_ADC_DMAEN			0x2
> +#define VF610_ADC_ACREN			0x4
> +#define VF610_ADC_ACFGT			0x8
> +#define VF610_ADC_ACFE			0x10
> +#define VF610_ADC_AVGEN			0x20
> +#define VF610_ADC_ADCON			0x40
> +#define VF610_ADC_CAL			0x80
> +
> +/* Other field define */
> +#define VF610_ADC_ADCHC(x)		((x) & 0xF)
> +#define VF610_ADC_AIEN			(0x1 << 7)
> +#define VF610_ADC_CONV_DISABLE		0x1F
> +#define VF610_ADC_HS_COCO0		0x1
> +#define VF610_ADC_CALF			0x2
> +#define VF610_ADC_TIMEOUT		msecs_to_jiffies(100)
> +
> +enum clk_sel {
> +	VF610_ADCIOC_BUSCLK_SET,
> +	VF610_ADCIOC_ALTCLK_SET,
> +	VF610_ADCIOC_ADACK_SET,
> +};
> +
> +enum vol_ref {
> +	VF610_ADCIOC_VR_VREF_SET,
> +	VF610_ADCIOC_VR_VALT_SET,
> +	VF610_ADCIOC_VR_VBG_SET,
> +};
> +
> +enum average_sel {
> +	VF610_ADC_SAMPLE_1,
> +	VF610_ADC_SAMPLE_4,
> +	VF610_ADC_SAMPLE_8,
> +	VF610_ADC_SAMPLE_16,
> +	VF610_ADC_SAMPLE_32,
> +};

> +
> +struct vf610_adc_feature {
> +	enum clk_sel	clk_sel;
> +	enum vol_ref	vol_ref;
> +
> +	int	clk_div;
> +	int     sample_rate;
> +	int	res_mode;
> +
> +	bool	lpm;
> +	bool	calibration;
> +	bool	ovwren;
> +};
> +
> +struct vf610_adc {
> +	struct device *dev;
> +	void __iomem *regs;
> +	struct clk *clk;
> +
> +	u32 vref_uv;
> +	u32 value;
> +	struct regulator *vref;
> +	struct vf610_adc_feature adc_feature;
> +
> +	struct completion completion;
> +};
> +
> +#define VF610_ADC_CHAN(_idx, _chan_type) {			\
> +	.type = (_chan_type),					\
> +	.indexed = 1,						\
> +	.channel = (_idx),					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> +				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> +}
> +
> +static const struct iio_chan_spec vf610_adc_iio_channels[] = {
> +	VF610_ADC_CHAN(0, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(1, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(2, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(3, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(4, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(5, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(6, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(7, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(8, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(9, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(10, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(11, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(12, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(13, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(14, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(15, IIO_VOLTAGE),
> +	/* sentinel */
> +};
> +
> +/*
> + * ADC sample frequency, unit is ADCK cycles.
> + * ADC clk source is ipg clock, which is the same as bus clock.
> + *
> + * ADC conversion time = SFCAdder + AverageNum x (BCT + LSTAdder)
> + * SFCAdder: fixed to 6 ADCK cycles
> + * AverageNum: 1, 4, 8, 16, 32 samples for hardware average.
> + * BCT (Base Conversion Time): fixed to 25 ADCK cycles for 12 bit mode
> + * LSTAdder(Long Sample Time): fixed to 3 ADCK cycles
> + *
> + * By default, enable 12 bit resolution mode, clock source
> + * set to ipg clock, So get below frequency group:
> + */
> +static const u32 vf610_sample_freq_avail[5] =
> +{1941176, 559332, 286957, 145374, 73171};
> +
> +static inline void vf610_adc_cfg_init(struct vf610_adc *info)
> +{
> +	/* set default Configuration for ADC controller */
> +	info->adc_feature.clk_sel = VF610_ADCIOC_BUSCLK_SET;
> +	info->adc_feature.vol_ref = VF610_ADCIOC_VR_VREF_SET;
> +
> +	info->adc_feature.calibration = true;
> +	info->adc_feature.ovwren = true;
> +
> +	info->adc_feature.clk_div = 1;
> +	info->adc_feature.res_mode = 12;
> +	info->adc_feature.sample_rate = 1;
> +	info->adc_feature.lpm = true;
> +}
> +
> +static void vf610_adc_cfg_post_set(struct vf610_adc *info)
> +{
> +	struct vf610_adc_feature *adc_feature = &info->adc_feature;
> +	int cfg_data = 0;
> +	int gc_data = 0;
> +
> +	switch (adc_feature->clk_sel) {
> +	case VF610_ADCIOC_ALTCLK_SET:
> +		cfg_data |= VF610_ADC_ALTCLK_SEL;
> +		break;
> +	case VF610_ADCIOC_ADACK_SET:
> +		cfg_data |= VF610_ADC_ADACK_SEL;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	/* low power set for calibration */
> +	cfg_data |= VF610_ADC_ADLPC_EN;
> +
> +	/* enable high speed for calibration */
> +	cfg_data |= VF610_ADC_ADHSC_EN;
> +
> +	/* voltage reference */
> +	switch (adc_feature->vol_ref) {
> +	case VF610_ADCIOC_VR_VREF_SET:
> +		break;
> +	case VF610_ADCIOC_VR_VALT_SET:
> +		cfg_data |= VF610_ADC_REFSEL_VALT;
> +		break;
> +	case VF610_ADCIOC_VR_VBG_SET:
> +		cfg_data |= VF610_ADC_REFSEL_VBG;
> +		break;
> +	default:
> +		dev_err(info->dev, "error voltage reference\n");
> +	}
> +
> +	/* data overwrite enable */
> +	if (adc_feature->ovwren)
> +		cfg_data |= VF610_ADC_OVWREN;
> +
> +	writel(cfg_data, info->regs + VF610_REG_ADC_CFG);
> +	writel(gc_data, info->regs + VF610_REG_ADC_GC);
> +}
> +
> +static void vf610_adc_calibration(struct vf610_adc *info)
> +{
> +	int adc_gc, hc_cfg;
> +	int timeout;
> +
> +	if (!info->adc_feature.calibration)
> +		return;
> +
> +	/* enable calibration interrupt */
> +	hc_cfg = VF610_ADC_AIEN | VF610_ADC_CONV_DISABLE;
> +	writel(hc_cfg, info->regs + VF610_REG_ADC_HC0);
> +
> +	adc_gc = readl(info->regs + VF610_REG_ADC_GC);
> +	writel(adc_gc | VF610_ADC_CAL, info->regs + VF610_REG_ADC_GC);
> +
> +	timeout = wait_for_completion_timeout
> +			(&info->completion, VF610_ADC_TIMEOUT);
> +	if (timeout == 0)
> +		dev_err(info->dev, "Timeout for adc calibration\n");
> +
> +	adc_gc = readl(info->regs + VF610_REG_ADC_GS);
> +	if (adc_gc & VF610_ADC_CALF)
> +		dev_err(info->dev, "ADC calibration failed\n");
> +
> +	info->adc_feature.calibration = false;
> +}
> +
> +static void vf610_adc_cfg_set(struct vf610_adc *info)
> +{
> +	struct vf610_adc_feature *adc_feature = &(info->adc_feature);
> +	int cfg_data;
> +
> +	cfg_data = readl(info->regs + VF610_REG_ADC_CFG);
> +
> +	/* low power configuration */
> +	cfg_data &= ~VF610_ADC_ADLPC_EN;
> +	if (adc_feature->lpm)
> +		cfg_data |= VF610_ADC_ADLPC_EN;
> +
> +	/* disable high speed */
> +	cfg_data &= ~VF610_ADC_ADHSC_EN;
> +
> +	writel(cfg_data, info->regs + VF610_REG_ADC_CFG);
> +}
> +
> +static void vf610_adc_sample_set(struct vf610_adc *info)
> +{
> +	struct vf610_adc_feature *adc_feature = &(info->adc_feature);
> +	int cfg_data, gc_data;
> +
> +	cfg_data = readl(info->regs + VF610_REG_ADC_CFG);
> +	gc_data = readl(info->regs + VF610_REG_ADC_GC);
> +
> +	/* resolution mode */
> +	cfg_data &= ~VF610_ADC_MODE_MASK;
> +	switch (adc_feature->res_mode) {
> +	case 8:
> +		cfg_data |= VF610_ADC_MODE_BIT8;
> +		break;
> +	case 10:
> +		cfg_data |= VF610_ADC_MODE_BIT10;
> +		break;
> +	case 12:
> +		cfg_data |= VF610_ADC_MODE_BIT12;
> +		break;
> +	default:
> +		dev_err(info->dev, "error resolution mode\n");
> +		break;
> +	}
> +
> +	/* clock select and clock divider */
> +	cfg_data &= ~(VF610_ADC_CLK_MASK | VF610_ADC_ADCCLK_MASK);
> +	switch (adc_feature->clk_div) {
> +	case 1:
> +		break;
> +	case 2:
> +		cfg_data |= VF610_ADC_CLK_DIV2;
> +		break;
> +	case 4:
> +		cfg_data |= VF610_ADC_CLK_DIV4;
> +		break;
> +	case 8:
> +		cfg_data |= VF610_ADC_CLK_DIV8;
> +		break;
> +	case 16:
> +		switch (adc_feature->clk_sel) {
> +		case VF610_ADCIOC_BUSCLK_SET:
> +			cfg_data |= VF610_ADC_BUSCLK2_SEL | VF610_ADC_CLK_DIV8;
> +			break;
> +		default:
> +			dev_err(info->dev, "error clk divider\n");
> +			break;
> +		}
> +		break;
> +	}
> +
> +	/* Use the short sample mode */
> +	cfg_data &= ~(VF610_ADC_ADLSMP_LONG | VF610_ADC_ADSTS_MASK);
> +
> +	/* update hardware average selection */
> +	cfg_data &= ~VF610_ADC_AVGS_MASK;
> +	gc_data &= ~VF610_ADC_AVGEN;
> +	switch (adc_feature->sample_rate) {
> +	case VF610_ADC_SAMPLE_1:
> +		break;
> +	case VF610_ADC_SAMPLE_4:
> +		gc_data |= VF610_ADC_AVGEN;
> +		break;
> +	case VF610_ADC_SAMPLE_8:
> +		gc_data |= VF610_ADC_AVGEN;
> +		cfg_data |= VF610_ADC_AVGS_8;
> +		break;
> +	case VF610_ADC_SAMPLE_16:
> +		gc_data |= VF610_ADC_AVGEN;
> +		cfg_data |= VF610_ADC_AVGS_16;
> +		break;
> +	case VF610_ADC_SAMPLE_32:
> +		gc_data |= VF610_ADC_AVGEN;
> +		cfg_data |= VF610_ADC_AVGS_32;
> +		break;
> +	default:
> +		dev_err(info->dev,
> +			"error hardware sample average select\n");
> +	}
> +
> +	writel(cfg_data, info->regs + VF610_REG_ADC_CFG);
> +	writel(gc_data, info->regs + VF610_REG_ADC_GC);
> +}
> +
> +static void vf610_adc_hw_init(struct vf610_adc *info)
> +{
> +	/* CFG: Feature set */
> +	vf610_adc_cfg_post_set(info);
> +	vf610_adc_sample_set(info);
> +
> +	/* adc calibration */
> +	vf610_adc_calibration(info);
> +
> +	/* CFG: power and speed set */
> +	vf610_adc_cfg_set(info);
> +}
> +
> +static int vf610_adc_read_data(struct vf610_adc *info)
> +{
> +	int result;
> +
> +	result = readl(info->regs + VF610_REG_ADC_R0);
> +
> +	switch (info->adc_feature.res_mode) {
> +	case 8:
> +		result &= 0xFF;
> +		break;
> +	case 10:
> +		result &= 0x3FF;
> +		break;
> +	case 12:
> +		result &= 0xFFF;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return result;
> +}
> +
> +static irqreturn_t vf610_adc_isr(int irq, void *dev_id)
> +{
> +	struct vf610_adc *info = (struct vf610_adc *)dev_id;
> +	int coco;
> +
> +	coco = readl(info->regs + VF610_REG_ADC_HS);
> +	if (coco & VF610_ADC_HS_COCO0) {
> +		info->value = vf610_adc_read_data(info);
> +		complete(&info->completion);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("1941176, 559332, 286957, 145374, 73171");
> +
> +static struct attribute *vf610_attributes[] = {
> +	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group vf610_attribute_group = {
> +	.attrs = vf610_attributes,
> +};
> +
> +static int vf610_read_raw(struct iio_dev *indio_dev,
> +			struct iio_chan_spec const *chan,
> +			int *val,
> +			int *val2,
> +			long mask)
> +{
> +	struct vf610_adc *info = iio_priv(indio_dev);
> +	unsigned int hc_cfg;
> +	unsigned long ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&indio_dev->mlock);
> +		reinit_completion(&info->completion);
> +
> +		hc_cfg = VF610_ADC_ADCHC(chan->channel);
> +		hc_cfg |= VF610_ADC_AIEN;
> +		writel(hc_cfg, info->regs + VF610_REG_ADC_HC0);
> +		ret = wait_for_completion_interruptible_timeout
> +				(&info->completion, VF610_ADC_TIMEOUT);
> +		if (ret == 0)
The mutex is still held here.
> +			return -ETIMEDOUT;
> +		if (ret < 0) {
> +			mutex_unlock(&indio_dev->mlock);
> +			return ret;
> +		}
> +
> +		*val = info->value;
> +		mutex_unlock(&indio_dev->mlock);
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = info->vref_uv / 1000;
> +		*val2 = info->adc_feature.res_mode;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = vf610_sample_freq_avail[info->adc_feature.sample_rate];
> +		*val2 = 0;
> +		return IIO_VAL_INT;
> +
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int vf610_write_raw(struct iio_dev *indio_dev,
> +			struct iio_chan_spec const *chan,
> +			int val,
> +			int val2,
> +			long mask)
> +{
> +	struct vf610_adc *info = iio_priv(indio_dev);
> +	int i;
> +
> +	switch (mask) {
> +		case IIO_CHAN_INFO_SAMP_FREQ:
> +			for (i = 0;
> +				i < ARRAY_SIZE(vf610_sample_freq_avail);
> +				i++)
> +				if (val == vf610_sample_freq_avail[i]) {
> +					info->adc_feature.sample_rate = i;
> +					vf610_adc_sample_set(info);
> +					return 0;
> +				}
> +			break;
> +
> +		default:
> +			break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int vf610_adc_reg_access(struct iio_dev *indio_dev,
> +			unsigned reg, unsigned writeval,
> +			unsigned *readval)
> +{
> +	struct vf610_adc *info = iio_priv(indio_dev);
> +
> +	if ((readval == NULL) ||
> +		(!(reg % 4) || (reg > VF610_REG_ADC_PCTL)))
> +		return -EINVAL;
> +
> +	*readval = readl(info->regs + reg);
> +
> +	return 0;
> +}
> +
> +static const struct iio_info vf610_adc_iio_info = {
> +	.driver_module = THIS_MODULE,
> +	.read_raw = &vf610_read_raw,
> +	.write_raw = &vf610_write_raw,
> +	.debugfs_reg_access = &vf610_adc_reg_access,
> +	.attrs = &vf610_attribute_group,
> +};
> +
> +static const struct of_device_id vf610_adc_match[] = {
> +	{ .compatible = "fsl,vf610-adc", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, vf610_adc_dt_ids);
> +
> +static int vf610_adc_probe(struct platform_device *pdev)
> +{
> +	struct vf610_adc *info;
> +	struct iio_dev *indio_dev;
> +	struct resource *mem;
> +	int irq;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(struct vf610_adc));
> +	if (!indio_dev) {
> +		dev_err(&pdev->dev, "Failed allocating iio device\n");
> +		return -ENOMEM;
> +	}
> +
> +	info = iio_priv(indio_dev);
> +	info->dev = &pdev->dev;
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	info->regs = devm_ioremap_resource(&pdev->dev, mem);
> +	if (IS_ERR(info->regs))
> +		return PTR_ERR(info->regs);
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq <= 0) {
> +		dev_err(&pdev->dev, "no irq resource?\n");
> +		return -EINVAL;
> +	}
> +
Hmm. Devm requests for irqs always make me nervous.  I 'think' this one
is fine, but it is rather too easy to use something in the handler that
is freed via a route other than managed resources...  Still if you are sure
then leave it be.
> +	ret = devm_request_irq(info->dev, irq,
> +				vf610_adc_isr, 0,
> +				dev_name(&pdev->dev), info);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed requesting irq, irq = %d\n", irq);
> +		return ret;
> +	}
> +
> +	info->clk = devm_clk_get(&pdev->dev, "adc");
> +	if (IS_ERR(info->clk)) {
> +		dev_err(&pdev->dev, "failed getting clock, err = %ld\n",
> +						PTR_ERR(info->clk));
> +		ret = PTR_ERR(info->clk);
> +		return ret;
> +	}
> +
> +	info->vref = devm_regulator_get(&pdev->dev, "vref");
> +	if (IS_ERR(info->vref))
> +		return PTR_ERR(info->vref);
> +
> +	ret = regulator_enable(info->vref);
> +	if (ret)
> +		return ret;
> +
> +	info->vref_uv = regulator_get_voltage(info->vref);
> +
> +	platform_set_drvdata(pdev, indio_dev);
> +
> +	init_completion(&info->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 = &vf610_adc_iio_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = vf610_adc_iio_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(vf610_adc_iio_channels);
> +
> +	ret = clk_prepare_enable(info->clk);
> +	if (ret) {
> +		dev_err(&pdev->dev,
> +			"Could not prepare or enable the clock.\n");
> +		goto error_adc_clk_enable;
> +	}
> +
> +	vf610_adc_cfg_init(info);
> +	vf610_adc_hw_init(info);
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Couldn't register the device.\n");
> +		goto error_iio_device_register;
> +	}
> +
> +	return 0;
> +
> +
> +error_iio_device_register:
> +	clk_disable_unprepare(info->clk);
> +error_adc_clk_enable:
> +	regulator_disable(info->vref);
> +
> +	return ret;
> +}
> +
> +static int vf610_adc_remove(struct platform_device *pdev)
> +{
> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +	struct vf610_adc *info = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +	regulator_disable(info->vref);
> +	clk_disable_unprepare(info->clk);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int vf610_adc_suspend(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct vf610_adc *info = iio_priv(indio_dev);
> +	int hc_cfg;
> +
> +	/* ADC controller enters to stop mode */
> +	hc_cfg = readl(info->regs + VF610_REG_ADC_HC0);
> +	hc_cfg |= VF610_ADC_CONV_DISABLE;
> +	writel(hc_cfg, info->regs + VF610_REG_ADC_HC0);
> +
> +	clk_disable_unprepare(info->clk);
> +	regulator_disable(info->vref);
> +
> +	return 0;
> +}
> +
> +static int vf610_adc_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct vf610_adc *info = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = regulator_enable(info->vref);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(info->clk);
> +	if (ret)
> +		return ret;
> +
> +	vf610_adc_hw_init(info);
> +
> +	return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(vf610_adc_pm_ops,
> +			vf610_adc_suspend,
> +			vf610_adc_resume);
> +
> +static struct platform_driver vf610_adc_driver = {
> +	.probe          = vf610_adc_probe,
> +	.remove         = vf610_adc_remove,
> +	.driver         = {
> +		.name   = DRIVER_NAME,
> +		.owner  = THIS_MODULE,
> +		.of_match_table = vf610_adc_match,
> +		.pm     = &vf610_adc_pm_ops,
> +	},
> +};
> +
> +module_platform_driver(vf610_adc_driver);
> +
> +MODULE_AUTHOR("Fugang Duan <B38611@freescale.com>");
> +MODULE_DESCRIPTION("Freescale VF610 ADC driver");
> +MODULE_LICENSE("GPL v2");
>

Re: [PATCH v5 3/3] Documentation: add the binding file for Freescale vf610 ADC driver

[Jonathan Cameron]

On 26/01/14 05:39, Fugang Duan wrote:
> The patch adds the binding file for Freescale vf610 ADC driver.
>
> CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> CC: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> CC: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> CC: Otavio Salvador <otavio-fKevB0iiKLMBZ+LybsDmbA@public.gmane.org>
> CC: Peter Meerwald <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>
> CC: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
> Signed-off-by: Fugang Duan <B38611-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
It's been near enough 3 weeks without a comment from the Device Tree maintainers.
(admittedly it wasn't cc'd to the devicetree list).  I'm going to take this through
the IIO tree.

Applied to the togreg branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
though will go out briefly as the testing branch to let the autobuilders play.

> ---
>   .../devicetree/bindings/iio/adc/vf610-adc.txt      |   22 ++++++++++++++++++++
>   1 files changed, 22 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> new file mode 100644
> index 0000000..dcebff1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> @@ -0,0 +1,22 @@
> +Freescale vf610 Analog to Digital Converter bindings
> +
> +The devicetree bindings are for the new ADC driver written for
> +vf610/i.MX6slx and upward SoCs from Freescale.
> +
> +Required properties:
> +- compatible: Should contain "fsl,vf610-adc"
> +- reg: Offset and length of the register set for the device
> +- interrupts: Should contain the interrupt for the device
> +- clocks: The clock is needed by the ADC controller, ADC clock source is ipg clock.
> +- clock-names: Must contain "adc", matching entry in the clocks property.
> +- vref-supply: The regulator supply ADC refrence voltage.
> +
> +Example:
> +adc0: adc@4003b000 {
> +	compatible = "fsl,vf610-adc";
> +	reg = <0x4003b000 0x1000>;
> +	interrupts = <0 53 0x04>;
> +	clocks = <&clks VF610_CLK_ADC0>;
> +	clock-names = "adc";
> +	vref-supply = <&reg_vcc_3v3_mcu>;
> +};
>

Re: [PATCH v5 3/3] Documentation: add the binding file for Freescale vf610 ADC driver

[Jonathan Cameron]

On 26/01/14 05:39, Fugang Duan wrote:
> The patch adds the binding file for Freescale vf610 ADC driver.
>
> CC: Shawn Guo <shawn.guo@linaro.org>
> CC: Jonathan Cameron <jic23@kernel.org>
> CC: Mark Rutland <mark.rutland@arm.com>
> CC: Otavio Salvador <otavio@ossystems.com.br>
> CC: Peter Meerwald <pmeerw@pmeerw.net>
> CC: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Fugang Duan <B38611@freescale.com>
It's been near enough 3 weeks without a comment from the Device Tree maintainers.
(admittedly it wasn't cc'd to the devicetree list).  I'm going to take this through
the IIO tree.

Applied to the togreg branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
though will go out briefly as the testing branch to let the autobuilders play.

> ---
>   .../devicetree/bindings/iio/adc/vf610-adc.txt      |   22 ++++++++++++++++++++
>   1 files changed, 22 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> new file mode 100644
> index 0000000..dcebff1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> @@ -0,0 +1,22 @@
> +Freescale vf610 Analog to Digital Converter bindings
> +
> +The devicetree bindings are for the new ADC driver written for
> +vf610/i.MX6slx and upward SoCs from Freescale.
> +
> +Required properties:
> +- compatible: Should contain "fsl,vf610-adc"
> +- reg: Offset and length of the register set for the device
> +- interrupts: Should contain the interrupt for the device
> +- clocks: The clock is needed by the ADC controller, ADC clock source is ipg clock.
> +- clock-names: Must contain "adc", matching entry in the clocks property.
> +- vref-supply: The regulator supply ADC refrence voltage.
> +
> +Example:
> +adc0: adc@4003b000 {
> +	compatible = "fsl,vf610-adc";
> +	reg = <0x4003b000 0x1000>;
> +	interrupts = <0 53 0x04>;
> +	clocks = <&clks VF610_CLK_ADC0>;
> +	clock-names = "adc";
> +	vref-supply = <&reg_vcc_3v3_mcu>;
> +};
>

Re: [PATCH v5 2/3] iio:adc:imx: add Freescale Vybrid vf610 adc driver

[Jonathan Cameron]

On 15/02/14 10:18, Jonathan Cameron wrote:
> On 26/01/14 05:39, Fugang Duan wrote:
>> Add Freescale Vybrid vf610 adc driver. The driver only support
>> ADC software trigger.
>>
>> VF610 ADC device documentation is available at below reference manual
>> (chapter 37):
>> http://cache.freescale.com/files/32bit/doc/ref_manual/VYBRIDRM.pdf?
>> fpsp=1&WT_TYPE=Reference%20Manuals&WT_VENDOR=FREESCALE&WT_FILE_FORMAT
>> =pdf&WT_ASSET=Documentation
>>
>> CC: Shawn Guo <shawn.guo@linaro.org>
>> CC: Jonathan Cameron <jic23@kernel.org>
>> CC: Mark Rutland <mark.rutland@arm.com>
>> CC: Otavio Salvador <otavio@ossystems.com.br>
>> CC: Peter Meerwald <pmeerw@pmeerw.net>
>> CC: Lars-Peter Clausen <lars@metafoo.de>
>> Signed-off-by: Fugang Duan <B38611@freescale.com>
>
> I very much like the simplified approach you have taken for this initial
> merge.  Much easier to start simple and build up to the more complex
> functionality.
>
> There was one missing mutex unlock in an error path (noted below). I have
> fixed that and applied to the togreg branch of iio.git.   This will initially
> go out as the testing branch for the autobuilders to play with it.
>
> Also devm_irq requests always make me nervous, but I think this one is fine
> so have left it alone
>
> Jonathan
As an update I also fixed up the build - see below.  One day I'll no send
out applied messages until after my local build tests have finished!

Note that it is this sort of thing that has led me to pushing out a testing
branch as that catches all the weird combinations that I don't.

>> +
>> +static const struct iio_info vf610_adc_iio_info = {
>> +    .driver_module = THIS_MODULE,
>> +    .read_raw = &vf610_read_raw,
>> +    .write_raw = &vf610_write_raw,
>> +    .debugfs_reg_access = &vf610_adc_reg_access,
>> +    .attrs = &vf610_attribute_group,
>> +};
>> +
>> +static const struct of_device_id vf610_adc_match[] = {
>> +    { .compatible = "fsl,vf610-adc", },
>> +    { /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, vf610_adc_dt_ids);
Your MODULE_DEVICE_TABLE needs to take a second variable that actually exists...
Also needs to depend in the KCONFIG on OF to avoid build issues on other platforms.

I've added both.

>> +
>> +static int vf610_adc_probe(struct platform_device *pdev)
>> +{
>> +    struct vf610_adc *info;
>> +    struct iio_dev *indio_dev;
>> +    struct resource *mem;
>> +    int irq;
>> +    int ret;
>> +
>> +    indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(struct vf610_adc));
>> +    if (!indio_dev) {
>> +        dev_err(&pdev->dev, "Failed allocating iio device\n");
>> +        return -ENOMEM;
>> +    }
>> +
>> +    info = iio_priv(indio_dev);
>> +    info->dev = &pdev->dev;
>> +
>> +    mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +    info->regs = devm_ioremap_resource(&pdev->dev, mem);
>> +    if (IS_ERR(info->regs))
>> +        return PTR_ERR(info->regs);
>> +
>> +    irq = platform_get_irq(pdev, 0);
>> +    if (irq <= 0) {
>> +        dev_err(&pdev->dev, "no irq resource?\n");
>> +        return -EINVAL;
>> +    }
>> +
> Hmm. Devm requests for irqs always make me nervous.  I 'think' this one
> is fine, but it is rather too easy to use something in the handler that
> is freed via a route other than managed resources...  Still if you are sure
> then leave it be.
>> +    ret = devm_request_irq(info->dev, irq,
>> +                vf610_adc_isr, 0,
>> +                dev_name(&pdev->dev), info);
>> +    if (ret < 0) {
>> +        dev_err(&pdev->dev, "failed requesting irq, irq = %d\n", irq);
>> +        return ret;
>> +    }
>> +
>> +    info->clk = devm_clk_get(&pdev->dev, "adc");
>> +    if (IS_ERR(info->clk)) {
>> +        dev_err(&pdev->dev, "failed getting clock, err = %ld\n",
>> +                        PTR_ERR(info->clk));
>> +        ret = PTR_ERR(info->clk);
>> +        return ret;
>> +    }
>> +
>> +    info->vref = devm_regulator_get(&pdev->dev, "vref");
>> +    if (IS_ERR(info->vref))
>> +        return PTR_ERR(info->vref);
>> +
>> +    ret = regulator_enable(info->vref);
>> +    if (ret)
>> +        return ret;
>> +
>> +    info->vref_uv = regulator_get_voltage(info->vref);
>> +
>> +    platform_set_drvdata(pdev, indio_dev);
>> +
>> +    init_completion(&info->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 = &vf610_adc_iio_info;
>> +    indio_dev->modes = INDIO_DIRECT_MODE;
>> +    indio_dev->channels = vf610_adc_iio_channels;
>> +    indio_dev->num_channels = ARRAY_SIZE(vf610_adc_iio_channels);
>> +
>> +    ret = clk_prepare_enable(info->clk);
>> +    if (ret) {
>> +        dev_err(&pdev->dev,
>> +            "Could not prepare or enable the clock.\n");
>> +        goto error_adc_clk_enable;
>> +    }
>> +
>> +    vf610_adc_cfg_init(info);
>> +    vf610_adc_hw_init(info);
>> +
>> +    ret = iio_device_register(indio_dev);
>> +    if (ret) {
>> +        dev_err(&pdev->dev, "Couldn't register the device.\n");
>> +        goto error_iio_device_register;
>> +    }
>> +
>> +    return 0;
>> +
>> +
>> +error_iio_device_register:
>> +    clk_disable_unprepare(info->clk);
>> +error_adc_clk_enable:
>> +    regulator_disable(info->vref);
>> +
>> +    return ret;
>> +}
>> +
>> +static int vf610_adc_remove(struct platform_device *pdev)
>> +{
>> +    struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>> +    struct vf610_adc *info = iio_priv(indio_dev);
>> +
>> +    iio_device_unregister(indio_dev);
>> +    regulator_disable(info->vref);
>> +    clk_disable_unprepare(info->clk);
>> +
>> +    return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int vf610_adc_suspend(struct device *dev)
>> +{
>> +    struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> +    struct vf610_adc *info = iio_priv(indio_dev);
>> +    int hc_cfg;
>> +
>> +    /* ADC controller enters to stop mode */
>> +    hc_cfg = readl(info->regs + VF610_REG_ADC_HC0);
>> +    hc_cfg |= VF610_ADC_CONV_DISABLE;
>> +    writel(hc_cfg, info->regs + VF610_REG_ADC_HC0);
>> +
>> +    clk_disable_unprepare(info->clk);
>> +    regulator_disable(info->vref);
>> +
>> +    return 0;
>> +}
>> +
>> +static int vf610_adc_resume(struct device *dev)
>> +{
>> +    struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> +    struct vf610_adc *info = iio_priv(indio_dev);
>> +    int ret;
>> +
>> +    ret = regulator_enable(info->vref);
>> +    if (ret)
>> +        return ret;
>> +
>> +    ret = clk_prepare_enable(info->clk);
>> +    if (ret)
>> +        return ret;
>> +
>> +    vf610_adc_hw_init(info);
>> +
>> +    return 0;
>> +}
>> +#endif
>> +
>> +static SIMPLE_DEV_PM_OPS(vf610_adc_pm_ops,
>> +            vf610_adc_suspend,
>> +            vf610_adc_resume);
>> +
>> +static struct platform_driver vf610_adc_driver = {
>> +    .probe          = vf610_adc_probe,
>> +    .remove         = vf610_adc_remove,
>> +    .driver         = {
>> +        .name   = DRIVER_NAME,
>> +        .owner  = THIS_MODULE,
>> +        .of_match_table = vf610_adc_match,
>> +        .pm     = &vf610_adc_pm_ops,
>> +    },
>> +};
>> +
>> +module_platform_driver(vf610_adc_driver);
>> +
>> +MODULE_AUTHOR("Fugang Duan <B38611@freescale.com>");
>> +MODULE_DESCRIPTION("Freescale VF610 ADC driver");
>> +MODULE_LICENSE("GPL v2");
>>
>
> --
> 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

Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[Jonathan Cameron]

On 26/01/14 05:39, Fugang Duan wrote:
> vf610 has two ADC controllers, and vf610-twr board ADC0_SE5 pin connect
> to sliding rheostat for ADC test, other ADC pins connect to connectors for
> future use.
>
> Add support for ADC0_SE5.
>
> CC: Shawn Guo <shawn.guo@linaro.org>
> CC: Jonathan Cameron <jic23@kernel.org>
> CC: Mark Rutland <mark.rutland@arm.com>
> CC: Otavio Salvador <otavio@ossystems.com.br>
> CC: Peter Meerwald <pmeerw@pmeerw.net>
> CC: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Fugang Duan <B38611@freescale.com>
I've applied the driver patches and device docs to the IIO git tree.
They'll filter through to linux-next via staging-next in the next
few days.  This patch probably wants to go via the relevant arch trees.
> ---
>   arch/arm/boot/dts/vf610-twr.dts |   21 +++++++++++++++++++++
>   arch/arm/boot/dts/vf610.dtsi    |   26 ++++++++++++++++++++++++++
>   2 files changed, 47 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
> index c8047ca..d867be3 100644
> --- a/arch/arm/boot/dts/vf610-twr.dts
> +++ b/arch/arm/boot/dts/vf610-twr.dts
> @@ -34,6 +34,27 @@
>   		};
>   	};
>
> +	regulators {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		reg_vcc_3v3_mcu: regulator@0 {
> +			compatible = "regulator-fixed";
> +			reg = <0>;
> +			regulator-name = "vcc_3v3_mcu";
> +			regulator-min-microvolt = <3300000>;
> +			regulator-max-microvolt = <3300000>;
> +		};
> +	};
> +
> +};
> +
> +&adc0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_adc0_ad5>;
> +	vref-supply = <&reg_vcc_3v3_mcu>;
> +	status = "okay";
>   };
>
>   &dspi0 {
> diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
> index d31ce1b..b5b21ea 100644
> --- a/arch/arm/boot/dts/vf610.dtsi
> +++ b/arch/arm/boot/dts/vf610.dtsi
> @@ -152,6 +152,15 @@
>   				clock-names = "pit";
>   			};
>
> +			adc0: adc@4003b000 {
> +				compatible = "fsl,vf610-adc";
> +				reg = <0x4003b000 0x1000>;
> +				interrupts = <0 53 0x04>;
> +				clocks = <&clks VF610_CLK_ADC0>;
> +				clock-names = "adc";
> +				status = "disabled";
> +			};
> +
>   			wdog@4003e000 {
>   				compatible = "fsl,vf610-wdt", "fsl,imx21-wdt";
>   				reg = <0x4003e000 0x1000>;
> @@ -178,6 +187,14 @@
>
>   				/* functions and groups pins */
>
> +				adc0 {
> +					pinctrl_adc0_ad5: adc0_ad5 {
> +						fsl,pins = <
> +						VF610_PAD_PTC30__ADC0_SE5	0xa1
> +						>;
> +					};
> +				};
> +
>   				dcu0 {
>   					pinctrl_dcu0_1: dcu0grp_1 {
>   						fsl,pins = <
> @@ -450,6 +467,15 @@
>   				status = "disabled";
>   			};
>
> +			adc1: adc@400bb000 {
> +				compatible = "fsl,vf610-adc";
> +				reg = <0x400bb000 0x1000>;
> +				interrupts = <0 54 0x04>;
> +				clocks = <&clks VF610_CLK_ADC1>;
> +				clock-names = "adc";
> +				status = "disabled";
> +			};
> +
>   			fec0: ethernet@400d0000 {
>   				compatible = "fsl,mvf600-fec";
>   				reg = <0x400d0000 0x1000>;
>

Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[Shawn Guo]

Copy more DT folks and lists, as I want to make sure everyone agrees on
how the fixed regulators should organized in the device tree sources,
before I apply the patch.

On Sun, Jan 26, 2014 at 01:39:31PM +0800, Fugang Duan wrote:
> vf610 has two ADC controllers, and vf610-twr board ADC0_SE5 pin connect
> to sliding rheostat for ADC test, other ADC pins connect to connectors for
> future use.
> 
> Add support for ADC0_SE5.
> 
> CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> CC: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> CC: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> CC: Otavio Salvador <otavio-fKevB0iiKLMBZ+LybsDmbA@public.gmane.org>
> CC: Peter Meerwald <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>
> CC: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
> Signed-off-by: Fugang Duan <B38611-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
>  arch/arm/boot/dts/vf610-twr.dts |   21 +++++++++++++++++++++
>  arch/arm/boot/dts/vf610.dtsi    |   26 ++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
> index c8047ca..d867be3 100644
> --- a/arch/arm/boot/dts/vf610-twr.dts
> +++ b/arch/arm/boot/dts/vf610-twr.dts
> @@ -34,6 +34,27 @@
>  		};
>  	};
>  
> +	regulators {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		reg_vcc_3v3_mcu: regulator@0 {
> +			compatible = "regulator-fixed";
> +			reg = <0>;
> +			regulator-name = "vcc_3v3_mcu";
> +			regulator-min-microvolt = <3300000>;
> +			regulator-max-microvolt = <3300000>;
> +		};
> +	};

Per discussion [1], Mark Rutland suggests that instead of organizing
the fixed regulator nodes in a simple-bus container, it should be put
under root node directly like below.

/ {
	reg_vcc_3v3_mcu: regulator_0 {
		compatible = "regulator-fixed";
		regulator-name = "vcc_3v3_mcu";
		regulator-min-microvolt = <3300000>;
		regulator-max-microvolt = <3300000>;
	};
};

Is this what all DT folks agree on?  At least the node name should be
'regulator-0' since it's more idiomatic to use '-' than '_' in node
name?

Shawn

[1] http://thread.gmane.org/gmane.linux.drivers.devicetree/61467/focus=300895

> +
> +};
> +
> +&adc0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_adc0_ad5>;
> +	vref-supply = <&reg_vcc_3v3_mcu>;
> +	status = "okay";
>  };
>  
>  &dspi0 {
> diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
> index d31ce1b..b5b21ea 100644
> --- a/arch/arm/boot/dts/vf610.dtsi
> +++ b/arch/arm/boot/dts/vf610.dtsi
> @@ -152,6 +152,15 @@
>  				clock-names = "pit";
>  			};
>  
> +			adc0: adc@4003b000 {
> +				compatible = "fsl,vf610-adc";
> +				reg = <0x4003b000 0x1000>;
> +				interrupts = <0 53 0x04>;
> +				clocks = <&clks VF610_CLK_ADC0>;
> +				clock-names = "adc";
> +				status = "disabled";
> +			};
> +
>  			wdog@4003e000 {
>  				compatible = "fsl,vf610-wdt", "fsl,imx21-wdt";
>  				reg = <0x4003e000 0x1000>;
> @@ -178,6 +187,14 @@
>  
>  				/* functions and groups pins */
>  
> +				adc0 {
> +					pinctrl_adc0_ad5: adc0_ad5 {
> +						fsl,pins = <
> +						VF610_PAD_PTC30__ADC0_SE5	0xa1
> +						>;
> +					};
> +				};
> +
>  				dcu0 {
>  					pinctrl_dcu0_1: dcu0grp_1 {
>  						fsl,pins = <
> @@ -450,6 +467,15 @@
>  				status = "disabled";
>  			};
>  
> +			adc1: adc@400bb000 {
> +				compatible = "fsl,vf610-adc";
> +				reg = <0x400bb000 0x1000>;
> +				interrupts = <0 54 0x04>;
> +				clocks = <&clks VF610_CLK_ADC1>;
> +				clock-names = "adc";
> +				status = "disabled";
> +			};
> +
>  			fec0: ethernet@400d0000 {
>  				compatible = "fsl,mvf600-fec";
>  				reg = <0x400d0000 0x1000>;
> -- 
> 1.7.2.rc3
> 
> 

Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[Shawn Guo]

Copy more DT folks and lists, as I want to make sure everyone agrees on
how the fixed regulators should organized in the device tree sources,
before I apply the patch.

On Sun, Jan 26, 2014 at 01:39:31PM +0800, Fugang Duan wrote:
> vf610 has two ADC controllers, and vf610-twr board ADC0_SE5 pin connect
> to sliding rheostat for ADC test, other ADC pins connect to connectors for
> future use.
> 
> Add support for ADC0_SE5.
> 
> CC: Shawn Guo <shawn.guo@linaro.org>
> CC: Jonathan Cameron <jic23@kernel.org>
> CC: Mark Rutland <mark.rutland@arm.com>
> CC: Otavio Salvador <otavio@ossystems.com.br>
> CC: Peter Meerwald <pmeerw@pmeerw.net>
> CC: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Fugang Duan <B38611@freescale.com>
> ---
>  arch/arm/boot/dts/vf610-twr.dts |   21 +++++++++++++++++++++
>  arch/arm/boot/dts/vf610.dtsi    |   26 ++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
> index c8047ca..d867be3 100644
> --- a/arch/arm/boot/dts/vf610-twr.dts
> +++ b/arch/arm/boot/dts/vf610-twr.dts
> @@ -34,6 +34,27 @@
>  		};
>  	};
>  
> +	regulators {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		reg_vcc_3v3_mcu: regulator@0 {
> +			compatible = "regulator-fixed";
> +			reg = <0>;
> +			regulator-name = "vcc_3v3_mcu";
> +			regulator-min-microvolt = <3300000>;
> +			regulator-max-microvolt = <3300000>;
> +		};
> +	};

Per discussion [1], Mark Rutland suggests that instead of organizing
the fixed regulator nodes in a simple-bus container, it should be put
under root node directly like below.

/ {
	reg_vcc_3v3_mcu: regulator_0 {
		compatible = "regulator-fixed";
		regulator-name = "vcc_3v3_mcu";
		regulator-min-microvolt = <3300000>;
		regulator-max-microvolt = <3300000>;
	};
};

Is this what all DT folks agree on?  At least the node name should be
'regulator-0' since it's more idiomatic to use '-' than '_' in node
name?

Shawn

[1] http://thread.gmane.org/gmane.linux.drivers.devicetree/61467/focus=300895

> +
> +};
> +
> +&adc0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_adc0_ad5>;
> +	vref-supply = <&reg_vcc_3v3_mcu>;
> +	status = "okay";
>  };
>  
>  &dspi0 {
> diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
> index d31ce1b..b5b21ea 100644
> --- a/arch/arm/boot/dts/vf610.dtsi
> +++ b/arch/arm/boot/dts/vf610.dtsi
> @@ -152,6 +152,15 @@
>  				clock-names = "pit";
>  			};
>  
> +			adc0: adc@4003b000 {
> +				compatible = "fsl,vf610-adc";
> +				reg = <0x4003b000 0x1000>;
> +				interrupts = <0 53 0x04>;
> +				clocks = <&clks VF610_CLK_ADC0>;
> +				clock-names = "adc";
> +				status = "disabled";
> +			};
> +
>  			wdog@4003e000 {
>  				compatible = "fsl,vf610-wdt", "fsl,imx21-wdt";
>  				reg = <0x4003e000 0x1000>;
> @@ -178,6 +187,14 @@
>  
>  				/* functions and groups pins */
>  
> +				adc0 {
> +					pinctrl_adc0_ad5: adc0_ad5 {
> +						fsl,pins = <
> +						VF610_PAD_PTC30__ADC0_SE5	0xa1
> +						>;
> +					};
> +				};
> +
>  				dcu0 {
>  					pinctrl_dcu0_1: dcu0grp_1 {
>  						fsl,pins = <
> @@ -450,6 +467,15 @@
>  				status = "disabled";
>  			};
>  
> +			adc1: adc@400bb000 {
> +				compatible = "fsl,vf610-adc";
> +				reg = <0x400bb000 0x1000>;
> +				interrupts = <0 54 0x04>;
> +				clocks = <&clks VF610_CLK_ADC1>;
> +				clock-names = "adc";
> +				status = "disabled";
> +			};
> +
>  			fec0: ethernet@400d0000 {
>  				compatible = "fsl,mvf600-fec";
>  				reg = <0x400d0000 0x1000>;
> -- 
> 1.7.2.rc3
> 
> 

[PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[Shawn Guo]

Copy more DT folks and lists, as I want to make sure everyone agrees on
how the fixed regulators should organized in the device tree sources,
before I apply the patch.

On Sun, Jan 26, 2014 at 01:39:31PM +0800, Fugang Duan wrote:
> vf610 has two ADC controllers, and vf610-twr board ADC0_SE5 pin connect
> to sliding rheostat for ADC test, other ADC pins connect to connectors for
> future use.
> 
> Add support for ADC0_SE5.
> 
> CC: Shawn Guo <shawn.guo@linaro.org>
> CC: Jonathan Cameron <jic23@kernel.org>
> CC: Mark Rutland <mark.rutland@arm.com>
> CC: Otavio Salvador <otavio@ossystems.com.br>
> CC: Peter Meerwald <pmeerw@pmeerw.net>
> CC: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Fugang Duan <B38611@freescale.com>
> ---
>  arch/arm/boot/dts/vf610-twr.dts |   21 +++++++++++++++++++++
>  arch/arm/boot/dts/vf610.dtsi    |   26 ++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
> index c8047ca..d867be3 100644
> --- a/arch/arm/boot/dts/vf610-twr.dts
> +++ b/arch/arm/boot/dts/vf610-twr.dts
> @@ -34,6 +34,27 @@
>  		};
>  	};
>  
> +	regulators {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		reg_vcc_3v3_mcu: regulator at 0 {
> +			compatible = "regulator-fixed";
> +			reg = <0>;
> +			regulator-name = "vcc_3v3_mcu";
> +			regulator-min-microvolt = <3300000>;
> +			regulator-max-microvolt = <3300000>;
> +		};
> +	};

Per discussion [1], Mark Rutland suggests that instead of organizing
the fixed regulator nodes in a simple-bus container, it should be put
under root node directly like below.

/ {
	reg_vcc_3v3_mcu: regulator_0 {
		compatible = "regulator-fixed";
		regulator-name = "vcc_3v3_mcu";
		regulator-min-microvolt = <3300000>;
		regulator-max-microvolt = <3300000>;
	};
};

Is this what all DT folks agree on?  At least the node name should be
'regulator-0' since it's more idiomatic to use '-' than '_' in node
name?

Shawn

[1] http://thread.gmane.org/gmane.linux.drivers.devicetree/61467/focus=300895

> +
> +};
> +
> +&adc0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_adc0_ad5>;
> +	vref-supply = <&reg_vcc_3v3_mcu>;
> +	status = "okay";
>  };
>  
>  &dspi0 {
> diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
> index d31ce1b..b5b21ea 100644
> --- a/arch/arm/boot/dts/vf610.dtsi
> +++ b/arch/arm/boot/dts/vf610.dtsi
> @@ -152,6 +152,15 @@
>  				clock-names = "pit";
>  			};
>  
> +			adc0: adc at 4003b000 {
> +				compatible = "fsl,vf610-adc";
> +				reg = <0x4003b000 0x1000>;
> +				interrupts = <0 53 0x04>;
> +				clocks = <&clks VF610_CLK_ADC0>;
> +				clock-names = "adc";
> +				status = "disabled";
> +			};
> +
>  			wdog at 4003e000 {
>  				compatible = "fsl,vf610-wdt", "fsl,imx21-wdt";
>  				reg = <0x4003e000 0x1000>;
> @@ -178,6 +187,14 @@
>  
>  				/* functions and groups pins */
>  
> +				adc0 {
> +					pinctrl_adc0_ad5: adc0_ad5 {
> +						fsl,pins = <
> +						VF610_PAD_PTC30__ADC0_SE5	0xa1
> +						>;
> +					};
> +				};
> +
>  				dcu0 {
>  					pinctrl_dcu0_1: dcu0grp_1 {
>  						fsl,pins = <
> @@ -450,6 +467,15 @@
>  				status = "disabled";
>  			};
>  
> +			adc1: adc at 400bb000 {
> +				compatible = "fsl,vf610-adc";
> +				reg = <0x400bb000 0x1000>;
> +				interrupts = <0 54 0x04>;
> +				clocks = <&clks VF610_CLK_ADC1>;
> +				clock-names = "adc";
> +				status = "disabled";
> +			};
> +
>  			fec0: ethernet at 400d0000 {
>  				compatible = "fsl,mvf600-fec";
>  				reg = <0x400d0000 0x1000>;
> -- 
> 1.7.2.rc3
> 
> 

Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[Shawn Guo]

On Sun, Jan 26, 2014 at 01:39:31PM +0800, Fugang Duan wrote:
> @@ -178,6 +187,14 @@
>  
>  				/* functions and groups pins */
>  
> +				adc0 {
> +					pinctrl_adc0_ad5: adc0_ad5 {
> +						fsl,pins = <
> +						VF610_PAD_PTC30__ADC0_SE5	0xa1
> +						>;
> +					};

We have been moving pinctrl setting nodes into board dts.  Please rebase
the patch against my for-next branch.

Shawn

> +				};
> +

RE: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[fugang.duan]

From: Shawn Guo <shawn.guo@linaro.org>
Data: Sunday, February 16, 2014 3:50 PM

>To: Duan Fugang-B38611
>Cc: jic23@kernel.org; sachin.kamat@linaro.org; pmeerw@pmeerw.net;
>lars@metafoo.de; mark.rutland@arm.com; linux-iio@vger.kernel.org
>Subject: Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support
>
>On Sun, Jan 26, 2014 at 01:39:31PM +0800, Fugang Duan wrote:
>> @@ -178,6 +187,14 @@
>>
>>  				/* functions and groups pins */
>>
>> +				adc0 {
>> +					pinctrl_adc0_ad5: adc0_ad5 {
>> +						fsl,pins = <
>> +						VF610_PAD_PTC30__ADC0_SE5	0xa1
>> +						>;
>> +					};
>
>We have been moving pinctrl setting nodes into board dts.  Please rebase the
>patch against my for-next branch.
>
>Shawn
>
>> +				};
>> +
>

Ok, thanks shawn.

RE: [PATCH v5 2/3] iio:adc:imx: add Freescale Vybrid vf610 adc driver

[fugang.duan]

From: Jonathan Cameron <jic23@kernel.org>
Data: Saturday, February 15, 2014 6:29 PM

>To: Duan Fugang-B38611
>Cc: shawn.guo@linaro.org; sachin.kamat@linaro.org; pmeerw@pmeerw.net;
>lars@metafoo.de; mark.rutland@arm.com; linux-iio@vger.kernel.org
>Subject: Re: [PATCH v5 2/3] iio:adc:imx: add Freescale Vybrid vf610 adc driver
>
>On 15/02/14 10:18, Jonathan Cameron wrote:
>> On 26/01/14 05:39, Fugang Duan wrote:
>>> Add Freescale Vybrid vf610 adc driver. The driver only support ADC
>>> software trigger.
>>>
>>> VF610 ADC device documentation is available at below reference manual
>>> (chapter 37):
>>> http://cache.freescale.com/files/32bit/doc/ref_manual/VYBRIDRM.pdf?
>>> fpsp=1&WT_TYPE=Reference%20Manuals&WT_VENDOR=FREESCALE&WT_FILE_FORMAT
>>> =pdf&WT_ASSET=Documentation
>>>
>>> CC: Shawn Guo <shawn.guo@linaro.org>
>>> CC: Jonathan Cameron <jic23@kernel.org>
>>> CC: Mark Rutland <mark.rutland@arm.com>
>>> CC: Otavio Salvador <otavio@ossystems.com.br>
>>> CC: Peter Meerwald <pmeerw@pmeerw.net>
>>> CC: Lars-Peter Clausen <lars@metafoo.de>
>>> Signed-off-by: Fugang Duan <B38611@freescale.com>
>>
>> I very much like the simplified approach you have taken for this
>> initial merge.  Much easier to start simple and build up to the more
>> complex functionality.
>>
>> There was one missing mutex unlock in an error path (noted below). I have
>> fixed that and applied to the togreg branch of iio.git.   This will initially
>> go out as the testing branch for the autobuilders to play with it.
>>
>> Also devm_irq requests always make me nervous, but I think this one is
>> fine so have left it alone
>>
>> Jonathan
>As an update I also fixed up the build - see below.  One day I'll no send out
>applied messages until after my local build tests have finished!
>
>Note that it is this sort of thing that has led me to pushing out a testing
>branch as that catches all the weird combinations that I don't.
>
>>> +
>>> +static const struct iio_info vf610_adc_iio_info = {
>>> +    .driver_module = THIS_MODULE,
>>> +    .read_raw = &vf610_read_raw,
>>> +    .write_raw = &vf610_write_raw,
>>> +    .debugfs_reg_access = &vf610_adc_reg_access,
>>> +    .attrs = &vf610_attribute_group, };
>>> +
>>> +static const struct of_device_id vf610_adc_match[] = {
>>> +    { .compatible = "fsl,vf610-adc", },
>>> +    { /* sentinel */ }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, vf610_adc_dt_ids);
>Your MODULE_DEVICE_TABLE needs to take a second variable that actually exists...
>Also needs to depend in the KCONFIG on OF to avoid build issues on other
>platforms.
>
>I've added both.
>

Jonathan,  thanks!

Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[Shawn Guo]

On Sun, Feb 16, 2014 at 03:48:53PM +0800, Shawn Guo wrote:
> Copy more DT folks and lists, as I want to make sure everyone agrees on
> how the fixed regulators should organized in the device tree sources,
> before I apply the patch.
> 
> On Sun, Jan 26, 2014 at 01:39:31PM +0800, Fugang Duan wrote:
> > vf610 has two ADC controllers, and vf610-twr board ADC0_SE5 pin connect
> > to sliding rheostat for ADC test, other ADC pins connect to connectors for
> > future use.
> > 
> > Add support for ADC0_SE5.
> > 
> > CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > CC: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > CC: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> > CC: Otavio Salvador <otavio-fKevB0iiKLMBZ+LybsDmbA@public.gmane.org>
> > CC: Peter Meerwald <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>
> > CC: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
> > Signed-off-by: Fugang Duan <B38611-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/vf610-twr.dts |   21 +++++++++++++++++++++
> >  arch/arm/boot/dts/vf610.dtsi    |   26 ++++++++++++++++++++++++++
> >  2 files changed, 47 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
> > index c8047ca..d867be3 100644
> > --- a/arch/arm/boot/dts/vf610-twr.dts
> > +++ b/arch/arm/boot/dts/vf610-twr.dts
> > @@ -34,6 +34,27 @@
> >  		};
> >  	};
> >  
> > +	regulators {
> > +		compatible = "simple-bus";
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		reg_vcc_3v3_mcu: regulator@0 {
> > +			compatible = "regulator-fixed";
> > +			reg = <0>;
> > +			regulator-name = "vcc_3v3_mcu";
> > +			regulator-min-microvolt = <3300000>;
> > +			regulator-max-microvolt = <3300000>;
> > +		};
> > +	};
> 
> Per discussion [1], Mark Rutland suggests that instead of organizing
> the fixed regulator nodes in a simple-bus container, it should be put
> under root node directly like below.
> 
> / {
> 	reg_vcc_3v3_mcu: regulator_0 {
> 		compatible = "regulator-fixed";
> 		regulator-name = "vcc_3v3_mcu";
> 		regulator-min-microvolt = <3300000>;
> 		regulator-max-microvolt = <3300000>;
> 	};
> };
> 
> Is this what all DT folks agree on?  At least the node name should be
> 'regulator-0' since it's more idiomatic to use '-' than '_' in node
> name?

It looks that Mark is giving up [1].  And we're fine with the original
code then.

Shawn

[1] http://thread.gmane.org/gmane.linux.drivers.devicetree/61894/focus=62405

Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[Shawn Guo]

On Sun, Feb 16, 2014 at 03:48:53PM +0800, Shawn Guo wrote:
> Copy more DT folks and lists, as I want to make sure everyone agrees on
> how the fixed regulators should organized in the device tree sources,
> before I apply the patch.
> 
> On Sun, Jan 26, 2014 at 01:39:31PM +0800, Fugang Duan wrote:
> > vf610 has two ADC controllers, and vf610-twr board ADC0_SE5 pin connect
> > to sliding rheostat for ADC test, other ADC pins connect to connectors for
> > future use.
> > 
> > Add support for ADC0_SE5.
> > 
> > CC: Shawn Guo <shawn.guo@linaro.org>
> > CC: Jonathan Cameron <jic23@kernel.org>
> > CC: Mark Rutland <mark.rutland@arm.com>
> > CC: Otavio Salvador <otavio@ossystems.com.br>
> > CC: Peter Meerwald <pmeerw@pmeerw.net>
> > CC: Lars-Peter Clausen <lars@metafoo.de>
> > Signed-off-by: Fugang Duan <B38611@freescale.com>
> > ---
> >  arch/arm/boot/dts/vf610-twr.dts |   21 +++++++++++++++++++++
> >  arch/arm/boot/dts/vf610.dtsi    |   26 ++++++++++++++++++++++++++
> >  2 files changed, 47 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
> > index c8047ca..d867be3 100644
> > --- a/arch/arm/boot/dts/vf610-twr.dts
> > +++ b/arch/arm/boot/dts/vf610-twr.dts
> > @@ -34,6 +34,27 @@
> >  		};
> >  	};
> >  
> > +	regulators {
> > +		compatible = "simple-bus";
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		reg_vcc_3v3_mcu: regulator@0 {
> > +			compatible = "regulator-fixed";
> > +			reg = <0>;
> > +			regulator-name = "vcc_3v3_mcu";
> > +			regulator-min-microvolt = <3300000>;
> > +			regulator-max-microvolt = <3300000>;
> > +		};
> > +	};
> 
> Per discussion [1], Mark Rutland suggests that instead of organizing
> the fixed regulator nodes in a simple-bus container, it should be put
> under root node directly like below.
> 
> / {
> 	reg_vcc_3v3_mcu: regulator_0 {
> 		compatible = "regulator-fixed";
> 		regulator-name = "vcc_3v3_mcu";
> 		regulator-min-microvolt = <3300000>;
> 		regulator-max-microvolt = <3300000>;
> 	};
> };
> 
> Is this what all DT folks agree on?  At least the node name should be
> 'regulator-0' since it's more idiomatic to use '-' than '_' in node
> name?

It looks that Mark is giving up [1].  And we're fine with the original
code then.

Shawn

[1] http://thread.gmane.org/gmane.linux.drivers.devicetree/61894/focus=62405

[PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[Shawn Guo]

On Sun, Feb 16, 2014 at 03:48:53PM +0800, Shawn Guo wrote:
> Copy more DT folks and lists, as I want to make sure everyone agrees on
> how the fixed regulators should organized in the device tree sources,
> before I apply the patch.
> 
> On Sun, Jan 26, 2014 at 01:39:31PM +0800, Fugang Duan wrote:
> > vf610 has two ADC controllers, and vf610-twr board ADC0_SE5 pin connect
> > to sliding rheostat for ADC test, other ADC pins connect to connectors for
> > future use.
> > 
> > Add support for ADC0_SE5.
> > 
> > CC: Shawn Guo <shawn.guo@linaro.org>
> > CC: Jonathan Cameron <jic23@kernel.org>
> > CC: Mark Rutland <mark.rutland@arm.com>
> > CC: Otavio Salvador <otavio@ossystems.com.br>
> > CC: Peter Meerwald <pmeerw@pmeerw.net>
> > CC: Lars-Peter Clausen <lars@metafoo.de>
> > Signed-off-by: Fugang Duan <B38611@freescale.com>
> > ---
> >  arch/arm/boot/dts/vf610-twr.dts |   21 +++++++++++++++++++++
> >  arch/arm/boot/dts/vf610.dtsi    |   26 ++++++++++++++++++++++++++
> >  2 files changed, 47 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
> > index c8047ca..d867be3 100644
> > --- a/arch/arm/boot/dts/vf610-twr.dts
> > +++ b/arch/arm/boot/dts/vf610-twr.dts
> > @@ -34,6 +34,27 @@
> >  		};
> >  	};
> >  
> > +	regulators {
> > +		compatible = "simple-bus";
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		reg_vcc_3v3_mcu: regulator at 0 {
> > +			compatible = "regulator-fixed";
> > +			reg = <0>;
> > +			regulator-name = "vcc_3v3_mcu";
> > +			regulator-min-microvolt = <3300000>;
> > +			regulator-max-microvolt = <3300000>;
> > +		};
> > +	};
> 
> Per discussion [1], Mark Rutland suggests that instead of organizing
> the fixed regulator nodes in a simple-bus container, it should be put
> under root node directly like below.
> 
> / {
> 	reg_vcc_3v3_mcu: regulator_0 {
> 		compatible = "regulator-fixed";
> 		regulator-name = "vcc_3v3_mcu";
> 		regulator-min-microvolt = <3300000>;
> 		regulator-max-microvolt = <3300000>;
> 	};
> };
> 
> Is this what all DT folks agree on?  At least the node name should be
> 'regulator-0' since it's more idiomatic to use '-' than '_' in node
> name?

It looks that Mark is giving up [1].  And we're fine with the original
code then.

Shawn

[1] http://thread.gmane.org/gmane.linux.drivers.devicetree/61894/focus=62405

RE: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[fugang.duan-KZfg59tc24xl57MIdRCFDg]

From: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Data: Wednesday, February 19, 2014 1:07 PM + 0800

>To: Duan Fugang-B38611
>Cc: jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; sachin.kamat-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org;
>lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org; mark.rutland-5wv7dgnIgG8@public.gmane.org; linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Rob Herring;
>Grant Likely; Pawel Moll; Ian Campbell; Kumar Gala; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
>linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>Subject: Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support
>
>On Sun, Feb 16, 2014 at 03:48:53PM +0800, Shawn Guo wrote:
>> Copy more DT folks and lists, as I want to make sure everyone agrees
>> on how the fixed regulators should organized in the device tree
>> sources, before I apply the patch.
>>
>> On Sun, Jan 26, 2014 at 01:39:31PM +0800, Fugang Duan wrote:
>> > vf610 has two ADC controllers, and vf610-twr board ADC0_SE5 pin
>> > connect to sliding rheostat for ADC test, other ADC pins connect to
>> > connectors for future use.
>> >
>> > Add support for ADC0_SE5.
>> >
>> > CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> > CC: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> > CC: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>> > CC: Otavio Salvador <otavio-fKevB0iiKLMBZ+LybsDmbA@public.gmane.org>
>> > CC: Peter Meerwald <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>
>> > CC: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
>> > Signed-off-by: Fugang Duan <B38611-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>> > ---
>> >  arch/arm/boot/dts/vf610-twr.dts |   21 +++++++++++++++++++++
>> >  arch/arm/boot/dts/vf610.dtsi    |   26 ++++++++++++++++++++++++++
>> >  2 files changed, 47 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/arch/arm/boot/dts/vf610-twr.dts
>> > b/arch/arm/boot/dts/vf610-twr.dts index c8047ca..d867be3 100644
>> > --- a/arch/arm/boot/dts/vf610-twr.dts
>> > +++ b/arch/arm/boot/dts/vf610-twr.dts
>> > @@ -34,6 +34,27 @@
>> >  		};
>> >  	};
>> >
>> > +	regulators {
>> > +		compatible = "simple-bus";
>> > +		#address-cells = <1>;
>> > +		#size-cells = <0>;
>> > +
>> > +		reg_vcc_3v3_mcu: regulator@0 {
>> > +			compatible = "regulator-fixed";
>> > +			reg = <0>;
>> > +			regulator-name = "vcc_3v3_mcu";
>> > +			regulator-min-microvolt = <3300000>;
>> > +			regulator-max-microvolt = <3300000>;
>> > +		};
>> > +	};
>>
>> Per discussion [1], Mark Rutland suggests that instead of organizing
>> the fixed regulator nodes in a simple-bus container, it should be put
>> under root node directly like below.
>>
>> / {
>> 	reg_vcc_3v3_mcu: regulator_0 {
>> 		compatible = "regulator-fixed";
>> 		regulator-name = "vcc_3v3_mcu";
>> 		regulator-min-microvolt = <3300000>;
>> 		regulator-max-microvolt = <3300000>;
>> 	};
>> };
>>
>> Is this what all DT folks agree on?  At least the node name should be
>> 'regulator-0' since it's more idiomatic to use '-' than '_' in node
>> name?
>
>It looks that Mark is giving up [1].  And we're fine with the original code
>then.
>
>Shawn

So, you apply the original patch ? Do I need to resend it again ?

Thanks,
Andy

RE: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[fugang.duan]

From: Shawn Guo <shawn.guo@linaro.org>
Data: Wednesday, February 19, 2014 1:07 PM + 0800

>To: Duan Fugang-B38611
>Cc: jic23@kernel.org; sachin.kamat@linaro.org; pmeerw@pmeerw.net;
>lars@metafoo.de; mark.rutland@arm.com; linux-iio@vger.kernel.org; Rob Herring;
>Grant Likely; Pawel Moll; Ian Campbell; Kumar Gala; devicetree@vger.kernel.org;
>linux-arm-kernel@lists.infradead.org
>Subject: Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support
>
>On Sun, Feb 16, 2014 at 03:48:53PM +0800, Shawn Guo wrote:
>> Copy more DT folks and lists, as I want to make sure everyone agrees
>> on how the fixed regulators should organized in the device tree
>> sources, before I apply the patch.
>>
>> On Sun, Jan 26, 2014 at 01:39:31PM +0800, Fugang Duan wrote:
>> > vf610 has two ADC controllers, and vf610-twr board ADC0_SE5 pin
>> > connect to sliding rheostat for ADC test, other ADC pins connect to
>> > connectors for future use.
>> >
>> > Add support for ADC0_SE5.
>> >
>> > CC: Shawn Guo <shawn.guo@linaro.org>
>> > CC: Jonathan Cameron <jic23@kernel.org>
>> > CC: Mark Rutland <mark.rutland@arm.com>
>> > CC: Otavio Salvador <otavio@ossystems.com.br>
>> > CC: Peter Meerwald <pmeerw@pmeerw.net>
>> > CC: Lars-Peter Clausen <lars@metafoo.de>
>> > Signed-off-by: Fugang Duan <B38611@freescale.com>
>> > ---
>> >  arch/arm/boot/dts/vf610-twr.dts |   21 +++++++++++++++++++++
>> >  arch/arm/boot/dts/vf610.dtsi    |   26 ++++++++++++++++++++++++++
>> >  2 files changed, 47 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/arch/arm/boot/dts/vf610-twr.dts
>> > b/arch/arm/boot/dts/vf610-twr.dts index c8047ca..d867be3 100644
>> > --- a/arch/arm/boot/dts/vf610-twr.dts
>> > +++ b/arch/arm/boot/dts/vf610-twr.dts
>> > @@ -34,6 +34,27 @@
>> >  		};
>> >  	};
>> >
>> > +	regulators {
>> > +		compatible = "simple-bus";
>> > +		#address-cells = <1>;
>> > +		#size-cells = <0>;
>> > +
>> > +		reg_vcc_3v3_mcu: regulator@0 {
>> > +			compatible = "regulator-fixed";
>> > +			reg = <0>;
>> > +			regulator-name = "vcc_3v3_mcu";
>> > +			regulator-min-microvolt = <3300000>;
>> > +			regulator-max-microvolt = <3300000>;
>> > +		};
>> > +	};
>>
>> Per discussion [1], Mark Rutland suggests that instead of organizing
>> the fixed regulator nodes in a simple-bus container, it should be put
>> under root node directly like below.
>>
>> / {
>> 	reg_vcc_3v3_mcu: regulator_0 {
>> 		compatible = "regulator-fixed";
>> 		regulator-name = "vcc_3v3_mcu";
>> 		regulator-min-microvolt = <3300000>;
>> 		regulator-max-microvolt = <3300000>;
>> 	};
>> };
>>
>> Is this what all DT folks agree on?  At least the node name should be
>> 'regulator-0' since it's more idiomatic to use '-' than '_' in node
>> name?
>
>It looks that Mark is giving up [1].  And we're fine with the original code
>then.
>
>Shawn

So, you apply the original patch ? Do I need to resend it again ?

Thanks,
Andy

[PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[fugang.duan at freescale.com]

From: Shawn Guo <shawn.guo@linaro.org>
Data: Wednesday, February 19, 2014 1:07 PM + 0800

>To: Duan Fugang-B38611
>Cc: jic23 at kernel.org; sachin.kamat at linaro.org; pmeerw at pmeerw.net;
>lars at metafoo.de; mark.rutland at arm.com; linux-iio at vger.kernel.org; Rob Herring;
>Grant Likely; Pawel Moll; Ian Campbell; Kumar Gala; devicetree at vger.kernel.org;
>linux-arm-kernel at lists.infradead.org
>Subject: Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support
>
>On Sun, Feb 16, 2014 at 03:48:53PM +0800, Shawn Guo wrote:
>> Copy more DT folks and lists, as I want to make sure everyone agrees
>> on how the fixed regulators should organized in the device tree
>> sources, before I apply the patch.
>>
>> On Sun, Jan 26, 2014 at 01:39:31PM +0800, Fugang Duan wrote:
>> > vf610 has two ADC controllers, and vf610-twr board ADC0_SE5 pin
>> > connect to sliding rheostat for ADC test, other ADC pins connect to
>> > connectors for future use.
>> >
>> > Add support for ADC0_SE5.
>> >
>> > CC: Shawn Guo <shawn.guo@linaro.org>
>> > CC: Jonathan Cameron <jic23@kernel.org>
>> > CC: Mark Rutland <mark.rutland@arm.com>
>> > CC: Otavio Salvador <otavio@ossystems.com.br>
>> > CC: Peter Meerwald <pmeerw@pmeerw.net>
>> > CC: Lars-Peter Clausen <lars@metafoo.de>
>> > Signed-off-by: Fugang Duan <B38611@freescale.com>
>> > ---
>> >  arch/arm/boot/dts/vf610-twr.dts |   21 +++++++++++++++++++++
>> >  arch/arm/boot/dts/vf610.dtsi    |   26 ++++++++++++++++++++++++++
>> >  2 files changed, 47 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/arch/arm/boot/dts/vf610-twr.dts
>> > b/arch/arm/boot/dts/vf610-twr.dts index c8047ca..d867be3 100644
>> > --- a/arch/arm/boot/dts/vf610-twr.dts
>> > +++ b/arch/arm/boot/dts/vf610-twr.dts
>> > @@ -34,6 +34,27 @@
>> >  		};
>> >  	};
>> >
>> > +	regulators {
>> > +		compatible = "simple-bus";
>> > +		#address-cells = <1>;
>> > +		#size-cells = <0>;
>> > +
>> > +		reg_vcc_3v3_mcu: regulator at 0 {
>> > +			compatible = "regulator-fixed";
>> > +			reg = <0>;
>> > +			regulator-name = "vcc_3v3_mcu";
>> > +			regulator-min-microvolt = <3300000>;
>> > +			regulator-max-microvolt = <3300000>;
>> > +		};
>> > +	};
>>
>> Per discussion [1], Mark Rutland suggests that instead of organizing
>> the fixed regulator nodes in a simple-bus container, it should be put
>> under root node directly like below.
>>
>> / {
>> 	reg_vcc_3v3_mcu: regulator_0 {
>> 		compatible = "regulator-fixed";
>> 		regulator-name = "vcc_3v3_mcu";
>> 		regulator-min-microvolt = <3300000>;
>> 		regulator-max-microvolt = <3300000>;
>> 	};
>> };
>>
>> Is this what all DT folks agree on?  At least the node name should be
>> 'regulator-0' since it's more idiomatic to use '-' than '_' in node
>> name?
>
>It looks that Mark is giving up [1].  And we're fine with the original code
>then.
>
>Shawn

So, you apply the original patch ? Do I need to resend it again ?

Thanks,
Andy

Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[Shawn Guo]

On Fri, Feb 21, 2014 at 05:17:20AM +0000, fugang.duan-KZfg59tc24xl57MIdRCFDg@public.gmane.org wrote:
> So, you apply the original patch ? Do I need to resend it again ?

You still need to rebase it against my for-next branch to resolve my
comment on pinctrl setting.

Shawn

Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[Shawn Guo]

On Fri, Feb 21, 2014 at 05:17:20AM +0000, fugang.duan@freescale.com wrote:
> So, you apply the original patch ? Do I need to resend it again ?

You still need to rebase it against my for-next branch to resolve my
comment on pinctrl setting.

Shawn

[PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[Shawn Guo]

On Fri, Feb 21, 2014 at 05:17:20AM +0000, fugang.duan at freescale.com wrote:
> So, you apply the original patch ? Do I need to resend it again ?

You still need to rebase it against my for-next branch to resolve my
comment on pinctrl setting.

Shawn

RE: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[fugang.duan]

From: Shawn Guo <shawn.guo@linaro.org>
Data: Friday, February 21, 2014 1:39 PM + 0800

>To: Duan Fugang-B38611
>Cc: jic23@kernel.org; sachin.kamat@linaro.org; pmeerw@pmeerw.net;
>lars@metafoo.de; mark.rutland@arm.com; linux-iio@vger.kernel.org; Rob Herring;
>Grant Likely; Pawel Moll; Ian Campbell; Kumar Gala; devicetree@vger.kernel.org;
>linux-arm-kernel@lists.infradead.org
>Subject: Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support
>
>On Fri, Feb 21, 2014 at 05:17:20AM +0000, fugang.duan@freescale.com wrote:
>> So, you apply the original patch ? Do I need to resend it again ?
>
>You still need to rebase it against my for-next branch to resolve my comment on
>pinctrl setting.
>
>Shawn

I send the patch to you after moving pinctrl setting to board dts.

Thanks,
Andy

RE: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[fugang.duan]

From: Shawn Guo <shawn.guo@linaro.org>
Data: Friday, February 21, 2014 1:39 PM + 0800

>To: Duan Fugang-B38611
>Cc: jic23@kernel.org; sachin.kamat@linaro.org; pmeerw@pmeerw.net;
>lars@metafoo.de; mark.rutland@arm.com; linux-iio@vger.kernel.org; Rob Herring;
>Grant Likely; Pawel Moll; Ian Campbell; Kumar Gala; devicetree@vger.kernel.org;
>linux-arm-kernel@lists.infradead.org
>Subject: Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support
>
>On Fri, Feb 21, 2014 at 05:17:20AM +0000, fugang.duan@freescale.com wrote:
>> So, you apply the original patch ? Do I need to resend it again ?
>
>You still need to rebase it against my for-next branch to resolve my comment on
>pinctrl setting.
>
>Shawn

I send the patch to you after moving pinctrl setting to board dts.

Thanks,
Andy

[PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support

[fugang.duan at freescale.com]

From: Shawn Guo <shawn.guo@linaro.org>
Data: Friday, February 21, 2014 1:39 PM + 0800

>To: Duan Fugang-B38611
>Cc: jic23 at kernel.org; sachin.kamat at linaro.org; pmeerw at pmeerw.net;
>lars at metafoo.de; mark.rutland at arm.com; linux-iio at vger.kernel.org; Rob Herring;
>Grant Likely; Pawel Moll; Ian Campbell; Kumar Gala; devicetree at vger.kernel.org;
>linux-arm-kernel at lists.infradead.org
>Subject: Re: [PATCH v5 1/3] ARM: dts: vf610-twr: Add ADC support
>
>On Fri, Feb 21, 2014 at 05:17:20AM +0000, fugang.duan at freescale.com wrote:
>> So, you apply the original patch ? Do I need to resend it again ?
>
>You still need to rebase it against my for-next branch to resolve my comment on
>pinctrl setting.
>
>Shawn

I send the patch to you after moving pinctrl setting to board dts.

Thanks,
Andy