LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v3 0/3] iio: imx8qxp-adc: Add driver support for NXP IMX8QXP ADC
@ 2021-09-07  1:57 Cai Huoqing
  2021-09-07  1:57 ` [PATCH v3 1/3] " Cai Huoqing
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Cai Huoqing @ 2021-09-07  1:57 UTC (permalink / raw)
  To: caihuoqing
  Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, linux-iio, devicetree, linux-arm-kernel,
	linux-kernel

The NXP i.MX 8QuadXPlus SOC has a new ADC IP. These patches add
driver support for this ADC

dt-bindings: iio: adc:
v1->v2: *Fix some indentation issues.
        *Mark status as okay.
        *Change clock2 source.

iio: imx8qxp-adc:
v1->v2:	*Squash patches 1, 2, 3, and 5 into a single patch.
	*Add device specific prefix.
	*Remove the brackets around individual numbers.
	*Make use of FIELD_PREP() and FIELD_GET().
	*Remove a lot of cache values.
	*Replace mlock with adc->lock.
	*Move adc->value read from isr to the completion.
	*Set pm_runtime_disable/_put_noidle() before adc_disable.
	*Add error handler-err_disable_reg/err_unprepare_clk.
v2->v3: *Add "return 0" to adc_runtime_resume()

v1 link:
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20210830172140.414-4-caihuoqing@baidu.com/

Cai Huoqing (3):
  iio: imx8qxp-adc: Add driver support for NXP IMX8QXP ADC
  dt-bindings: iio: adc: Add the binding documentation for NXP IMX8QXP
    ADC
  MAINTAINERS:  Add the driver info of the NXP IMX8QXP ADC

 .../bindings/iio/adc/nxp,imx8qxp-adc.yaml     |  85 ++++
 MAINTAINERS                                   |   7 +
 drivers/iio/adc/Kconfig                       |  10 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/imx8qxp-adc.c                 | 470 ++++++++++++++++++
 5 files changed, 573 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
 create mode 100644 drivers/iio/adc/imx8qxp-adc.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 1/3] iio: imx8qxp-adc: Add driver support for NXP IMX8QXP ADC
  2021-09-07  1:57 [PATCH v3 0/3] iio: imx8qxp-adc: Add driver support for NXP IMX8QXP ADC Cai Huoqing
@ 2021-09-07  1:57 ` Cai Huoqing
  2021-09-11 17:28   ` Jonathan Cameron
  2021-09-07  1:57 ` [PATCH v3 2/3] dt-bindings: iio: adc: Add the binding documentation " Cai Huoqing
  2021-09-07  1:57 ` [PATCH v3 3/3] MAINTAINERS: Add the driver info of the " Cai Huoqing
  2 siblings, 1 reply; 7+ messages in thread
From: Cai Huoqing @ 2021-09-07  1:57 UTC (permalink / raw)
  To: caihuoqing
  Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, linux-iio, devicetree, linux-arm-kernel,
	linux-kernel

The NXP i.MX 8QuadXPlus SOC has a new ADC IP. These patches add
driver support for this ADC.

Signed-off-by: Cai Huoqing <caihuoqing@baidu.com>
---
v1->v2:	*Squash patches 1, 2, 3, and 5 into a single patch.
	*Add device specific prefix.
	*Remove the brackets around individual numbers.
	*Make use of FIELD_PREP() and FIELD_GET().
	*Remove a lot of cache values.
	*Replace mlock with adc->lock.
	*Move adc->value read from isr to the completion.
	*Set pm_runtime_disable/_put_noidle() before adc_disable.
	*Add error handler-err_disable_reg/err_unprepare_clk.
v2->v3:	*Add "return 0" to adc_runtime_resume().
v1 link:
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20210830172140.414-4-caihuoqing@baidu.com/

 drivers/iio/adc/Kconfig       |  10 +
 drivers/iio/adc/Makefile      |   1 +
 drivers/iio/adc/imx8qxp-adc.c | 470 ++++++++++++++++++++++++++++++++++
 3 files changed, 481 insertions(+)
 create mode 100644 drivers/iio/adc/imx8qxp-adc.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index af168e1c9fdb..fa8ad63d6ac2 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -530,6 +530,16 @@ config IMX7D_ADC
 	  This driver can also be built as a module. If so, the module will be
 	  called imx7d_adc.
 
+config IMX8QXP_ADC
+	tristate "NXP IMX8QXP ADC driver"
+	depends on ARCH_MXC_ARM64 || COMPILE_TEST
+	depends on HAS_IOMEM
+	help
+	  Say yes here to build support for IMX8QXP ADC.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called imx8qxp-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 d68550f493e3..d3f53549720c 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_FSL_MX25_ADC) += fsl-imx25-gcq.o
 obj-$(CONFIG_HI8435) += hi8435.o
 obj-$(CONFIG_HX711) += hx711.o
 obj-$(CONFIG_IMX7D_ADC) += imx7d_adc.o
+obj-$(CONFIG_IMX8QXP_ADC) += imx8qxp-adc.o
 obj-$(CONFIG_INA2XX_ADC) += ina2xx-adc.o
 obj-$(CONFIG_INGENIC_ADC) += ingenic-adc.o
 obj-$(CONFIG_INTEL_MRFLD_ADC) += intel_mrfld_adc.o
diff --git a/drivers/iio/adc/imx8qxp-adc.c b/drivers/iio/adc/imx8qxp-adc.c
new file mode 100644
index 000000000000..9ba77af360f3
--- /dev/null
+++ b/drivers/iio/adc/imx8qxp-adc.c
@@ -0,0 +1,470 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * NXP i.MX8QXP ADC driver
+ *
+ * Based on the work of Haibo Chen <haibo.chen@nxp.com>
+ * The initial developer of the original code is Haibo Chen.
+ * Portions created by Haibo Chen are Copyright (C) 2018 NXP.
+ * All Rights Reserved.
+ *
+ * Copyright (C) 2018 NXP
+ * Copyright (C) 2021 Cai Huoqing
+ */
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/driver.h>
+#include <linux/iio/sysfs.h>
+
+#define ADC_DRIVER_NAME		"imx8qxp-adc"
+
+/* Register map definition */
+#define IMX8QXP_ADR_ADC_CTRL		0x10
+#define IMX8QXP_ADR_ADC_STAT		0x14
+#define IMX8QXP_ADR_ADC_IE		0x18
+#define IMX8QXP_ADR_ADC_DE		0x1c
+#define IMX8QXP_ADR_ADC_CFG		0x20
+#define IMX8QXP_ADR_ADC_FCTRL		0x30
+#define IMX8QXP_ADR_ADC_SWTRIG		0x34
+#define IMX8QXP_ADR_ADC_TCTRL(tid)	(0xc0 + tid * 4)
+#define IMX8QXP_ADR_ADC_CMDH(cid)	(0x100 + cid * 8)
+#define IMX8QXP_ADR_ADC_CMDL(cid)	(0x104 + cid * 8)
+#define IMX8QXP_ADR_ADC_RESFIFO		0x300
+#define IMX8QXP_ADR_ADC_TST		0xffc
+
+/* ADC bit shift */
+#define IMX8QXP_ADC_IE_FWMIE_MASK		GENMASK(1, 0)
+#define IMX8QXP_ADC_CTRL_FIFO_RESET_MASK	BIT(8)
+#define IMX8QXP_ADC_CTRL_SOFTWARE_RESET_MASK	BIT(1)
+#define IMX8QXP_ADC_CTRL_ADC_EN_MASK		BIT(0)
+#define IMX8QXP_ADC_TCTRL_TCMD_MASK		GENMASK(31, 24)
+#define IMX8QXP_ADC_TCTRL_TDLY_MASK		GENMASK(23, 16)
+#define IMX8QXP_ADC_TCTRL_TPRI_MASK		GENMASK(15, 8)
+#define IMX8QXP_ADC_TCTRL_HTEN_MASK		GENMASK(7, 0)
+#define IMX8QXP_ADC_CMDL_CSCALE_MASK		GENMASK(13, 8)
+#define IMX8QXP_ADC_CMDL_MODE_MASK		BIT(7)
+#define IMX8QXP_ADC_CMDL_DIFF_MASK		BIT(6)
+#define IMX8QXP_ADC_CMDL_ABSEL_MASK		BIT(5)
+#define IMX8QXP_ADC_CMDL_ADCH_MASK		GENMASK(2, 0)
+#define IMX8QXP_ADC_CMDH_NEXT_MASK		GENMASK(31, 24)
+#define IMX8QXP_ADC_CMDH_LOOP_MASK		GENMASK(23, 16)
+#define IMX8QXP_ADC_CMDH_AVGS_MASK		GENMASK(15, 12)
+#define IMX8QXP_ADC_CMDH_STS_MASK		BIT(8)
+#define IMX8QXP_ADC_CMDH_LWI_MASK		GENMASK(7, 7)
+#define IMX8QXP_ADC_CMDH_CMPEN_MASK		GENMASK(0, 0)
+#define IMX8QXP_ADC_CFG_PWREN_MASK		BIT(28)
+#define IMX8QXP_ADC_CFG_PUDLY_MASK		GENMASK(23, 16)
+#define IMX8QXP_ADC_CFG_REFSEL_MASK		GENMASK(7, 6)
+#define IMX8QXP_ADC_CFG_PWRSEL_MASK		GENMASK(5, 4)
+#define IMX8QXP_ADC_CFG_TPRICTRL_MASK		GENMASK(3, 0)
+#define IMX8QXP_ADC_FCTRL_FWMARK_MASK		GENMASK(20, 16)
+#define IMX8QXP_ADC_FCTRL_FCOUNT_MASK		GENMASK(4, 0)
+#define IMX8QXP_ADC_RESFIFO_VAL_MASK		GENMASK(18, 3)
+
+/* ADC PARAMETER*/
+#define IMX8QXP_ADC_CMDL_CHANNEL_SCALE_FULL		GENMASK(5, 0)
+#define IMX8QXP_ADC_CMDL_SEL_A_A_B_CHANNEL		0
+#define IMX8QXP_ADC_CMDL_STANDARD_RESOLUTION		0
+#define IMX8QXP_ADC_CMDL_MODE_SINGLE			0
+#define IMX8QXP_ADC_CMDH_LWI_INCREMENT_DIS		0
+#define IMX8QXP_ADC_CMDH_CMPEN_DIS			0
+#define IMX8QXP_ADC_PAUSE_EN				BIT(31)
+#define IMX8QXP_ADC_TCTRL_TPRI_PRIORITY_HIGH		0
+
+#define IMX8QXP_ADC_TCTRL_HTEN_HW_TIRG_DIS		0
+
+#define IMX8QXP_ADC_TIMEOUT		msecs_to_jiffies(100)
+
+struct imx8qxp_adc {
+	struct device *dev;
+	void __iomem *regs;
+	struct clk *clk;
+	struct clk *ipg_clk;
+	struct regulator *vref;
+	struct mutex lock;
+	struct completion completion;
+};
+
+#define IMX8QXP_ADC_CHAN(_idx) {				\
+	.type = IIO_VOLTAGE,					\
+	.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 imx8qxp_adc_iio_channels[] = {
+	IMX8QXP_ADC_CHAN(0),
+	IMX8QXP_ADC_CHAN(1),
+	IMX8QXP_ADC_CHAN(2),
+	IMX8QXP_ADC_CHAN(3),
+	IMX8QXP_ADC_CHAN(4),
+	IMX8QXP_ADC_CHAN(5),
+	IMX8QXP_ADC_CHAN(6),
+	IMX8QXP_ADC_CHAN(7),
+};
+
+static void imx8qxp_adc_reset(struct imx8qxp_adc *adc)
+{
+	u32 ctrl;
+
+	/*software reset, need to clear the set bit*/
+	ctrl = readl(adc->regs + IMX8QXP_ADR_ADC_CTRL);
+	ctrl |= FIELD_PREP(IMX8QXP_ADC_CTRL_SOFTWARE_RESET_MASK, 1);
+	writel(ctrl, adc->regs + IMX8QXP_ADR_ADC_CTRL);
+	udelay(10);
+	ctrl &= ~FIELD_PREP(IMX8QXP_ADC_CTRL_SOFTWARE_RESET_MASK, 1);
+	writel(ctrl, adc->regs + IMX8QXP_ADR_ADC_CTRL);
+
+	/* reset the fifo */
+	ctrl |= FIELD_PREP(IMX8QXP_ADC_CTRL_FIFO_RESET_MASK, 1);
+	writel(ctrl, adc->regs + IMX8QXP_ADR_ADC_CTRL);
+}
+
+static void imx8qxp_adc_reg_config(struct imx8qxp_adc *adc, int channel)
+{
+	u32 adc_cfg, adc_tctrl, adc_cmdl, adc_cmdh;
+
+	/* ADC configuration */
+	adc_cfg = FIELD_PREP(IMX8QXP_ADC_CFG_PWREN_MASK, 1) |
+		  FIELD_PREP(IMX8QXP_ADC_CFG_PUDLY_MASK, 0x80)|
+		  FIELD_PREP(IMX8QXP_ADC_CFG_REFSEL_MASK, 0) |
+		  FIELD_PREP(IMX8QXP_ADC_CFG_PWRSEL_MASK, 3) |
+		  FIELD_PREP(IMX8QXP_ADC_CFG_TPRICTRL_MASK, 0);
+	writel(adc_cfg, adc->regs + IMX8QXP_ADR_ADC_CFG);
+
+	/* config the trigger control */
+	adc_tctrl = FIELD_PREP(IMX8QXP_ADC_TCTRL_TCMD_MASK, 1) |
+		    FIELD_PREP(IMX8QXP_ADC_TCTRL_TDLY_MASK, 0) |
+		    FIELD_PREP(IMX8QXP_ADC_TCTRL_TPRI_MASK, IMX8QXP_ADC_TCTRL_TPRI_PRIORITY_HIGH) |
+		    FIELD_PREP(IMX8QXP_ADC_TCTRL_HTEN_MASK, IMX8QXP_ADC_TCTRL_HTEN_HW_TIRG_DIS);
+	writel(adc_cfg, adc->regs + IMX8QXP_ADR_ADC_TCTRL(0));
+
+	/* config the cmd */
+	adc_cmdl = FIELD_PREP(IMX8QXP_ADC_CMDL_CSCALE_MASK, IMX8QXP_ADC_CMDL_CHANNEL_SCALE_FULL) |
+		   FIELD_PREP(IMX8QXP_ADC_CMDL_MODE_MASK, IMX8QXP_ADC_CMDL_STANDARD_RESOLUTION) |
+		   FIELD_PREP(IMX8QXP_ADC_CMDL_DIFF_MASK, IMX8QXP_ADC_CMDL_MODE_SINGLE) |
+		   FIELD_PREP(IMX8QXP_ADC_CMDL_ABSEL_MASK, IMX8QXP_ADC_CMDL_SEL_A_A_B_CHANNEL) |
+		   FIELD_PREP(IMX8QXP_ADC_CMDL_ADCH_MASK, channel);
+	writel(adc_cmdl, adc->regs + IMX8QXP_ADR_ADC_CMDL(0));
+
+	adc_cmdh = FIELD_PREP(IMX8QXP_ADC_CMDH_NEXT_MASK, 0) |
+		   FIELD_PREP(IMX8QXP_ADC_CMDH_LOOP_MASK, 0) |
+		   FIELD_PREP(IMX8QXP_ADC_CMDH_AVGS_MASK, 7) |
+		   FIELD_PREP(IMX8QXP_ADC_CMDH_STS_MASK, 0) |
+		   FIELD_PREP(IMX8QXP_ADC_CMDH_LWI_MASK, IMX8QXP_ADC_CMDH_LWI_INCREMENT_DIS) |
+		   FIELD_PREP(IMX8QXP_ADC_CMDH_CMPEN_MASK, IMX8QXP_ADC_CMDH_CMPEN_DIS);
+	writel(adc_cmdh, adc->regs + IMX8QXP_ADR_ADC_CMDH(0));
+}
+
+static void imx8qxp_adc_fifo_config(struct imx8qxp_adc *adc)
+{
+	u32 fifo_ctrl, interrupt_en;
+
+	fifo_ctrl = readl(adc->regs + IMX8QXP_ADR_ADC_FCTRL);
+	fifo_ctrl &= ~IMX8QXP_ADC_FCTRL_FWMARK_MASK;
+	/* set the watermark level to 1 */
+	fifo_ctrl |= FIELD_PREP(IMX8QXP_ADC_FCTRL_FWMARK_MASK, 0);
+	writel(fifo_ctrl, adc->regs + IMX8QXP_ADR_ADC_FCTRL);
+
+	/* FIFO Watermark Interrupt Enable */
+	interrupt_en = readl(adc->regs + IMX8QXP_ADR_ADC_IE);
+	interrupt_en |= FIELD_PREP(IMX8QXP_ADC_IE_FWMIE_MASK, 1);
+	writel(interrupt_en, adc->regs + IMX8QXP_ADR_ADC_IE);
+}
+
+static void imx8qxp_adc_disable(struct imx8qxp_adc *adc)
+{
+	u32 ctrl;
+
+	ctrl = readl(adc->regs + IMX8QXP_ADR_ADC_CTRL);
+	ctrl &= ~FIELD_PREP(IMX8QXP_ADC_CTRL_ADC_EN_MASK, 1);
+	writel(ctrl, adc->regs + IMX8QXP_ADR_ADC_CTRL);
+}
+
+static int imx8qxp_adc_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+				int *val, int *val2, long mask)
+{
+	struct imx8qxp_adc *adc = iio_priv(indio_dev);
+	struct device *dev = adc->dev;
+
+	u32 ctrl, vref_uv;
+	long ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		pm_runtime_get_sync(dev);
+
+		mutex_lock(&adc->lock);
+		reinit_completion(&adc->completion);
+
+		imx8qxp_adc_reg_config(adc, chan->channel);
+
+		imx8qxp_adc_fifo_config(adc);
+
+		/* adc enable */
+		ctrl = readl(adc->regs + IMX8QXP_ADR_ADC_CTRL);
+		ctrl |= FIELD_PREP(IMX8QXP_ADC_CTRL_ADC_EN_MASK, 1);
+		writel(ctrl, adc->regs + IMX8QXP_ADR_ADC_CTRL);
+		/* adc start */
+		writel(1, adc->regs + IMX8QXP_ADR_ADC_SWTRIG);
+
+		ret = wait_for_completion_interruptible_timeout(&adc->completion,
+								IMX8QXP_ADC_TIMEOUT);
+
+		pm_runtime_mark_last_busy(dev);
+		pm_runtime_put_sync_autosuspend(dev);
+
+		if (ret == 0) {
+			mutex_unlock(&adc->lock);
+			return -ETIMEDOUT;
+		}
+		if (ret < 0) {
+			mutex_unlock(&adc->lock);
+			return ret;
+		}
+
+		*val = FIELD_GET(IMX8QXP_ADC_RESFIFO_VAL_MASK,
+				 readl(adc->regs + IMX8QXP_ADR_ADC_RESFIFO));
+
+		mutex_unlock(&adc->lock);
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		vref_uv = regulator_get_voltage(adc->vref);
+		*val = vref_uv / 1000;
+		*val2 = 12;
+		return IIO_VAL_FRACTIONAL_LOG2;
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = clk_get_rate(adc->clk) / 3;
+		return IIO_VAL_INT;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static irqreturn_t imx8qxp_adc_isr(int irq, void *dev_id)
+{
+	struct imx8qxp_adc *adc = (struct imx8qxp_adc *)dev_id;
+
+	u32 fifo_count;
+
+	fifo_count = FIELD_GET(IMX8QXP_ADC_FCTRL_FCOUNT_MASK,
+			       readl(adc->regs + IMX8QXP_ADR_ADC_FCTRL));
+
+	if (fifo_count)
+		complete(&adc->completion);
+
+	return IRQ_HANDLED;
+}
+
+static int imx8qxp_adc_reg_access(struct iio_dev *indio_dev, unsigned int reg,
+				  unsigned int writeval, unsigned int *readval)
+{
+	struct imx8qxp_adc *adc = iio_priv(indio_dev);
+	struct device *dev = adc->dev;
+
+	if (!readval || reg % 4 || reg > IMX8QXP_ADR_ADC_TST)
+		return -EINVAL;
+
+	pm_runtime_get_sync(dev);
+
+	*readval = readl(adc->regs + reg);
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_sync_autosuspend(dev);
+
+	return 0;
+}
+
+static const struct iio_info imx8qxp_adc_iio_info = {
+	.read_raw = &imx8qxp_adc_read_raw,
+	.debugfs_reg_access = &imx8qxp_adc_reg_access,
+};
+
+static int imx8qxp_adc_probe(struct platform_device *pdev)
+{
+	struct imx8qxp_adc *adc;
+	struct iio_dev *indio_dev;
+	struct device *dev = &pdev->dev;
+	int irq;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*adc));
+	if (!indio_dev) {
+		dev_err(dev, "Failed allocating iio device\n");
+		return -ENOMEM;
+	}
+
+	adc = iio_priv(indio_dev);
+	adc->dev = dev;
+
+	mutex_init(&adc->lock);
+	adc->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(adc->regs))
+		return PTR_ERR(adc->regs);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	adc->clk = devm_clk_get(dev, "per");
+	if (IS_ERR(adc->clk)) {
+		ret = PTR_ERR(adc->clk);
+		dev_err(dev, "Failed getting clock, err = %d\n", ret);
+		return ret;
+	}
+
+	adc->ipg_clk = devm_clk_get(dev, "ipg");
+	if (IS_ERR(adc->ipg_clk)) {
+		ret = PTR_ERR(adc->ipg_clk);
+		dev_err(dev, "Failed getting clock, err = %d\n", ret);
+		return ret;
+	}
+
+	adc->vref = devm_regulator_get(dev, "vref");
+	if (IS_ERR(adc->vref)) {
+		ret = PTR_ERR(adc->vref);
+		dev_err(dev,
+			"Failed getting reference voltage, err = %d\n", ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, indio_dev);
+
+	init_completion(&adc->completion);
+
+	indio_dev->name = dev_name(dev);
+	indio_dev->info = &imx8qxp_adc_iio_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = imx8qxp_adc_iio_channels;
+	indio_dev->num_channels = ARRAY_SIZE(imx8qxp_adc_iio_channels);
+
+	ret = devm_request_irq(dev, irq, imx8qxp_adc_isr, 0, dev_name(dev), adc);
+	if (ret < 0) {
+		dev_err(dev, "Failed requesting irq, irq = %d\n", irq);
+		return ret;
+	}
+
+	imx8qxp_adc_reset(adc);
+
+	ret = iio_device_register(indio_dev);
+	if (ret) {
+		imx8qxp_adc_disable(adc);
+		dev_err(dev, "Couldn't register the device.\n");
+		return ret;
+	}
+
+	pm_runtime_set_active(dev);
+	pm_runtime_set_autosuspend_delay(dev, 50);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_enable(dev);
+
+	return 0;
+}
+
+static int imx8qxp_adc_remove(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+	struct imx8qxp_adc *adc = iio_priv(indio_dev);
+	struct device *dev = adc->dev;
+
+	pm_runtime_get_sync(dev);
+
+	iio_device_unregister(indio_dev);
+
+	pm_runtime_disable(dev);
+	pm_runtime_put_noidle(dev);
+
+	imx8qxp_adc_disable(adc);
+
+	return 0;
+}
+
+static int imx8qxp_adc_runtime_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct imx8qxp_adc *adc = iio_priv(indio_dev);
+
+	imx8qxp_adc_disable(adc);
+
+	clk_disable_unprepare(adc->clk);
+	clk_disable_unprepare(adc->ipg_clk);
+	regulator_disable(adc->vref);
+
+	return 0;
+}
+
+static int imx8qxp_adc_runtime_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct imx8qxp_adc *adc = iio_priv(indio_dev);
+	int ret;
+
+	ret = regulator_enable(adc->vref);
+	if (ret) {
+		dev_err(dev, "Can't enable adc reference top voltage, err = %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(adc->clk);
+	if (ret) {
+		dev_err(dev, "Could not prepare or enable clock.\n");
+		goto err_disable_reg;
+	}
+
+	ret = clk_prepare_enable(adc->ipg_clk);
+	if (ret) {
+		dev_err(dev, "Could not prepare or enable clock.\n");
+		goto err_unprepare_clk;
+	}
+	imx8qxp_adc_reset(adc);
+	return 0;
+err_unprepare_clk:
+	clk_disable_unprepare(adc->clk);
+
+err_disable_reg:
+	regulator_disable(adc->vref);
+
+	return ret;
+}
+
+static const struct dev_pm_ops imx8qxp_adc_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
+	SET_RUNTIME_PM_OPS(imx8qxp_adc_runtime_suspend, imx8qxp_adc_runtime_resume, NULL)
+};
+
+static const struct of_device_id imx8qxp_adc_match[] = {
+	{ .compatible = "nxp,imx8qxp-adc", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx8qxp_adc_match);
+
+static struct platform_driver imx8qxp_adc_driver = {
+	.probe		= imx8qxp_adc_probe,
+	.remove		= imx8qxp_adc_remove,
+	.driver		= {
+		.name	= ADC_DRIVER_NAME,
+		.of_match_table = imx8qxp_adc_match,
+		.pm	= &imx8qxp_adc_pm_ops,
+	},
+};
+
+module_platform_driver(imx8qxp_adc_driver);
+
+MODULE_DESCRIPTION("i.MX8QuadXPlus ADC driver");
+MODULE_LICENSE("GPL v2");
-- 
2.25.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 2/3] dt-bindings: iio: adc: Add the binding documentation for NXP IMX8QXP ADC
  2021-09-07  1:57 [PATCH v3 0/3] iio: imx8qxp-adc: Add driver support for NXP IMX8QXP ADC Cai Huoqing
  2021-09-07  1:57 ` [PATCH v3 1/3] " Cai Huoqing
@ 2021-09-07  1:57 ` Cai Huoqing
  2021-09-07 11:49   ` Rob Herring
  2021-09-07 16:46   ` Rob Herring
  2021-09-07  1:57 ` [PATCH v3 3/3] MAINTAINERS: Add the driver info of the " Cai Huoqing
  2 siblings, 2 replies; 7+ messages in thread
From: Cai Huoqing @ 2021-09-07  1:57 UTC (permalink / raw)
  To: caihuoqing
  Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, linux-iio, devicetree, linux-arm-kernel,
	linux-kernel

The NXP i.MX 8QuadXPlus SOC has a new ADC IP, so add the binding
documentation for NXP IMX8QXP ADC.

Signed-off-by: Cai Huoqing <caihuoqing@baidu.com>
---
v1->v2: *Fix some indentation issues.
        *Mark status as okay.
        *Change clock2 source.
v1 link:
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20210830172140.414-5-caihuoqing@baidu.com/

 .../bindings/iio/adc/nxp,imx8qxp-adc.yaml     | 85 +++++++++++++++++++
 1 file changed, 85 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
new file mode 100644
index 000000000000..77501898a563
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
@@ -0,0 +1,85 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/nxp,imx8qxp-adc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP IMX8QXP ADC bindings
+
+maintainers:
+  - Cai Huoqing <caihuoqing@baidu.com>
+
+description:
+  Supports the ADC found on the IMX8QXP SoC.
+
+properties:
+  compatible:
+    const: nxp,imx8qxp-adc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: per
+      - const: ipg
+
+  assigned-clocks:
+    maxItems: 1
+
+  assigned-clocks-rate:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  status:
+    const: disable
+
+  "#io-channel-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupts-parent
+  - clocks
+  - clock-names
+  - assigned-clocks
+  - assigned-clock-rates
+  - power-domains
+  - state
+  - "#address-cells"
+  - "#size-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/firmware/imx/rsrc.h>
+    soc {
+        #address-cells = <1>;
+        #size-cells = <1>;
+        adc@5a880000 {
+            compatible = "nxp,imx8qxp-adc";
+            reg = <0x0 0x5a880000 0x0 0x10000>;
+            interrupts = <GIC_SPI 240 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&clk IMX_SC_R_ADC_0>,
+                     <&clk IMX_SC_R_ADC_0>;
+            clock-names = "per", "ipg";
+            assigned-clocks = <&clk IMX_SC_R_ADC_0>;
+            assigned-clock-rates = <24000000>;
+            power-domains = <&pd IMX_SC_R_ADC_0>;
+            status = "okay";
+            #io-channel-cells = <1>;
+        };
+    };
+...
-- 
2.25.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 3/3] MAINTAINERS:  Add the driver info of the NXP IMX8QXP ADC
  2021-09-07  1:57 [PATCH v3 0/3] iio: imx8qxp-adc: Add driver support for NXP IMX8QXP ADC Cai Huoqing
  2021-09-07  1:57 ` [PATCH v3 1/3] " Cai Huoqing
  2021-09-07  1:57 ` [PATCH v3 2/3] dt-bindings: iio: adc: Add the binding documentation " Cai Huoqing
@ 2021-09-07  1:57 ` Cai Huoqing
  2 siblings, 0 replies; 7+ messages in thread
From: Cai Huoqing @ 2021-09-07  1:57 UTC (permalink / raw)
  To: caihuoqing
  Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, linux-iio, devicetree, linux-arm-kernel,
	linux-kernel

The NXP i.MX 8QuadXPlus SOC has a new ADC IP. After adding
the driver support for it, I add the driver info of the
NXP IMX8QXP ADC to MAINTAINERS file.

Signed-off-by: Cai Huoqing <caihuoqing@baidu.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f800abca74b0..de98fcf75b01 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13475,6 +13475,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml
 F:	drivers/gpu/drm/imx/dcss/
 
+NXP i.MX 8QXP ADC DRIVER
+M:	Cai Huoqing <caihuoqing@baidu.com>
+L:	linux-iio@vger.kernel.org
+S:	Supported
+F:	Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
+F:	drivers/iio/adc/imx8qxp-adc.c
+
 NXP PF8100/PF8121A/PF8200 PMIC REGULATOR DEVICE DRIVER
 M:	Jagan Teki <jagan@amarulasolutions.com>
 S:	Maintained
-- 
2.25.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/3] dt-bindings: iio: adc: Add the binding documentation for NXP IMX8QXP ADC
  2021-09-07  1:57 ` [PATCH v3 2/3] dt-bindings: iio: adc: Add the binding documentation " Cai Huoqing
@ 2021-09-07 11:49   ` Rob Herring
  2021-09-07 16:46   ` Rob Herring
  1 sibling, 0 replies; 7+ messages in thread
From: Rob Herring @ 2021-09-07 11:49 UTC (permalink / raw)
  To: Cai Huoqing
  Cc: NXP Linux Team, Lars-Peter Clausen, Fabio Estevam, linux-kernel,
	Sascha Hauer, Rob Herring, Shawn Guo, devicetree,
	Pengutronix Kernel Team, Jonathan Cameron, linux-iio,
	linux-arm-kernel

On Tue, 07 Sep 2021 09:57:22 +0800, Cai Huoqing wrote:
> The NXP i.MX 8QuadXPlus SOC has a new ADC IP, so add the binding
> documentation for NXP IMX8QXP ADC.
> 
> Signed-off-by: Cai Huoqing <caihuoqing@baidu.com>
> ---
> v1->v2: *Fix some indentation issues.
>         *Mark status as okay.
>         *Change clock2 source.
> v1 link:
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20210830172140.414-5-caihuoqing@baidu.com/
> 
>  .../bindings/iio/adc/nxp,imx8qxp-adc.yaml     | 85 +++++++++++++++++++
>  1 file changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.example.dt.yaml: soc: adc@5a880000:reg:0: [0, 1518862336, 0, 65536] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.example.dt.yaml: adc@5a880000: 'interrupts-parent' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.example.dt.yaml: adc@5a880000: 'state' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.example.dt.yaml: adc@5a880000: '#address-cells' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.example.dt.yaml: adc@5a880000: '#size-cells' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.example.dt.yaml: adc@5a880000: 'assigned-clock-rates' does not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1525098

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/3] dt-bindings: iio: adc: Add the binding documentation for NXP IMX8QXP ADC
  2021-09-07  1:57 ` [PATCH v3 2/3] dt-bindings: iio: adc: Add the binding documentation " Cai Huoqing
  2021-09-07 11:49   ` Rob Herring
@ 2021-09-07 16:46   ` Rob Herring
  1 sibling, 0 replies; 7+ messages in thread
From: Rob Herring @ 2021-09-07 16:46 UTC (permalink / raw)
  To: Cai Huoqing
  Cc: Jonathan Cameron, Lars-Peter Clausen, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	linux-iio, devicetree, linux-arm-kernel, linux-kernel

On Tue, Sep 07, 2021 at 09:57:22AM +0800, Cai Huoqing wrote:
> The NXP i.MX 8QuadXPlus SOC has a new ADC IP, so add the binding
> documentation for NXP IMX8QXP ADC.
> 
> Signed-off-by: Cai Huoqing <caihuoqing@baidu.com>
> ---
> v1->v2: *Fix some indentation issues.
>         *Mark status as okay.
>         *Change clock2 source.
> v1 link:
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20210830172140.414-5-caihuoqing@baidu.com/
> 
>  .../bindings/iio/adc/nxp,imx8qxp-adc.yaml     | 85 +++++++++++++++++++
>  1 file changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
> new file mode 100644
> index 000000000000..77501898a563
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
> @@ -0,0 +1,85 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/nxp,imx8qxp-adc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP IMX8QXP ADC bindings
> +
> +maintainers:
> +  - Cai Huoqing <caihuoqing@baidu.com>
> +
> +description:
> +  Supports the ADC found on the IMX8QXP SoC.
> +
> +properties:
> +  compatible:
> +    const: nxp,imx8qxp-adc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      - const: per
> +      - const: ipg
> +
> +  assigned-clocks:
> +    maxItems: 1
> +
> +  assigned-clocks-rate:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  status:
> +    const: disable

???

You don't need 'status' in bindings. Plus this would cause 'status = 
"okay"' to cause an error in your dts files.

> +
> +  "#io-channel-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupts-parent

It is valid for interrupt-parent to be in a parent node, so it is never 
required.

> +  - clocks
> +  - clock-names
> +  - assigned-clocks
> +  - assigned-clock-rates
> +  - power-domains
> +  - state

Not documented.

> +  - "#address-cells"
> +  - "#size-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/firmware/imx/rsrc.h>
> +    soc {
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        adc@5a880000 {
> +            compatible = "nxp,imx8qxp-adc";
> +            reg = <0x0 0x5a880000 0x0 0x10000>;
> +            interrupts = <GIC_SPI 240 IRQ_TYPE_LEVEL_HIGH>;
> +            clocks = <&clk IMX_SC_R_ADC_0>,
> +                     <&clk IMX_SC_R_ADC_0>;
> +            clock-names = "per", "ipg";
> +            assigned-clocks = <&clk IMX_SC_R_ADC_0>;
> +            assigned-clock-rates = <24000000>;
> +            power-domains = <&pd IMX_SC_R_ADC_0>;
> +            status = "okay";

Remove status from examples.

> +            #io-channel-cells = <1>;
> +        };
> +    };
> +...
> -- 
> 2.25.1
> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/3] iio: imx8qxp-adc: Add driver support for NXP IMX8QXP ADC
  2021-09-07  1:57 ` [PATCH v3 1/3] " Cai Huoqing
@ 2021-09-11 17:28   ` Jonathan Cameron
  0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2021-09-11 17:28 UTC (permalink / raw)
  To: Cai Huoqing
  Cc: Lars-Peter Clausen, Rob Herring, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	linux-iio, devicetree, linux-arm-kernel, linux-kernel

On Tue, 7 Sep 2021 09:57:21 +0800
Cai Huoqing <caihuoqing@baidu.com> wrote:

> The NXP i.MX 8QuadXPlus SOC has a new ADC IP. These patches add
> driver support for this ADC.
> 
> Signed-off-by: Cai Huoqing <caihuoqing@baidu.com>

Hi,

A few minor things inline.  This is looking pretty good to me!

Jonathan

> ---
> v1->v2:	*Squash patches 1, 2, 3, and 5 into a single patch.
> 	*Add device specific prefix.
> 	*Remove the brackets around individual numbers.
> 	*Make use of FIELD_PREP() and FIELD_GET().
> 	*Remove a lot of cache values.
> 	*Replace mlock with adc->lock.
> 	*Move adc->value read from isr to the completion.
> 	*Set pm_runtime_disable/_put_noidle() before adc_disable.
> 	*Add error handler-err_disable_reg/err_unprepare_clk.
> v2->v3:	*Add "return 0" to adc_runtime_resume().
> v1 link:
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20210830172140.414-4-caihuoqing@baidu.com/
> 
>  drivers/iio/adc/Kconfig       |  10 +
>  drivers/iio/adc/Makefile      |   1 +
>  drivers/iio/adc/imx8qxp-adc.c | 470 ++++++++++++++++++++++++++++++++++
>  3 files changed, 481 insertions(+)
>  create mode 100644 drivers/iio/adc/imx8qxp-adc.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index af168e1c9fdb..fa8ad63d6ac2 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -530,6 +530,16 @@ config IMX7D_ADC
....
> diff --git a/drivers/iio/adc/imx8qxp-adc.c b/drivers/iio/adc/imx8qxp-adc.c
> new file mode 100644
> index 000000000000..9ba77af360f3
> --- /dev/null
> +++ b/drivers/iio/adc/imx8qxp-adc.c
> @@ -0,0 +1,470 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * NXP i.MX8QXP ADC driver
> + *
> + * Based on the work of Haibo Chen <haibo.chen@nxp.com>
> + * The initial developer of the original code is Haibo Chen.
> + * Portions created by Haibo Chen are Copyright (C) 2018 NXP.
> + * All Rights Reserved.
> + *
> + * Copyright (C) 2018 NXP
> + * Copyright (C) 2021 Cai Huoqing
> + */
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/delay.h>

Not in alphabetical order.

> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>

...

> +static irqreturn_t imx8qxp_adc_isr(int irq, void *dev_id)
> +{
> +	struct imx8qxp_adc *adc = (struct imx8qxp_adc *)dev_id;

Never any need to explicitly cast from void * in c.

	struct imx8qpx_adc *adc = dev_id;

> +
> +	u32 fifo_count;
> +
> +	fifo_count = FIELD_GET(IMX8QXP_ADC_FCTRL_FCOUNT_MASK,
> +			       readl(adc->regs + IMX8QXP_ADR_ADC_FCTRL));
> +
> +	if (fifo_count)
> +		complete(&adc->completion);
> +
> +	return IRQ_HANDLED;
> +}
> +

...

> +
> +static int imx8qxp_adc_probe(struct platform_device *pdev)
> +{
> +	struct imx8qxp_adc *adc;
> +	struct iio_dev *indio_dev;
> +	struct device *dev = &pdev->dev;
> +	int irq;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*adc));
> +	if (!indio_dev) {
> +		dev_err(dev, "Failed allocating iio device\n");
> +		return -ENOMEM;
> +	}
> +
> +	adc = iio_priv(indio_dev);
> +	adc->dev = dev;
> +
> +	mutex_init(&adc->lock);
> +	adc->regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(adc->regs))
> +		return PTR_ERR(adc->regs);
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0)
> +		return irq;
> +
> +	adc->clk = devm_clk_get(dev, "per");
> +	if (IS_ERR(adc->clk)) {
> +		ret = PTR_ERR(adc->clk);
> +		dev_err(dev, "Failed getting clock, err = %d\n", ret);
> +		return ret;
> +	}
> +
> +	adc->ipg_clk = devm_clk_get(dev, "ipg");
> +	if (IS_ERR(adc->ipg_clk)) {
> +		ret = PTR_ERR(adc->ipg_clk);
> +		dev_err(dev, "Failed getting clock, err = %d\n", ret);
> +		return ret;
> +	}
> +
> +	adc->vref = devm_regulator_get(dev, "vref");
> +	if (IS_ERR(adc->vref)) {
> +		ret = PTR_ERR(adc->vref);
> +		dev_err(dev,
		return dev_err_probe(dev, PTR_ERR(adc->vref), "Failed getting reference voltage\n");

as might be a deferred case and then we don't want errors filling up the log.
This function also deals with reporting error codes etc for you.
Same for anything else that might be deferred because of driver load ordering
(clks for example)


> +			"Failed getting reference voltage, err = %d\n", ret);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, indio_dev);
> +
> +	init_completion(&adc->completion);
> +
> +	indio_dev->name = dev_name(dev);
> +	indio_dev->info = &imx8qxp_adc_iio_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = imx8qxp_adc_iio_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(imx8qxp_adc_iio_channels);
> +
> +	ret = devm_request_irq(dev, irq, imx8qxp_adc_isr, 0, dev_name(dev), adc);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed requesting irq, irq = %d\n", irq);
> +		return ret;
> +	}
> +
> +	imx8qxp_adc_reset(adc);
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret) {
> +		imx8qxp_adc_disable(adc);
> +		dev_err(dev, "Couldn't register the device.\n");
> +		return ret;
> +	}
> +
> +	pm_runtime_set_active(dev);
> +	pm_runtime_set_autosuspend_delay(dev, 50);
> +	pm_runtime_use_autosuspend(dev);
> +	pm_runtime_enable(dev);
> +
> +	return 0;
> +}
> +

...

> +static int imx8qxp_adc_runtime_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct imx8qxp_adc *adc = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = regulator_enable(adc->vref);
> +	if (ret) {
> +		dev_err(dev, "Can't enable adc reference top voltage, err = %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(adc->clk);
> +	if (ret) {
> +		dev_err(dev, "Could not prepare or enable clock.\n");
> +		goto err_disable_reg;
> +	}
> +
> +	ret = clk_prepare_enable(adc->ipg_clk);
> +	if (ret) {
> +		dev_err(dev, "Could not prepare or enable clock.\n");
> +		goto err_unprepare_clk;
> +	}

Blank lines here (to separate statement + error handler from next statement)

> +	imx8qxp_adc_reset(adc);

here

> +	return 0;

and here would help readability (slightly!)

> +err_unprepare_clk:
> +	clk_disable_unprepare(adc->clk);
> +
> +err_disable_reg:
> +	regulator_disable(adc->vref);
> +
> +	return ret;
> +}
> +
...

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-09-11 17:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-07  1:57 [PATCH v3 0/3] iio: imx8qxp-adc: Add driver support for NXP IMX8QXP ADC Cai Huoqing
2021-09-07  1:57 ` [PATCH v3 1/3] " Cai Huoqing
2021-09-11 17:28   ` Jonathan Cameron
2021-09-07  1:57 ` [PATCH v3 2/3] dt-bindings: iio: adc: Add the binding documentation " Cai Huoqing
2021-09-07 11:49   ` Rob Herring
2021-09-07 16:46   ` Rob Herring
2021-09-07  1:57 ` [PATCH v3 3/3] MAINTAINERS: Add the driver info of the " Cai Huoqing

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).