All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Primoz Fiser <primoz.fiser@norik.com>
To: Andrej Picej <andrej.picej@norik.com>,
	Jonathan Cameron <jic23@kernel.org>
Cc: devicetree@vger.kernel.org, conor+dt@kernel.org, lars@metafoo.de,
	krzysztof.kozlowski+dt@linaro.org, imx@lists.linux.dev,
	linux-iio@vger.kernel.org, festevam@gmail.com,
	s.hauer@pengutronix.de, upstream@lists.phytec.de,
	linux-kernel@vger.kernel.org, haibo.chen@nxp.com,
	kernel@pengutronix.de, shawnguo@kernel.org, robh@kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [Upstream] [PATCH 0/2] i.MX93 ADC calibration settings
Date: Mon, 25 Mar 2024 09:55:23 +0100	[thread overview]
Message-ID: <44ac8977-cf98-46a5-be15-1bec330c6a2e@norik.com> (raw)
In-Reply-To: <3423ea96-859d-4c4b-a9a7-e0d9c3c00727@norik.com>

Hi Jonathan,

On 25. 03. 24 09:32, Andrej Picej wrote:
> Hi Jonathan,
> 
> On 24. 03. 24 14:55, Jonathan Cameron wrote:
>> On Wed, 20 Mar 2024 11:04:04 +0100
>> Andrej Picej <andrej.picej@norik.com> wrote:
>>
>>> Hi all,
>>>
>>> we had some problems with failing ADC calibration on the i.MX93 boards.
>>> Changing default calibration settings fixed this. The board where this
>>> patches are useful is not yet upstream but will be soon (hopefully).
>>
>> Tell us more.  My initial instinct is that this shouldn't be board
>> specific.
>> What's the trade off we are making here?  Time vs precision of
>> calibration or
>> something else?  If these are set to a level by default that doesn't work
>> for our board, maybe we should just change them for all devices?
>>

The imx93_adc driver is quite new.

If you look at line #162, you will find a comment by the original author:

> 	/*
> 	 * TODO: we use the default TSAMP/NRSMPL/AVGEN in MCR,
> 	 * can add the setting of these bit if need in future.
> 	 */

URL:
https://github.com/torvalds/linux/blob/master/drivers/iio/adc/imx93_adc.c#L162

So, for most use-cases the default setting should work, but why not make
them configurable?

So this patch-series just implement what was missing from the beginning
/ was planned for later.

BR,
Primoz


> 
> So we have two different boards with the same SoC. On one, the
> calibration works with the default values, on the second one the
> calibration fails, which makes the ADC unusable. What the ADC lines
> measure differ between the boards though. But the implementation is
> nothing out of the ordinary.
> 
> We tried different things but the only thing that helped is to use
> different calibration properties. We tried deferring the probe and
> calibration until later boot and after boot, but it did not help.
> 
> In the Reference Manual [1] (chapter 72.5.1) it is written:
> 
>> 4. Configure desired calibration settings (default values kept for
>> highest accuracy maximum time).
> 
> So your assumption is correct, longer calibration time (more averaging
> samples) -> higher precision. The default values go for a high accuracy.
> And since we use a NRSMPL (Number of Averaging Samples) of 32 instead of
> default 512, we reduce the accuracy so the calibration values pass the
> internal defined limits.
> 
> I'm not sure that changing default values is the right solution here. We
> saw default values work with one of the boards. And since the NXP kept
> these values adjustable I think there is a reason behind it.
> 
> Note: When I say one of the boards I mean one board form. So same board
> version, but different HW.
> 
> Best regards,
> Andrej
> 
> [1] i.MX 93 Applications Processor Reference Manual, Rev. 4, 12/2023
> _______________________________________________
> upstream mailing list
> upstream@lists.phytec.de
> http://lists.phytec.de/cgi-bin/mailman/listinfo/upstream


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Primoz Fiser <primoz.fiser@norik.com>
To: Andrej Picej <andrej.picej@norik.com>,
	Jonathan Cameron <jic23@kernel.org>
Cc: devicetree@vger.kernel.org, conor+dt@kernel.org, lars@metafoo.de,
	krzysztof.kozlowski+dt@linaro.org, imx@lists.linux.dev,
	linux-iio@vger.kernel.org, festevam@gmail.com,
	s.hauer@pengutronix.de, upstream@lists.phytec.de,
	linux-kernel@vger.kernel.org, haibo.chen@nxp.com,
	kernel@pengutronix.de, shawnguo@kernel.org, robh@kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [Upstream] [PATCH 0/2] i.MX93 ADC calibration settings
Date: Mon, 25 Mar 2024 09:55:23 +0100	[thread overview]
Message-ID: <44ac8977-cf98-46a5-be15-1bec330c6a2e@norik.com> (raw)
In-Reply-To: <3423ea96-859d-4c4b-a9a7-e0d9c3c00727@norik.com>

Hi Jonathan,

On 25. 03. 24 09:32, Andrej Picej wrote:
> Hi Jonathan,
> 
> On 24. 03. 24 14:55, Jonathan Cameron wrote:
>> On Wed, 20 Mar 2024 11:04:04 +0100
>> Andrej Picej <andrej.picej@norik.com> wrote:
>>
>>> Hi all,
>>>
>>> we had some problems with failing ADC calibration on the i.MX93 boards.
>>> Changing default calibration settings fixed this. The board where this
>>> patches are useful is not yet upstream but will be soon (hopefully).
>>
>> Tell us more.  My initial instinct is that this shouldn't be board
>> specific.
>> What's the trade off we are making here?  Time vs precision of
>> calibration or
>> something else?  If these are set to a level by default that doesn't work
>> for our board, maybe we should just change them for all devices?
>>

The imx93_adc driver is quite new.

If you look at line #162, you will find a comment by the original author:

> 	/*
> 	 * TODO: we use the default TSAMP/NRSMPL/AVGEN in MCR,
> 	 * can add the setting of these bit if need in future.
> 	 */

URL:
https://github.com/torvalds/linux/blob/master/drivers/iio/adc/imx93_adc.c#L162

So, for most use-cases the default setting should work, but why not make
them configurable?

So this patch-series just implement what was missing from the beginning
/ was planned for later.

BR,
Primoz


> 
> So we have two different boards with the same SoC. On one, the
> calibration works with the default values, on the second one the
> calibration fails, which makes the ADC unusable. What the ADC lines
> measure differ between the boards though. But the implementation is
> nothing out of the ordinary.
> 
> We tried different things but the only thing that helped is to use
> different calibration properties. We tried deferring the probe and
> calibration until later boot and after boot, but it did not help.
> 
> In the Reference Manual [1] (chapter 72.5.1) it is written:
> 
>> 4. Configure desired calibration settings (default values kept for
>> highest accuracy maximum time).
> 
> So your assumption is correct, longer calibration time (more averaging
> samples) -> higher precision. The default values go for a high accuracy.
> And since we use a NRSMPL (Number of Averaging Samples) of 32 instead of
> default 512, we reduce the accuracy so the calibration values pass the
> internal defined limits.
> 
> I'm not sure that changing default values is the right solution here. We
> saw default values work with one of the boards. And since the NXP kept
> these values adjustable I think there is a reason behind it.
> 
> Note: When I say one of the boards I mean one board form. So same board
> version, but different HW.
> 
> Best regards,
> Andrej
> 
> [1] i.MX 93 Applications Processor Reference Manual, Rev. 4, 12/2023
> _______________________________________________
> upstream mailing list
> upstream@lists.phytec.de
> http://lists.phytec.de/cgi-bin/mailman/listinfo/upstream


  reply	other threads:[~2024-03-25  8:55 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-20 10:04 [PATCH 0/2] i.MX93 ADC calibration settings Andrej Picej
2024-03-20 10:04 ` Andrej Picej
2024-03-20 10:04 ` [PATCH 1/2] iio: adc: imx93: Make calibration properties configurable Andrej Picej
2024-03-20 10:04   ` Andrej Picej
2024-03-24 14:02   ` Jonathan Cameron
2024-03-24 14:02     ` Jonathan Cameron
2024-03-20 10:04 ` [PATCH 2/2] dt-bindings: iio: adc: nxp,imx93-adc.yaml: Add calibration properties Andrej Picej
2024-03-20 10:04   ` Andrej Picej
2024-03-20 10:26   ` Krzysztof Kozlowski
2024-03-20 10:26     ` Krzysztof Kozlowski
2024-03-20 12:05     ` Andrej Picej
2024-03-20 12:05       ` Andrej Picej
2024-03-20 12:15       ` Krzysztof Kozlowski
2024-03-20 12:15         ` Krzysztof Kozlowski
2024-03-22  7:39         ` Andrej Picej
2024-03-22  7:39           ` Andrej Picej
2024-03-22  8:14           ` Krzysztof Kozlowski
2024-03-22  8:14             ` Krzysztof Kozlowski
2024-03-22  9:58             ` Andrej Picej
2024-03-22  9:58               ` Andrej Picej
2024-03-24 13:54               ` Jonathan Cameron
2024-03-24 13:54                 ` Jonathan Cameron
2024-03-25  9:58               ` Krzysztof Kozlowski
2024-03-25  9:58                 ` Krzysztof Kozlowski
2024-03-25 14:38                 ` Jonathan Cameron
2024-03-25 14:38                   ` Jonathan Cameron
2024-03-20 21:41   ` Rob Herring
2024-03-20 21:41     ` Rob Herring
2024-03-22  6:47   ` kernel test robot
2024-03-22  6:47     ` kernel test robot
2024-03-24 13:55 ` [PATCH 0/2] i.MX93 ADC calibration settings Jonathan Cameron
2024-03-24 13:55   ` Jonathan Cameron
2024-03-25  8:32   ` Andrej Picej
2024-03-25  8:32     ` Andrej Picej
2024-03-25  8:55     ` Primoz Fiser [this message]
2024-03-25  8:55       ` [Upstream] " Primoz Fiser
2024-03-25 14:45       ` Jonathan Cameron
2024-03-25 14:45         ` Jonathan Cameron
2024-03-29  7:58         ` Primoz Fiser
2024-03-29  7:58           ` Primoz Fiser

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=44ac8977-cf98-46a5-be15-1bec330c6a2e@norik.com \
    --to=primoz.fiser@norik.com \
    --cc=andrej.picej@norik.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=festevam@gmail.com \
    --cc=haibo.chen@nxp.com \
    --cc=imx@lists.linux.dev \
    --cc=jic23@kernel.org \
    --cc=kernel@pengutronix.de \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=lars@metafoo.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=upstream@lists.phytec.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.