spi engine and ad7944 backport #2446
Conversation
dlech
force-pushed
the
adi/dlech/spi-engine-and-ad7944-backport
branch
2 times, most recently
from
5993ad3 to
3d73efb
Compare
|
I have gone through the checkpatch CI failures and fixed what I could. The rest are stuff that is already upstream or false positives that can be ignored. |
There was a problem hiding this comment.
Then for the AXI SPI Engine driver, since there are breaking changes compared to the current ADI tree version, I didn't update spi-axi-spi-engine.c in the ADI tree but rather added a new spi-axi-spi-engine-ex.c module that can coexist side-by-side with the old version and changed the DT compatible strings to use an adi-ex, prefix instead of adi,. This way, we don't have to try to fix all of the drivers depending on the old version before we can add and use the new version.
This much I agree and it's fine. We don't have that many users, so it should be not too bad.
Also, the hope is that having the -ex.c file will save time in the future by avoiding merge conflicts when updating the ADI tree to newer kernel versions.
Note that my plan is to get rid of the -ex drivers as soon as all users are converted. The conflicts are just then part of life :). Hopefully, they also remind us of the importance of just push things upstream.
Similarly, for backporting the upstream version of the ad7944 driver, I placed the driver in ad7944-ex.c instead of ad7944.c so that we can extended it in the ADI tree as needed without worrying about conflicts with upstream changes down the road. And similarly, I changed the DT compatible string prefix to adi-ex, so that we don't accidentally end up trying to use bindings that have been extended out of tree with the mainline kernel
In the above spirit, just use the driver as-is upstream. If we place proper comments on the out of tree code, it should be not too bad to handle the conflicts. And, anyways, I'll be the one taking the pain but I still prefer that than having duplicated drivers.
Yeah, no need to "fix" stuff that is also upstream... |
OK, I'll fix it up like that. |
dlech
force-pushed
the
adi/dlech/spi-engine-and-ad7944-backport
branch
3 times, most recently
from
6f9edd1 to
3be6b87
Compare
|
I have updated this to revert #2280 to avoid duplicate drivers for the same part. And the AD7944 driver backport now just cherry-picks the upstream patches instead of squashing them into a single commit (so no more "-ex" suffix on this one). |
| static struct attribute *ad7944_attrs[] = { | ||
| &iio_dev_attr_sampling_frequency.dev_attr.attr, | ||
| NULL | ||
| }; |
There was a problem hiding this comment.
Any reason for not using typical IIO attributes (read_raw(), write_raw())?
There was a problem hiding this comment.
I guess I just saw IIO_DEV_ATTR_SAMP_FREQ first and didn't notice IIO_CHAN_INFO_SAMP_FREQ.
I didn't implement it yet, but as the REVISIT comment says, we could actually add a runtime check to hide the attribute when using a SPI controller without offload support. So if that seems like a nice thing to have, that could be a reason for leaving it this way.
There was a problem hiding this comment.
So if that seems like a nice thing to have, that could be a reason for leaving it this way.
You can still have that with the normal API even though you'll need more code. Anyways, I don't care thaaat much about it. Just that when upstreaming, Jonathan will likely want to have the "normal" API.
There was a problem hiding this comment.
Makes sense. I will keep that in mind.
| static IIO_DEV_ATTR_SAMP_FREQ(0644, ad7944_sampling_frequency_show, | ||
| ad7944_sampling_frequency_store); | ||
|
|
||
| static struct attribute *ad7944_attrs[] = { |
There was a problem hiding this comment.
As a side note your 'REVISIT' implies ABI breakage which is a no-go upstream (note that some apps could be relying on the read to return -ENODEV to infer that there's no offload support).
Having said the above, ABI breakage is acceptable in ADI tree when the reasoning behind it is upstreaming out of tree stuff (which would be this case).
Anyways, if you can do the .is_visible stuff already, it would be great. If you have plenty of other stuff also fine for me to take it as-is
There was a problem hiding this comment.
I added the implementation.
dlech
force-pushed
the
adi/dlech/spi-engine-and-ad7944-backport
branch
from
3be6b87 to
c94d50c
Compare
Add spi_split_transfers_maxwords() function that splits spi_transfers transparently into multiple transfers that are below a given number of SPI words. This function reuses most of its code from spi_split_transfers_maxsize() and for transfers with eight or less bits per word actually behaves the same. Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de> Link: https://lore.kernel.org/r/20230310092053.1006459-1-l.goehrs@pengutronix.de Signed-off-by: Mark Brown <broonie@kernel.org>
Previously, __spi_sync() and __spi_async() set message->spi to the spi device independently after calling __spi_validate(). __spi_validate() also would conditionally set this if it needed to split the message since it wasn't set yet. Since both __spi_sync() and __spi_async() call __spi_validate(), we can consolidate this into only setting message->spi once (unconditionally) in __spi_validate(). This will also save any future callers of __spi_validate() from also needing to set message->spi. Signed-off-by: David Lechner <dlechner@baylibre.com> Link: https://msgid.link/r/20240123214946.2616786-1-dlechner@baylibre.com Signed-off-by: Mark Brown <broonie@kernel.org>
In __spi_pump_transfer_message(), the message was not finalized in the first error return as it is in the other error return paths. Not finalizing the message could cause anything waiting on the message to complete to hang forever. This adds the missing call to spi_finalize_current_message(). Fixes: ae7d234 ("spi: Don't use the message queue if possible in spi_sync") Signed-off-by: David Lechner <dlechner@baylibre.com> Link: https://msgid.link/r/20240125205312.3458541-2-dlechner@baylibre.com Signed-off-by: Mark Brown <broonie@kernel.org>
The __spi_sync() function calls __spi_validate() early in the function. Later, it can call spi_async_locked() which calls __spi_validate() again. __spi_validate() is an expensive function, so we can improve performance measurably by avoiding calling it twice. Instead of calling spi_async_locked(), we can call __spi_async() with the spin lock held. spi_async_locked() is removed since there are no more callers. Signed-off-by: David Lechner <dlechner@baylibre.com> Link: https://msgid.link/r/20240125234732.3530278-2-dlechner@baylibre.com Signed-off-by: Mark Brown <broonie@kernel.org>
This moves splitting transfers for CS_WORD software emulation to the same place where we split transfers for controller-specific reasons. This fixes a few subtle bugs. The calculation for maxsize was wrong for bit sizes between 17 and 24. This is fixed by making use of spi_split_transfers_maxwords() which already has the correct calculation. Also, since this indirectly calls spi_res_alloc(), to avoid leaking resources, spi_finalize_current_message() would need to be called on all error paths in __spi_validate() and callers of __spi_validate() would need to do the same. This is fixed by moving the call to __spi_pump_transfer_message() where it is already splitting transfers for other reasons and correctly releases resources in the subsequent error paths. Fixes: cbaa62e ("spi: add software implementation for SPI_CS_WORD") Signed-off-by: David Lechner <dlechner@baylibre.com> Link: https://lore.kernel.org/r/20240126212358.3916280-2-dlechner@baylibre.com Signed-off-by: Mark Brown <broonie@kernel.org>
The __spi_split_transfer_maxsize() function has a gpf argument to allow callers to specify the type of memory allocation that needs to be used. However, this function only allocates struct spi_transfer and is not intended to be used from atomic contexts so this type should always be GFP_KERNEL, so we can just drop the argument. Some callers of these functions also passed GFP_DMA, but since only struct spi_transfer is allocated and not any tx/rx buffers, this is not actually necessary and is removed in this commit. Signed-off-by: David Lechner <dlechner@baylibre.com> Link: https://lore.kernel.org/r/20240206200648.1782234-1-dlechner@baylibre.com Signed-off-by: Mark Brown <broonie@kernel.org>
This adds a new spi_optimize_message() function that can be used to optimize SPI messages that are used more than once. Peripheral drivers that use the same message multiple times can use this API to perform SPI message validation and controller-specific optimizations once and then reuse the message while avoiding the overhead of revalidating the message on each spi_(a)sync() call. Internally, the SPI core will also call this function for each message if the peripheral driver did not explicitly call it. This is done to so that controller drivers don't have to have multiple code paths for optimized and non-optimized messages. A hook is provided for controller drivers to perform controller-specific optimizations. Suggested-by: Martin Sperl <kernel@martin.sperl.org> Link: https://lore.kernel.org/linux-spi/39DEC004-10A1-47EF-9D77-276188D2580C@martin.sperl.org/ Signed-off-by: David Lechner <dlechner@baylibre.com> Link: https://msgid.link/r/20240219-mainline-spi-precook-message-v2-1-4a762c6701b9@baylibre.com Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Signed-off-by: Mark Brown <broonie@kernel.org>
Splitting transfers is an expensive operation so we can potentially optimize it by doing it only once per optimization of the message instead of repeating each time the message is transferred. The transfer splitting functions are currently the only user of spi_res_alloc() so spi_res_release() can be safely moved at this time from spi_finalize_current_message() to spi_unoptimize_message(). The doc comments of the public functions for splitting transfers are also updated so that callers will know when it is safe to call them to ensure proper resource management. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Signed-off-by: David Lechner <dlechner@baylibre.com> Link: https://msgid.link/r/20240219-mainline-spi-precook-message-v2-2-4a762c6701b9@baylibre.com Signed-off-by: Mark Brown <broonie@kernel.org>
This converts the axi-spi-engine binding to yaml. There are a few minor fixes in the conversion: * Added maintainers. * Added descriptions for the clocks. * Fixed the double "@" in the example. * Added a comma between the clocks in the example. Signed-off-by: David Lechner <dlechner@baylibre.com> Reviewed-by: Rob Herring <robh@kernel.org> Link: https://lore.kernel.org/r/20231117-axi-spi-engine-series-1-v1-1-cc59db999b87@baylibre.com Signed-off-by: Mark Brown <broonie@kernel.org>
This adds a copy of the mainline DT bindings for the AXI SPI Engine with the adi-ex vendor prefix for use in the ADI tree. Signed-off-by: David Lechner <dlechner@baylibre.com>
This is a copy of the upstream (v6.9) spi-axi-spi-engine.c backported to the ADI tree and with the -ex suffix added to indicate that this file will have out of tree changes. Signed-off-by: David Lechner <dlechner@baylibre.com>
This ports the existing out-of-tree offload functions from spi-axi-spi-engine to spi-axi-spi-engine-ex. API is the same. Function names contain "ex" to avoid conflicts with the existing driver. Signed-off-by: David Lechner <dlechner@baylibre.com>
This reverts commit c86eef0. This functionality will be replaced by a backport of the upstream ad7944 driver, so we no longer need this. Signed-off-by: David Lechner <dlechner@baylibre.com>
This reverts commit 9168247. We will be replacing this functionality with the a backport of the mainline ad7944 driver. Signed-off-by: David Lechner <dlechner@baylibre.com>
This reverts commit b3a4218. We will be replacing this with a backport of the upstream adi,ad7944 bindings. Signed-off-by: David Lechner <dlechner@baylibre.com>
This adds a new binding for the Analog Devices, Inc. AD7944, AD7985, and AD7986 ADCs. Reviewed-by: Rob Herring <robh@kernel.org> Signed-off-by: David Lechner <dlechner@baylibre.com> Link: https://lore.kernel.org/r/20240304-ad7944-mainline-v5-1-f0a38cea8901@baylibre.com Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
This adds a driver for the Analog Devices Inc. AD7944, AD7985, and AD7986 ADCs. These are a family of pin-compatible ADCs that can sample at rates up to 2.5 MSPS. The initial driver adds support for sampling at lower rates using the usual IIO triggered buffer and can handle all 3 possible reference voltage configurations. Signed-off-by: David Lechner <dlechner@baylibre.com> Reviewed-by: Nuno Sa <nuno.sa@analog.com> Link: https://lore.kernel.org/r/20240304-ad7944-mainline-v5-2-f0a38cea8901@baylibre.com Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
This adds support for AD7944 ADCs wired in "3-wire mode". (NOTE: 3-wire is the datasheet name for this wiring configuration and has nothing to do with SPI_3WIRE.) In the 3-wire mode, the SPI controller CS line can be wired to the CNV line on the ADC and used to trigger conversions rather that using a separate GPIO line. Signed-off-by: David Lechner <dlechner@baylibre.com>
This modifies the ad7944 driver to use spi_optimize_message() to reduce CPU usage and increase the max sample rate by avoiding repeating validation of the spi message on each transfer. Signed-off-by: David Lechner <dlechner@baylibre.com>
This adds the new AD7944 driver to the list of drivers that are enabled by default in the ADI tree. Signed-off-by: David Lechner <dlechner@baylibre.com>
This updates the ad4744/85/86 eval board dts files for the new spi-axi-spi-engine-ex and ad4744 drivers and 3-wire variant of the HDL. Signed-off-by: David Lechner <dlechner@baylibre.com>
This adds support for SPI Engine offload to the ad7944-ex driver. This allows reaching the max sample rate of 2/2.5 MSPS when the chip is wired in 3-wire mode. Signed-off-by: David Lechner <dlechner@baylibre.com>
dlech
force-pushed
the
adi/dlech/spi-engine-and-ad7944-backport
branch
from
c94d50c to
d73dba4
Compare
Since it is probably going to take a while to finish getting SPI Engine offload support added upstream, I've gone ahead and started a backport of what has been upstreamed already.
The first 9 commits here are core spi changes cherry-picked from upstream (up to v6.9). I just picked the ones I know we need and not everything. So, there were a few merge conflicts, but it wasn't too bad. When merging newer upstream kernels in to the ADI tree in the future, we should be able to just say use upstream's version of
spi.cand ignore any local diff.Then for the AXI SPI Engine driver, since there are breaking changes compared to the current ADI tree version, I didn't update
spi-axi-spi-engine.cin the ADI tree but rather added a newspi-axi-spi-engine-ex.cmodule that can coexist side-by-side with the old version and changed the DT compatible strings to use anadi-ex,prefix instead ofadi,. This way, we don't have to try to fix all of the drivers depending on the old version before we can add and use the new version. Also, the hope is that having the-ex.cfile will save time in the future by avoiding merge conflicts when updating the ADI tree to newer kernel versions.Similarly, for backporting the upstream version of the ad7944 driver, I placed the driver in
ad7944-ex.cinstead ofad7944.cso that we can extended it in the ADI tree as needed without worrying about conflicts with upstream changes down the road. And similarly, I changed the DT compatible string prefix toadi-ex,so that we don't accidentally end up trying to use bindings that have been extended out of tree with the mainline kernel.So basically, the only changes here that aren't already upstream (or planned to submit upstream soon) other than the
-exrenames I already mentioned are "spi: axi-spi-engine-ex: port offload functions" and "iio: adc: ad7944-ex: add spi offload support". These are just keeping the status quo of how SPI Engine offload is done in the ADI tree already by extending the SPI peripheral DT node with the pwms and dmas properties and adding some APIs directly to the SPI engine driver for offload support instead of going through the SPI core for that part.