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

[Primoz Fiser <primoz.fiser@norik.com>]

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