Hi Jonathan, On 25. 03. 24 15:45, Jonathan Cameron wrote: > On Mon, 25 Mar 2024 09:55:23 +0100 > Primoz Fiser <primoz.fiser@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxxxxxxxx >>> http://lists.phytec.de/cgi-bin/mailman/listinfo/upstream >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx >> 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@xxxxxxxxx <mailto:info@xxxxxxxxx> | Slovenia, EU | phone: +386-41-540-545 <tel:+386-41-540-545> |
