Message-ID: <561A6ACD.7000704@kernel.org>
Date:	Sun, 11 Oct 2015 14:57:33 +0100
From:	Jonathan Cameron <jic23@...nel.org>
To:	Haibo Chen <haibo.chen@...escale.com>, robh+dt@...nel.org,
	pawel.moll@....com, mark.rutland@....com,
	ijc+devicetree@...lion.org.uk, galak@...eaurora.org,
	shawnguo@...nel.org, kernel@...gutronix.de,
	mturquette@...libre.com, sboyd@...eaurora.org
Cc:	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-clk@...r.kernel.org,
	linux-iio@...r.kernel.org
Subject: Re: [PATCH 1/5] iio: adc: add IMX7D ADC driver support

On 08/10/15 11:59, Haibo Chen wrote:
> Freescale i.MX7D soc contains a new ADC IP. This patch add this ADC
> driver support, and the driver only support ADC software trigger.
> 
> Signed-off-by: Haibo Chen <haibo.chen@...escale.com>
Hi Haibo,

A very nice clean driver. I've noted a few minor stylistic things inline that
I would like you to clean up.

In particular we are fairly strict in IIO about prefixing register defines
etc.  It is much easier to just do this be default than to constantly
be wary of where a naming clash might occur with a core header in the
future.

Thanks,

Jonathan
> ---
>  drivers/iio/adc/Kconfig     |   9 +
>  drivers/iio/adc/Makefile    |   1 +
>  drivers/iio/adc/imx7d_adc.c | 586 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 596 insertions(+)
>  create mode 100644 drivers/iio/adc/imx7d_adc.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 7868c74..bf0611c 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -194,6 +194,15 @@ config HI8435
>  	  This driver can also be built as a module. If so, the module will be
>  	  called hi8435.
>  
> +config IMX7D_ADC
> +	tristate "IMX7D ADC driver"
> +	depends on OF
> +	help
> +	  Say yes here to build support for IMX7D ADC.
> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called imx7d_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 99b37a9..282ffc01 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_CC10001_ADC) += cc10001_adc.o
>  obj-$(CONFIG_DA9150_GPADC) += da9150-gpadc.o
>  obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
>  obj-$(CONFIG_HI8435) += hi8435.o
> +obj-$(CONFIG_IMX7D_ADC) += imx7d_adc.o
>  obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
>  obj-$(CONFIG_MAX1027) += max1027.o
>  obj-$(CONFIG_MAX1363) += max1363.o
> diff --git a/drivers/iio/adc/imx7d_adc.c b/drivers/iio/adc/imx7d_adc.c
> new file mode 100644
> index 0000000..8be8bf8
> --- /dev/null
> +++ b/drivers/iio/adc/imx7d_adc.c
> @@ -0,0 +1,586 @@
> +/*
> + * Freescale ADC driver
> + *
> + * Copyright (C) 2015 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.
> + */
> +
> +#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 "imx7d_adc"
> +
> +/* ADC register */
Please prefix defines with IMX7D_ or similar.
Avoids possible naming clashes in the future....

> +#define REG_ADC_CH_A_CFG1		0x00
> +#define REG_ADC_CH_A_CFG2		0x10
> +#define REG_ADC_CH_B_CFG1		0x20
> +#define REG_ADC_CH_B_CFG2		0x30
> +#define REG_ADC_CH_C_CFG1		0x40
> +#define REG_ADC_CH_C_CFG2		0x50
> +#define REG_ADC_CH_D_CFG1		0x60
> +#define REG_ADC_CH_D_CFG2		0x70
> +#define REG_ADC_CH_SW_CFG		0x80
> +#define REG_ADC_TIMER_UNIT		0x90
> +#define REG_ADC_DMA_FIFO		0xa0
> +#define REG_ADC_FIFO_STATUS		0xb0
> +#define REG_ADC_INT_SIG_EN		0xc0
> +#define REG_ADC_INT_EN			0xd0
> +#define REG_ADC_INT_STATUS		0xe0
> +#define REG_ADC_CHA_B_CNV_RSLT		0xf0
> +#define REG_ADC_CHC_D_CNV_RSLT		0x100
> +#define REG_ADC_CH_SW_CNV_RSLT		0x110
> +#define REG_ADC_DMA_FIFO_DAT		0x120
> +#define REG_ADC_ADC_CFG			0x130
> +
> +#define CHANNEL_REG_SHIF		0x20
> +
> +#define CHANNEL_EN			(0x1 << 31)
> +#define CHANNEL_DISABLE			(0x0 << 31)
> +#define CHANNEL_SINGLE			(0x1 << 30)
> +#define CHANNEL_AVG_EN			(0x1 << 29)
> +#define CHANNEL_SEL_SHIF		24
> +
> +#define PRE_DIV_4			(0x0 << 29)
> +#define PRE_DIV_8			(0x1 << 29)
> +#define PRE_DIV_16			(0x2 << 29)
> +#define PRE_DIV_32			(0x3 << 29)
> +#define PRE_DIV_64			(0x4 << 29)
> +#define PRE_DIV_128			(0x5 << 29)

Please name these in a fashion that makes it clear which
register they are fields within.
IMX7D_REG_ADC_TIMER_UNIT_ADC_CLK_DOWN for example.
> +
> +#define ADC_CLK_DOWN			(0x1 << 31)
> +#define ADC_POWER_DOWN			(0x1 << 1)
> +#define ADC_EN				0x1
> +
> +#define AVG_NUM_4			(0x0 << 12)
> +#define AVG_NUM_8			(0x1 << 12)
> +#define AVG_NUM_16			(0x2 << 12)
> +#define AVG_NUM_32			(0x3 << 12)
> +
Where it is really a single bit, please use the BIT macro
to make that clear.
> +#define CHA_COV_INT_EN			(0x1 << 8)
> +#define CHB_COV_INT_EN			(0x1 << 9)
> +#define CHC_COV_INT_EN			(0x1 << 10)
> +#define CHD_COV_INT_EN			(0x1 << 11)
> +#define CHANNEL_INT_EN			(CHA_COV_INT_EN | CHB_COV_INT_EN | \
> +					CHC_COV_INT_EN | CHD_COV_INT_EN)
> +#define CHANNEL_INT_STATUS		0xf00
> +
> +#define IMX7D_ADC_TIMEOUT		msecs_to_jiffies(100)
> +
> +#define IMX7D_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),	\
> +}
I'd prefer this to be futher down.  Preferabbly just before it is
used so that when reviewing there is no need to jump backwards
and forwards through the file.
> +
> +enum clk_pre_div {
> +	CLK_PRE_DIV_4,
> +	CLK_PRE_DIV_8,
> +	CLK_PRE_DIV_16,
> +	CLK_PRE_DIV_32,
> +	CLK_PRE_DIV_64,
> +	CLK_PRE_DIV_128,
> +};
> +
> +enum average_num {
> +	AVERAGE_NUM_4,
> +	AVERAGE_NUM_8,
> +	AVERAGE_NUM_16,
> +	AVERAGE_NUM_32,
> +};
> +
> +struct imx7d_adc_feature {
> +	enum clk_pre_div clk_pre_div;
> +	enum average_num avg_num;
> +
> +	u32 core_time_unit;	/* define the sample rate */
> +
> +	bool dma_en;
> +	bool average_en;
> +};
> +
> +struct imx7d_adc {
> +	struct device *dev;
> +	void __iomem *regs;
> +	struct clk *clk;
> +
> +	u32 vref_uv;
> +	u32 value;
> +	u32 channel;
> +	u32 pre_div_num;
> +
> +	struct regulator *vref;
> +	struct imx7d_adc_feature adc_feature;
> +
> +	struct completion completion;
> +};
> +
> +static const struct iio_chan_spec imx7d_adc_iio_channels[] = {
> +	IMX7D_ADC_CHAN(0, IIO_VOLTAGE),
> +	IMX7D_ADC_CHAN(1, IIO_VOLTAGE),
> +	IMX7D_ADC_CHAN(2, IIO_VOLTAGE),
> +	IMX7D_ADC_CHAN(3, IIO_VOLTAGE),
> +	IMX7D_ADC_CHAN(4, IIO_VOLTAGE),
> +	IMX7D_ADC_CHAN(5, IIO_VOLTAGE),
> +	IMX7D_ADC_CHAN(6, IIO_VOLTAGE),
> +	IMX7D_ADC_CHAN(7, IIO_VOLTAGE),
> +	IMX7D_ADC_CHAN(8, IIO_VOLTAGE),
> +	IMX7D_ADC_CHAN(9, IIO_VOLTAGE),
> +	IMX7D_ADC_CHAN(10, IIO_VOLTAGE),
> +	IMX7D_ADC_CHAN(11, IIO_VOLTAGE),
> +	IMX7D_ADC_CHAN(12, IIO_VOLTAGE),
> +	IMX7D_ADC_CHAN(13, IIO_VOLTAGE),
> +	IMX7D_ADC_CHAN(14, IIO_VOLTAGE),
> +	IMX7D_ADC_CHAN(15, IIO_VOLTAGE),
> +	/* sentinel */
There isn't a sentinel here so drop the comment.
> +};
> +
> +static void imx7d_feature_config(struct imx7d_adc *info)
> +{
> +	info->adc_feature.clk_pre_div = CLK_PRE_DIV_4;
> +	info->adc_feature.avg_num = AVERAGE_NUM_32;
> +
> +	info->adc_feature.core_time_unit = 1;
> +
> +	info->adc_feature.dma_en = false;
> +	info->adc_feature.average_en = true;
Right now there doesn't seem to be any means fo modifying these settings
so it might be best to drop them for now and bring them back in when
this functionality is added.

Lars-Peter Clausen has dma buffer patches under review at the moment
that might be of interest to you.
> +}
> +
> +static void imx7d_adc_sample_set(struct imx7d_adc *info)
> +{
> +	struct imx7d_adc_feature *adc_feature = &info->adc_feature;
> +	u32 i;
> +	u32 time_cfg = 0;
> +
> +	/*
> +	 * Before sample set, disable channel A,B,C,D. Here we
> +	 * set the bit 31 of register REG_ADC_CH_A\B\C\D_CFG1.
> +	 */
You are clearing the bit I think rather than setting it?
> +	for (i = 0; i < 4; i++)
> +		writel(CHANNEL_DISABLE, info->regs + i * CHANNEL_REG_SHIF);
> +
Perhaps this switch would be cleaner as static const array of structures
used as a look up table?
> +	switch (adc_feature->clk_pre_div) {
> +	case CLK_PRE_DIV_4:
> +		time_cfg |= PRE_DIV_4;
> +		info->pre_div_num = 4;
> +		break;
> +	case CLK_PRE_DIV_8:
> +		time_cfg |= PRE_DIV_8;
> +		info->pre_div_num = 8;
> +		break;
> +	case CLK_PRE_DIV_16:
> +		time_cfg |= PRE_DIV_16;
> +		info->pre_div_num = 16;
> +		break;
> +	case CLK_PRE_DIV_32:
> +		time_cfg |= PRE_DIV_32;
> +		info->pre_div_num = 32;
> +		break;
> +	case CLK_PRE_DIV_64:
> +		time_cfg |= PRE_DIV_64;
> +		info->pre_div_num = 64;
> +		break;
> +	case CLK_PRE_DIV_128:
> +		time_cfg |= PRE_DIV_128;
> +		info->pre_div_num = 128;
> +		break;
> +	default:
> +		dev_err(info->dev, "error pre div!\n");
> +	}
> +
> +	time_cfg |= adc_feature->core_time_unit;
> +	writel(time_cfg, info->regs + REG_ADC_TIMER_UNIT);
> +}
> +
> +static void imx7d_hw_init(struct imx7d_adc *info)
> +{
> +	u32 cfg;
blank line here please.
> +	/* power up and enable adc analogue core */
> +	cfg = readl(info->regs + REG_ADC_ADC_CFG);
> +	cfg &= ~(ADC_CLK_DOWN | ADC_POWER_DOWN);
> +	cfg |= ADC_EN;
> +	writel(cfg, info->regs + REG_ADC_ADC_CFG);
> +
> +	/* enable channel A,B,C,D interrupt */
> +	writel(CHANNEL_INT_EN, info->regs + REG_ADC_INT_SIG_EN);
> +	writel(CHANNEL_INT_EN, info->regs + REG_ADC_INT_EN);
> +
> +	imx7d_adc_sample_set(info);
> +}
> +
> +static void imx7d_channel_set(struct imx7d_adc *info)
> +{
> +	u32 cfg1 = 0;
> +	u32 cfg2;
> +	u32 channel;
> +
> +	channel = info->channel;
> +
> +	/* the channel choose single conversion, and enable average mode */
> +	cfg1 |= (CHANNEL_EN | CHANNEL_SINGLE);
> +	if (info->adc_feature.average_en)
> +		cfg1 |= CHANNEL_AVG_EN;
> +
> +	/*
> +	 * physical channel 0 chose logical channel A
> +	 * physical channel 1 chose logical channel B
> +	 * physical channel 2 chose logical channel C
> +	 * physical channel 3 chose logical channel D
> +	 */
> +	cfg1 |= (channel << CHANNEL_SEL_SHIF);
> +
> +	/* read register REG_ADC_CH_A\B\C\D_CFG2, according to the channel choosed */
> +	cfg2 = readl(info->regs + CHANNEL_REG_SHIF * channel + 0x10);
> +

Perhaps this switch be neater as a look up table?
> +	switch (info->adc_feature.avg_num) {
> +	case AVERAGE_NUM_4:
> +		cfg2 |= AVG_NUM_4;
> +		break;
> +	case AVERAGE_NUM_8:
> +		cfg2 |= AVG_NUM_8;
> +		break;
> +	case AVERAGE_NUM_16:
> +		cfg2 |= AVG_NUM_16;
> +		break;
> +	case AVERAGE_NUM_32:
> +		cfg2 |= AVG_NUM_32;
> +		break;
> +	default:
> +		dev_err(info->dev, "error average number!\n");
> +		break;
> +	}
> +
> +	/* write the register REG_ADC_CH_A\B\C\D_CFG2, according to the channel choosed */
chosen (silly bit of English!)  Same elsewhere in the driver.

> +	writel(cfg2, info->regs + CHANNEL_REG_SHIF * channel + 0x10);
> +	writel(cfg1, info->regs + CHANNEL_REG_SHIF * channel);
> +}
> +
> +static u32 imx7d_get_sample_rate(struct imx7d_adc *info)
> +{
> +	/* input clock is always 24MHz */
> +	u32 input_clk = 24000000;
> +	u32 analogue_core_clk;
> +	u32 core_time_unit = info->adc_feature.core_time_unit;
> +	u32 sample_clk;
> +	u32 tmp;
> +
> +	analogue_core_clk = input_clk / info->pre_div_num;
> +	tmp = (core_time_unit + 1) * 6;
> +	sample_clk = analogue_core_clk / tmp;
> +
> +	return sample_clk;
> +}
> +
> +static int imx7d_read_raw(struct iio_dev *indio_dev,
> +			struct iio_chan_spec const *chan,
> +			int *val,
> +			int *val2,
> +			long mask)
> +{
> +	struct imx7d_adc *info = iio_priv(indio_dev);
> +
> +	u32 channel;
> +	long ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&indio_dev->mlock);
> +		reinit_completion(&info->completion);
> +
> +		channel = (chan->channel) & 0x0f;
> +		info->channel = channel;
> +		imx7d_channel_set(info);
> +
> +		ret = wait_for_completion_interruptible_timeout
> +				(&info->completion, IMX7D_ADC_TIMEOUT);
> +		if (ret == 0) {
> +			mutex_unlock(&indio_dev->mlock);
> +			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 = 12;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = imx7d_get_sample_rate(info);
> +		*val2 = 0;
> +		return IIO_VAL_INT;
> +
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int imx7d_adc_read_data(struct imx7d_adc *info)
> +{
> +	u32 channel;
> +	u32 value;
> +
> +	channel = info->channel;
> +
> +	/*
> +	 * channel A and B conversion result share one register,
> +	 * bit[27~16] is the channel B conversion result,
> +	 * bit[11~0] is the channel A conversion result.
> +	 * channel C and D is the same.
> +	 */
> +	if (channel < 2)
> +		value = readl(info->regs + REG_ADC_CHA_B_CNV_RSLT);
> +	else
> +		value = readl(info->regs + REG_ADC_CHC_D_CNV_RSLT);
> +	if (channel & 0x1)	/* channel B or D */
> +		value = (value >> 16) & 0xFFF;
> +	else			/* channel A or C */
> +		value &= 0xFFF;
> +
> +	return value;
> +}
> +
> +static irqreturn_t imx7d_adc_isr(int irq, void *dev_id)
> +{
> +	struct imx7d_adc *info = (struct imx7d_adc *)dev_id;
> +	int status;
> +
> +	status = readl(info->regs + REG_ADC_INT_STATUS);
> +	if (status & CHANNEL_INT_STATUS) {
> +		info->value = imx7d_adc_read_data(info);
> +		complete(&info->completion);
> +	}
> +	writel(0, info->regs + REG_ADC_INT_STATUS);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int imx7d_adc_reg_access(struct iio_dev *indio_dev,
> +			unsigned reg, unsigned writeval,
> +			unsigned *readval)
> +{
> +	struct imx7d_adc *info = iio_priv(indio_dev);
> +
> +	if ((readval == NULL) ||
> +		((reg % 4) || (reg > REG_ADC_ADC_CFG)))
> +		return -EINVAL;
> +
> +	*readval = readl(info->regs + reg);
nitpick.  Blank line here.
> +	return 0;
> +}
> +
> +static const struct iio_info imx7d_adc_iio_info = {
> +	.driver_module = THIS_MODULE,
> +	.read_raw = &imx7d_read_raw,
> +	.debugfs_reg_access = &imx7d_adc_reg_access,
> +};
> +
> +static const struct of_device_id imx7d_adc_match[] = {
> +	{ .compatible = "fsl,imx7d-adc", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imx7d_adc_match);
> +
> +static int imx7d_adc_probe(struct platform_device *pdev)
> +{
> +	struct imx7d_adc *info;
> +	struct iio_dev *indio_dev;
> +	struct resource *mem;
> +	int irq;
> +	int ret;
> +	u32 channels;
> +
> +	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(struct imx7d_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)) {
> +		ret = PTR_ERR(info->regs);
> +		dev_err(&pdev->dev, "failed to remap adc memory: %d\n", ret);
> +		return ret;
> +	}
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0) {
> +		dev_err(&pdev->dev, "no irq resource?\n");
> +		return irq;
> +	}
> +
> +	ret = devm_request_irq(info->dev, irq,
> +				imx7d_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)) {
> +		ret = PTR_ERR(info->clk);
> +		dev_err(&pdev->dev, "failed getting clock, err = %d\n", ret);
> +		return ret;
why not
return PTR_ERR(info->clk);
Similar elsewhere.

> +	}
> +
> +	info->vref = devm_regulator_get(&pdev->dev, "vref");
> +	if (IS_ERR(info->vref)) {
> +		ret = PTR_ERR(info->vref);
> +		dev_err(&pdev->dev, "failed getting reference voltage: %d\n", ret);
> +		return ret;
> +	}
> +
> +	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);
> +
> +	ret  = of_property_read_u32(pdev->dev.of_node,
> +					"num-channels", &channels);
> +	if (ret)
> +		channels = ARRAY_SIZE(imx7d_adc_iio_channels);
> +
> +	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 = &imx7d_adc_iio_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = imx7d_adc_iio_channels;
> +	indio_dev->num_channels = (int)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;
> +	}
> +
> +	imx7d_feature_config(info);
> +	imx7d_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 imx7d_adc_remove(struct platform_device *pdev)
> +{
> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +	struct imx7d_adc *info = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +	regulator_disable(info->vref);
> +	clk_disable_unprepare(info->clk);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused imx7d_adc_suspend(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct imx7d_adc *info = iio_priv(indio_dev);
> +	u32 adc_cfg;
> +
> +	adc_cfg = readl(info->regs + REG_ADC_ADC_CFG);
> +	adc_cfg |= ADC_CLK_DOWN | ADC_POWER_DOWN;
> +	adc_cfg &= ~ADC_EN;
> +	writel(adc_cfg, info->regs + REG_ADC_ADC_CFG);
> +
> +	clk_disable_unprepare(info->clk);
> +	regulator_disable(info->vref);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused imx7d_adc_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct imx7d_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) {
> +		dev_err(info->dev,
> +			"Could not prepare or enable clock.\n");
> +		regulator_disable(info->vref);
> +		return ret;
> +	}
> +
> +	imx7d_hw_init(info);
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(imx7d_adc_pm_ops, imx7d_adc_suspend, imx7d_adc_resume);
> +
> +static struct platform_driver imx7d_driver = {
> +	.probe		= imx7d_adc_probe,
> +	.remove		= imx7d_adc_remove,
> +	.driver		= {
> +		.name	= DRIVER_NAME,
> +		.of_match_table = imx7d_adc_match,
> +		.pm	= &imx7d_adc_pm_ops,
> +	},
> +};
> +
> +module_platform_driver(imx7d_driver);
> +
> +MODULE_AUTHOR("Haibo Chen <haibo.chen@...escale.com>");
> +MODULE_DESCRIPTION("Freeacale IMX7D ADC driver");
> +MODULE_LICENSE("GPL v2");
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists