From: Primoz Fiser <primoz.fiser@norik.com>
To: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Cc: Andrej Picej <andrej.picej@norik.com>,
Jonathan Cameron <jic23@kernel.org>,
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: Fri, 29 Mar 2024 08:58:42 +0100 [thread overview]
Message-ID: <d9a233d6-95cd-4901-9c06-d8319f2eb3b4@norik.com> (raw)
In-Reply-To: <20240325144555.00002d16@Huawei.com>
Hi Jonathan,
On 25. 03. 24 15:45, Jonathan Cameron wrote:
> On Mon, 25 Mar 2024 09:55:23 +0100
> Primoz Fiser <primoz.fiser@norik.com> wrote:
>
>> 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.
> Hi Primoz,
>
> I doubt anyone reviewed the comment closely enough to say if what it was
> suggesting was sensible or not, so the fact it was listed as a todo
> doesn't directly impact this discussion.
I agree.
However on the other hand, since we stumbled upon a use-case that
requires adjusting the driver provided default settings of the i.MX93
ADC, this TODO to us is and was a clear indication from the original
author that the driver needs little TLC.
Anyhow, a stance from the author/NXP would be highly welcoming in this
situation.
BR,
Primoz
>
>>
>> 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.
>
> Ouch. Let me try to dig into this. Is this effectively relaxing the
> constraints? I guess because a value that is perhaps always biased one way
> is considered within bounds if those acceptable bounds are wider because
> of lower precision?
>
> I was assuming it was the other way around and the device had fixed constraint
> limits and you needed to take more samples due to higher noise. Seems the
> opposite is true here and that worries me.
>
> I'll definitely need input from NXP on this as a workaround and their
> strong support to consider it.
>
>>>
>>> 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.
>
> I'd assume trade off between time and calibration precision, not the
> sort of use I think you are describing.
>
>>>
>>> Note: When I say one of the boards I mean one board form. So same board
>>> version, but different HW.
>
> Superficially I'm struggling to not see this as broken hardware that it
> is out of expected tolerances in some fashion. Maybe I misunderstood
> the issue.
>
> Jonathan
>
>>>
>>> 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
>
--
Primoz Fiser | phone: +386-41-390-545
<tel:+386-41-390-545> |
---------------------------------------------------------|
Norik systems d.o.o. | https://www.norik.com
<https://www.norik.com> |
Your embedded software partner | email: info@norik.com
<mailto:info@norik.com> |
Slovenia, EU | phone: +386-41-540-545
<tel:+386-41-540-545> |
prev parent reply other threads:[~2024-03-29 7:58 UTC|newest]
Thread overview: 20+ 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 ` [PATCH 1/2] iio: adc: imx93: Make calibration properties configurable Andrej Picej
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:26 ` Krzysztof Kozlowski
2024-03-20 12:05 ` Andrej Picej
2024-03-20 12:15 ` Krzysztof Kozlowski
2024-03-22 7:39 ` Andrej Picej
2024-03-22 8:14 ` Krzysztof Kozlowski
2024-03-22 9:58 ` Andrej Picej
2024-03-24 13:54 ` Jonathan Cameron
2024-03-25 9:58 ` Krzysztof Kozlowski
2024-03-25 14:38 ` Jonathan Cameron
2024-03-20 21:41 ` Rob Herring
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-25 8:32 ` Andrej Picej
2024-03-25 8:55 ` [Upstream] " Primoz Fiser
2024-03-25 14:45 ` Jonathan Cameron
2024-03-29 7:58 ` Primoz Fiser [this message]
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=d9a233d6-95cd-4901-9c06-d8319f2eb3b4@norik.com \
--to=primoz.fiser@norik.com \
--cc=Jonathan.Cameron@Huawei.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).