[PATCH 0/2] Make i.MX 93 ADC calibration params configurable

[PATCH 0/2] Make i.MX 93 ADC calibration params configurable

[Primoz Fiser]

The i.MX 93 ADC calibration parameters are configurable in the MCR
(Main Configuration Register). One can tweak bits:

 - MCR[AVGEN]: Enable calibration averaging function,
 - MCR[NRSMPL]: Select number of calibration samples,
 - MCR[TSAMP]: Select sample time of calibration conversions, 

Make it possible to configure those parameters via the corresponding
device-tree properties.

This patch series is based on the one we submitted in March 2024:
https://lore.kernel.org/linux-arm-kernel/20240320100407.1639082-1-andrej.picej@norik.com/

Changes since the original submission:
- rebase on linux-next
- reorder patches (bindings before users)
- fix checkpatch errors
- reword commit subject & body
- implement feedback from reviewers


Andrej Picej (2):
  dt-bindings: iio: adc: imx93: Add calibration properties
  iio: adc: imx93: Make calibration parameters configurable

 .../bindings/iio/adc/nxp,imx93-adc.yaml       | 21 ++++++
 drivers/iio/adc/imx93_adc.c                   | 75 +++++++++++++++++--
 2 files changed, 91 insertions(+), 5 deletions(-)

-- 
2.34.1

[PATCH 1/2] dt-bindings: iio: adc: imx93: Add calibration properties

[Primoz Fiser]

From: Andrej Picej <andrej.picej@norik.com>

Document i.MX93 ADC calibration properties and how to set them.

Signed-off-by: Andrej Picej <andrej.picej@norik.com>
Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
---
 .../bindings/iio/adc/nxp,imx93-adc.yaml       | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
index c2e5ff418920..d1c04cf85fe6 100644
--- a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
@@ -52,6 +52,27 @@ properties:
   "#io-channel-cells":
     const: 1
 
+  nxp,calib-avg-en:
+    default: 1
+    description:
+      Enable or disable calibration averaging function (AVGEN).
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [ 0, 1 ]
+
+  nxp,calib-nr-samples:
+    default: 512
+    description:
+      Selects number of samples (NRSMPL) to be used during calibration.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [ 16, 32, 128, 512 ]
+
+  nxp,calib-t-sample:
+    default: 22
+    description:
+      Selects sample time (TSAMP) of calibration conversions in ADC clock cycles
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [ 8, 16, 22, 32 ]
+
 required:
   - compatible
   - reg
-- 
2.34.1

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

[Primoz Fiser]

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

[Andy Shevchenko]

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

[Primoz Fiser]

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

[Andy Shevchenko]

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

Re: [PATCH 1/2] dt-bindings: iio: adc: imx93: Add calibration properties

[Frank Li]

On Thu, Jul 10, 2025 at 09:39:03AM +0200, Primoz Fiser wrote:
> From: Andrej Picej <andrej.picej@norik.com>
>
> Document i.MX93 ADC calibration properties and how to set them.
>
> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> ---
>  .../bindings/iio/adc/nxp,imx93-adc.yaml       | 21 +++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> index c2e5ff418920..d1c04cf85fe6 100644
> --- a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> @@ -52,6 +52,27 @@ properties:
>    "#io-channel-cells":
>      const: 1
>
> +  nxp,calib-avg-en:
> +    default: 1
> +    description:
> +      Enable or disable calibration averaging function (AVGEN).
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [ 0, 1 ]

bool type should be enough

> +
> +  nxp,calib-nr-samples:
> +    default: 512
> +    description:
> +      Selects number of samples (NRSMPL) to be used during calibration.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [ 16, 32, 128, 512 ]
> +
> +  nxp,calib-t-sample:
> +    default: 22
> +    description:
> +      Selects sample time (TSAMP) of calibration conversions in ADC clock cycles
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [ 8, 16, 22, 32 ]

Need some judgement in commit message, such as difference board need
difference nxp,calib-nr-samples value.

Frank
> +
>  required:
>    - compatible
>    - reg
> --
> 2.34.1
>

Re: [PATCH 1/2] dt-bindings: iio: adc: imx93: Add calibration properties

[David Lechner]

On 7/10/25 2:39 AM, Primoz Fiser wrote:
> From: Andrej Picej <andrej.picej@norik.com>
> 
> Document i.MX93 ADC calibration properties and how to set them.
> 
> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> ---
>  .../bindings/iio/adc/nxp,imx93-adc.yaml       | 21 +++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> index c2e5ff418920..d1c04cf85fe6 100644
> --- a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> @@ -52,6 +52,27 @@ properties:
>    "#io-channel-cells":
>      const: 1
>  
> +  nxp,calib-avg-en:
> +    default: 1
> +    description:
> +      Enable or disable calibration averaging function (AVGEN).
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [ 0, 1 ]
> +
> +  nxp,calib-nr-samples:
> +    default: 512
> +    description:
> +      Selects number of samples (NRSMPL) to be used during calibration.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [ 16, 32, 128, 512 ]
> +
> +  nxp,calib-t-sample:
> +    default: 22
> +    description:
> +      Selects sample time (TSAMP) of calibration conversions in ADC clock cycles
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [ 8, 16, 22, 32 ]
> +
>  required:
>    - compatible
>    - reg

This seem like things that should be set at runtime rather than
in the devicetree. Unless there is some justification on why
these values depend on how the chip is wired up?

Re: [PATCH 1/2] dt-bindings: iio: adc: imx93: Add calibration properties

[Jonathan Cameron]

On Thu, 10 Jul 2025 10:46:44 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 7/10/25 2:39 AM, Primoz Fiser wrote:
> > From: Andrej Picej <andrej.picej@norik.com>
> > 
> > Document i.MX93 ADC calibration properties and how to set them.
> > 
> > Signed-off-by: Andrej Picej <andrej.picej@norik.com>
> > Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> > ---
> >  .../bindings/iio/adc/nxp,imx93-adc.yaml       | 21 +++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> > index c2e5ff418920..d1c04cf85fe6 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> > @@ -52,6 +52,27 @@ properties:
> >    "#io-channel-cells":
> >      const: 1
> >  
> > +  nxp,calib-avg-en:
> > +    default: 1
> > +    description:
> > +      Enable or disable calibration averaging function (AVGEN).
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [ 0, 1 ]
> > +
> > +  nxp,calib-nr-samples:
> > +    default: 512
> > +    description:
> > +      Selects number of samples (NRSMPL) to be used during calibration.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [ 16, 32, 128, 512 ]

Allow 1 as a value and drop the enabled above.   Averaging over 1 sample
is same as no averaging and gives simpler binding.

> > +
> > +  nxp,calib-t-sample:
> > +    default: 22
> > +    description:
> > +      Selects sample time (TSAMP) of calibration conversions in ADC clock cycles
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [ 8, 16, 22, 32 ]
> > +
> >  required:
> >    - compatible
> >    - reg  
> 
> This seem like things that should be set at runtime rather than
> in the devicetree. Unless there is some justification on why
> these values depend on how the chip is wired up?

Further to that, I'd like to see some explanation of why we care
to change it at all. Is it ever a bad idea to enable averaging and
pick a large number of samples for calibration?

> 
> 

Re: [PATCH 1/2] dt-bindings: iio: adc: imx93: Add calibration properties

[Primoz Fiser]

Hi all,

On 13. 07. 25 17:02, Jonathan Cameron wrote:
> On Thu, 10 Jul 2025 10:46:44 -0500
> David Lechner <dlechner@baylibre.com> wrote:
> 
>> On 7/10/25 2:39 AM, Primoz Fiser wrote:
>>> From: Andrej Picej <andrej.picej@norik.com>
>>>
>>> Document i.MX93 ADC calibration properties and how to set them.
>>>
>>> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
>>> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
>>> ---
>>>  .../bindings/iio/adc/nxp,imx93-adc.yaml       | 21 +++++++++++++++++++
>>>  1 file changed, 21 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>> index c2e5ff418920..d1c04cf85fe6 100644
>>> --- a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>> +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>> @@ -52,6 +52,27 @@ properties:
>>>    "#io-channel-cells":
>>>      const: 1
>>>  
>>> +  nxp,calib-avg-en:
>>> +    default: 1
>>> +    description:
>>> +      Enable or disable calibration averaging function (AVGEN).
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    enum: [ 0, 1 ]
>>> +
>>> +  nxp,calib-nr-samples:
>>> +    default: 512
>>> +    description:
>>> +      Selects number of samples (NRSMPL) to be used during calibration.
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    enum: [ 16, 32, 128, 512 ]
> 
> Allow 1 as a value and drop the enabled above.   Averaging over 1 sample
> is same as no averaging and gives simpler binding.
> 
>>> +
>>> +  nxp,calib-t-sample:
>>> +    default: 22
>>> +    description:
>>> +      Selects sample time (TSAMP) of calibration conversions in ADC clock cycles
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    enum: [ 8, 16, 22, 32 ]
>>> +
>>>  required:
>>>    - compatible
>>>    - reg  
>>
>> This seem like things that should be set at runtime rather than
>> in the devicetree. Unless there is some justification on why
>> these values depend on how the chip is wired up?

It depends how ADC 1.8V Vref is wired up, especially how noisy it is.

> 
> Further to that, I'd like to see some explanation of why we care
> to change it at all. Is it ever a bad idea to enable averaging and
> pick a large number of samples for calibration?

This is a snippet from the i.MX93 TRM, chapter Analog-to-Digital
Converter (SAR_ADC) describing calibration steps:

1. Wait for deassertion of functional reset.
2. Configure SAR controller operating clock (MCR[ADCLKSE] = 0).
3. Bring ADC out of Power-down state (MCR[PWDN] = 0).
4. Configure desired calibration settings (default values kept for
highest accuracy maximum time).
• MCR[TSAMP]: Sample time for calibration conversion
• MCR[NRSMPL]: Number of samples in averaging
• MCR[AVGEN]: Averaging function enable in calibration
5. Run calibration by writing a one to MCR[CALSTART].
6. Check calibration run status in MSR[CALBUSY]—wait until MSR[CALBUSY]
= 0; alternatively, MSR[ADCSTAT] can be
used to check status.
7. Check calibration pass/fail status in MSR[CALFAIL] field. If
MSR[CALFAIL] = 1 then calibration failed. Detailed status
can be checked in CALSTAT.


See point 4).

Not sure why would there be an option to configure i.MX93 ADC
calibration parameters if one use-case (max accuracy max time) to rule
them all?

On the other hand, public TRM doesn't give much more information and
input from NXP would be highly desired.

@Haibo Chen your thoughts?

BR,
Primoz

> 
>>
>>
> 

-- 
Primoz Fiser
phone: +386-41-390-545
email: primoz.fiser@norik.com
--
Norik systems d.o.o.
Your embedded software partner
Slovenia, EU
phone: +386-41-540-545
email: info@norik.com

Re: [PATCH 1/2] dt-bindings: iio: adc: imx93: Add calibration properties

[Nuno Sá]

On Mon, 2025-07-14 at 07:56 +0200, Primoz Fiser wrote:
> Hi all,
> 
> On 13. 07. 25 17:02, Jonathan Cameron wrote:
> > On Thu, 10 Jul 2025 10:46:44 -0500
> > David Lechner <dlechner@baylibre.com> wrote:
> > 
> > > On 7/10/25 2:39 AM, Primoz Fiser wrote:
> > > > From: Andrej Picej <andrej.picej@norik.com>
> > > > 
> > > > Document i.MX93 ADC calibration properties and how to set them.
> > > > 
> > > > Signed-off-by: Andrej Picej <andrej.picej@norik.com>
> > > > Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> > > > ---
> > > >  .../bindings/iio/adc/nxp,imx93-adc.yaml       | 21 +++++++++++++++++++
> > > >  1 file changed, 21 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-
> > > > adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> > > > index c2e5ff418920..d1c04cf85fe6 100644
> > > > --- a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> > > > +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> > > > @@ -52,6 +52,27 @@ properties:
> > > >    "#io-channel-cells":
> > > >      const: 1
> > > >  
> > > > +  nxp,calib-avg-en:
> > > > +    default: 1
> > > > +    description:
> > > > +      Enable or disable calibration averaging function (AVGEN).
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    enum: [ 0, 1 ]
> > > > +
> > > > +  nxp,calib-nr-samples:
> > > > +    default: 512
> > > > +    description:
> > > > +      Selects number of samples (NRSMPL) to be used during calibration.
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    enum: [ 16, 32, 128, 512 ]
> > 
> > Allow 1 as a value and drop the enabled above.   Averaging over 1 sample
> > is same as no averaging and gives simpler binding.
> > 
> > > > +
> > > > +  nxp,calib-t-sample:
> > > > +    default: 22
> > > > +    description:
> > > > +      Selects sample time (TSAMP) of calibration conversions in ADC
> > > > clock cycles
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    enum: [ 8, 16, 22, 32 ]
> > > > +
> > > >  required:
> > > >    - compatible
> > > >    - reg  
> > > 
> > > This seem like things that should be set at runtime rather than
> > > in the devicetree. Unless there is some justification on why
> > > these values depend on how the chip is wired up?
> 
> It depends how ADC 1.8V Vref is wired up, especially how noisy it is.
> 
> > 
> > Further to that, I'd like to see some explanation of why we care
> > to change it at all. Is it ever a bad idea to enable averaging and
> > pick a large number of samples for calibration?
> 
> This is a snippet from the i.MX93 TRM, chapter Analog-to-Digital
> Converter (SAR_ADC) describing calibration steps:
> 
> 1. Wait for deassertion of functional reset.
> 2. Configure SAR controller operating clock (MCR[ADCLKSE] = 0).
> 3. Bring ADC out of Power-down state (MCR[PWDN] = 0).
> 4. Configure desired calibration settings (default values kept for
> highest accuracy maximum time).
> • MCR[TSAMP]: Sample time for calibration conversion
> • MCR[NRSMPL]: Number of samples in averaging
> • MCR[AVGEN]: Averaging function enable in calibration
> 5. Run calibration by writing a one to MCR[CALSTART].
> 6. Check calibration run status in MSR[CALBUSY]—wait until MSR[CALBUSY]
> = 0; alternatively, MSR[ADCSTAT] can be
> used to check status.
> 7. Check calibration pass/fail status in MSR[CALFAIL] field. If
> MSR[CALFAIL] = 1 then calibration failed. Detailed status
> can be checked in CALSTAT.
> 
> 
> See point 4).
> 
> Not sure why would there be an option to configure i.MX93 ADC
> calibration parameters if one use-case (max accuracy max time) to rule
> them all?
> 

Sometimes HW guys just want to give you some options. Does not mean we have to
use them all :).

I guess what Jonathan is interested in, is to understand in what conditions the
defaults are no good for the calibration? If we can have a set of values that
should pretty much always work, no need to further complicate the bindings or
the driver.

- Nuno Sá 
> On the other hand, public TRM doesn't give much more information and
> > 

Re: [PATCH 1/2] dt-bindings: iio: adc: imx93: Add calibration properties

[Primoz Fiser]

Hi Nuno,

On 14. 07. 25 18:11, Nuno Sá wrote:
> On Mon, 2025-07-14 at 07:56 +0200, Primoz Fiser wrote:
>> Hi all,
>>
>> On 13. 07. 25 17:02, Jonathan Cameron wrote:
>>> On Thu, 10 Jul 2025 10:46:44 -0500
>>> David Lechner <dlechner@baylibre.com> wrote:
>>>
>>>> On 7/10/25 2:39 AM, Primoz Fiser wrote:
>>>>> From: Andrej Picej <andrej.picej@norik.com>
>>>>>
>>>>> Document i.MX93 ADC calibration properties and how to set them.
>>>>>
>>>>> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
>>>>> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
>>>>> ---
>>>>>  .../bindings/iio/adc/nxp,imx93-adc.yaml       | 21 +++++++++++++++++++
>>>>>  1 file changed, 21 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-
>>>>> adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>>>> index c2e5ff418920..d1c04cf85fe6 100644
>>>>> --- a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>>>> +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>>>> @@ -52,6 +52,27 @@ properties:
>>>>>    "#io-channel-cells":
>>>>>      const: 1
>>>>>  
>>>>> +  nxp,calib-avg-en:
>>>>> +    default: 1
>>>>> +    description:
>>>>> +      Enable or disable calibration averaging function (AVGEN).
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>>> +    enum: [ 0, 1 ]
>>>>> +
>>>>> +  nxp,calib-nr-samples:
>>>>> +    default: 512
>>>>> +    description:
>>>>> +      Selects number of samples (NRSMPL) to be used during calibration.
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>>> +    enum: [ 16, 32, 128, 512 ]
>>>
>>> Allow 1 as a value and drop the enabled above.   Averaging over 1 sample
>>> is same as no averaging and gives simpler binding.
>>>
>>>>> +
>>>>> +  nxp,calib-t-sample:
>>>>> +    default: 22
>>>>> +    description:
>>>>> +      Selects sample time (TSAMP) of calibration conversions in ADC
>>>>> clock cycles
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>>> +    enum: [ 8, 16, 22, 32 ]
>>>>> +
>>>>>  required:
>>>>>    - compatible
>>>>>    - reg  
>>>>
>>>> This seem like things that should be set at runtime rather than
>>>> in the devicetree. Unless there is some justification on why
>>>> these values depend on how the chip is wired up?
>>
>> It depends how ADC 1.8V Vref is wired up, especially how noisy it is.
>>
>>>
>>> Further to that, I'd like to see some explanation of why we care
>>> to change it at all. Is it ever a bad idea to enable averaging and
>>> pick a large number of samples for calibration?
>>
>> This is a snippet from the i.MX93 TRM, chapter Analog-to-Digital
>> Converter (SAR_ADC) describing calibration steps:
>>
>> 1. Wait for deassertion of functional reset.
>> 2. Configure SAR controller operating clock (MCR[ADCLKSE] = 0).
>> 3. Bring ADC out of Power-down state (MCR[PWDN] = 0).
>> 4. Configure desired calibration settings (default values kept for
>> highest accuracy maximum time).
>> • MCR[TSAMP]: Sample time for calibration conversion
>> • MCR[NRSMPL]: Number of samples in averaging
>> • MCR[AVGEN]: Averaging function enable in calibration
>> 5. Run calibration by writing a one to MCR[CALSTART].
>> 6. Check calibration run status in MSR[CALBUSY]—wait until MSR[CALBUSY]
>> = 0; alternatively, MSR[ADCSTAT] can be
>> used to check status.
>> 7. Check calibration pass/fail status in MSR[CALFAIL] field. If
>> MSR[CALFAIL] = 1 then calibration failed. Detailed status
>> can be checked in CALSTAT.
>>
>>
>> See point 4).
>>
>> Not sure why would there be an option to configure i.MX93 ADC
>> calibration parameters if one use-case (max accuracy max time) to rule
>> them all?
>>
> 
> Sometimes HW guys just want to give you some options. Does not mean we have to
> use them all :).
> 
> I guess what Jonathan is interested in, is to understand in what conditions the
> defaults are no good for the calibration? If we can have a set of values that
> should pretty much always work, no need to further complicate the bindings or
> the driver.

In case you have a noisy Vref you can adjust the parameters to pass the
calibration and have a working ADC.

The trade-off is a less precise ADC but at least a working one.

In ideal case you would have Vref supplied by the dedicated LDO and tons
of decoupling caps, but in real-world you have it connected to a noisy
SMPS and you need to adjust the parameters accordingly.

That's it :)

BR,
Primoz

-- 
Primoz Fiser
phone: +386-41-390-545
email: primoz.fiser@norik.com
--
Norik systems d.o.o.
Your embedded software partner
Slovenia, EU
phone: +386-41-540-545
email: info@norik.com

Re: [PATCH 1/2] dt-bindings: iio: adc: imx93: Add calibration properties

[Jonathan Cameron]

On Tue, 15 Jul 2025 07:46:44 +0200
Primoz Fiser <primoz.fiser@norik.com> wrote:

> Hi Nuno,
> 
> On 14. 07. 25 18:11, Nuno Sá wrote:
> > On Mon, 2025-07-14 at 07:56 +0200, Primoz Fiser wrote:  
> >> Hi all,
> >>
> >> On 13. 07. 25 17:02, Jonathan Cameron wrote:  
> >>> On Thu, 10 Jul 2025 10:46:44 -0500
> >>> David Lechner <dlechner@baylibre.com> wrote:
> >>>  
> >>>> On 7/10/25 2:39 AM, Primoz Fiser wrote:  
> >>>>> From: Andrej Picej <andrej.picej@norik.com>
> >>>>>
> >>>>> Document i.MX93 ADC calibration properties and how to set them.
> >>>>>
> >>>>> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
> >>>>> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> >>>>> ---
> >>>>>  .../bindings/iio/adc/nxp,imx93-adc.yaml       | 21 +++++++++++++++++++
> >>>>>  1 file changed, 21 insertions(+)
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-
> >>>>> adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> >>>>> index c2e5ff418920..d1c04cf85fe6 100644
> >>>>> --- a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> >>>>> +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> >>>>> @@ -52,6 +52,27 @@ properties:
> >>>>>    "#io-channel-cells":
> >>>>>      const: 1
> >>>>>  
> >>>>> +  nxp,calib-avg-en:
> >>>>> +    default: 1
> >>>>> +    description:
> >>>>> +      Enable or disable calibration averaging function (AVGEN).
> >>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>>>> +    enum: [ 0, 1 ]
> >>>>> +
> >>>>> +  nxp,calib-nr-samples:
> >>>>> +    default: 512
> >>>>> +    description:
> >>>>> +      Selects number of samples (NRSMPL) to be used during calibration.
> >>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>>>> +    enum: [ 16, 32, 128, 512 ]  
> >>>
> >>> Allow 1 as a value and drop the enabled above.   Averaging over 1 sample
> >>> is same as no averaging and gives simpler binding.
> >>>  
> >>>>> +
> >>>>> +  nxp,calib-t-sample:
> >>>>> +    default: 22
> >>>>> +    description:
> >>>>> +      Selects sample time (TSAMP) of calibration conversions in ADC
> >>>>> clock cycles
> >>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>>>> +    enum: [ 8, 16, 22, 32 ]
> >>>>> +
> >>>>>  required:
> >>>>>    - compatible
> >>>>>    - reg    
> >>>>
> >>>> This seem like things that should be set at runtime rather than
> >>>> in the devicetree. Unless there is some justification on why
> >>>> these values depend on how the chip is wired up?  
> >>
> >> It depends how ADC 1.8V Vref is wired up, especially how noisy it is.
> >>  
> >>>
> >>> Further to that, I'd like to see some explanation of why we care
> >>> to change it at all. Is it ever a bad idea to enable averaging and
> >>> pick a large number of samples for calibration?  
> >>
> >> This is a snippet from the i.MX93 TRM, chapter Analog-to-Digital
> >> Converter (SAR_ADC) describing calibration steps:
> >>
> >> 1. Wait for deassertion of functional reset.
> >> 2. Configure SAR controller operating clock (MCR[ADCLKSE] = 0).
> >> 3. Bring ADC out of Power-down state (MCR[PWDN] = 0).
> >> 4. Configure desired calibration settings (default values kept for
> >> highest accuracy maximum time).
> >> • MCR[TSAMP]: Sample time for calibration conversion
> >> • MCR[NRSMPL]: Number of samples in averaging
> >> • MCR[AVGEN]: Averaging function enable in calibration
> >> 5. Run calibration by writing a one to MCR[CALSTART].
> >> 6. Check calibration run status in MSR[CALBUSY]—wait until MSR[CALBUSY]
> >> = 0; alternatively, MSR[ADCSTAT] can be
> >> used to check status.
> >> 7. Check calibration pass/fail status in MSR[CALFAIL] field. If
> >> MSR[CALFAIL] = 1 then calibration failed. Detailed status
> >> can be checked in CALSTAT.
> >>
> >>
> >> See point 4).
> >>
> >> Not sure why would there be an option to configure i.MX93 ADC
> >> calibration parameters if one use-case (max accuracy max time) to rule
> >> them all?
> >>  
> > 
> > Sometimes HW guys just want to give you some options. Does not mean we have to
> > use them all :).
> > 
> > I guess what Jonathan is interested in, is to understand in what conditions the
> > defaults are no good for the calibration? If we can have a set of values that
> > should pretty much always work, no need to further complicate the bindings or
> > the driver.  
> 
> In case you have a noisy Vref you can adjust the parameters to pass the
> calibration and have a working ADC.

That's a fairly odd sounding situation.  Is this a case of it will always
pass because there is some drift going on or something low frequency like that?

Or is it a case of retry until it passes?

Jonathan


> 
> The trade-off is a less precise ADC but at least a working one.
> 
> In ideal case you would have Vref supplied by the dedicated LDO and tons
> of decoupling caps, but in real-world you have it connected to a noisy
> SMPS and you need to adjust the parameters accordingly.
> 
> That's it :)
> 
> BR,
> Primoz
> 

RE: [PATCH 1/2] dt-bindings: iio: adc: imx93: Add calibration properties

[Bough Chen]

> -----Original Message-----
> From: Peng Fan (OSS) <peng.fan@oss.nxp.com>
> Sent: 2025年7月21日 17:39
> To: Nuno S? <noname.nuno@gmail.com>
> Cc: Primoz Fiser <primoz.fiser@norik.com>; Jonathan Cameron
> <jic23@kernel.org>; David Lechner <dlechner@baylibre.com>; Bough Chen
> <haibo.chen@nxp.com>; Nuno Sa <nuno.sa@analog.com>; Andy Shevchenko
> <andy@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn Guo
> <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; linux-iio@vger.kernel.org; imx@lists.linux.dev;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; upstream@lists.phytec.de;
> andrej.picej@norik.com
> Subject: Re: [PATCH 1/2] dt-bindings: iio: adc: imx93: Add calibration properties
> 
> On Mon, Jul 14, 2025 at 05:11:31PM +0100, Nuno S? wrote:
> >On Mon, 2025-07-14 at 07:56 +0200, Primoz Fiser wrote:
> >> Hi all,
> >>
> >> On 13. 07. 25 17:02, Jonathan Cameron wrote:
> >> > On Thu, 10 Jul 2025 10:46:44 -0500
> >> > David Lechner <dlechner@baylibre.com> wrote:
> >> >
> >> > > On 7/10/25 2:39 AM, Primoz Fiser wrote:
> >> > > > From: Andrej Picej <andrej.picej@norik.com>
> >> > > >
> >> > > > Document i.MX93 ADC calibration properties and how to set them.
> >> > > >
> >> > > > Signed-off-by: Andrej Picej <andrej.picej@norik.com>
> >> > > > Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> >> > > > ---
> >> > > > ??.../bindings/iio/adc/nxp,imx93-adc.yaml???????????? | 21
> >> > > > +++++++++++++++++++
> >> > > > ??1 file changed, 21 insertions(+)
> >> > > >
> >> > > > diff --git
> >> > > > a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-
> >> > > > adc.yaml
> >> > > > b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> >> > > > index c2e5ff418920..d1c04cf85fe6 100644
> >> > > > ---
> >> > > > a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> >> > > > +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.y
> >> > > > +++ aml
> >> > > > @@ -52,6 +52,27 @@ properties:
> >> > > > ???? "#io-channel-cells":
> >> > > > ???????? const: 1
> >> > > > ??
> >> > > > +?? nxp,calib-avg-en:
> >> > > > +?????? default: 1
> >> > > > +?????? description:
> >> > > > +?????????? Enable or disable calibration averaging function (AVGEN).
> >> > > > +?????? $ref: /schemas/types.yaml#/definitions/uint32
> >> > > > +?????? enum: [ 0, 1 ]
> >> > > > +
> >> > > > +?? nxp,calib-nr-samples:
> >> > > > +?????? default: 512
> >> > > > +?????? description:
> >> > > > +?????????? Selects number of samples (NRSMPL) to be used during
> calibration.
> >> > > > +?????? $ref: /schemas/types.yaml#/definitions/uint32
> >> > > > +?????? enum: [ 16, 32, 128, 512 ]
> >> >
> >> > Allow 1 as a value and drop the enabled above.???? Averaging over 1
> >> > sample is same as no averaging and gives simpler binding.
> >> >
> >> > > > +
> >> > > > +?? nxp,calib-t-sample:
> >> > > > +?????? default: 22
> >> > > > +?????? description:
> >> > > > +?????????? Selects sample time (TSAMP) of calibration
> >> > > > +conversions in ADC
> >> > > > clock cycles
> >> > > > +?????? $ref: /schemas/types.yaml#/definitions/uint32
> >> > > > +?????? enum: [ 8, 16, 22, 32 ]
> >> > > > +
> >> > > > ??required:
> >> > > > ???? - compatible
> >> > > > ???? - reg??
> >> > >
> >> > > This seem like things that should be set at runtime rather than
> >> > > in the devicetree. Unless there is some justification on why
> >> > > these values depend on how the chip is wired up?
> >>
> >> It depends how ADC 1.8V Vref is wired up, especially how noisy it is.
> >>
> >> >
> >> > Further to that, I'd like to see some explanation of why we care to
> >> > change it at all. Is it ever a bad idea to enable averaging and
> >> > pick a large number of samples for calibration?
> >>
> >> This is a snippet from the i.MX93 TRM, chapter Analog-to-Digital
> >> Converter (SAR_ADC) describing calibration steps:
> >>
> >> 1. Wait for deassertion of functional reset.
> >> 2. Configure SAR controller operating clock (MCR[ADCLKSE] = 0).
> >> 3. Bring ADC out of Power-down state (MCR[PWDN] = 0).
> >> 4. Configure desired calibration settings (default values kept for
> >> highest accuracy maximum time).
> >> ??? MCR[TSAMP]: Sample time for calibration conversion ???
> >> MCR[NRSMPL]: Number of samples in averaging ??? MCR[AVGEN]: Averaging
> >> function enable in calibration 5. Run calibration by writing a one to
> >> MCR[CALSTART].
> >> 6. Check calibration run status in MSR[CALBUSY]???wait until
> >> MSR[CALBUSY] = 0; alternatively, MSR[ADCSTAT] can be used to check
> >> status.
> >> 7. Check calibration pass/fail status in MSR[CALFAIL] field. If
> >> MSR[CALFAIL] = 1 then calibration failed. Detailed status can be
> >> checked in CALSTAT.
> >>
> >>
> >> See point 4).
> >>
> >> Not sure why would there be an option to configure i.MX93 ADC
> >> calibration parameters if one use-case (max accuracy max time) to
> >> rule them all?
> >>
> >
> >Sometimes HW guys just want to give you some options. Does not mean we
> >have to use them all :).
> >
> >I guess what Jonathan is interested in, is to understand in what
> >conditions the defaults are no good for the calibration? If we can have
> >a set of values that should pretty much always work, no need to further
> >complicate the bindings or the driver.
> 
> Just my understanding, it is hard to use one fixed settings to fit all kinds of
> conditions.
> 
>                  Noise induced from PCB tracks  Electro- magnetic noise
> 		     |                           |
> 		     V                           V
>  ---------
> |SOC(ADC)|   <---------------------------------<- (~) external Signal
>  ---------
>                  ^                 ^
>                  |                 |
>              I/O coupled noise    Internal noise
> 
> 
> So OEM A's board may needs different settings compared with OEM B's board.

The noise on Vref did impact the calibration, we did get report from customer, and IC guys suggested to do like the following patch, what's your comments?

https://patchwork.kernel.org/project/linux-iio/patch/20250423-adcpatch-v1-1-b0e84c27ae98@nxp.com/

Regards
Haibo Chen 
> 
> Regards,
> Peng
> 
> >
> >- Nuno S??
> >> On the other hand, public TRM doesn't give much more information and
> >> >

Re: [PATCH 1/2] dt-bindings: iio: adc: imx93: Add calibration properties

[Peng Fan]

On Mon, Jul 14, 2025 at 05:11:31PM +0100, Nuno S? wrote:
>On Mon, 2025-07-14 at 07:56 +0200, Primoz Fiser wrote:
>> Hi all,
>> 
>> On 13. 07. 25 17:02, Jonathan Cameron wrote:
>> > On Thu, 10 Jul 2025 10:46:44 -0500
>> > David Lechner <dlechner@baylibre.com> wrote:
>> > 
>> > > On 7/10/25 2:39 AM, Primoz Fiser wrote:
>> > > > From: Andrej Picej <andrej.picej@norik.com>
>> > > > 
>> > > > Document i.MX93 ADC calibration properties and how to set them.
>> > > > 
>> > > > Signed-off-by: Andrej Picej <andrej.picej@norik.com>
>> > > > Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
>> > > > ---
>> > > > ??.../bindings/iio/adc/nxp,imx93-adc.yaml???????????? | 21 +++++++++++++++++++
>> > > > ??1 file changed, 21 insertions(+)
>> > > > 
>> > > > diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-
>> > > > adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>> > > > index c2e5ff418920..d1c04cf85fe6 100644
>> > > > --- a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>> > > > +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>> > > > @@ -52,6 +52,27 @@ properties:
>> > > > ???? "#io-channel-cells":
>> > > > ???????? const: 1
>> > > > ??
>> > > > +?? nxp,calib-avg-en:
>> > > > +?????? default: 1
>> > > > +?????? description:
>> > > > +?????????? Enable or disable calibration averaging function (AVGEN).
>> > > > +?????? $ref: /schemas/types.yaml#/definitions/uint32
>> > > > +?????? enum: [ 0, 1 ]
>> > > > +
>> > > > +?? nxp,calib-nr-samples:
>> > > > +?????? default: 512
>> > > > +?????? description:
>> > > > +?????????? Selects number of samples (NRSMPL) to be used during calibration.
>> > > > +?????? $ref: /schemas/types.yaml#/definitions/uint32
>> > > > +?????? enum: [ 16, 32, 128, 512 ]
>> > 
>> > Allow 1 as a value and drop the enabled above.???? Averaging over 1 sample
>> > is same as no averaging and gives simpler binding.
>> > 
>> > > > +
>> > > > +?? nxp,calib-t-sample:
>> > > > +?????? default: 22
>> > > > +?????? description:
>> > > > +?????????? Selects sample time (TSAMP) of calibration conversions in ADC
>> > > > clock cycles
>> > > > +?????? $ref: /schemas/types.yaml#/definitions/uint32
>> > > > +?????? enum: [ 8, 16, 22, 32 ]
>> > > > +
>> > > > ??required:
>> > > > ???? - compatible
>> > > > ???? - reg?? 
>> > > 
>> > > This seem like things that should be set at runtime rather than
>> > > in the devicetree. Unless there is some justification on why
>> > > these values depend on how the chip is wired up?
>> 
>> It depends how ADC 1.8V Vref is wired up, especially how noisy it is.
>> 
>> > 
>> > Further to that, I'd like to see some explanation of why we care
>> > to change it at all. Is it ever a bad idea to enable averaging and
>> > pick a large number of samples for calibration?
>> 
>> This is a snippet from the i.MX93 TRM, chapter Analog-to-Digital
>> Converter (SAR_ADC) describing calibration steps:
>> 
>> 1. Wait for deassertion of functional reset.
>> 2. Configure SAR controller operating clock (MCR[ADCLKSE] = 0).
>> 3. Bring ADC out of Power-down state (MCR[PWDN] = 0).
>> 4. Configure desired calibration settings (default values kept for
>> highest accuracy maximum time).
>> ??? MCR[TSAMP]: Sample time for calibration conversion
>> ??? MCR[NRSMPL]: Number of samples in averaging
>> ??? MCR[AVGEN]: Averaging function enable in calibration
>> 5. Run calibration by writing a one to MCR[CALSTART].
>> 6. Check calibration run status in MSR[CALBUSY]???wait until MSR[CALBUSY]
>> = 0; alternatively, MSR[ADCSTAT] can be
>> used to check status.
>> 7. Check calibration pass/fail status in MSR[CALFAIL] field. If
>> MSR[CALFAIL] = 1 then calibration failed. Detailed status
>> can be checked in CALSTAT.
>> 
>> 
>> See point 4).
>> 
>> Not sure why would there be an option to configure i.MX93 ADC
>> calibration parameters if one use-case (max accuracy max time) to rule
>> them all?
>> 
>
>Sometimes HW guys just want to give you some options. Does not mean we have to
>use them all :).
>
>I guess what Jonathan is interested in, is to understand in what conditions the
>defaults are no good for the calibration? If we can have a set of values that
>should pretty much always work, no need to further complicate the bindings or
>the driver.

Just my understanding, it is hard to use one fixed settings to fit all
kinds of conditions.

                 Noise induced from PCB tracks  Electro- magnetic noise
		     |                           |
		     V                           V
 ---------
|SOC(ADC)|   <---------------------------------<- (~) external Signal
 ---------
                 ^                 ^
                 |                 |
             I/O coupled noise    Internal noise


So OEM A's board may needs different settings compared with OEM B's board.

Regards,
Peng

>
>- Nuno S?? 
>> On the other hand, public TRM doesn't give much more information and
>> > 

Re: [PATCH 1/2] dt-bindings: iio: adc: imx93: Add calibration properties

[Jonathan Cameron]

On Mon, 21 Jul 2025 09:09:06 +0000
Bough Chen <haibo.chen@nxp.com> wrote:

> > -----Original Message-----
> > From: Peng Fan (OSS) <peng.fan@oss.nxp.com>
> > Sent: 2025年7月21日 17:39
> > To: Nuno S? <noname.nuno@gmail.com>
> > Cc: Primoz Fiser <primoz.fiser@norik.com>; Jonathan Cameron
> > <jic23@kernel.org>; David Lechner <dlechner@baylibre.com>; Bough Chen
> > <haibo.chen@nxp.com>; Nuno Sa <nuno.sa@analog.com>; Andy Shevchenko
> > <andy@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> > <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn Guo
> > <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> > Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> > <festevam@gmail.com>; linux-iio@vger.kernel.org; imx@lists.linux.dev;
> > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > linux-kernel@vger.kernel.org; upstream@lists.phytec.de;
> > andrej.picej@norik.com
> > Subject: Re: [PATCH 1/2] dt-bindings: iio: adc: imx93: Add calibration properties
> > 
> > On Mon, Jul 14, 2025 at 05:11:31PM +0100, Nuno S? wrote:  
> > >On Mon, 2025-07-14 at 07:56 +0200, Primoz Fiser wrote:  
> > >> Hi all,
> > >>
> > >> On 13. 07. 25 17:02, Jonathan Cameron wrote:  
> > >> > On Thu, 10 Jul 2025 10:46:44 -0500
> > >> > David Lechner <dlechner@baylibre.com> wrote:
> > >> >  
> > >> > > On 7/10/25 2:39 AM, Primoz Fiser wrote:  
> > >> > > > From: Andrej Picej <andrej.picej@norik.com>
> > >> > > >
> > >> > > > Document i.MX93 ADC calibration properties and how to set them.
> > >> > > >
> > >> > > > Signed-off-by: Andrej Picej <andrej.picej@norik.com>
> > >> > > > Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> > >> > > > ---
> > >> > > > ??.../bindings/iio/adc/nxp,imx93-adc.yaml???????????? | 21
> > >> > > > +++++++++++++++++++
> > >> > > > ??1 file changed, 21 insertions(+)
> > >> > > >
> > >> > > > diff --git
> > >> > > > a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-
> > >> > > > adc.yaml
> > >> > > > b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> > >> > > > index c2e5ff418920..d1c04cf85fe6 100644
> > >> > > > ---
> > >> > > > a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> > >> > > > +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.y
> > >> > > > +++ aml
> > >> > > > @@ -52,6 +52,27 @@ properties:
> > >> > > > ???? "#io-channel-cells":
> > >> > > > ???????? const: 1
> > >> > > > ??
> > >> > > > +?? nxp,calib-avg-en:
> > >> > > > +?????? default: 1
> > >> > > > +?????? description:
> > >> > > > +?????????? Enable or disable calibration averaging function (AVGEN).
> > >> > > > +?????? $ref: /schemas/types.yaml#/definitions/uint32
> > >> > > > +?????? enum: [ 0, 1 ]
> > >> > > > +
> > >> > > > +?? nxp,calib-nr-samples:
> > >> > > > +?????? default: 512
> > >> > > > +?????? description:
> > >> > > > +?????????? Selects number of samples (NRSMPL) to be used during  
> > calibration.  
> > >> > > > +?????? $ref: /schemas/types.yaml#/definitions/uint32
> > >> > > > +?????? enum: [ 16, 32, 128, 512 ]  
> > >> >
> > >> > Allow 1 as a value and drop the enabled above.???? Averaging over 1
> > >> > sample is same as no averaging and gives simpler binding.
> > >> >  
> > >> > > > +
> > >> > > > +?? nxp,calib-t-sample:
> > >> > > > +?????? default: 22
> > >> > > > +?????? description:
> > >> > > > +?????????? Selects sample time (TSAMP) of calibration
> > >> > > > +conversions in ADC
> > >> > > > clock cycles
> > >> > > > +?????? $ref: /schemas/types.yaml#/definitions/uint32
> > >> > > > +?????? enum: [ 8, 16, 22, 32 ]
> > >> > > > +
> > >> > > > ??required:
> > >> > > > ???? - compatible
> > >> > > > ???? - reg??  
> > >> > >
> > >> > > This seem like things that should be set at runtime rather than
> > >> > > in the devicetree. Unless there is some justification on why
> > >> > > these values depend on how the chip is wired up?  
> > >>
> > >> It depends how ADC 1.8V Vref is wired up, especially how noisy it is.
> > >>  
> > >> >
> > >> > Further to that, I'd like to see some explanation of why we care to
> > >> > change it at all. Is it ever a bad idea to enable averaging and
> > >> > pick a large number of samples for calibration?  
> > >>
> > >> This is a snippet from the i.MX93 TRM, chapter Analog-to-Digital
> > >> Converter (SAR_ADC) describing calibration steps:
> > >>
> > >> 1. Wait for deassertion of functional reset.
> > >> 2. Configure SAR controller operating clock (MCR[ADCLKSE] = 0).
> > >> 3. Bring ADC out of Power-down state (MCR[PWDN] = 0).
> > >> 4. Configure desired calibration settings (default values kept for
> > >> highest accuracy maximum time).
> > >> ??? MCR[TSAMP]: Sample time for calibration conversion ???
> > >> MCR[NRSMPL]: Number of samples in averaging ??? MCR[AVGEN]: Averaging
> > >> function enable in calibration 5. Run calibration by writing a one to
> > >> MCR[CALSTART].
> > >> 6. Check calibration run status in MSR[CALBUSY]???wait until
> > >> MSR[CALBUSY] = 0; alternatively, MSR[ADCSTAT] can be used to check
> > >> status.
> > >> 7. Check calibration pass/fail status in MSR[CALFAIL] field. If
> > >> MSR[CALFAIL] = 1 then calibration failed. Detailed status can be
> > >> checked in CALSTAT.
> > >>
> > >>
> > >> See point 4).
> > >>
> > >> Not sure why would there be an option to configure i.MX93 ADC
> > >> calibration parameters if one use-case (max accuracy max time) to
> > >> rule them all?
> > >>  
> > >
> > >Sometimes HW guys just want to give you some options. Does not mean we
> > >have to use them all :).
> > >
> > >I guess what Jonathan is interested in, is to understand in what
> > >conditions the defaults are no good for the calibration? If we can have
> > >a set of values that should pretty much always work, no need to further
> > >complicate the bindings or the driver.  
> > 
> > Just my understanding, it is hard to use one fixed settings to fit all kinds of
> > conditions.
> > 
> >                  Noise induced from PCB tracks  Electro- magnetic noise
> > 		     |                           |
> > 		     V                           V
> >  ---------
> > |SOC(ADC)|   <---------------------------------<- (~) external Signal
> >  ---------
> >                  ^                 ^
> >                  |                 |
> >              I/O coupled noise    Internal noise
> > 
> > 
> > So OEM A's board may needs different settings compared with OEM B's board.  
> 
> The noise on Vref did impact the calibration, we did get report from customer, and IC guys suggested to do like the following patch, what's your comments?
> 
> https://patchwork.kernel.org/project/linux-iio/patch/20250423-adcpatch-v1-1-b0e84c27ae98@nxp.com/

To me warning and accepting a failed calibration is better than tweaking settings.

The reason is I'm still failing to understand why we should (for example) reduce the
time over which the signal is averaged.  Why would that make it more likely to pass
in some noise conditions than another set?  It might increase the chance of passing
I guess, but it's still likely to fail sometimes.

Jonathan

> 
> Regards
> Haibo Chen 
> > 
> > Regards,
> > Peng
> >   
> > >
> > >- Nuno S??  
> > >> On the other hand, public TRM doesn't give much more information and  
> > >> >  

Re: [PATCH 1/2] dt-bindings: iio: adc: imx93: Add calibration properties

[Primoz Fiser]

Hi,

On 21. 07. 25 11:09, Bough Chen wrote:
>> -----Original Message-----
>> From: Peng Fan (OSS) <peng.fan@oss.nxp.com>
>> Sent: 2025年7月21日 17:39
>> To: Nuno S? <noname.nuno@gmail.com>
>> Cc: Primoz Fiser <primoz.fiser@norik.com>; Jonathan Cameron
>> <jic23@kernel.org>; David Lechner <dlechner@baylibre.com>; Bough Chen
>> <haibo.chen@nxp.com>; Nuno Sa <nuno.sa@analog.com>; Andy Shevchenko
>> <andy@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
>> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn Guo
>> <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
>> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
>> <festevam@gmail.com>; linux-iio@vger.kernel.org; imx@lists.linux.dev;
>> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>> linux-kernel@vger.kernel.org; upstream@lists.phytec.de;
>> andrej.picej@norik.com
>> Subject: Re: [PATCH 1/2] dt-bindings: iio: adc: imx93: Add calibration properties
>>
>> On Mon, Jul 14, 2025 at 05:11:31PM +0100, Nuno S? wrote:
>>> On Mon, 2025-07-14 at 07:56 +0200, Primoz Fiser wrote:
>>>> Hi all,
>>>>
>>>> On 13. 07. 25 17:02, Jonathan Cameron wrote:
>>>>> On Thu, 10 Jul 2025 10:46:44 -0500
>>>>> David Lechner <dlechner@baylibre.com> wrote:
>>>>>
>>>>>> On 7/10/25 2:39 AM, Primoz Fiser wrote:
>>>>>>> From: Andrej Picej <andrej.picej@norik.com>
>>>>>>>
>>>>>>> Document i.MX93 ADC calibration properties and how to set them.
>>>>>>>
>>>>>>> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
>>>>>>> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
>>>>>>> ---
>>>>>>> ??.../bindings/iio/adc/nxp,imx93-adc.yaml???????????? | 21
>>>>>>> +++++++++++++++++++
>>>>>>> ??1 file changed, 21 insertions(+)
>>>>>>>
>>>>>>> diff --git
>>>>>>> a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-
>>>>>>> adc.yaml
>>>>>>> b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>>>>>> index c2e5ff418920..d1c04cf85fe6 100644
>>>>>>> ---
>>>>>>> a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>>>>>> +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.y
>>>>>>> +++ aml
>>>>>>> @@ -52,6 +52,27 @@ properties:
>>>>>>> ???? "#io-channel-cells":
>>>>>>> ???????? const: 1
>>>>>>> ??
>>>>>>> +?? nxp,calib-avg-en:
>>>>>>> +?????? default: 1
>>>>>>> +?????? description:
>>>>>>> +?????????? Enable or disable calibration averaging function (AVGEN).
>>>>>>> +?????? $ref: /schemas/types.yaml#/definitions/uint32
>>>>>>> +?????? enum: [ 0, 1 ]
>>>>>>> +
>>>>>>> +?? nxp,calib-nr-samples:
>>>>>>> +?????? default: 512
>>>>>>> +?????? description:
>>>>>>> +?????????? Selects number of samples (NRSMPL) to be used during
>> calibration.
>>>>>>> +?????? $ref: /schemas/types.yaml#/definitions/uint32
>>>>>>> +?????? enum: [ 16, 32, 128, 512 ]
>>>>>
>>>>> Allow 1 as a value and drop the enabled above.???? Averaging over 1
>>>>> sample is same as no averaging and gives simpler binding.
>>>>>
>>>>>>> +
>>>>>>> +?? nxp,calib-t-sample:
>>>>>>> +?????? default: 22
>>>>>>> +?????? description:
>>>>>>> +?????????? Selects sample time (TSAMP) of calibration
>>>>>>> +conversions in ADC
>>>>>>> clock cycles
>>>>>>> +?????? $ref: /schemas/types.yaml#/definitions/uint32
>>>>>>> +?????? enum: [ 8, 16, 22, 32 ]
>>>>>>> +
>>>>>>> ??required:
>>>>>>> ???? - compatible
>>>>>>> ???? - reg??
>>>>>>
>>>>>> This seem like things that should be set at runtime rather than
>>>>>> in the devicetree. Unless there is some justification on why
>>>>>> these values depend on how the chip is wired up?
>>>>
>>>> It depends how ADC 1.8V Vref is wired up, especially how noisy it is.
>>>>
>>>>>
>>>>> Further to that, I'd like to see some explanation of why we care to
>>>>> change it at all. Is it ever a bad idea to enable averaging and
>>>>> pick a large number of samples for calibration?
>>>>
>>>> This is a snippet from the i.MX93 TRM, chapter Analog-to-Digital
>>>> Converter (SAR_ADC) describing calibration steps:
>>>>
>>>> 1. Wait for deassertion of functional reset.
>>>> 2. Configure SAR controller operating clock (MCR[ADCLKSE] = 0).
>>>> 3. Bring ADC out of Power-down state (MCR[PWDN] = 0).
>>>> 4. Configure desired calibration settings (default values kept for
>>>> highest accuracy maximum time).
>>>> ??? MCR[TSAMP]: Sample time for calibration conversion ???
>>>> MCR[NRSMPL]: Number of samples in averaging ??? MCR[AVGEN]: Averaging
>>>> function enable in calibration 5. Run calibration by writing a one to
>>>> MCR[CALSTART].
>>>> 6. Check calibration run status in MSR[CALBUSY]???wait until
>>>> MSR[CALBUSY] = 0; alternatively, MSR[ADCSTAT] can be used to check
>>>> status.
>>>> 7. Check calibration pass/fail status in MSR[CALFAIL] field. If
>>>> MSR[CALFAIL] = 1 then calibration failed. Detailed status can be
>>>> checked in CALSTAT.
>>>>
>>>>
>>>> See point 4).
>>>>
>>>> Not sure why would there be an option to configure i.MX93 ADC
>>>> calibration parameters if one use-case (max accuracy max time) to
>>>> rule them all?
>>>>
>>>
>>> Sometimes HW guys just want to give you some options. Does not mean we
>>> have to use them all :).
>>>
>>> I guess what Jonathan is interested in, is to understand in what
>>> conditions the defaults are no good for the calibration? If we can have
>>> a set of values that should pretty much always work, no need to further
>>> complicate the bindings or the driver.
>>
>> Just my understanding, it is hard to use one fixed settings to fit all kinds of
>> conditions.
>>
>>                  Noise induced from PCB tracks  Electro- magnetic noise
>> 		     |                           |
>> 		     V                           V
>>  ---------
>> |SOC(ADC)|   <---------------------------------<- (~) external Signal
>>  ---------
>>                  ^                 ^
>>                  |                 |
>>              I/O coupled noise    Internal noise
>>
>>
>> So OEM A's board may needs different settings compared with OEM B's board.
> 
> The noise on Vref did impact the calibration, we did get report from customer, and IC guys suggested to do like the following patch, what's your comments?
> 
> https://patchwork.kernel.org/project/linux-iio/patch/20250423-adcpatch-v1-1-b0e84c27ae98@nxp.com/

With this patch we still get calibration warning however ADC is
eventually working.

Where can we get mapping for this register:

#define IMX93_ADC_CALCFG0	0X3A0

It seems that public i.MX 93 TRM goes up to offset 0x39C only?

BR,
Primoz

> 
> Regards
> Haibo Chen 
>>
>> Regards,
>> Peng
>>
>>>
>>> - Nuno S??
>>>> On the other hand, public TRM doesn't give much more information and
>>>>>

-- 
Primoz Fiser
phone: +386-41-390-545
email: primoz.fiser@norik.com
--
Norik systems d.o.o.
Your embedded software partner
Slovenia, EU
phone: +386-41-540-545
email: info@norik.com

RE: [PATCH 1/2] dt-bindings: iio: adc: imx93: Add calibration properties

[Bough Chen]

> -----Original Message-----
> From: Primoz Fiser <primoz.fiser@norik.com>
> Sent: 2025年8月7日 12:37
> To: Bough Chen <haibo.chen@nxp.com>; Peng Fan (OSS)
> <peng.fan@oss.nxp.com>; Nuno S? <noname.nuno@gmail.com>
> Cc: Jonathan Cameron <jic23@kernel.org>; David Lechner
> <dlechner@baylibre.com>; Nuno Sa <nuno.sa@analog.com>; Andy Shevchenko
> <andy@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn Guo
> <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; linux-iio@vger.kernel.org; imx@lists.linux.dev;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; upstream@lists.phytec.de;
> andrej.picej@norik.com
> Subject: Re: [PATCH 1/2] dt-bindings: iio: adc: imx93: Add calibration properties
> 
> Hi,
> 
> On 21. 07. 25 11:09, Bough Chen wrote:
> >> -----Original Message-----
> >> From: Peng Fan (OSS) <peng.fan@oss.nxp.com>
> >> Sent: 2025年7月21日 17:39
> >> To: Nuno S? <noname.nuno@gmail.com>
> >> Cc: Primoz Fiser <primoz.fiser@norik.com>; Jonathan Cameron
> >> <jic23@kernel.org>; David Lechner <dlechner@baylibre.com>; Bough Chen
> >> <haibo.chen@nxp.com>; Nuno Sa <nuno.sa@analog.com>; Andy
> Shevchenko
> >> <andy@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> >> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn Guo
> >> <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> >> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> >> <festevam@gmail.com>; linux-iio@vger.kernel.org; imx@lists.linux.dev;
> >> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> >> linux-kernel@vger.kernel.org; upstream@lists.phytec.de;
> >> andrej.picej@norik.com
> >> Subject: Re: [PATCH 1/2] dt-bindings: iio: adc: imx93: Add
> >> calibration properties
> >>
> >> On Mon, Jul 14, 2025 at 05:11:31PM +0100, Nuno S? wrote:
> >>> On Mon, 2025-07-14 at 07:56 +0200, Primoz Fiser wrote:
> >>>> Hi all,
> >>>>
> >>>> On 13. 07. 25 17:02, Jonathan Cameron wrote:
> >>>>> On Thu, 10 Jul 2025 10:46:44 -0500 David Lechner
> >>>>> <dlechner@baylibre.com> wrote:
> >>>>>
> >>>>>> On 7/10/25 2:39 AM, Primoz Fiser wrote:
> >>>>>>> From: Andrej Picej <andrej.picej@norik.com>
> >>>>>>>
> >>>>>>> Document i.MX93 ADC calibration properties and how to set them.
> >>>>>>>
> >>>>>>> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
> >>>>>>> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> >>>>>>> ---
> >>>>>>> ??.../bindings/iio/adc/nxp,imx93-adc.yaml???????????? | 21
> >>>>>>> +++++++++++++++++++
> >>>>>>> ??1 file changed, 21 insertions(+)
> >>>>>>>
> >>>>>>> diff --git
> >>>>>>> a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-
> >>>>>>> adc.yaml
> >>>>>>> b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> >>>>>>> index c2e5ff418920..d1c04cf85fe6 100644
> >>>>>>> ---
> >>>>>>> a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> >>>>>>> +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.y
> >>>>>>> +++ aml
> >>>>>>> @@ -52,6 +52,27 @@ properties:
> >>>>>>> ???? "#io-channel-cells":
> >>>>>>> ???????? const: 1
> >>>>>>> ??
> >>>>>>> +?? nxp,calib-avg-en:
> >>>>>>> +?????? default: 1
> >>>>>>> +?????? description:
> >>>>>>> +?????????? Enable or disable calibration averaging function (AVGEN).
> >>>>>>> +?????? $ref: /schemas/types.yaml#/definitions/uint32
> >>>>>>> +?????? enum: [ 0, 1 ]
> >>>>>>> +
> >>>>>>> +?? nxp,calib-nr-samples:
> >>>>>>> +?????? default: 512
> >>>>>>> +?????? description:
> >>>>>>> +?????????? Selects number of samples (NRSMPL) to be used during
> >> calibration.
> >>>>>>> +?????? $ref: /schemas/types.yaml#/definitions/uint32
> >>>>>>> +?????? enum: [ 16, 32, 128, 512 ]
> >>>>>
> >>>>> Allow 1 as a value and drop the enabled above.???? Averaging over
> >>>>> 1 sample is same as no averaging and gives simpler binding.
> >>>>>
> >>>>>>> +
> >>>>>>> +?? nxp,calib-t-sample:
> >>>>>>> +?????? default: 22
> >>>>>>> +?????? description:
> >>>>>>> +?????????? Selects sample time (TSAMP) of calibration
> >>>>>>> +conversions in ADC
> >>>>>>> clock cycles
> >>>>>>> +?????? $ref: /schemas/types.yaml#/definitions/uint32
> >>>>>>> +?????? enum: [ 8, 16, 22, 32 ]
> >>>>>>> +
> >>>>>>> ??required:
> >>>>>>> ???? - compatible
> >>>>>>> ???? - reg??
> >>>>>>
> >>>>>> This seem like things that should be set at runtime rather than
> >>>>>> in the devicetree. Unless there is some justification on why
> >>>>>> these values depend on how the chip is wired up?
> >>>>
> >>>> It depends how ADC 1.8V Vref is wired up, especially how noisy it is.
> >>>>
> >>>>>
> >>>>> Further to that, I'd like to see some explanation of why we care
> >>>>> to change it at all. Is it ever a bad idea to enable averaging and
> >>>>> pick a large number of samples for calibration?
> >>>>
> >>>> This is a snippet from the i.MX93 TRM, chapter Analog-to-Digital
> >>>> Converter (SAR_ADC) describing calibration steps:
> >>>>
> >>>> 1. Wait for deassertion of functional reset.
> >>>> 2. Configure SAR controller operating clock (MCR[ADCLKSE] = 0).
> >>>> 3. Bring ADC out of Power-down state (MCR[PWDN] = 0).
> >>>> 4. Configure desired calibration settings (default values kept for
> >>>> highest accuracy maximum time).
> >>>> ??? MCR[TSAMP]: Sample time for calibration conversion ???
> >>>> MCR[NRSMPL]: Number of samples in averaging ??? MCR[AVGEN]:
> >>>> Averaging function enable in calibration 5. Run calibration by
> >>>> writing a one to MCR[CALSTART].
> >>>> 6. Check calibration run status in MSR[CALBUSY]???wait until
> >>>> MSR[CALBUSY] = 0; alternatively, MSR[ADCSTAT] can be used to check
> >>>> status.
> >>>> 7. Check calibration pass/fail status in MSR[CALFAIL] field. If
> >>>> MSR[CALFAIL] = 1 then calibration failed. Detailed status can be
> >>>> checked in CALSTAT.
> >>>>
> >>>>
> >>>> See point 4).
> >>>>
> >>>> Not sure why would there be an option to configure i.MX93 ADC
> >>>> calibration parameters if one use-case (max accuracy max time) to
> >>>> rule them all?
> >>>>
> >>>
> >>> Sometimes HW guys just want to give you some options. Does not mean
> >>> we have to use them all :).
> >>>
> >>> I guess what Jonathan is interested in, is to understand in what
> >>> conditions the defaults are no good for the calibration? If we can
> >>> have a set of values that should pretty much always work, no need to
> >>> further complicate the bindings or the driver.
> >>
> >> Just my understanding, it is hard to use one fixed settings to fit
> >> all kinds of conditions.
> >>
> >>                  Noise induced from PCB tracks  Electro- magnetic
> noise
> >> 		     |                           |
> >> 		     V                           V
> >>  ---------
> >> |SOC(ADC)|   <---------------------------------<- (~) external Signal
> >>  ---------
> >>                  ^                 ^
> >>                  |                 |
> >>              I/O coupled noise    Internal noise
> >>
> >>
> >> So OEM A's board may needs different settings compared with OEM B's
> board.
> >
> > The noise on Vref did impact the calibration, we did get report from customer,
> and IC guys suggested to do like the following patch, what's your comments?
> >
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> > hwork.kernel.org%2Fproject%2Flinux-iio%2Fpatch%2F20250423-adcpatch-v1-
> >
> 1-b0e84c27ae98%40nxp.com%2F&data=05%7C02%7Chaibo.chen%40nxp.com
> %7C2101
> >
> ebdd274b4dc119fa08ddd56c1e7f%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%7C
> >
> 0%7C638901382543508027%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcG
> kiOnRydWU
> >
> sIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D
> %
> >
> 7C0%7C%7C%7C&sdata=gW%2F7FPCrcyVSduyj0dK%2BVm4rQJ4hviEsTlTJ06uvH
> 0M%3D&
> > reserved=0
> 
> With this patch we still get calibration warning however ADC is eventually
> working.
> 
> Where can we get mapping for this register:
> 
> #define IMX93_ADC_CALCFG0	0X3A0
> 
> It seems that public i.MX 93 TRM goes up to offset 0x39C only?

You can get the RM our nxp.com, the latest RM already add register 3A0 definition, here is the link,:
https://www.nxp.com/products/processors-and-microcontrollers/arm-processors/i-mx-applications-processors/i-mx-9-processors/i-mx-93-applications-processor-family-arm-cortex-a55-ml-acceleration-power-efficient-mpu:i.MX93

I will send out v2 of this patch.

Regards
Haibo Chen
> 
> BR,
> Primoz
> 
> >
> > Regards
> > Haibo Chen
> >>
> >> Regards,
> >> Peng
> >>
> >>>
> >>> - Nuno S??
> >>>> On the other hand, public TRM doesn't give much more information
> >>>> and
> >>>>>
> 
> --
> Primoz Fiser
> phone: +386-41-390-545
> email: primoz.fiser@norik.com
> --
> Norik systems d.o.o.
> Your embedded software partner
> Slovenia, EU
> phone: +386-41-540-545
> email: info@norik.com