linux-kernel - Re: [PATCH 7/7] iio: adc: ad7606: add support for AD7606C-{16,18} parts

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: <bf12e626-d052-421f-a7e7-ec52577d3297@baylibre.com>
Date: Fri, 23 Aug 2024 13:04:00 -0500
From: David Lechner <dlechner@...libre.com>
To: Alexandru Ardelean <aardelean@...libre.com>
Cc: linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org, jic23@...nel.org, krzk+dt@...nel.org,
 robh@...nel.org, lars@...afoo.de, michael.hennerich@...log.com,
 gstols@...libre.com, Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH 7/7] iio: adc: ad7606: add support for AD7606C-{16,18}
 parts

On 8/23/24 10:54 AM, Alexandru Ardelean wrote:
> On Mon, Aug 19, 2024 at 6:33 PM David Lechner <dlechner@...libre.com> wrote:
>>
>> On 8/19/24 1:47 AM, Alexandru Ardelean wrote:
>>> The AD7606C-16 and AD7606C-18 are pretty similar with the AD7606B.
>>> The main difference between AD7606C-16 & AD7606C-18 is the precision in
>>> bits (16 vs 18).
>>> Because of that, some scales need to be defined for the 18-bit variants, as
>>> they need to be computed against 2**18 (vs 2**16 for the 16 bit-variants).
>>>
>>> Because the AD7606C-16,18 also supports bipolar & differential channels,
>>> for SW-mode, the default range of 10 V or ±10V should be set at probe.
>>> On reset, the default range (in the registers) is set to value 0x3 which
>>> corresponds to '±10 V single-ended range', regardless of bipolar or
>>> differential configuration.
>>>
>>> Aside from the scale/ranges, the AD7606C-16 is similar to the AD7606B.
>>>
>>> And the AD7606C-18 variant offers 18-bit precision. The unfortunate effect
>>> of this 18-bit sample size, is that there is no simple/neat way to get the
>>> samples into a 32-bit array without having to do a home-brewed bit-buffer.
>>> The ADC must read all samples (from all 8 channels) in order to get the
>>> N-th sample (this could be reworked to do up-to-N-th sample for scan-direct).
>>> There doesn't seem to be any quick-trick to be usable to pad the samples
>>> up to at least 24 bits.
>>> Even the optional status-header is 8-bits, which would mean 26-bits of data
>>> per sample.
>>> That means that when using a simple SPI controller (which can usually read
>>> 8 bit multiples) a simple bit-buffer trick is required.
>>>
>> Maybe it would be better to just use .bits_per_word = 18 for the 18-bit
>> ADC and not worry about "simple" SPI controller support for that one?
>>
> 
> +cc Mark Brown for some input on the SPI stuff
> 
> I'm generally fine with choosing to not support SPI controllers that
> can't do padding to 16/32 bit arrays
> 
> But, at the same time: would it be an interesting topic to implement
> (in the SPI framework) some SW implementation for padding a series of
> 18-bit samples to 32-bit arrays?
> (Similarly, this could work for 10-15 bit samples into 16 bit arrays).
> 
> Apologies if this is already implemented and I missed it.
> 
> But if there isn't such a functionality (padding done in SW inside the
> SPI framework), then I could probably spin-up a proposal.
> I think that the functionality could be spun-up in a separate
> patch-set/discussion; and this patchset would just go with
> "bits_per_word = 18".
> 
> It could be done as a new field in the "struct spi_transfer", or
> something else like "spi_pad_rx_to_nbits(struct spi_device *)"
> Or other suggestions welcome
> 
> Thanks
> Alex

Seems like it would be tricky to do something in the core code to
emulate "odd" sized words in general since what is permissible
likely depends on how the individual peripheral works. For example,

total_bits = xfer->bits_per_word * (xfer->len  /
	roundup_pow_of_two(BITS_TO_BYTES(xfer->bits_per_word)))

If total_bits % 8 != 0, then there will be extra trailing
clock cycles that could be problematic on some peripherals
but not others.

And there are other incompatibilities to consider, like this
could not be used with a peripheral that have the CS_WORD flag
set (highly unlikely, but still something to consider if we
are integrating this into the core).

But if you want to look into it more, another use case for this
could be SPI TFT displays. There are a number of these that use
9-bit data words. Right now emulation is handled in the peripheral
driver code. For example, see mipi_dbi_spi1e_transfer() and
fbtft_write_reg8_bus9().