linux-kernel - Re: [PATCH v2 2/2] iio: adc: ad9467: support write/read offset

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: <c1cce165-0c34-4277-89b4-b0117ebb4bba@vaisala.com>
Date: Tue, 2 Dec 2025 17:01:08 +0200
From: Tomas Melin <tomas.melin@...sala.com>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>,
 Michael Hennerich <Michael.Hennerich@...log.com>,
 Nuno Sa <nuno.sa@...log.com>, Jonathan Cameron <jic23@...nel.org>,
 David Lechner <dlechner@...libre.com>, Andy Shevchenko <andy@...nel.org>,
 Alexandru Ardelean <alexandru.ardelean@...log.com>,
 Jonathan Cameron <Jonathan.Cameron@...wei.com>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] iio: adc: ad9467: support write/read offset



On 02/12/2025 16:11, Andy Shevchenko wrote:
> On Tue, Dec 02, 2025 at 12:53:09PM +0000, Tomas Melin wrote:
>> Support configuring output calibration value. Among the devices
>> currently supported by this driver, this setting is specific to
>> ad9434. The offset can be used to calibrate the output against
>> a known input. The register is called offset, but the procedure
>> is best mapped internally with calibbias operation.
> 
> ...
> 
>>  static const struct iio_chan_spec ad9434_channels[] = {
>> -	AD9467_CHAN(0, BIT(IIO_CHAN_INFO_SCALE), 0, 12, 's'),
>> +	{
>> +		.type = IIO_VOLTAGE,
>> +		.indexed = 1,
>> +		.channel = 0,
>> +		.info_mask_shared_by_type =
>> +		BIT(IIO_CHAN_INFO_SCALE) |
>> +		BIT(IIO_CHAN_INFO_SAMP_FREQ) |
>> +		BIT(IIO_CHAN_INFO_CALIBBIAS),
> 
> Wrong indentation.

Can you please provide example of your preferred indentation for this
particular case? This is used in several places around the code and
seemed like one of the more readable.

> 
>> +		.info_mask_shared_by_type_available =
>> +		BIT(IIO_CHAN_INFO_SCALE) |
>> +		BIT(IIO_CHAN_INFO_CALIBBIAS),
> 
> Ditto.
> 
>> +		.scan_index = 0,
>> +		.scan_type = {
>> +			.sign = 's',
>> +			.realbits = 12,
>> +			.storagebits = 16,
>> +		},
>> +	},
>>  };
> 
> I'm not sure about macro-less approach here, I think that we want more
> consistency and hence before doing this change probably we want to clean up
> the existing macro, then split it to two, and add another one here based on
> the low-level, which was split in the previous clean up.

As mentioned, this is only needed for a single channel, and since it is
different than the other, it needs to be separated. Do You think we
actually need another macro for this?

> 
> ...
> 
>> +	return ad9467_spi_write(st, AN877_ADC_REG_TRANSFER,
>> +				AN877_ADC_TRANSFER_SYNC);
> 
> I would make it one line, despite on being 85 characters long.
> But it's up to you and maintainers.
I would like to not fight against checkpatch here.

>