linux-kernel - Re: [PATCH 2/2] iio: adc: imx93: Make calibration parameters configurable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <42dd658e-555a-43f7-a7bf-e5365d508f4a@norik.com>
Date: Thu, 10 Jul 2025 12:23:58 +0200
From: Primoz Fiser <primoz.fiser@...ik.com>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: Haibo Chen <haibo.chen@....com>, Jonathan Cameron <jic23@...nel.org>,
 David Lechner <dlechner@...libre.com>, Nuno Sa <nuno.sa@...log.com>,
 Andy Shevchenko <andy@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
 Sascha Hauer <s.hauer@...gutronix.de>,
 Pengutronix Kernel Team <kernel@...gutronix.de>,
 Fabio Estevam <festevam@...il.com>, linux-iio@...r.kernel.org,
 imx@...ts.linux.dev, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 upstream@...ts.phytec.de, andrej.picej@...ik.com
Subject: Re: [PATCH 2/2] iio: adc: imx93: Make calibration parameters
 configurable

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