[PATCH 1/3] iio: adc: add imx93 adc support

[PATCH 1/3] iio: adc: add imx93 adc support

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

The ADC in i.mx93 is a total new ADC IP, add a driver to support
this ADC.

Currently, only support one shot normal conversion triggered by
software. For other mode, will add in future.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 MAINTAINERS                 |   4 +-
 drivers/iio/adc/Kconfig     |  10 +
 drivers/iio/adc/Makefile    |   1 +
 drivers/iio/adc/imx93_adc.c | 448 ++++++++++++++++++++++++++++++++++++
 4 files changed, 462 insertions(+), 1 deletion(-)
 create mode 100644 drivers/iio/adc/imx93_adc.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 779f599f9abf..b0724635fb49 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14620,14 +14620,16 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/adc/nxp,imx8qxp-adc.yaml
 F:	drivers/iio/adc/imx8qxp-adc.c
 
-NXP i.MX 7D/6SX/6UL AND VF610 ADC DRIVER
+NXP i.MX 7D/6SX/6UL/93 AND VF610 ADC DRIVER
 M:	Haibo Chen <haibo.chen@nxp.com>
 L:	linux-iio@vger.kernel.org
 L:	linux-imx@nxp.com
 S:	Maintained
 F:	Documentation/devicetree/bindings/iio/adc/fsl,imx7d-adc.yaml
 F:	Documentation/devicetree/bindings/iio/adc/fsl,vf610-adc.yaml
+F:	Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
 F:	drivers/iio/adc/imx7d_adc.c
+F:	drivers/iio/adc/imx93_adc.c
 F:	drivers/iio/adc/vf610_adc.c
 
 NXP PF8100/PF8121A/PF8200 PMIC REGULATOR DEVICE DRIVER
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 7fe5930891e0..95399a0acbf4 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -550,6 +550,16 @@ config IMX8QXP_ADC
 	  This driver can also be built as a module. If so, the module will be
 	  called imx8qxp-adc.
 
+config IMX93_ADC
+	tristate "IMX93 ADC driver"
+	depends on ARCH_MXC || COMPILE_TEST
+	depends on HAS_IOMEM
+	help
+	  Say yes here to build support for IMX93 ADC.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called imx93_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 1772a549a3c8..64e17b21228b 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -48,6 +48,7 @@ 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_IMX93_ADC) += imx93_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/imx93_adc.c b/drivers/iio/adc/imx93_adc.c
new file mode 100644
index 000000000000..6e3cf6d3e629
--- /dev/null
+++ b/drivers/iio/adc/imx93_adc.c
@@ -0,0 +1,448 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * NXP i.MX93 ADC driver
+ *
+ * Copyright 2022 NXP
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/err.h>
+#include <linux/iio/iio.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+
+#define IMX93_ADC_DRIVER_NAME	"imx93-adc"
+
+/* Register map definition */
+#define IMX93_ADC_MCR		0x00
+#define IMX93_ADC_MSR		0x04
+#define IMX93_ADC_ISR		0x10
+#define IMX93_ADC_IMR		0x20
+#define IMX93_ADC_CIMR0		0x24
+#define IMX93_ADC_CTR0		0x94
+#define IMX93_ADC_NCMR0		0xA4
+#define IMX93_ADC_PCDR0		0x100
+#define IMX93_ADC_PCDR1		0x104
+#define IMX93_ADC_PCDR2		0x108
+#define IMX93_ADC_PCDR3		0x10c
+#define IMX93_ADC_PCDR4		0x110
+#define IMX93_ADC_PCDR5		0x114
+#define IMX93_ADC_PCDR6		0x118
+#define IMX93_ADC_PCDR7		0x11c
+#define IMX93_ADC_CALSTAT	0x39C
+
+/* ADC bit shift */
+#define IMX93_ADC_MCR_MODE_MASK			BIT(29)
+#define IMX93_ADC_MCR_NSTART_MASK		BIT(24)
+#define IMX93_ADC_MCR_CALSTART_MASK		BIT(14)
+#define IMX93_ADC_MCR_ADCLKSE_MASK		BIT(8)
+#define IMX93_ADC_MCR_PWDN_MASK			BIT(0)
+#define IMX93_ADC_MSR_CALFAIL_MASK		BIT(30)
+#define IMX93_ADC_MSR_CALBUSY_MASK		BIT(29)
+#define IMX93_ADC_MSR_ADCSTATUS_MASK		GENMASK(2, 0)
+#define IMX93_ADC_ISR_EOC_MASK			BIT(1)
+#define IMX93_ADC_IMR_JEOC_MASK			BIT(3)
+#define IMX93_ADC_IMR_JECH_MASK			BIT(2)
+#define IMX93_ADC_IMR_EOC_MASK			BIT(1)
+#define IMX93_ADC_IMR_ECH_MASK			BIT(0)
+#define IMX93_ADC_PCDR_CDATA_MASK		GENMASK(11, 0)
+
+/* ADC status */
+#define IDLE			0
+#define POWER_DOWN		1
+#define WAIT_STATE		2
+#define BUSY_IN_CALIBRATION	3
+#define SAMPLE			4
+#define CONVERSION		6
+
+#define IMX93_ADC_TIMEOUT		msecs_to_jiffies(100)
+
+struct imx93_adc {
+	struct device *dev;
+	void __iomem *regs;
+	struct clk *ipg_clk;
+	struct regulator *vref;
+	struct mutex lock;
+	struct completion completion;
+};
+
+#define IMX93_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 imx93_adc_iio_channels[] = {
+	IMX93_ADC_CHAN(0),
+	IMX93_ADC_CHAN(1),
+	IMX93_ADC_CHAN(2),
+	IMX93_ADC_CHAN(3),
+};
+
+static void imx93_adc_power_down(struct imx93_adc *adc)
+{
+	u32 mcr, msr;
+	int ret;
+
+	mcr = readl(adc->regs + IMX93_ADC_MCR);
+	mcr |= FIELD_PREP(IMX93_ADC_MCR_PWDN_MASK, 1);
+	writel(mcr, adc->regs + IMX93_ADC_MCR);
+
+	ret = readl_poll_timeout(adc->regs + IMX93_ADC_MSR, msr,
+		((msr & IMX93_ADC_MSR_ADCSTATUS_MASK) == POWER_DOWN), 1, 50);
+	if (ret == -ETIMEDOUT)
+		dev_warn(adc->dev,
+			 "ADC do not in power down mode, current MSR is %x\n",
+			 msr);
+}
+
+static void imx93_adc_config_ad_clk(struct imx93_adc *adc)
+{
+	u32 mcr;
+
+	/* put adc in power down mode */
+	imx93_adc_power_down(adc);
+
+	/* config the AD_CLK equal to bus clock */
+	mcr = readl(adc->regs + IMX93_ADC_MCR);
+	mcr |= FIELD_PREP(IMX93_ADC_MCR_ADCLKSE_MASK, 1);
+	writel(mcr, adc->regs + IMX93_ADC_MCR);
+
+	/* bring ADC out of power down state, in idle state */
+	mcr = readl(adc->regs + IMX93_ADC_MCR);
+	mcr &= ~FIELD_PREP(IMX93_ADC_MCR_PWDN_MASK, 1);
+	writel(mcr, adc->regs + IMX93_ADC_MCR);
+}
+
+static int imx93_adc_calibration(struct imx93_adc *adc)
+{
+	u32 mcr, msr, adc_status;
+	int ret;
+
+	/* make sure ADC in power down mode */
+	msr = readl(adc->regs + IMX93_ADC_MSR);
+	adc_status = FIELD_GET(IMX93_ADC_MSR_ADCSTATUS_MASK, msr);
+	if (adc_status != POWER_DOWN)
+		imx93_adc_power_down(adc);
+
+	/* config SAR controller operating clock */
+	mcr = readl(adc->regs + IMX93_ADC_MCR);
+	mcr &= ~FIELD_PREP(IMX93_ADC_MCR_ADCLKSE_MASK, 1);
+	writel(mcr, adc->regs + IMX93_ADC_MCR);
+
+	/* bring ADC out of power down state */
+	mcr = readl(adc->regs + IMX93_ADC_MCR);
+	mcr &= ~FIELD_PREP(IMX93_ADC_MCR_PWDN_MASK, 1);
+	writel(mcr, adc->regs + IMX93_ADC_MCR);
+
+	/*
+	 * TODO: we use the default TSAMP/NRSMPL/AVGEN in MCR,
+	 * can add the setting of these bit if need in future.
+	 */
+
+	/* run calibration */
+	mcr = readl(adc->regs + IMX93_ADC_MCR);
+	mcr |= FIELD_PREP(IMX93_ADC_MCR_CALSTART_MASK, 1);
+	writel(mcr, adc->regs + IMX93_ADC_MCR);
+
+	/* wait calibration to be finished */
+	ret = readl_poll_timeout(adc->regs + IMX93_ADC_MSR, msr,
+		!(msr & IMX93_ADC_MSR_CALBUSY_MASK), 1000, 2000000);
+	if (ret == -ETIMEDOUT) {
+		dev_warn(adc->dev, "ADC do not finish calibration in 1 min!\n");
+		return ret;
+	}
+
+	/* check whether calbration is success or not */
+	msr = readl(adc->regs + IMX93_ADC_MSR);
+	if (msr & IMX93_ADC_MSR_CALFAIL_MASK) {
+		dev_warn(adc->dev, "ADC calibration failed!\n");
+		return -EAGAIN;
+	}
+
+	return 0;
+}
+
+static int imx93_adc_read_raw(struct iio_dev *indio_dev,
+				struct iio_chan_spec const *chan,
+				int *val, int *val2, long mask)
+{
+	struct imx93_adc *adc = iio_priv(indio_dev);
+	struct device *dev = adc->dev;
+	u32 channel;
+	u32 imr, mcr, pcda;
+	long ret;
+	u32 vref_uv;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		pm_runtime_get_sync(dev);
+		mutex_lock(&adc->lock);
+
+		reinit_completion(&adc->completion);
+
+		/* config channel mask register */
+		channel = 1 << chan->channel;
+		writel(channel, adc->regs + IMX93_ADC_NCMR0);
+
+		/* TODO: can config desired sample time in CTRn if need */
+
+		/* config interrupt mask */
+		imr = FIELD_PREP(IMX93_ADC_IMR_EOC_MASK, 1);
+		writel(imr, adc->regs + IMX93_ADC_IMR);
+		writel(channel, adc->regs + IMX93_ADC_CIMR0);
+
+		/* config one-shot mode */
+		mcr = readl(adc->regs + IMX93_ADC_MCR);
+		mcr &= ~FIELD_PREP(IMX93_ADC_MCR_MODE_MASK, 1);
+		writel(mcr, adc->regs + IMX93_ADC_MCR);
+
+		/* start normal conversion */
+		mcr = readl(adc->regs + IMX93_ADC_MCR);
+		mcr |= FIELD_PREP(IMX93_ADC_MCR_NSTART_MASK, 1);
+		writel(mcr, adc->regs + IMX93_ADC_MCR);
+
+		ret = wait_for_completion_interruptible_timeout(&adc->completion,
+								IMX93_ADC_TIMEOUT);
+		if (ret == 0) {
+			mutex_unlock(&adc->lock);
+			return -ETIMEDOUT;
+		}
+		if (ret < 0) {
+			mutex_unlock(&adc->lock);
+			return ret;
+		}
+
+		pcda = readl(adc->regs + IMX93_ADC_PCDR0 + chan->channel * 4);
+
+		*val = FIELD_GET(IMX93_ADC_PCDR_CDATA_MASK, pcda);
+
+		mutex_unlock(&adc->lock);
+		pm_runtime_mark_last_busy(dev);
+		pm_runtime_put_sync_autosuspend(dev);
+		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->ipg_clk);
+		return IIO_VAL_INT;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static irqreturn_t imx93_adc_isr(int irq, void *dev_id)
+{
+	struct imx93_adc *adc = dev_id;
+	u32 isr;
+
+	isr = readl(adc->regs + IMX93_ADC_ISR);
+	writel(isr, adc->regs + IMX93_ADC_ISR);
+
+	if (FIELD_GET(IMX93_ADC_ISR_EOC_MASK, isr))
+		complete(&adc->completion);
+
+	return IRQ_HANDLED;
+}
+
+static const struct iio_info imx93_adc_iio_info = {
+	.read_raw = &imx93_adc_read_raw,
+};
+
+static int imx93_adc_probe(struct platform_device *pdev)
+{
+	struct imx93_adc *adc;
+	struct iio_dev *indio_dev;
+	struct device *dev = &pdev->dev;
+	int irq, 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);
+
+	/* The third irq is for ADC conversion usage */
+	irq = platform_get_irq(pdev, 2);
+	if (irq < 0)
+		return irq;
+
+	adc->ipg_clk = devm_clk_get(dev, "ipg");
+	if (IS_ERR(adc->ipg_clk))
+		return dev_err_probe(dev, PTR_ERR(adc->ipg_clk),
+				     "Failed getting clock\n");
+
+	adc->vref = devm_regulator_get(dev, "vref");
+	if (IS_ERR(adc->vref))
+		return dev_err_probe(dev, PTR_ERR(adc->vref),
+				     "Failed getting reference voltage\n");
+
+	ret = regulator_enable(adc->vref);
+	if (ret) {
+		dev_err(dev, "Can't enable adc reference top voltage\n");
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, indio_dev);
+
+	init_completion(&adc->completion);
+
+	indio_dev->name = IMX93_ADC_DRIVER_NAME;
+	indio_dev->info = &imx93_adc_iio_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = imx93_adc_iio_channels;
+	indio_dev->num_channels = ARRAY_SIZE(imx93_adc_iio_channels);
+
+	ret = clk_prepare_enable(adc->ipg_clk);
+	if (ret) {
+		dev_err(&pdev->dev, "Could not prepare or enable the clock.\n");
+		return ret;
+	}
+
+	ret = devm_request_irq(dev, irq, imx93_adc_isr, 0, IMX93_ADC_DRIVER_NAME, adc);
+	if (ret < 0) {
+		dev_err(dev, "Failed requesting irq, irq = %d\n", irq);
+		goto error_ipg_clk_disable;
+	}
+
+	ret = imx93_adc_calibration(adc);
+	if (ret < 0)
+		goto error_ipg_clk_disable;
+
+	imx93_adc_config_ad_clk(adc);
+
+	ret = iio_device_register(indio_dev);
+	if (ret) {
+		dev_err(dev, "Couldn't register the device.\n");
+		goto error_ipg_clk_disable;
+	}
+
+	pm_runtime_set_active(dev);
+	pm_runtime_set_autosuspend_delay(dev, 50);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_enable(dev);
+
+	return 0;
+
+error_ipg_clk_disable:
+	clk_disable_unprepare(adc->ipg_clk);
+	regulator_disable(adc->vref);
+
+	return ret;
+}
+
+static int imx93_adc_remove(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+	struct imx93_adc *adc = iio_priv(indio_dev);
+	struct device *dev = adc->dev;
+
+	pm_runtime_get_sync(dev);
+
+	iio_device_unregister(indio_dev);
+	imx93_adc_power_down(adc);
+	clk_disable_unprepare(adc->ipg_clk);
+	regulator_disable(adc->vref);
+
+	pm_runtime_disable(dev);
+	pm_runtime_put_noidle(dev);
+
+	return 0;
+}
+
+static int imx93_adc_runtime_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct imx93_adc *adc = iio_priv(indio_dev);
+
+	imx93_adc_power_down(adc);
+	clk_disable_unprepare(adc->ipg_clk);
+	regulator_disable(adc->vref);
+
+	return 0;
+}
+
+static int imx93_adc_runtime_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct imx93_adc *adc = iio_priv(indio_dev);
+	int ret;
+	u32 mcr;
+
+	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->ipg_clk);
+	if (ret) {
+		dev_err(dev, "Could not prepare or enable clock.\n");
+		goto err_disable_reg;
+	}
+
+	/* bring ADC out of power down state, in idle state */
+	mcr = readl(adc->regs + IMX93_ADC_MCR);
+	mcr &= ~FIELD_PREP(IMX93_ADC_MCR_PWDN_MASK, 1);
+	writel(mcr, adc->regs + IMX93_ADC_MCR);
+
+	return 0;
+
+err_disable_reg:
+	regulator_disable(adc->vref);
+
+	return ret;
+}
+
+static DEFINE_RUNTIME_DEV_PM_OPS(imx93_adc_pm_ops,
+				 imx93_adc_runtime_suspend,
+				 imx93_adc_runtime_resume, NULL);
+
+static const struct of_device_id imx93_adc_match[] = {
+	{ .compatible = "nxp,imx93-adc", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx93_adc_match);
+
+static struct platform_driver imx93_adc_driver = {
+	.probe		= imx93_adc_probe,
+	.remove		= imx93_adc_remove,
+	.driver		= {
+		.name	= IMX93_ADC_DRIVER_NAME,
+		.of_match_table = imx93_adc_match,
+		.pm	= pm_ptr(&imx93_adc_pm_ops),
+	},
+};
+
+module_platform_driver(imx93_adc_driver);
+
+MODULE_DESCRIPTION("NXP i.MX93 ADC driver");
+MODULE_AUTHOR("Haibo Chen <haibo.chen@nxp.com>");
+MODULE_LICENSE("GPL");
-- 
2.25.1

[PATCH 2/3] dt-bindings: iio: adc: Add binding documentation for NXP IMX93 ADC

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

The IMX93 SoC has a new ADC IP, so add binding documentation
for NXP IMX93 ADC.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 .../bindings/iio/adc/nxp,imx93-adc.yaml       | 65 +++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
new file mode 100644
index 000000000000..e0eac5aa81d7
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/nxp,imx93-adc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP ADC found on the imx93 SoC
+
+maintainers:
+  - Haibo Chen <haibo.chen@nxp.com>
+
+properties:
+  compatible:
+    const: nxp,imx93-adc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 4
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: ipg
+
+  vref-supply: true
+
+  "#io-channel-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - vref-supply
+  - "#io-channel-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/clock/imx93-clock.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    soc {
+        #address-cells = <1>;
+        #size-cells = <1>;
+        adc@44530000 {
+            compatible = "nxp,imx93-adc";
+            reg = <0x44530000 0x10000>;
+            interrupts = <GIC_SPI 217 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 218 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&clk IMX93_CLK_ADC1_GATE>;
+            clock-names = "ipg";
+            vref-supply = <&reg_vref_1v8>;
+            status = "disabled";
+        };
+    };
+...
-- 
2.25.1

[PATCH 3/3] arm64: dts: imx93: add ADC support

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

Add ADC support for imx93-11x11-evk board.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts | 12 ++++++++++++
 arch/arm64/boot/dts/freescale/imx93.dtsi          | 12 ++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts b/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts
index 69786c326db0..cab5f4d66bf9 100644
--- a/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts
@@ -15,6 +15,13 @@ chosen {
 		stdout-path = &lpuart1;
 	};
 
+	reg_vref_1v8: regulator-adc-vref {
+		compatible = "regulator-fixed";
+		regulator-name = "vref_1v8";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+	};
+
 	reg_usdhc2_vmmc: regulator-usdhc2 {
 		compatible = "regulator-fixed";
 		pinctrl-names = "default";
@@ -27,6 +34,11 @@ reg_usdhc2_vmmc: regulator-usdhc2 {
 	};
 };
 
+&adc1 {
+	vref-supply = <&reg_vref_1v8>;
+	status = "okay";
+};
+
 &mu1 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi b/arch/arm64/boot/dts/freescale/imx93.dtsi
index f83a07c7c9b1..78182f3a7942 100644
--- a/arch/arm64/boot/dts/freescale/imx93.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
@@ -165,6 +165,18 @@ anatop: anatop@44480000 {
 				compatible = "fsl,imx93-anatop", "syscon";
 				reg = <0x44480000 0x10000>;
 			};
+
+			adc1: adc@44530000 {
+				compatible = "nxp,imx93-adc";
+				reg = <0x44530000 0x10000>;
+				interrupts = <GIC_SPI 217 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 218 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk IMX93_CLK_ADC1_GATE>;
+				clock-names = "ipg";
+				status = "disabled";
+			};
 		};
 
 		aips2: bus@42000000 {
-- 
2.25.1

Re: [PATCH 2/3] dt-bindings: iio: adc: Add binding documentation for NXP IMX93 ADC

[Krzysztof Kozlowski]

On 03/08/2022 11:12, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> The IMX93 SoC has a new ADC IP, so add binding documentation
> for NXP IMX93 ADC.
> 
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  .../bindings/iio/adc/nxp,imx93-adc.yaml       | 65 +++++++++++++++++++
>  1 file changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> new file mode 100644
> index 000000000000..e0eac5aa81d7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/nxp,imx93-adc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP ADC found on the imx93 SoC

How different it is from ADC in imx8qxp?

> +
> +maintainers:
> +  - Haibo Chen <haibo.chen@nxp.com>
> +
> +properties:
> +  compatible:
> +    const: nxp,imx93-adc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 4

You need to describe items.

> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: ipg

No need for clock-names in such case.

> +
> +  vref-supply: true

Missing description.

> +
> +  "#io-channel-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - vref-supply
> +  - "#io-channel-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/clock/imx93-clock.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    soc {
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        adc@44530000 {
> +            compatible = "nxp,imx93-adc";
> +            reg = <0x44530000 0x10000>;
> +            interrupts = <GIC_SPI 217 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 218 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>;
> +            clocks = <&clk IMX93_CLK_ADC1_GATE>;
> +            clock-names = "ipg";
> +            vref-supply = <&reg_vref_1v8>;
> +            status = "disabled";

No status in the example.



Best regards,
Krzysztof

RE: [PATCH 2/3] dt-bindings: iio: adc: Add binding documentation for NXP IMX93 ADC

[Bough Chen]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: 2022年8月3日 18:20
> To: Bough Chen <haibo.chen@nxp.com>; jic23@kernel.org; lars@metafoo.de;
> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de
> Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> <linux-imx@nxp.com>; linux-iio@vger.kernel.org; devicetree@vger.kernel.org
> Subject: Re: [PATCH 2/3] dt-bindings: iio: adc: Add binding documentation for
> NXP IMX93 ADC
> 
> On 03/08/2022 11:12, haibo.chen@nxp.com wrote:
> > From: Haibo Chen <haibo.chen@nxp.com>
> >
> > The IMX93 SoC has a new ADC IP, so add binding documentation for NXP
> > IMX93 ADC.
> >
> > Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> > ---
> >  .../bindings/iio/adc/nxp,imx93-adc.yaml       | 65
> +++++++++++++++++++
> >  1 file changed, 65 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> > b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> > new file mode 100644
> > index 000000000000..e0eac5aa81d7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> > @@ -0,0 +1,65 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fschemas%2Fiio%2Fadc%2Fnxp%2Cimx93-adc.yaml%23&amp;d
> ata=0
> >
> +5%7C01%7Chaibo.chen%40nxp.com%7Ca11cd128f8814929684b08da7539b
> dbc%7C68
> >
> +6ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637951188101491669%
> 7CUnknown
> >
> +%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
> WwiLC
> >
> +JXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=JFNr4telb4AovE62YaHQu
> KNr1ywL%2
> > +Blc0dJMFNN1OA1U%3D&amp;reserved=0
> > +$schema:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=05%7C01%7Chaib
> o.che
> >
> +n%40nxp.com%7Ca11cd128f8814929684b08da7539bdbc%7C686ea1d3bc2
> b4c6fa92c
> >
> +d99c5c301635%7C0%7C0%7C637951188101491669%7CUnknown%7CTWF
> pbGZsb3d8eyJ
> >
> +WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> 7C300
> >
> +0%7C%7C%7C&amp;sdata=A1PPlSkOsS7nWFOPAokyA1F8%2BYFSZj5dY%2FO
> blm0U4UA%
> > +3D&amp;reserved=0
> > +
> > +title: NXP ADC found on the imx93 SoC
> 
> How different it is from ADC in imx8qxp?

They are totally two different ADC IP, no similar with each other.

> 
> > +
> > +maintainers:
> > +  - Haibo Chen <haibo.chen@nxp.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: nxp,imx93-adc
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 4
> 
> You need to describe items.

Will add.

> 
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    const: ipg
> 
> No need for clock-names in such case.

Okay

> 
> > +
> > +  vref-supply: true
> 
> Missing description.

Will add

> 
> > +
> > +  "#io-channel-cells":
> > +    const: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +  - vref-supply
> > +  - "#io-channel-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <dt-bindings/clock/imx93-clock.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    soc {
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +        adc@44530000 {
> > +            compatible = "nxp,imx93-adc";
> > +            reg = <0x44530000 0x10000>;
> > +            interrupts = <GIC_SPI 217 IRQ_TYPE_LEVEL_HIGH>,
> > +                         <GIC_SPI 218 IRQ_TYPE_LEVEL_HIGH>,
> > +                         <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>,
> > +                         <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>;
> > +            clocks = <&clk IMX93_CLK_ADC1_GATE>;
> > +            clock-names = "ipg";
> > +            vref-supply = <&reg_vref_1v8>;
> > +            status = "disabled";
> 
> No status in the example.

Okay.

Best Regards
Haibo Chen
> 
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH 2/3] dt-bindings: iio: adc: Add binding documentation for NXP IMX93 ADC

[Krzysztof Kozlowski]

On 04/08/2022 03:05, Bough Chen wrote:
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: 2022年8月3日 18:20
>> To: Bough Chen <haibo.chen@nxp.com>; jic23@kernel.org; lars@metafoo.de;
>> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
>> shawnguo@kernel.org; s.hauer@pengutronix.de
>> Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
>> <linux-imx@nxp.com>; linux-iio@vger.kernel.org; devicetree@vger.kernel.org
>> Subject: Re: [PATCH 2/3] dt-bindings: iio: adc: Add binding documentation for
>> NXP IMX93 ADC
>>
>> On 03/08/2022 11:12, haibo.chen@nxp.com wrote:
>>> From: Haibo Chen <haibo.chen@nxp.com>
>>>
>>> The IMX93 SoC has a new ADC IP, so add binding documentation for NXP
>>> IMX93 ADC.
>>>
>>> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
>>> ---
>>>  .../bindings/iio/adc/nxp,imx93-adc.yaml       | 65
>> +++++++++++++++++++
>>>  1 file changed, 65 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>> b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>> new file mode 100644
>>> index 000000000000..e0eac5aa81d7
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>> @@ -0,0 +1,65 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
>>> +---
>>> +$id:
>>> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
>>>
>> +cetree.org%2Fschemas%2Fiio%2Fadc%2Fnxp%2Cimx93-adc.yaml%23&amp;d
>> ata=0
>>>
>> +5%7C01%7Chaibo.chen%40nxp.com%7Ca11cd128f8814929684b08da7539b
>> dbc%7C68
>>>
>> +6ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637951188101491669%
>> 7CUnknown
>>>
>> +%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
>> WwiLC
>>>
>> +JXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=JFNr4telb4AovE62YaHQu
>> KNr1ywL%2
>>> +Blc0dJMFNN1OA1U%3D&amp;reserved=0
>>> +$schema:
>>> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
>>>
>> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=05%7C01%7Chaib
>> o.che
>>>
>> +n%40nxp.com%7Ca11cd128f8814929684b08da7539bdbc%7C686ea1d3bc2
>> b4c6fa92c
>>>
>> +d99c5c301635%7C0%7C0%7C637951188101491669%7CUnknown%7CTWF
>> pbGZsb3d8eyJ
>>>
>> +WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
>> 7C300
>>>
>> +0%7C%7C%7C&amp;sdata=A1PPlSkOsS7nWFOPAokyA1F8%2BYFSZj5dY%2FO
>> blm0U4UA%
>>> +3D&amp;reserved=0
>>> +
>>> +title: NXP ADC found on the imx93 SoC
>>
>> How different it is from ADC in imx8qxp?
> 
> They are totally two different ADC IP, no similar with each other.

Each submitter responds like that... how much different? What is
different? Driver has lots of copied pieces, so actually could be
unified as well.



Best regards,
Krzysztof

Re: [PATCH 1/3] iio: adc: add imx93 adc support

[Jonathan Cameron]

On Wed,  3 Aug 2022 17:12:25 +0800
haibo.chen@nxp.com wrote:

> From: Haibo Chen <haibo.chen@nxp.com>
> 
> The ADC in i.mx93 is a total new ADC IP, add a driver to support
> this ADC.
> 
> Currently, only support one shot normal conversion triggered by
> software. For other mode, will add in future.
> 
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>

Nice.

A few comments inline.

Thanks,

Jonathan


> diff --git a/drivers/iio/adc/imx93_adc.c b/drivers/iio/adc/imx93_adc.c
> new file mode 100644
> index 000000000000..6e3cf6d3e629
> --- /dev/null
> +++ b/drivers/iio/adc/imx93_adc.c
> @@ -0,0 +1,448 @@

...

> +
> +/* ADC status */
> +#define IDLE			0

Very generic names that are likely to clash with some define in a header
introduced in the future.  Prefix them all with IMX93_ADC_ to scope
them to this driver.

> +#define POWER_DOWN		1
> +#define WAIT_STATE		2
> +#define BUSY_IN_CALIBRATION	3
> +#define SAMPLE			4
> +#define CONVERSION		6
> +
> +#define IMX93_ADC_TIMEOUT		msecs_to_jiffies(100)
> +
> +struct imx93_adc {
> +	struct device *dev;
> +	void __iomem *regs;
> +	struct clk *ipg_clk;
> +	struct regulator *vref;
> +	struct mutex lock;

All locks should have documentation to explain what data they are protecting
(that can be in memory, or in the registers etc, or on a bus, but it's normally
some sort of data).

> +	struct completion completion;
> +};
> +
> +#define IMX93_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 imx93_adc_iio_channels[] = {
> +	IMX93_ADC_CHAN(0),
> +	IMX93_ADC_CHAN(1),
> +	IMX93_ADC_CHAN(2),
> +	IMX93_ADC_CHAN(3),
> +};
> +
> +static void imx93_adc_power_down(struct imx93_adc *adc)
> +{
> +	u32 mcr, msr;
> +	int ret;
> +
> +	mcr = readl(adc->regs + IMX93_ADC_MCR);
> +	mcr |= FIELD_PREP(IMX93_ADC_MCR_PWDN_MASK, 1);
> +	writel(mcr, adc->regs + IMX93_ADC_MCR);
> +
> +	ret = readl_poll_timeout(adc->regs + IMX93_ADC_MSR, msr,
> +		((msr & IMX93_ADC_MSR_ADCSTATUS_MASK) == POWER_DOWN), 1, 50);
> +	if (ret == -ETIMEDOUT)
> +		dev_warn(adc->dev,
> +			 "ADC do not in power down mode, current MSR is %x\n",
> +			 msr);
> +}
> +
> +static void imx93_adc_config_ad_clk(struct imx93_adc *adc)
> +{
> +	u32 mcr;
> +
> +	/* put adc in power down mode */
> +	imx93_adc_power_down(adc);
> +
> +	/* config the AD_CLK equal to bus clock */
> +	mcr = readl(adc->regs + IMX93_ADC_MCR);
> +	mcr |= FIELD_PREP(IMX93_ADC_MCR_ADCLKSE_MASK, 1);
> +	writel(mcr, adc->regs + IMX93_ADC_MCR);
> +
> +	/* bring ADC out of power down state, in idle state */
> +	mcr = readl(adc->regs + IMX93_ADC_MCR);
> +	mcr &= ~FIELD_PREP(IMX93_ADC_MCR_PWDN_MASK, 1);
> +	writel(mcr, adc->regs + IMX93_ADC_MCR);
> +}
> +
> +static int imx93_adc_calibration(struct imx93_adc *adc)
> +{
> +	u32 mcr, msr, adc_status;
> +	int ret;
> +
> +	/* make sure ADC in power down mode */

Is the (noop) write that would otherwise occur worth the code complexity here.
Putting it another way, why not just call imx93_adc_powerdown() unconditionally
and simplify this code?

> +	msr = readl(adc->regs + IMX93_ADC_MSR);
> +	adc_status = FIELD_GET(IMX93_ADC_MSR_ADCSTATUS_MASK, msr);
> +	if (adc_status != POWER_DOWN)
> +		imx93_adc_power_down(adc);
> +
> +	/* config SAR controller operating clock */
> +	mcr = readl(adc->regs + IMX93_ADC_MCR);
> +	mcr &= ~FIELD_PREP(IMX93_ADC_MCR_ADCLKSE_MASK, 1);
> +	writel(mcr, adc->regs + IMX93_ADC_MCR);
> +
> +	/* bring ADC out of power down state */
> +	mcr = readl(adc->regs + IMX93_ADC_MCR);
> +	mcr &= ~FIELD_PREP(IMX93_ADC_MCR_PWDN_MASK, 1);
> +	writel(mcr, adc->regs + IMX93_ADC_MCR);
> +
> +	/*
> +	 * TODO: we use the default TSAMP/NRSMPL/AVGEN in MCR,
> +	 * can add the setting of these bit if need in future.
> +	 */
> +
> +	/* run calibration */
> +	mcr = readl(adc->regs + IMX93_ADC_MCR);
> +	mcr |= FIELD_PREP(IMX93_ADC_MCR_CALSTART_MASK, 1);
> +	writel(mcr, adc->regs + IMX93_ADC_MCR);
> +
> +	/* wait calibration to be finished */
> +	ret = readl_poll_timeout(adc->regs + IMX93_ADC_MSR, msr,
> +		!(msr & IMX93_ADC_MSR_CALBUSY_MASK), 1000, 2000000);
> +	if (ret == -ETIMEDOUT) {
> +		dev_warn(adc->dev, "ADC do not finish calibration in 1 min!\n");
> +		return ret;
> +	}
> +
> +	/* check whether calbration is success or not */
> +	msr = readl(adc->regs + IMX93_ADC_MSR);
> +	if (msr & IMX93_ADC_MSR_CALFAIL_MASK) {
> +		dev_warn(adc->dev, "ADC calibration failed!\n");
> +		return -EAGAIN;
> +	}
> +
> +	return 0;
> +}
> +
> +static int imx93_adc_read_raw(struct iio_dev *indio_dev,
> +				struct iio_chan_spec const *chan,
> +				int *val, int *val2, long mask)
> +{
> +	struct imx93_adc *adc = iio_priv(indio_dev);
> +	struct device *dev = adc->dev;
> +	u32 channel;
> +	u32 imr, mcr, pcda;
> +	long ret;
> +	u32 vref_uv;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:

This is a big block.  For readability I'd suggest factoring the channel read part out as
a separate function that you then call from here.  Perhaps leaving hte runtime pm code and
locking external to that call.

> +		pm_runtime_get_sync(dev);
> +		mutex_lock(&adc->lock);
> +
> +		reinit_completion(&adc->completion);
> +
> +		/* config channel mask register */
> +		channel = 1 << chan->channel;
> +		writel(channel, adc->regs + IMX93_ADC_NCMR0);
> +
> +		/* TODO: can config desired sample time in CTRn if need */
> +
> +		/* config interrupt mask */
> +		imr = FIELD_PREP(IMX93_ADC_IMR_EOC_MASK, 1);
> +		writel(imr, adc->regs + IMX93_ADC_IMR);
> +		writel(channel, adc->regs + IMX93_ADC_CIMR0);
> +
> +		/* config one-shot mode */
> +		mcr = readl(adc->regs + IMX93_ADC_MCR);
> +		mcr &= ~FIELD_PREP(IMX93_ADC_MCR_MODE_MASK, 1);
> +		writel(mcr, adc->regs + IMX93_ADC_MCR);
> +
> +		/* start normal conversion */
> +		mcr = readl(adc->regs + IMX93_ADC_MCR);
> +		mcr |= FIELD_PREP(IMX93_ADC_MCR_NSTART_MASK, 1);
> +		writel(mcr, adc->regs + IMX93_ADC_MCR);
> +
> +		ret = wait_for_completion_interruptible_timeout(&adc->completion,
> +								IMX93_ADC_TIMEOUT);
> +		if (ret == 0) {
> +			mutex_unlock(&adc->lock);
> +			return -ETIMEDOUT;
> +		}
> +		if (ret < 0) {
> +			mutex_unlock(&adc->lock);
> +			return ret;
> +		}
> +
> +		pcda = readl(adc->regs + IMX93_ADC_PCDR0 + chan->channel * 4);
> +
> +		*val = FIELD_GET(IMX93_ADC_PCDR_CDATA_MASK, pcda);
> +
> +		mutex_unlock(&adc->lock);
> +		pm_runtime_mark_last_busy(dev);
> +		pm_runtime_put_sync_autosuspend(dev);
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		vref_uv = regulator_get_voltage(adc->vref);

IIRC Can return an error.  You should check and pass that error on if there
is one.

> +		*val = vref_uv / 1000;
> +		*val2 = 12;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = clk_get_rate(adc->ipg_clk);
> +		return IIO_VAL_INT;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static irqreturn_t imx93_adc_isr(int irq, void *dev_id)
> +{
> +	struct imx93_adc *adc = dev_id;
> +	u32 isr;
> +
> +	isr = readl(adc->regs + IMX93_ADC_ISR);
> +	writel(isr, adc->regs + IMX93_ADC_ISR);

I'm 'guessing' that ISR had multiple bits reflecting different
causes of the interrupt.  Normally we'd only want to clear
those bits that correspond to things we are expecting +
return IRQ_NONE if none of the bits we expect to see set are
(spurious interrupt).

> +
> +	if (FIELD_GET(IMX93_ADC_ISR_EOC_MASK, isr))
> +		complete(&adc->completion);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static const struct iio_info imx93_adc_iio_info = {
> +	.read_raw = &imx93_adc_read_raw,
> +};
> +
> +static int imx93_adc_probe(struct platform_device *pdev)
> +{
> +	struct imx93_adc *adc;
> +	struct iio_dev *indio_dev;
> +	struct device *dev = &pdev->dev;
> +	int irq, 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);
> +
> +	/* The third irq is for ADC conversion usage */
> +	irq = platform_get_irq(pdev, 2);
> +	if (irq < 0)
> +		return irq;
> +
> +	adc->ipg_clk = devm_clk_get(dev, "ipg");
> +	if (IS_ERR(adc->ipg_clk))
> +		return dev_err_probe(dev, PTR_ERR(adc->ipg_clk),
> +				     "Failed getting clock\n");
> +
> +	adc->vref = devm_regulator_get(dev, "vref");
> +	if (IS_ERR(adc->vref))
> +		return dev_err_probe(dev, PTR_ERR(adc->vref),
> +				     "Failed getting reference voltage\n");
> +
> +	ret = regulator_enable(adc->vref);
> +	if (ret) {
> +		dev_err(dev, "Can't enable adc reference top voltage\n");
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, indio_dev);
> +
> +	init_completion(&adc->completion);
> +
> +	indio_dev->name = IMX93_ADC_DRIVER_NAME;
> +	indio_dev->info = &imx93_adc_iio_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = imx93_adc_iio_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(imx93_adc_iio_channels);
> +
> +	ret = clk_prepare_enable(adc->ipg_clk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Could not prepare or enable the clock.\n");
> +		return ret;

Regulator left powered up.  You need another label in the error handling below
and appropriate goto.


> +	}
> +
> +	ret = devm_request_irq(dev, irq, imx93_adc_isr, 0, IMX93_ADC_DRIVER_NAME, adc);

Do not mix devm managed and non devm managed code.  That just needs to difficult
to reason about ordering in remove.  Two options to make this easy.

1) After the first non devm_ managed call (here the regulator_enable() above,
  stop using devm_ calls at all.
2) Use devm_add_action_or_reset() to register custom cleanup callbacks so that
   you can get rid of the remove() function and still guarantee the ordering
   is what is expected (almost always should be reverse of what is in probe())

I prefer option 2 personally but either is fine.  It can get a little fiddly to
get runtime pm tear down right when using devm to do it.


> +	if (ret < 0) {
> +		dev_err(dev, "Failed requesting irq, irq = %d\n", irq);
> +		goto error_ipg_clk_disable;
> +	}
> +
> +	ret = imx93_adc_calibration(adc);
> +	if (ret < 0)
> +		goto error_ipg_clk_disable;
> +
> +	imx93_adc_config_ad_clk(adc);
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret) {
> +		dev_err(dev, "Couldn't register the device.\n");
> +		goto error_ipg_clk_disable;
> +	}
> +
> +	pm_runtime_set_active(dev);
> +	pm_runtime_set_autosuspend_delay(dev, 50);
> +	pm_runtime_use_autosuspend(dev);
> +	pm_runtime_enable(dev);
> +
> +	return 0;
> +
> +error_ipg_clk_disable:
> +	clk_disable_unprepare(adc->ipg_clk);
> +	regulator_disable(adc->vref);
> +
> +	return ret;
> +}
> +
> +static int imx93_adc_remove(struct platform_device *pdev)
> +{
> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +	struct imx93_adc *adc = iio_priv(indio_dev);
> +	struct device *dev = adc->dev;
> +
> +	pm_runtime_get_sync(dev);

I would prefer to see  the runtime pm tear down all up here
as it corresponds to the setup at the end of probe()
I don't it will make any functional difference and it makes
the code easier to reason about in terms of ordering of calls.

> +
> +	iio_device_unregister(indio_dev);
> +	imx93_adc_power_down(adc);
> +	clk_disable_unprepare(adc->ipg_clk);
> +	regulator_disable(adc->vref);
> +
> +	pm_runtime_disable(dev);
> +	pm_runtime_put_noidle(dev);
> +
> +	return 0;
> +}
> +
> +static int imx93_adc_runtime_suspend(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct imx93_adc *adc = iio_priv(indio_dev);
> +
> +	imx93_adc_power_down(adc);
> +	clk_disable_unprepare(adc->ipg_clk);
> +	regulator_disable(adc->vref);
> +
> +	return 0;
> +}
> +
> +static int imx93_adc_runtime_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct imx93_adc *adc = iio_priv(indio_dev);
> +	int ret;
> +	u32 mcr;
> +
> +	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->ipg_clk);
> +	if (ret) {
> +		dev_err(dev, "Could not prepare or enable clock.\n");
> +		goto err_disable_reg;
> +	}
> +
> +	/* bring ADC out of power down state, in idle state */
> +	mcr = readl(adc->regs + IMX93_ADC_MCR);
> +	mcr &= ~FIELD_PREP(IMX93_ADC_MCR_PWDN_MASK, 1);
> +	writel(mcr, adc->regs + IMX93_ADC_MCR);

This 'power_up' code block occurs quite a few times. Probably
worth factoring out as a separate call alongside imx93_adc_power_down()

> +
> +	return 0;
> +
> +err_disable_reg:
> +	regulator_disable(adc->vref);
> +
> +	return ret;
> +}
> +
> +static DEFINE_RUNTIME_DEV_PM_OPS(imx93_adc_pm_ops,
> +				 imx93_adc_runtime_suspend,
> +				 imx93_adc_runtime_resume, NULL);
> +
> +static const struct of_device_id imx93_adc_match[] = {
> +	{ .compatible = "nxp,imx93-adc", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imx93_adc_match);
> +
> +static struct platform_driver imx93_adc_driver = {
> +	.probe		= imx93_adc_probe,
> +	.remove		= imx93_adc_remove,
> +	.driver		= {
> +		.name	= IMX93_ADC_DRIVER_NAME,
> +		.of_match_table = imx93_adc_match,
> +		.pm	= pm_ptr(&imx93_adc_pm_ops),
> +	},
> +};
> +
> +module_platform_driver(imx93_adc_driver);
> +
> +MODULE_DESCRIPTION("NXP i.MX93 ADC driver");
> +MODULE_AUTHOR("Haibo Chen <haibo.chen@nxp.com>");
> +MODULE_LICENSE("GPL");

RE: [PATCH 2/3] dt-bindings: iio: adc: Add binding documentation for NXP IMX93 ADC

[Bough Chen]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: 2022年8月4日 15:11
> To: Bough Chen <haibo.chen@nxp.com>; jic23@kernel.org; lars@metafoo.de;
> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de
> Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> <linux-imx@nxp.com>; linux-iio@vger.kernel.org; devicetree@vger.kernel.org
> Subject: Re: [PATCH 2/3] dt-bindings: iio: adc: Add binding documentation for
> NXP IMX93 ADC
> 
> On 04/08/2022 03:05, Bough Chen wrote:
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> Sent: 2022年8月3日 18:20
> >> To: Bough Chen <haibo.chen@nxp.com>; jic23@kernel.org;
> >> lars@metafoo.de;
> >> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> >> shawnguo@kernel.org; s.hauer@pengutronix.de
> >> Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> >> <linux-imx@nxp.com>; linux-iio@vger.kernel.org;
> >> devicetree@vger.kernel.org
> >> Subject: Re: [PATCH 2/3] dt-bindings: iio: adc: Add binding
> >> documentation for NXP IMX93 ADC
> >>
> >> On 03/08/2022 11:12, haibo.chen@nxp.com wrote:
> >>> From: Haibo Chen <haibo.chen@nxp.com>
> >>>
> >>> The IMX93 SoC has a new ADC IP, so add binding documentation for NXP
> >>> IMX93 ADC.
> >>>
> >>> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> >>> ---
> >>>  .../bindings/iio/adc/nxp,imx93-adc.yaml       | 65
> >> +++++++++++++++++++
> >>>  1 file changed, 65 insertions(+)
> >>>  create mode 100644
> >>> Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> >>>
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> >>> b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> >>> new file mode 100644
> >>> index 000000000000..e0eac5aa81d7
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> >>> @@ -0,0 +1,65 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> >>> +---
> >>> +$id:
> >>> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fde
> >>> +vi
> >>>
> >>
> +cetree.org%2Fschemas%2Fiio%2Fadc%2Fnxp%2Cimx93-adc.yaml%23&amp;d
> >> ata=0
> >>>
> >>
> +5%7C01%7Chaibo.chen%40nxp.com%7Ca11cd128f8814929684b08da7539b
> >> dbc%7C68
> >>>
> >> +6ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637951188101491669%
> >> 7CUnknown
> >>>
> >> +%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
> >> WwiLC
> >>>
> >>
> +JXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=JFNr4telb4AovE62YaHQu
> >> KNr1ywL%2
> >>> +Blc0dJMFNN1OA1U%3D&amp;reserved=0
> >>> +$schema:
> >>> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fde
> >>> +vi
> >>>
> >>
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=05%7C01%7Chaib
> >> o.che
> >>>
> >> +n%40nxp.com%7Ca11cd128f8814929684b08da7539bdbc%7C686ea1d3bc2
> >> b4c6fa92c
> >>>
> >> +d99c5c301635%7C0%7C0%7C637951188101491669%7CUnknown%7CTWF
> >> pbGZsb3d8eyJ
> >>>
> >>
> +WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> >> 7C300
> >>>
> >>
> +0%7C%7C%7C&amp;sdata=A1PPlSkOsS7nWFOPAokyA1F8%2BYFSZj5dY%2FO
> >> blm0U4UA%
> >>> +3D&amp;reserved=0
> >>> +
> >>> +title: NXP ADC found on the imx93 SoC
> >>
> >> How different it is from ADC in imx8qxp?
> >
> > They are totally two different ADC IP, no similar with each other.
> 
> Each submitter responds like that... how much different? What is different?
> Driver has lots of copied pieces, so actually could be unified as well.

HI Krzysztof,

Sorry for the delay, high loading on my current work.

For the difference, in general,
 First, the register define is totally different.
 Second, the ADC architecture is different, For imx8qxp, it contains ADC input ctrl + ADC core + ADC out control
        For imx93 ADC, it called SAR_ADC, contain ADCD + ADCA, in detail, it also contain calibration/self-test/watch dog timer IP logic, 
 Third, different conversion mode, 8QXP ADC support single and continue conversion, support average conversion.
        For imx93 ADC, it support normal mode, include single and average conversion, inject mode, hardware trigger mode.

These two drivers architecture looks similar, because they all under IIO subsystem.

For 8qxp ADC, it's feature list in RM:
? Support up to 16 analog inputs
? Support five conversion pairs, can work simultaneously, with different conversion
priority.
? Word size is 12-bits.
? Support Single and Continue conversion.
? Support Compare mode and channel auto disable if data match the requirement.
? Support Average conversion, Support flexible 4, 8, 16, 32 number of conversion
data.
? Configurable sample time and conversion speed / power. The ADC core clock can
vary from 300 kHz to 6 MHz, and the maximum sample rate is 1/6 ADC core clock.
? Conversion complete, hardware average complete, compare, DMA, time out flag and
interrupt.
? Automatic compare with interrupt for less than, greater than, and equal to, within
range, or out-of-range, programmable value.

For imx93 ADC, it's feature list in RM
? 4'd12-bit resolution
?Multiple modes of starting conversion (Normal, Injected)
―Normal mode supports One-Shot and Scan (continuous) conversions
―Injected mode supports One-Shot conversions only
?Software-initiated conversions in Normal and Injected modes, or external hardware trigger
?Two different abort features for either a single or chain conversion in Normal and Injected modes
?Independent data registers for each channel contain information about mode of conversion, data validity, overwrite status, and conversion data
?Alternate analog watchdog thresholds (threshold selected through input ports)
?Programmable DMA enables for each channel
?Individual interrupt flags for the following conditions:
―End of conversion of a single channel for Normal and Injected modes
―End of chain conversion for both Normal and Injected modes
―Watchdog threshold violations
?Programmable presampling for channels
?Auto-Clock-Off feature for improved power performance
?Power-Down mode to place the SAR_ADC in power-down state
?Programmable clock prescaler for SAR_ADC (bus clock, or bus clock divided by two or four)
?Software-initiated calibration
?Self-test feature

Best Regards
Haibo Chen
> 
> 
> 
> Best regards,
> Krzysztof

RE: [PATCH 1/3] iio: adc: add imx93 adc support

[Bough Chen]

> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: 2022年8月7日 0:03
> To: Bough Chen <haibo.chen@nxp.com>
> Cc: lars@metafoo.de; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>;
> linux-iio@vger.kernel.org; devicetree@vger.kernel.org
> Subject: Re: [PATCH 1/3] iio: adc: add imx93 adc support
> 
> On Wed,  3 Aug 2022 17:12:25 +0800
> haibo.chen@nxp.com wrote:
> 
> > From: Haibo Chen <haibo.chen@nxp.com>
> >
> > The ADC in i.mx93 is a total new ADC IP, add a driver to support this
> > ADC.
> >
> > Currently, only support one shot normal conversion triggered by
> > software. For other mode, will add in future.
> >
> > Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> 
> Nice.
> 
> A few comments inline.
> 
> Thanks,
> 
> Jonathan
> 
> 
> > diff --git a/drivers/iio/adc/imx93_adc.c b/drivers/iio/adc/imx93_adc.c
> > new file mode 100644 index 000000000000..6e3cf6d3e629
> > --- /dev/null
> > +++ b/drivers/iio/adc/imx93_adc.c
> > @@ -0,0 +1,448 @@
> 
> ..

.
...

> > +static int imx93_adc_remove(struct platform_device *pdev) {
> > +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> > +	struct imx93_adc *adc = iio_priv(indio_dev);
> > +	struct device *dev = adc->dev;
> > +
> > +	pm_runtime_get_sync(dev);
> 
> I would prefer to see  the runtime pm tear down all up here as it corresponds
> to the setup at the end of probe() I don't it will make any functional difference
> and it makes the code easier to reason about in terms of ordering of calls.

Hi Jonathan,

I can't get your point here, you mean call pm_runtime_disable and pm_runtime_put_noidle here?
I get sync the runtime pm here, because there are register operation in imx93_adc_power_down(), need to make sure clock is on.

Best Regards
Haibo Chen
> 
> > +
> > +	iio_device_unregister(indio_dev);
> > +	imx93_adc_power_down(adc);
> > +	clk_disable_unprepare(adc->ipg_clk);
> > +	regulator_disable(adc->vref);
> > +
> > +	pm_runtime_disable(dev);
> > +	pm_runtime_put_noidle(dev);
> > +
> > +	return 0;
> > +}
> > +

Re: [PATCH 2/3] dt-bindings: iio: adc: Add binding documentation for NXP IMX93 ADC

[Krzysztof Kozlowski]

On 12/08/2022 14:51, Bough Chen wrote:
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: 2022年8月4日 15:11
>> To: Bough Chen <haibo.chen@nxp.com>; jic23@kernel.org; lars@metafoo.de;
>> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
>> shawnguo@kernel.org; s.hauer@pengutronix.de
>> Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
>> <linux-imx@nxp.com>; linux-iio@vger.kernel.org; devicetree@vger.kernel.org
>> Subject: Re: [PATCH 2/3] dt-bindings: iio: adc: Add binding documentation for
>> NXP IMX93 ADC
>>
>> On 04/08/2022 03:05, Bough Chen wrote:
>>>> -----Original Message-----
>>>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> Sent: 2022年8月3日 18:20
>>>> To: Bough Chen <haibo.chen@nxp.com>; jic23@kernel.org;
>>>> lars@metafoo.de;
>>>> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
>>>> shawnguo@kernel.org; s.hauer@pengutronix.de
>>>> Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
>>>> <linux-imx@nxp.com>; linux-iio@vger.kernel.org;
>>>> devicetree@vger.kernel.org
>>>> Subject: Re: [PATCH 2/3] dt-bindings: iio: adc: Add binding
>>>> documentation for NXP IMX93 ADC
>>>>
>>>> On 03/08/2022 11:12, haibo.chen@nxp.com wrote:
>>>>> From: Haibo Chen <haibo.chen@nxp.com>
>>>>>
>>>>> The IMX93 SoC has a new ADC IP, so add binding documentation for NXP
>>>>> IMX93 ADC.
>>>>>
>>>>> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
>>>>> ---
>>>>>  .../bindings/iio/adc/nxp,imx93-adc.yaml       | 65
>>>> +++++++++++++++++++
>>>>>  1 file changed, 65 insertions(+)
>>>>>  create mode 100644
>>>>> Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>>>> b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..e0eac5aa81d7
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>>>> @@ -0,0 +1,65 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
>>>>> +---
>>>>> +$id:
>>>>> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fde
>>>>> +vi
>>>>>
>>>>
>> +cetree.org%2Fschemas%2Fiio%2Fadc%2Fnxp%2Cimx93-adc.yaml%23&amp;d
>>>> ata=0
>>>>>
>>>>
>> +5%7C01%7Chaibo.chen%40nxp.com%7Ca11cd128f8814929684b08da7539b
>>>> dbc%7C68
>>>>>
>>>> +6ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637951188101491669%
>>>> 7CUnknown
>>>>>
>>>> +%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
>>>> WwiLC
>>>>>
>>>>
>> +JXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=JFNr4telb4AovE62YaHQu
>>>> KNr1ywL%2
>>>>> +Blc0dJMFNN1OA1U%3D&amp;reserved=0
>>>>> +$schema:
>>>>> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fde
>>>>> +vi
>>>>>
>>>>
>> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=05%7C01%7Chaib
>>>> o.che
>>>>>
>>>> +n%40nxp.com%7Ca11cd128f8814929684b08da7539bdbc%7C686ea1d3bc2
>>>> b4c6fa92c
>>>>>
>>>> +d99c5c301635%7C0%7C0%7C637951188101491669%7CUnknown%7CTWF
>>>> pbGZsb3d8eyJ
>>>>>
>>>>
>> +WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
>>>> 7C300
>>>>>
>>>>
>> +0%7C%7C%7C&amp;sdata=A1PPlSkOsS7nWFOPAokyA1F8%2BYFSZj5dY%2FO
>>>> blm0U4UA%
>>>>> +3D&amp;reserved=0
>>>>> +
>>>>> +title: NXP ADC found on the imx93 SoC
>>>>
>>>> How different it is from ADC in imx8qxp?
>>>
>>> They are totally two different ADC IP, no similar with each other.
>>
>> Each submitter responds like that... how much different? What is different?
>> Driver has lots of copied pieces, so actually could be unified as well.
> 
> HI Krzysztof,
> 
> Sorry for the delay, high loading on my current work.
> 
> For the difference, in general,
>  First, the register define is totally different.
>  Second, the ADC architecture is different, For imx8qxp, it contains ADC input ctrl + ADC core + ADC out control
>         For imx93 ADC, it called SAR_ADC, contain ADCD + ADCA, in detail, it also contain calibration/self-test/watch dog timer IP logic, 
>  Third, different conversion mode, 8QXP ADC support single and continue conversion, support average conversion.
>         For imx93 ADC, it support normal mode, include single and average conversion, inject mode, hardware trigger mode.
> 
> These two drivers architecture looks similar, because they all under IIO subsystem.
> 
> For 8qxp ADC, it's feature list in RM:
> ? Support up to 16 analog inputs
> ? Support five conversion pairs, can work simultaneously, with different conversion
> priority.
> ? Word size is 12-bits.
> ? Support Single and Continue conversion.
> ? Support Compare mode and channel auto disable if data match the requirement.
> ? Support Average conversion, Support flexible 4, 8, 16, 32 number of conversion
> data.
> ? Configurable sample time and conversion speed / power. The ADC core clock can
> vary from 300 kHz to 6 MHz, and the maximum sample rate is 1/6 ADC core clock.
> ? Conversion complete, hardware average complete, compare, DMA, time out flag and
> interrupt.
> ? Automatic compare with interrupt for less than, greater than, and equal to, within
> range, or out-of-range, programmable value.
> 
> For imx93 ADC, it's feature list in RM
> ? 4'd12-bit resolution
> ?Multiple modes of starting conversion (Normal, Injected)
> —Normal mode supports One-Shot and Scan (continuous) conversions
> —Injected mode supports One-Shot conversions only
> ?Software-initiated conversions in Normal and Injected modes, or external hardware trigger
> ?Two different abort features for either a single or chain conversion in Normal and Injected modes
> ?Independent data registers for each channel contain information about mode of conversion, data validity, overwrite status, and conversion data
> ?Alternate analog watchdog thresholds (threshold selected through input ports)
> ?Programmable DMA enables for each channel
> ?Individual interrupt flags for the following conditions:
> —End of conversion of a single channel for Normal and Injected modes
> —End of chain conversion for both Normal and Injected modes
> —Watchdog threshold violations
> ?Programmable presampling for channels
> ?Auto-Clock-Off feature for improved power performance
> ?Power-Down mode to place the SAR_ADC in power-down state
> ?Programmable clock prescaler for SAR_ADC (bus clock, or bus clock divided by two or four)
> ?Software-initiated calibration
> ?Self-test feature

By pasting big pieces of description from RM you do not prove what are
the differences and it is not my task to find that one line which shows
the decisive difference (e.g. support or lack of support for DMA).

Best regards,
Krzysztof