[PATCH 2/2] iio: adc: imx93: Make calibration parameters configurable
Primoz Fiser
primoz.fiser at norik.com
Thu Jul 10 03:23:58 PDT 2025
Hi Andy,
On 10. 07. 25 11:22, Andy Shevchenko wrote:
> On Thu, Jul 10, 2025 at 09:39:04AM +0200, Primoz Fiser wrote:
>>
>> Make i.MX93 ADC calibration parameters:
>> - AVGEN: Enable calibration averaging function,
>> - NRSMPL: Select number of calibration samples,
>> - TSAMP: Select sample time of calibration conversions,
>>
>> in the MCR register configurable with the corresponding device-tree
>> properties:
>> - nxp,calib-avg-en,
>> - nxp,calib-nr-samples and
>> - nxp,calib-t-sample.
>
> ...
>
>> #include <linux/platform_device.h>
>> #include <linux/pm_runtime.h>
>> #include <linux/regulator/consumer.h>
>> +#include <linux/property.h>
>
> Keep it in order.
OK. Will fix for v2.
>
> ...
>
>> + ret = device_property_read_u32(adc->dev, "nxp,calib-avg-en", &val);
>> + if (!ret) {
>> + if (val != 0 && val != 1) {
>> + dev_err(adc->dev, "invalid nxp,calib-avg-en: %d\n", val);
>> + return -EINVAL;
>> + }
>> + reg = val;
>> + mcr &= ~IMX93_ADC_MCR_AVGEN_MASK;
>> + mcr |= FIELD_PREP(IMX93_ADC_MCR_AVGEN_MASK, reg);
>> + }
>
> Please, since it's optional, do other way around.
>
> val = $DEFAUTL;
> device_property_read_u32(adc->dev, "nxp,calib-avg-en", &val);
> FIELD_MODIFY(...)
>
> Similar approach may be used for the other properties.
OK, I guess I could implement it like you suggested to explicitly set
the default parameter values.
But in current implementation MCR values are read at the beginning of
imx93_adc_calibration(), meaning calibration parameters are register POR
defaults. With you suggestion, we put defaults in software rather than
reading them from the hw directly.
>
> ...
>
>> + /* write calibration settings to MCR */
>> + writel(mcr, adc->regs + IMX93_ADC_MCR);
>
> Please, factor out this to the function, so we won't see the direct IO in the
> ->probe().
Sorry I don't understand this part.
What do you mean by factoring out this writel()?
Do you perhaps suggest to implement function
imx93_adc_configure_calibration() and put all our changes into it?
But we are already in imx93_adc_calibration() which is separate from
probe().
Please explain.
BR,
Primoz
More information about the linux-arm-kernel
mailing list