Re: [PATCH v11 2/3] iio: adc: max14001: New driver

[David Lechner <dlechner@baylibre.com>]

On 9/17/25 3:12 AM, Andy Shevchenko wrote:
> On Wed, Sep 17, 2025 at 11:10:42AM +0300, Andy Shevchenko wrote:
>> On Tue, Sep 16, 2025 at 01:04:41PM -0500, David Lechner wrote:
>>> On 9/15/25 5:16 PM, Marilene Andrade Garcia wrote:
> 
> ...
> 
>>>> Change I was not able to do:
>>>> - I could not remove bitrev16 because I am using an SPI controller that
>>>> does not support SPI_LSB_FIRST. So I suggest keeping bitrev16 and not using
>>>> the spi-lsb-first devicetree property for now, since this driver currently
>>>> works for both types of controllers: those that support it and those that
>>>> do not. I left a TODO comment to address this issue as soon as the SPI
>>>> kernel code starts handling the bit-reverse operation for controllers that
>>>> do not have this support. Once I finish my work on this driver, if the SPI
>>>> code still does not include this handling, I can submit patches to add it.
>>>
>>> I looked more at what it would take to implement this in the SPI core code
>>> and found that it would actually be quite difficult to do in a generic way
>>> because there are so many edge/corner/n-dim cases. We can't change tx_buf
>>> data in-place because it might be const data that is in some memory area
>>> that can't be modified. And things would get complicated if different
>>> transfers pointed to the same buffer memory addresses anyway. So we would
>>> basically have to allocate new memory for all buffers, copy all tx data to
>>> that new memory, reverse all of the tx bits, and update all the pointers in
>>> the transfer structs. Then when the message was finished, we would have to
>>> reverse all of the rx bits, copy all of the rx buffers back to the original
>>> buffers and put all the buffer pointers back the way they were. But this
>>> could write over some of the original tx data if tx_buf and rx_buf point to
>>> the same original buffer, which would break things if a peripheral driver
>>> expected the tx data to persist.
>>
>> And what's the problem here? We do the same with bounce-buffers in case
>> of DMA/IOMMU (okay, without actual data modification, but it's possible
>> on-the-fly).

OK, maybe not as much problem as I thought. Just rather inefficient.
I might have another look. We could perhaps allocate the buffers
during the spi_optimize phase and only swap bits on each transfer to
make it as efficient as possible.

> 
> Actually, can this be done on a regmap level instead? We have a lot of custom
> regmap IO accessors, bulk accessor that does something to a data can be also
> implemented.
> 

Currently, if you have a peripheral that has the SPI_LSB_FIRST flag connected
to a controller that does not have that flag, the SPI core code will refuse to
make a SPI device for the peripheral. So to make anything work at all, the
core SPI code is going to have to be made aware one way or another.

>>> And we can't do this during the SPI optimize
>>> step because that currently allows the tx_buf data values to be modified after
>>> optimization.
>>
>> This I don't know, so perhaps it's indeed a showstopper.
>>
>>> So perhaps it is best to just handle it in the peripheral driver. It will
>>> be much more efficent that way anyway.
>>>
>>> However, we still do want to handle SPI_LSB_FIRST now so that people with
>>> hardware support can be more efficient and we don't want things to break
>>> if someone puts spi-lsb-first in the devicetree.
> 
>