From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id DEE5CC83F26 for ; Thu, 24 Jul 2025 18:34:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:MIME-Version:References:In-Reply-To:Message-ID:Subject:Cc:To: From:Date:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=y/JpBdh0M/EG+e/B96ZkkL3gSS4VjATa7TJ4jrZrXTk=; b=qrkoyI6LCSdJtfy2Mdu3MZwXCJ QxGok6t8bWjnjVFm5+whfF49xTehqJyb5ZjuVugFE2dRN66taJH1kJKguDrt6WIdGn3HlOq4lWB5g G6SphwNfggPjVPVP/V9Gw3ZWG9ck1GV5ZvXMR7hZ4MtPjYdEm5BBzyk/ufEh7K7JKx9QJiJfDLwuK 81WSMrP9iss46MSmPmipqXfqRmwTk6zETrv5IJLDRlivVw25n9fmQfk1umJhzgjC77e2x6zXD3+OL UcEcGCRXBn98/UazMXX4p8U2IJV3hGXqhK0WSaRCFgq79PWSFpcyhfkcjBvQooa9/C0+KY5CWE1wi KFtJ4ngA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uf0lb-00000008CoM-19Bp; Thu, 24 Jul 2025 18:34:07 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1uexiM-00000007pTd-0StM for linux-arm-kernel@lists.infradead.org; Thu, 24 Jul 2025 15:18:35 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 842595C658E; Thu, 24 Jul 2025 15:18:33 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B8B19C4CEED; Thu, 24 Jul 2025 15:18:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1753370313; bh=+bp6pfjFYbsKUp64YHHqLCc5uIl+4qZHCE84GxhUcHU=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=LoPzUSrn6r1oXxb3kNWN4MtXMDylnjukws8Eed4h76plciZYjuQdZKsPSyXZ+PRCs eIjt7QadV3qJM0q1SD5SBIqf6ZEq8Nt3zsXTV+pdJEAnj9SEpBnOKnrKdAStVn8ld8 vPe95RerhCKQdJai2hXcUGkECzpmsqM7Z4yEpoV8YqdPDFEI8eg9aGzzUJCZfgT4E0 /GzO3OZotadoLxuBlYAWpt6jv09cHslne3hPY0KfRCyhRJQdsS3VJycJtBNG1aSqSf pYjJjVlF2eaivil87NuFLKdBfd353lhVQH3/uenQQXSZ0ugrEdft7ZD9tZWnXBF6xj murJ3JuU0V2zg== Date: Thu, 24 Jul 2025 16:18:20 +0100 From: Jonathan Cameron To: Bough Chen Cc: "Peng Fan (OSS)" , Nuno S? , Primoz Fiser , David Lechner , Nuno Sa , Andy Shevchenko , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , "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 Message-ID: <20250724161820.66e37f6c@jic23-huawei> In-Reply-To: References: <20250710073905.1105417-1-primoz.fiser@norik.com> <20250710073905.1105417-2-primoz.fiser@norik.com> <2bcd758b-c2d0-488a-8ead-ec7fb39f93e2@baylibre.com> <20250713160247.0f22bbfe@jic23-huawei> <6b32118a13e9e28b7cf12152af33642c76367c34.camel@gmail.com> <20250721093847.GD4844@nxa18884-linux.ap.freescale.net> X-Mailer: Claws Mail 4.3.1 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250724_081834_233500_22390624 X-CRM114-Status: GOOD ( 43.49 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, 21 Jul 2025 09:09:06 +0000 Bough Chen wrote: > > -----Original Message----- > > From: Peng Fan (OSS) > > Sent: 2025=E5=B9=B47=E6=9C=8821=E6=97=A5 17:39 > > To: Nuno S? > > Cc: Primoz Fiser ; Jonathan Cameron > > ; David Lechner ; Bough Chen > > ; Nuno Sa ; Andy Shevchenko > > ; Rob Herring ; Krzysztof Kozlowski > > ; Conor Dooley ; Shawn Guo > > ; Sascha Hauer ; > > Pengutronix Kernel Team ; Fabio Estevam > > ; 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 > >=20 > > On Mon, Jul 14, 2025 at 05:11:31PM +0100, Nuno S? wrote: =20 > > >On Mon, 2025-07-14 at 07:56 +0200, Primoz Fiser wrote: =20 > > >> Hi all, > > >> > > >> On 13. 07. 25 17:02, Jonathan Cameron wrote: =20 > > >> > On Thu, 10 Jul 2025 10:46:44 -0500 > > >> > David Lechner wrote: > > >> > =20 > > >> > > On 7/10/25 2:39 AM, Primoz Fiser wrote: =20 > > >> > > > From: Andrej Picej > > >> > > > > > >> > > > Document i.MX93 ADC calibration properties and how to set them. > > >> > > > > > >> > > > Signed-off-by: Andrej Picej > > >> > > > Signed-off-by: Primoz Fiser > > >> > > > --- > > >> > > > ??.../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 duri= ng =20 > > calibration. =20 > > >> > > > +?????? $ref: /schemas/types.yaml#/definitions/uint32 > > >> > > > +?????? enum: [ 16, 32, 128, 512 ] =20 > > >> > > > >> > Allow 1 as a value and drop the enabled above.???? Averaging over 1 > > >> > sample is same as no averaging and gives simpler binding. > > >> > =20 > > >> > > > + > > >> > > > +?? 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?? =20 > > >> > > > > >> > > 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? =20 > > >> > > >> It depends how ADC 1.8V Vref is wired up, especially how noisy it is. > > >> =20 > > >> > > > >> > 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? =20 > > >> > > >> 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] =3D 0). > > >> 3. Bring ADC out of Power-down state (MCR[PWDN] =3D 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] =3D 0; alternatively, MSR[ADCSTAT] can be used to check > > >> status. > > >> 7. Check calibration pass/fail status in MSR[CALFAIL] field. If > > >> MSR[CALFAIL] =3D 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? > > >> =20 > > > > > >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. =20 > >=20 > > Just my understanding, it is hard to use one fixed settings to fit all = kinds of > > conditions. > >=20 > > Noise induced from PCB tracks Electro- magnetic noise > > | | > > V V > > --------- > > |SOC(ADC)| <---------------------------------<- (~) external Signal > > --------- > > ^ ^ > > | | > > I/O coupled noise Internal noise > >=20 > >=20 > > So OEM A's board may needs different settings compared with OEM B's boa= rd. =20 >=20 > The noise on Vref did impact the calibration, we did get report from cust= omer, and IC guys suggested to do like the following patch, what's your com= ments? >=20 > 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 se= ttings. The reason is I'm still failing to understand why we should (for example) r= educe 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 >=20 > Regards > Haibo Chen=20 > >=20 > > Regards, > > Peng > > =20 > > > > > >- Nuno S?? =20 > > >> On the other hand, public TRM doesn't give much more information and= =20 > > >> > =20