Message-ID: <aMptaOOmQ6SUoMLj@smile.fi.intel.com>
Date: Wed, 17 Sep 2025 11:12:24 +0300
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: David Lechner <dlechner@...libre.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 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).

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.

> > 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.


-- 
With Best Regards,
Andy Shevchenko

Powered by blists - more mailing lists