Message-ID: <19634e51-0568-421a-a3c4-b9988c2ccfb1@norik.com>
Date: Thu, 7 Aug 2025 06:37:27 +0200
From: Primoz Fiser <primoz.fiser@...ik.com>
To: Bough Chen <haibo.chen@....com>, "Peng Fan (OSS)" <peng.fan@....nxp.com>,
 Nuno S? <noname.nuno@...il.com>
Cc: Jonathan Cameron <jic23@...nel.org>, David Lechner
 <dlechner@...libre.com>, Nuno Sa <nuno.sa@...log.com>,
 Andy Shevchenko <andy@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
 Sascha Hauer <s.hauer@...gutronix.de>,
 Pengutronix Kernel Team <kernel@...gutronix.de>,
 Fabio Estevam <festevam@...il.com>,
 "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
 "imx@...ts.linux.dev" <imx@...ts.linux.dev>,
 "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
 "linux-arm-kernel@...ts.infradead.org"
 <linux-arm-kernel@...ts.infradead.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "upstream@...ts.phytec.de" <upstream@...ts.phytec.de>,
 "andrej.picej@...ik.com" <andrej.picej@...ik.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@....nxp.com>
>> Sent: 2025年7月21日 17:39
>> To: Nuno S? <noname.nuno@...il.com>
>> Cc: Primoz Fiser <primoz.fiser@...ik.com>; Jonathan Cameron
>> <jic23@...nel.org>; David Lechner <dlechner@...libre.com>; Bough Chen
>> <haibo.chen@....com>; Nuno Sa <nuno.sa@...log.com>; Andy Shevchenko
>> <andy@...nel.org>; Rob Herring <robh@...nel.org>; Krzysztof Kozlowski
>> <krzk+dt@...nel.org>; Conor Dooley <conor+dt@...nel.org>; Shawn Guo
>> <shawnguo@...nel.org>; Sascha Hauer <s.hauer@...gutronix.de>;
>> Pengutronix Kernel Team <kernel@...gutronix.de>; Fabio Estevam
>> <festevam@...il.com>; linux-iio@...r.kernel.org; imx@...ts.linux.dev;
>> devicetree@...r.kernel.org; linux-arm-kernel@...ts.infradead.org;
>> linux-kernel@...r.kernel.org; upstream@...ts.phytec.de;
>> andrej.picej@...ik.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@...libre.com> wrote:
>>>>>
>>>>>> On 7/10/25 2:39 AM, Primoz Fiser wrote:
>>>>>>> From: Andrej Picej <andrej.picej@...ik.com>
>>>>>>>
>>>>>>> Document i.MX93 ADC calibration properties and how to set them.
>>>>>>>
>>>>>>> Signed-off-by: Andrej Picej <andrej.picej@...ik.com>
>>>>>>> Signed-off-by: Primoz Fiser <primoz.fiser@...ik.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@...ik.com
--
Norik systems d.o.o.
Your embedded software partner
Slovenia, EU
phone: +386-41-540-545
email: info@...ik.com

Powered by blists - more mailing lists