1. Patchew
  2. linux
  3. [PATCH 0/2] Make i.MX 93 ADC calibration params configurable
  4. View patch

[PATCH 2/2] iio: adc: imx93: Make calibration parameters configurable

Primoz Fiser posted 2 patches 7 months ago
[PATCH 2/2] iio: adc: imx93: Make calibration parameters configurable
Posted by Primoz Fiser 7 months ago 
From: Andrej Picej <andrej.picej@norik.com>

Make i.MX93 ADC calibration parameters:
 - AVGEN: Enable calibration averaging function,
 - NRSMPL: Select number of calibration samples,
 - TSAMP: Select sample time of calibration conversions,

in the MCR register configurable with the corresponding device-tree
properties:
 - nxp,calib-avg-en,
 - nxp,calib-nr-samples and
 - nxp,calib-t-sample.

Signed-off-by: Andrej Picej <andrej.picej@norik.com>
Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
---
 drivers/iio/adc/imx93_adc.c | 75 ++++++++++++++++++++++++++++++++++---
 1 file changed, 70 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/imx93_adc.c b/drivers/iio/adc/imx93_adc.c
index 7feaafd2316f..da9b5c179240 100644
--- a/drivers/iio/adc/imx93_adc.c
+++ b/drivers/iio/adc/imx93_adc.c
@@ -18,6 +18,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
+#include <linux/property.h>
 
 #define IMX93_ADC_DRIVER_NAME	"imx93-adc"
 
@@ -43,6 +44,9 @@
 #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_AVGEN_MASK		BIT(13)
+#define IMX93_ADC_MCR_NRSMPL_MASK		GENMASK(12, 11)
+#define IMX93_ADC_MCR_TSAMP_MASK		GENMASK(10, 9)
 #define IMX93_ADC_MCR_ADCLKSE_MASK		BIT(8)
 #define IMX93_ADC_MCR_PWDN_MASK			BIT(0)
 #define IMX93_ADC_MSR_CALFAIL_MASK		BIT(30)
@@ -145,7 +149,7 @@ static void imx93_adc_config_ad_clk(struct imx93_adc *adc)
 
 static int imx93_adc_calibration(struct imx93_adc *adc)
 {
-	u32 mcr, msr;
+	u32 mcr, msr, val, reg;
 	int ret;
 
 	/* make sure ADC in power down mode */
@@ -156,12 +160,73 @@ static int imx93_adc_calibration(struct imx93_adc *adc)
 	mcr &= ~FIELD_PREP(IMX93_ADC_MCR_ADCLKSE_MASK, 1);
 	writel(mcr, adc->regs + IMX93_ADC_MCR);
 
-	imx93_adc_power_up(adc);
-
 	/*
-	 * TODO: we use the default TSAMP/NRSMPL/AVGEN in MCR,
-	 * can add the setting of these bit if need in future.
+	 * Optionally configure desired ADC calibration settings in MCR
+	 * - MCR[AVGEN]: Enable/disable calibration averaging function (default: on)
+	 * - MCR[NRSMPL]: Select the number of calibration samples (default: 512)
+	 * - MCR[TSAMP]: Select sample time of calibration conversions (default: 22)
 	 */
+	ret = device_property_read_u32(adc->dev, "nxp,calib-avg-en", &val);
+	if (!ret) {
+		if (val != 0 && val != 1) {
+			dev_err(adc->dev, "invalid nxp,calib-avg-en: %d\n", val);
+			return -EINVAL;
+		}
+		reg = val;
+		mcr &= ~IMX93_ADC_MCR_AVGEN_MASK;
+		mcr |= FIELD_PREP(IMX93_ADC_MCR_AVGEN_MASK, reg);
+	}
+
+	ret = device_property_read_u32(adc->dev, "nxp,calib-nr-samples", &val);
+	if (!ret) {
+		switch (val) {
+		case 16:
+			reg = 0x0;
+			break;
+		case 32:
+			reg = 0x1;
+			break;
+		case 128:
+			reg = 0x2;
+			break;
+		case 512:
+			reg = 0x3;
+			break;
+		default:
+			dev_err(adc->dev, "invalid nxp,calib-nr-samples: %d\n", val);
+			return -EINVAL;
+		}
+		mcr &= ~IMX93_ADC_MCR_NRSMPL_MASK;
+		mcr |= FIELD_PREP(IMX93_ADC_MCR_NRSMPL_MASK, reg);
+	}
+
+	ret = device_property_read_u32(adc->dev, "nxp,calib-t-sample", &val);
+	if (!ret) {
+		switch (val) {
+		case 8:
+			reg = 0x1;
+			break;
+		case 16:
+			reg = 0x2;
+			break;
+		case 22:
+			reg = 0x0;
+			break;
+		case 32:
+			reg = 0x3;
+			break;
+		default:
+			dev_err(adc->dev, "invalid nxp,calib-t-sample: %d\n", val);
+			return -EINVAL;
+		}
+		mcr &= ~IMX93_ADC_MCR_TSAMP_MASK;
+		mcr |= FIELD_PREP(IMX93_ADC_MCR_TSAMP_MASK, reg);
+	}
+
+	/* write calibration settings to MCR */
+	writel(mcr, adc->regs + IMX93_ADC_MCR);
+
+	imx93_adc_power_up(adc);
 
 	/* run calibration */
 	mcr = readl(adc->regs + IMX93_ADC_MCR);
-- 
2.34.1
Re: [PATCH 2/2] iio: adc: imx93: Make calibration parameters configurable
Posted by Andy Shevchenko 7 months ago 
On Thu, Jul 10, 2025 at 09:39:04AM +0200, Primoz Fiser wrote:
> 
> Make i.MX93 ADC calibration parameters:
>  - AVGEN: Enable calibration averaging function,
>  - NRSMPL: Select number of calibration samples,
>  - TSAMP: Select sample time of calibration conversions,
> 
> in the MCR register configurable with the corresponding device-tree
> properties:
>  - nxp,calib-avg-en,
>  - nxp,calib-nr-samples and
>  - nxp,calib-t-sample.

...

>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/property.h>

Keep it in order.

...

> +	ret = device_property_read_u32(adc->dev, "nxp,calib-avg-en", &val);
> +	if (!ret) {
> +		if (val != 0 && val != 1) {
> +			dev_err(adc->dev, "invalid nxp,calib-avg-en: %d\n", val);
> +			return -EINVAL;
> +		}
> +		reg = val;
> +		mcr &= ~IMX93_ADC_MCR_AVGEN_MASK;
> +		mcr |= FIELD_PREP(IMX93_ADC_MCR_AVGEN_MASK, reg);
> +	}

Please, since it's optional, do other way around.

	val = $DEFAUTL;
	device_property_read_u32(adc->dev, "nxp,calib-avg-en", &val);
	FIELD_MODIFY(...)

Similar approach may be used for the other properties.

...

> +	/* write calibration settings to MCR */
> +	writel(mcr, adc->regs + IMX93_ADC_MCR);

Please, factor out this to the function, so we won't see the direct IO in the
->probe().

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 2/2] iio: adc: imx93: Make calibration parameters configurable
Posted by Primoz Fiser 7 months ago 
Hi Andy,

On 10. 07. 25 11:22, Andy Shevchenko wrote:
> On Thu, Jul 10, 2025 at 09:39:04AM +0200, Primoz Fiser wrote:
>>
>> Make i.MX93 ADC calibration parameters:
>>  - AVGEN: Enable calibration averaging function,
>>  - NRSMPL: Select number of calibration samples,
>>  - TSAMP: Select sample time of calibration conversions,
>>
>> in the MCR register configurable with the corresponding device-tree
>> properties:
>>  - nxp,calib-avg-en,
>>  - nxp,calib-nr-samples and
>>  - nxp,calib-t-sample.
> 
> ...
> 
>>  #include <linux/platform_device.h>
>>  #include <linux/pm_runtime.h>
>>  #include <linux/regulator/consumer.h>
>> +#include <linux/property.h>
> 
> Keep it in order.

OK. Will fix for v2.

> 
> ...
> 
>> +	ret = device_property_read_u32(adc->dev, "nxp,calib-avg-en", &val);
>> +	if (!ret) {
>> +		if (val != 0 && val != 1) {
>> +			dev_err(adc->dev, "invalid nxp,calib-avg-en: %d\n", val);
>> +			return -EINVAL;
>> +		}
>> +		reg = val;
>> +		mcr &= ~IMX93_ADC_MCR_AVGEN_MASK;
>> +		mcr |= FIELD_PREP(IMX93_ADC_MCR_AVGEN_MASK, reg);
>> +	}
> 
> Please, since it's optional, do other way around.
> 
> 	val = $DEFAUTL;
> 	device_property_read_u32(adc->dev, "nxp,calib-avg-en", &val);
> 	FIELD_MODIFY(...)
> 
> Similar approach may be used for the other properties.

OK, I guess I could implement it like you suggested to explicitly set
the default parameter values.

But in current implementation MCR values are read at the beginning of
imx93_adc_calibration(), meaning calibration parameters are register POR
defaults. With you suggestion, we put defaults in software rather than
reading them from the hw directly.

> 
> ...
> 
>> +	/* write calibration settings to MCR */
>> +	writel(mcr, adc->regs + IMX93_ADC_MCR);
> 
> Please, factor out this to the function, so we won't see the direct IO in the
> ->probe().

Sorry I don't understand this part.

What do you mean by factoring out this writel()?

Do you perhaps suggest to implement function
imx93_adc_configure_calibration() and put all our changes into it?

But we are already in imx93_adc_calibration() which is separate from
probe().

Please explain.

BR,
Primoz
Re: [PATCH 2/2] iio: adc: imx93: Make calibration parameters configurable
Posted by Andy Shevchenko 7 months ago 
On Thu, Jul 10, 2025 at 12:23:58PM +0200, Primoz Fiser wrote:
> On 10. 07. 25 11:22, Andy Shevchenko wrote:
> > On Thu, Jul 10, 2025 at 09:39:04AM +0200, Primoz Fiser wrote:

...

> >> +	ret = device_property_read_u32(adc->dev, "nxp,calib-avg-en", &val);
> >> +	if (!ret) {
> >> +		if (val != 0 && val != 1) {
> >> +			dev_err(adc->dev, "invalid nxp,calib-avg-en: %d\n", val);
> >> +			return -EINVAL;
> >> +		}
> >> +		reg = val;
> >> +		mcr &= ~IMX93_ADC_MCR_AVGEN_MASK;
> >> +		mcr |= FIELD_PREP(IMX93_ADC_MCR_AVGEN_MASK, reg);
> >> +	}
> > 
> > Please, since it's optional, do other way around.
> > 
> > 	val = $DEFAUTL;
> > 	device_property_read_u32(adc->dev, "nxp,calib-avg-en", &val);
> > 	FIELD_MODIFY(...)
> > 
> > Similar approach may be used for the other properties.
> 
> OK, I guess I could implement it like you suggested to explicitly set
> the default parameter values.
> 
> But in current implementation MCR values are read at the beginning of
> imx93_adc_calibration(), meaning calibration parameters are register POR
> defaults. With you suggestion, we put defaults in software rather than
> reading them from the hw directly.

I see, then you need to read, do FIELD_GET()/device_property_read()/FIELD_MODIFY().
You got the idea.

...

> > Please, factor out this to the function, so we won't see the direct IO in the
> > ->probe().
> 
> Sorry I don't understand this part.
> 
> What do you mean by factoring out this writel()?
> 
> Do you perhaps suggest to implement function
> imx93_adc_configure_calibration() and put all our changes into it?
> 
> But we are already in imx93_adc_calibration() which is separate from
> probe().

Ah, sorry for the mistakenly read the function name. Ignore this comment.

-- 
With Best Regards,
Andy Shevchenko

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.