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@xxxxxxxxxxxxxx>
> Signed-off-by: Denis Carikli <denis@xxxxxxxxxx>
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@xxxxxxxxxxxxxx>
> + *
> + * 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?
> + 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.
> + 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", ®);
> + 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?
> + }
> + 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@xxxxxxxxxxxxxx>");
> +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?
I'm far from a device tree expert however, so perhaps this is the
preferred method.
> +
> +#endif
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
