[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <6ab0cee1-846f-4f90-bc61-141f74144a50@baylibre.com>
Date: Wed, 17 Sep 2025 08:21:23 -0500
From: David Lechner <dlechner@...libre.com>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: Marilene Andrade Garcia <marilene.agarcia@...il.com>,
linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, Kim Seer Paller <kimseer.paller@...log.com>,
Lars-Peter Clausen <lars@...afoo.de>,
Michael Hennerich <Michael.Hennerich@...log.com>,
Jonathan Cameron <jic23@...nel.org>, Nuno Sá
<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>,
Marcelo Schmitt <marcelo.schmitt1@...il.com>,
Marcelo Schmitt <Marcelo.Schmitt@...log.com>,
Ceclan Dumitru <dumitru.ceclan@...log.com>,
Jonathan Santos <Jonathan.Santos@...log.com>,
Dragos Bogdan <dragos.bogdan@...log.com>
Subject: Re: [PATCH v11 2/3] iio: adc: max14001: New driver
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.
>
>
Powered by blists - more mailing lists