Message-ID: <20250719124914.27536d4a@jic23-huawei>
Date: Sat, 19 Jul 2025 12:49:14 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Primoz Fiser <primoz.fiser@...ik.com>
Cc: David Lechner <dlechner@...libre.com>, Haibo Chen <haibo.chen@....com>,
 Nuno Sá <noname.nuno@...il.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 Tue, 15 Jul 2025 07:46:44 +0200
Primoz Fiser <primoz.fiser@...ik.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@...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.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
> 

Powered by blists - more mailing lists