devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: "Shawn Guo" <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	"Samuel Ortiz" <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>,
	"Dmitry Torokhov"
	<dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	"Fabio Estevam"
	<festevam-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	"Peter Meerwald" <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>,
	"Hartmut Knaack" <knaack.h-Mmb7MZpHnFY@public.gmane.org>,
	"Denis Carikli" <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>,
	"Eric Bénard" <eric-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>,
	"Sascha Hauer" <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	"Lee Jones" <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	"Lars-Peter Clausen"
	<lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	"Rob Herring" <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	"Pawel Moll" <pawel.moll-5wv7dgnIgG8@public.gmane.org>,
	"Mark Rutland" <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	"Ian Campbell"
	<ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>,
	"Kumar Gala" <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Subject: Re: [PATCH v5 3/6] iio: adc: fsl,imx25-gcq driver
Date: Sun, 08 Feb 2015 11:56:57 +0000	[thread overview]
Message-ID: <54D74F09.2060106@kernel.org> (raw)
In-Reply-To: <20150129101104.GE27422-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

On 29/01/15 10:11, Markus Pargmann wrote:
> On Tue, Jan 27, 2015 at 08:30:56PM +0000, Jonathan Cameron wrote:
>> On 24/01/15 14:01, Markus Pargmann wrote:
>>> This is a conversion queue driver for the mx25 SoC. It uses the central
>>> ADC which is used by two seperate independent queues. This driver
>>> prepares different conversion configurations for each possible input.
>>> For a conversion it creates a conversionqueue of one item with the
>>> correct configuration for the chosen channel. It then executes the queue
>>> once and disables the conversion queue afterwards.
>>>
>>> The reference voltages are configurable through devicetree subnodes,
>>> depending on the connections of the ADC inputs.
>>>
>>> Signed-off-by: Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>> Signed-off-by: Denis Carikli <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
>> A couple of queries inline.
>>> ---
>>>
>>> Notes:
>>>     Changes in v5:
>>>      - Fixed locking
>>>      - Removed module owner
>>>
>>>  .../devicetree/bindings/iio/adc/fsl,imx25-gcq.txt  |  51 +++
>>>  drivers/iio/adc/Kconfig                            |   7 +
>>>  drivers/iio/adc/Makefile                           |   1 +
>>>  drivers/iio/adc/fsl-imx25-gcq.c                    | 369 +++++++++++++++++++++
>>>  include/dt-bindings/iio/adc/fsl-imx25-gcq.h        |  11 +
>>>  5 files changed, 439 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/fsl,imx25-gcq.txt
>>>  create mode 100644 drivers/iio/adc/fsl-imx25-gcq.c
>>>  create mode 100644 include/dt-bindings/iio/adc/fsl-imx25-gcq.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/fsl,imx25-gcq.txt b/Documentation/devicetree/bindings/iio/adc/fsl,imx25-gcq.txt
>>> new file mode 100644
>>> index 000000000000..d55b6b751349
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/adc/fsl,imx25-gcq.txt
>>> @@ -0,0 +1,51 @@
>>> +Freescale i.MX25 ADC GCQ device
>>> +
>>> +This is a generic conversion queue device that can convert any of the
>>> +analog inputs using the ADC unit of the i.MX25.
>>> +
>>> +Required properties:
>>> + - compatible: Should be "fsl,imx25-gcq".
>>> + - reg: Should be the register range of the module.
>>> + - interrupts: Should be the interrupt number of the module.
>>> +   Typically this is <1>.
>>> + - interrupt-parent: phandle to the tsadc module of the i.MX25.
>>> + - #address-cells: Should be <1> (setting for the subnodes)
>>> + - #size-cells: Should be <0> (setting for the subnodes)
>>> +
>>> +Optional properties:
>>> + - vref-supply: The regulator supplying the ADC reference voltage.
>>> +   Required when at least one subnode uses the external reference.
>>> +
>>> +Sub-nodes:
>>> +Optionally you can define subnodes which define the reference voltage
>>> +for the analog inputs.
>>> +
>>> +Required properties for subnodes:
>>> + - reg: Should be the number of the analog input.
>>> +     0: xp
>>> +     1: yp
>>> +     2: xn
>>> +     3: yn
>>> +     4: wiper
>>> +     5: inaux0
>>> +     6: inaux1
>>> +     7: inaux2
>>> + - fsl,adc-ref: specifies the reference input as defined in
>>> +     <dt-bindings/iio/adc/fsl-imx25-gcq.h>
>>> +     MX25_ADC_REF_INT and MX25_ADC_REF_EXT flags are supported.
>>> +
>>> +Example:
>>> +
>>> +	adc: adc@50030800 {
>>> +		compatible = "fsl,imx25-gcq";
>>> +		reg = <0x50030800 0x60>;
>>> +		interrupt-parent = <&tscadc>;
>>> +		interrupts = <1>;
>>> +		#address-cells = <1>;
>>> +		#size-cells = <0>;
>>> +
>>> +		inaux@5 {
>>> +			reg = <5>;
>>> +			fsl,adc-ref = <MX25_ADC_REF_INT>;
>>> +		};
>>> +	};
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 0f79e4725763..fccbb4bf44cc 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -143,6 +143,13 @@ config EXYNOS_ADC
>>>  	  of SoCs for drivers such as the touchscreen and hwmon to use to share
>>>  	  this resource.
>>>  
>>> +config FSL_MX25_ADC
>>> +	tristate "Freescale MX25 ADC driver"
>>> +	depends on MFD_MX25_TSADC
>>> +	help
>>> +	  Generic Conversion Queue driver used for general purpose ADC in the
>>> +	  MX25. This driver supports single measurements using the MX25 ADC.
>>> +
>>>  config LP8788_ADC
>>>  	tristate "LP8788 ADC driver"
>>>  	depends on MFD_LP8788
>>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>>> index 701fdb7c96aa..acab8d956371 100644
>>> --- a/drivers/iio/adc/Makefile
>>> +++ b/drivers/iio/adc/Makefile
>>> @@ -16,6 +16,7 @@ obj-$(CONFIG_AD799X) += ad799x.o
>>>  obj-$(CONFIG_AT91_ADC) += at91_adc.o
>>>  obj-$(CONFIG_AXP288_ADC) += axp288_adc.o
>>>  obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
>>> +obj-$(CONFIG_FSL_MX25_ADC) += fsl-imx25-gcq.o
>>>  obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
>>>  obj-$(CONFIG_MAX1027) += max1027.o
>>>  obj-$(CONFIG_MAX1363) += max1363.o
>>> diff --git a/drivers/iio/adc/fsl-imx25-gcq.c b/drivers/iio/adc/fsl-imx25-gcq.c
>>> new file mode 100644
>>> index 000000000000..c1ac5af41ec5
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/fsl-imx25-gcq.c
>>> @@ -0,0 +1,369 @@
>>> +/*
>>> + * Copyright 2014 Markus Pargmann, Pengutronix <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>> + *
>>> + * The code contained herein is licensed under the GNU General Public
>>> + * License. You may obtain a copy of the GNU General Public License
>>> + * Version 2 or later at the following locations:
>>> + *
>>> + * http://www.opensource.org/licenses/gpl-license.html
>>> + * http://www.gnu.org/copyleft/gpl.html
>>> + *
>>> + * This is the driver for the imx25 GCQ (Generic Conversion Queue)
>>> + * connected to the imx25 ADC.
>>> + */
>>> +
>>> +#include <dt-bindings/iio/adc/fsl-imx25-gcq.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/mfd/imx25-tsadc.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/regulator/consumer.h>
>>> +
>>> +#define MX25_GCQ_TIMEOUT (msecs_to_jiffies(2000))
>>> +
>>> +enum mx25_gcq_cfgs {
>>> +	MX25_CFG_XP = 0,
>>> +	MX25_CFG_YP,
>>> +	MX25_CFG_XN,
>>> +	MX25_CFG_YN,
>>> +	MX25_CFG_WIPER,
>>> +	MX25_CFG_INAUX0,
>>> +	MX25_CFG_INAUX1,
>>> +	MX25_CFG_INAUX2,
>>> +	MX25_NUM_CFGS,
>>> +};
>>> +
>>> +struct mx25_gcq_priv {
>>> +	struct regmap *regs;
>>> +	struct completion completed;
>>> +	unsigned int settling_time;
>>> +	struct clk *clk;
>>> +	int irq;
>>> +	struct regulator *ext_vref;
>>> +	u32 channel_vref_mv[MX25_NUM_CFGS];
>>> +};
>>> +
>>> +#define MX25_CQG_CHAN(chan, id) {\
>>> +	.type = IIO_VOLTAGE,\
>>> +	.indexed = 1,\
>>> +	.channel = chan,\
>>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),\
>>> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
>>> +	.datasheet_name = id,\
>>> +}
>>> +
>>> +static const struct iio_chan_spec mx25_gcq_channels[MX25_NUM_CFGS] = {
>>> +	MX25_CQG_CHAN(0, "xp"),
>>> +	MX25_CQG_CHAN(1, "yp"),
>>> +	MX25_CQG_CHAN(2, "xn"),
>>> +	MX25_CQG_CHAN(3, "yn"),
>>> +	MX25_CQG_CHAN(4, "wiper"),
>>> +	MX25_CQG_CHAN(5, "inaux0"),
>>> +	MX25_CQG_CHAN(6, "inaux1"),
>>> +	MX25_CQG_CHAN(7, "inaux2"),
>>> +};
>>> +
>>> +static void mx25_gcq_disable_eoq(struct mx25_gcq_priv *priv)
>>> +{
>>> +	regmap_update_bits(priv->regs, MX25_ADCQ_MR, MX25_ADCQ_MR_EOQ_IRQ,
>>> +			   MX25_ADCQ_MR_EOQ_IRQ);
>>> +}
>>> +
>>> +static void mx25_gcq_enable_eoq(struct mx25_gcq_priv *priv)
>>> +{
>>> +	regmap_update_bits(priv->regs, MX25_ADCQ_MR,
>>> +			   MX25_ADCQ_MR_EOQ_IRQ, 0);
>>> +}
>>> +
>>> +static irqreturn_t mx25_gcq_irq(int irq, void *data)
>>> +{
>>> +	struct mx25_gcq_priv *priv = data;
>>> +	u32 stats;
>>> +
>>> +	regmap_read(priv->regs, MX25_ADCQ_SR, &stats);
>>> +
>>> +	if (stats & MX25_ADCQ_SR_EOQ) {
>>> +		mx25_gcq_disable_eoq(priv);
>>> +		complete(&priv->completed);
>>> +	}
>>> +
>>> +	/* Disable conversion queue run */
>>> +	regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_FQS, 0);
>>> +
>>> +	/* Acknowledge all possible irqs */
>>> +	regmap_write(priv->regs, MX25_ADCQ_SR, MX25_ADCQ_SR_FRR |
>>> +		     MX25_ADCQ_SR_FUR | MX25_ADCQ_SR_FOR |
>>> +		     MX25_ADCQ_SR_EOQ | MX25_ADCQ_SR_PD);
>>> +
>>> +	return IRQ_HANDLED;
>>> +}
>>> +
>>> +static int mx25_gcq_read_raw(struct iio_dev *indio_dev,
>>> +			     struct iio_chan_spec const *chan, int *val,
>>> +			     int *val2, long mask)
>>> +{
>>> +	struct mx25_gcq_priv *priv = iio_priv(indio_dev);
>>> +	long timeout;
>>> +	u32 data;
>>> +	int ret = 0;
>>> +
>>> +	switch (mask) {
>>> +	case IIO_CHAN_INFO_RAW:
>>> +		mutex_lock(&indio_dev->mlock);
>>> +
>>> +		/* Setup the configuration we want to use */
>>> +		regmap_write(priv->regs, MX25_ADCQ_ITEM_7_0,
>>> +			     MX25_ADCQ_ITEM(0, chan->channel));
>>> +
>>> +		mx25_gcq_enable_eoq(priv);
>>> +
>>> +		/* Trigger queue for one run */
>>> +		regmap_update_bits(priv->regs, MX25_ADCQ_CR, MX25_ADCQ_CR_FQS,
>>> +				   MX25_ADCQ_CR_FQS);
>>> +
>>> +		timeout = wait_for_completion_interruptible_timeout(
>>> +			&priv->completed, MX25_GCQ_TIMEOUT);
>>> +		if (timeout < 0) {
>>> +			dev_err(&indio_dev->dev,
>>> +				"ADC wait for measurement failed\n");
>>> +			ret = timeout;
>>> +			goto info_raw_out;
>>> +		} else if (timeout == 0) {
>>> +			dev_err(&indio_dev->dev, "ADC timed out\n");
>>> +			ret = -ETIMEDOUT;
>>> +			goto info_raw_out;
>>> +		}
>>> +
>>> +		regmap_read(priv->regs, MX25_ADCQ_FIFO, &data);
>>> +
>>> +		*val = MX25_ADCQ_FIFO_DATA(data);
>>> +		ret = IIO_VAL_INT;
>>> +info_raw_out:
>> Having this label and exit path in the middle is ugly.
>> Perhaps break out this case statement content as a separate function?
> 
> Sounds like a good idea.
> 
>>> +		mutex_unlock(&indio_dev->mlock);
>>> +
>>> +		return ret;
>>> +
>>> +	case IIO_CHAN_INFO_SCALE:
>>> +		*val = priv->channel_vref_mv[chan->channel];
>>> +		*val2 = 12;
>>> +		return IIO_VAL_FRACTIONAL_LOG2;
>>> +
>>> +	default:
>>> +		return -EINVAL;
>>> +	}
>>> +}
>>> +
>>> +static const struct iio_info mx25_gcq_iio_info = {
>>> +	.read_raw = mx25_gcq_read_raw,
>>> +};
>>> +
>>> +static const struct regmap_config mx25_gcq_regconfig = {
>>> +	.max_register = 0x5c,
>>> +	.reg_bits = 32,
>>> +	.val_bits = 32,
>>> +	.reg_stride = 4,
>>> +};
>>> +
>>> +static int mx25_gcq_setup_cfgs(struct platform_device *pdev,
>>> +			       struct mx25_gcq_priv *priv)
>>> +{
>>> +	struct device_node *np = pdev->dev.of_node;
>>> +	struct device_node *child;
>>> +	struct device *dev = &pdev->dev;
>>> +	int ret, i;
>>> +
>>> +	/*
>>> +	 * Setup all configurations registers with a default conversion
>>> +	 * configuration for each input
>>> +	 */
>>> +	for (i = 0; i < MX25_NUM_CFGS; ++i)
>>> +		regmap_write(priv->regs, MX25_ADCQ_CFG(i),
>>> +			     MX25_ADCQ_CFG_YPLL_OFF |
>>> +			     MX25_ADCQ_CFG_XNUR_OFF |
>>> +			     MX25_ADCQ_CFG_XPUL_OFF |
>>> +			     MX25_ADCQ_CFG_REFP_INT |
>>> +			     (i << 4) |
>> Again, given all the definitions i'd expect the shift to
>> be a defined constant as well.  Perhaps even a macro taking
>> i as an arguement.
> 
> Added a macro that shifts the value.
> 
>>> +			     MX25_ADCQ_CFG_REFN_NGND2);
>>> +
>>> +	for_each_child_of_node(np, child) {
>>> +		u32 reg;
>>> +		u32 refp;
>>> +		u32 adc_ref;
>>> +
>>> +		ret = of_property_read_u32(child, "reg", &reg);
>>> +		if (ret) {
>>> +			dev_err(dev, "Failed to get reg property\n");
>>> +			return ret;
>>> +		}
>>> +
>>> +		if (reg >= MX25_NUM_CFGS) {
>>> +			dev_err(dev,
>>> +				"reg value is greater than the number of available configuration registers\n");
>>> +			return -EINVAL;
>>> +		}
>>> +
>>> +		ret = of_property_read_u32(child, "fsl,adc-ref", &adc_ref);
>>> +		if (ret) {
>>> +			dev_err(dev, "Failed to get fsl,adc-ref property\n");
>>> +			return ret;
>>> +		}
>>> +
>>> +		if (adc_ref != MX25_ADC_REF_INT &&
>>> +		    adc_ref != MX25_ADC_REF_EXT) {
>>> +			dev_err(dev, "Invalid fsl,adc-refp property value %d\n",
>>> +				adc_ref);
>>> +			return -EINVAL;
>>> +		}
>>> +
>>> +		switch (adc_ref) {
>>> +		case MX25_ADC_REF_EXT:
>>> +			if (IS_ERR_OR_NULL(priv->ext_vref)) {
>>> +				dev_err(dev,
>>> +					"No regulator found for the external vref\n");
>>> +				return -EINVAL;
>>> +			}
>>> +			priv->channel_vref_mv[reg] =
>>> +				regulator_get_voltage(priv->ext_vref);
>>> +			refp = MX25_ADCQ_CFG_REFP_EXT;
>>> +			break;
>>> +		case MX25_ADC_REF_INT:
>>> +			priv->channel_vref_mv[reg] = 2500;
>>> +			refp = MX25_ADCQ_CFG_REFP_INT;
>>> +			break;
>>> +		default:
>>> +			return -EINVAL;
>>> +		}
>>> +
>>> +		regmap_update_bits(priv->regs, MX25_ADCQ_CFG(reg),
>>> +				   MX25_ADCQ_CFG_REFP_MASK |
>>> +				   MX25_ADCQ_CFG_REFN_MASK, (refp << 7) |
>>> +				   (MX25_ADCQ_CFG_REFN_NGND << 2));
>> It's a little inconsistent to define constants for the masks, but not
>> the shifts.  Particularly given these constants are already shifted in
>> their definitions...  Is that a bug?
> 
> Yes that definetly looks like a bug, thanks. Fixed and added a macro for
> refp.
> 
>>> +	}
>>> +	regmap_update_bits(priv->regs, MX25_ADCQ_CR,
>>> +			   MX25_ADCQ_CR_FRST | MX25_ADCQ_CR_QRST,
>>> +			   MX25_ADCQ_CR_FRST | MX25_ADCQ_CR_QRST);
>>> +
>>> +	regmap_write(priv->regs, MX25_ADCQ_CR,
>>> +		     MX25_ADCQ_CR_PDMSK |
>>> +		     MX25_ADCQ_CR_QSM_FQS);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int mx25_gcq_probe(struct platform_device *pdev)
>>> +{
>>> +	struct iio_dev *indio_dev;
>>> +	struct mx25_gcq_priv *priv;
>>> +	struct mx25_tsadc *tsadc = dev_get_drvdata(pdev->dev.parent);
>>> +	struct device *dev = &pdev->dev;
>>> +	struct resource *res;
>>> +	void __iomem *mem;
>>> +	int ret;
>>> +
>>> +	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
>>> +	if (!indio_dev)
>>> +		return -ENOMEM;
>>> +
>>> +	priv = iio_priv(indio_dev);
>>> +
>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +	mem = devm_ioremap_resource(dev, res);
>>> +	if (!mem)
>>> +		return -ENOMEM;
>>> +
>>> +	priv->regs = devm_regmap_init_mmio(dev, mem, &mx25_gcq_regconfig);
>>> +	if (IS_ERR(priv->regs)) {
>>> +		dev_err(dev, "Failed to initialize regmap\n");
>>> +		return PTR_ERR(priv->regs);
>>> +	}
>>> +
>>> +	init_completion(&priv->completed);
>>> +
>>> +	/* Optional external regulator */
>>> +	priv->ext_vref = devm_regulator_get(&pdev->dev, "vref");
>>> +	if (!IS_ERR_OR_NULL(priv->ext_vref)) {
>>> +		ret = regulator_enable(priv->ext_vref);
>>> +		if (ret)
>>> +			return ret;
>>> +	}
>>> +
>>> +	ret = mx25_gcq_setup_cfgs(pdev, priv);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	priv->clk = tsadc->clk;
>>> +	ret = clk_prepare_enable(priv->clk);
>>> +	if (ret) {
>>> +		dev_err(dev, "Failed to enable clock\n");
>>> +		return ret;
>>> +	}
>>> +
>>> +	priv->irq = platform_get_irq(pdev, 0);
>>> +	if (priv->irq <= 0) {
>>> +		dev_err(dev, "Failed to get IRQ\n");
>>> +		ret = priv->irq;
>>> +		goto err_clk_unprepare;
>>> +	}
>>> +
>>> +	ret = request_irq(priv->irq, mx25_gcq_irq, 0, pdev->name, priv);
>>> +	if (ret) {
>>> +		dev_err(dev, "Failed requesting IRQ\n");
>>> +		goto err_clk_unprepare;
>>> +	}
>>> +
>>> +	indio_dev->dev.parent = &pdev->dev;
>>> +	indio_dev->channels = mx25_gcq_channels;
>>> +	indio_dev->num_channels = ARRAY_SIZE(mx25_gcq_channels);
>>> +	indio_dev->info = &mx25_gcq_iio_info;
>>> +
>>> +	ret = iio_device_register(indio_dev);
>>> +	if (ret) {
>>> +		dev_err(dev, "Failed to register iio device\n");
>>> +		goto err_irq_free;
>>> +	}
>>> +
>>> +	platform_set_drvdata(pdev, priv);
>>> +
>>> +	return 0;
>>> +
>>> +err_irq_free:
>>> +	free_irq(priv->irq, (void *)priv);
>>> +err_clk_unprepare:
>>> +	clk_disable_unprepare(priv->clk);
>>> +	return ret;
>>> +}
>>> +
>>> +static int mx25_gcq_remove(struct platform_device *pdev)
>>> +{
>>> +	struct mx25_gcq_priv *priv = platform_get_drvdata(pdev);
>>> +	struct iio_dev *indio_dev = iio_priv_to_dev(pdev);
>>> +
>>> +	iio_device_unregister(indio_dev);
>>> +	free_irq(priv->irq, priv);
>>> +	clk_disable_unprepare(priv->clk);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static struct of_device_id mx25_gcq_ids[] = {
>>> +	{ .compatible = "fsl,imx25-gcq", },
>>> +	{ /* Sentinel */ }
>>> +};
>>> +
>>> +static struct platform_driver mx25_gcq_driver = {
>>> +	.driver		= {
>>> +		.name	= "mx25-gcq",
>>> +		.of_match_table = mx25_gcq_ids,
>>> +	},
>>> +	.probe		= mx25_gcq_probe,
>>> +	.remove		= mx25_gcq_remove,
>>> +};
>>> +module_platform_driver(mx25_gcq_driver);
>>> +
>>> +MODULE_DESCRIPTION("ADC driver for Freescale mx25");
>>> +MODULE_AUTHOR("Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>");
>>> +MODULE_LICENSE("GPL v2");
>>> diff --git a/include/dt-bindings/iio/adc/fsl-imx25-gcq.h b/include/dt-bindings/iio/adc/fsl-imx25-gcq.h
>>> new file mode 100644
>>> index 000000000000..486dce735ac9
>>> --- /dev/null
>>> +++ b/include/dt-bindings/iio/adc/fsl-imx25-gcq.h
>>> @@ -0,0 +1,11 @@
>>> +/*
>>> + * This header provides constants for configuring the I.MX25 ADC
>>> + */
>>> +
>>> +#ifndef _DT_BINDINGS_IIO_ADC_FS_IMX25_GCQ_H
>>> +#define _DT_BINDINGS_IIO_ADC_FS_IMX25_GCQ_H
>>> +
>>> +#define MX25_ADC_REF_INT	0 /* Internal voltage reference */
>>> +#define MX25_ADC_REF_EXT	1 /* External voltage reference */
>> Seems to me that this could be dealt with using a boolean property.
>> Afterall, we need to get a reference from somewhere, so if it isn't
>> internal it must be external?
> 
> The driver is currently very simple, the ADC itself is capable of much
> more combinations and complicated conversion queues. Internal and
> external references are not the only possible references.
Fair enough then.  Thanks for clarifying this!
> There is also
> YP and XP which are not supported by the driver yet. I will change this
> to the actual register values representing these references. At a second
> look on this it may also be better to change the DT-binding from
> 'fsl,adc-ref' to 'fsl,adc-refp' as it is actually the positive reference
> voltage. The negative reference is hardcoded in the driver for the
> moment but the DT-bindings could be extended later if necessary.
> 
> Added refp and refn now with all reference values.
> 
> Thanks,
> 
> Markus
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2015-02-08 11:56 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-24 14:01 [PATCH v5 0/6] imx25 adc and touchscreen driver Markus Pargmann
2015-01-24 14:01 ` [PATCH v5 1/6] mfd: fsl imx25 Touchscreen ADC driver Markus Pargmann
     [not found]   ` <1422108103-15052-2-git-send-email-mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2015-02-16 13:38     ` Lee Jones
2015-02-17 18:54       ` Fabio Estevam
2015-02-18  8:01         ` Lee Jones
2015-02-18 11:57           ` Fabio Estevam
2015-02-18 12:08             ` Lee Jones
2015-02-26 15:32               ` Markus Pargmann
2015-02-17 19:14     ` Uwe Kleine-König
2015-03-02 19:12       ` Markus Pargmann
2015-01-24 14:01 ` [PATCH v5 2/6] ARM: dt: Binding documentation for imx25 ADC/TSC Markus Pargmann
2015-01-24 14:01 ` [PATCH v5 3/6] iio: adc: fsl,imx25-gcq driver Markus Pargmann
2015-01-27 20:30   ` Jonathan Cameron
     [not found]     ` <54C7F580.1030901-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-01-29 10:11       ` Markus Pargmann
     [not found]         ` <20150129101104.GE27422-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2015-02-08 11:56           ` Jonathan Cameron [this message]
     [not found]   ` <1422108103-15052-4-git-send-email-mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2015-01-29 14:51     ` Fabio Estevam
2015-01-30  7:27       ` Markus Pargmann
2015-01-24 14:01 ` [PATCH v5 4/6] input: touchscreen: imx25 tcq driver Markus Pargmann
     [not found] ` <1422108103-15052-1-git-send-email-mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2015-01-24 14:01   ` [PATCH v5 5/6] ARM: dts: imx25: Add TSC and ADC support Markus Pargmann
2015-01-24 14:01 ` [PATCH v5 6/6] ARM: imx_v4_v5_defconfig: Add I.MX25 Touchscreen controller " Markus Pargmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54D74F09.2060106@kernel.org \
    --to=jic23-dgejt+ai2ygdnm+yrofe0a@public.gmane.org \
    --cc=denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=eric-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org \
    --cc=festevam-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org \
    --cc=kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org \
    --cc=knaack.h-Mmb7MZpHnFY@public.gmane.org \
    --cc=lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org \
    --cc=lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
    --cc=mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org \
    --cc=pawel.moll-5wv7dgnIgG8@public.gmane.org \
    --cc=pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org \
    --cc=shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).