mbox series

[v6,0/7] Add support for AD4000 series of ADCs

Message ID cover.1719686465.git.marcelo.schmitt@analog.com
Headers show
Series Add support for AD4000 series of ADCs | expand

Message

Marcelo Schmitt June 29, 2024, 7:04 p.m. UTC
This patch series extends the SPI bitbang, gpio, and spi-engine controllers to
support configurable MOSI line idle states.
It then introduces the ad4000 driver which uses the MOSI idle configuration to
provide improved support for the AD4000 series of ADCs.
Documentation is added describing the new extension to the SPI protocol.
The currently supported wiring modes for AD4000 devices were documented under
IIO documentation directory.

To apply this series, it requires the patches for SPI-Engine SPI_CS_HIGH feature
and the patches for devm_spi_optimize_message() helper.

89c2657429c4822a2697077bbb3a8d126d826ced "spi: axi-spi-engine: remove platform_set_drvdata()"
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/commit/?h=for-6.11&id=89c2657429c4822a2697077bbb3a8d126d826ced

7e74a45c7afdd8a9f82d14fd79ae0383bbaaed1e "spi: add EXPORT_SYMBOL_GPL(devm_spi_optimize_message)"
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/commit/?h=for-6.11&id=7e74a45c7afdd8a9f82d14fd79ae0383bbaaed1e

d4a0055fdc22381fa256e345095e88d134e354c5 "spi: add devm_spi_optimize_message() helper"
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/commit/?h=for-6.11&id=d4a0055fdc22381fa256e345095e88d134e354c5

6ecdb0aa4dca62d236a659426e11e6cf302e8f18 "spi: axi-spi-engine: Add SPI_CS_HIGH support"
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/commit/?h=for-6.11&id=6ecdb0aa4dca62d236a659426e11e6cf302e8f18

Change log v5 -> v6:
[SPI]
spi.h: Removed unused SPI_CONTROLLER_MOSI_IDLE_LOW and SPI_CONTROLLER_MOSI_IDLE_HIGH
spi-summay: Minor nit: inactive -> not asserted
spi-engine: Moved MOSI idle support check to IP core version check section
[IIO]
ad4000: Fixed ad4000_read_reg(). *val = st->tx_buf[1]; -> *val = st->rx_buf[1];
ad4000: Use devm_regulator_bulk_get_enable()
ad4000: Use iio_device_claim_direct_scoped() and guard() to protect scale update
ad4000: Fail probe if ad4000_config() fail
ad4000: Moved ad4000_prepare_..._message() closer to probe to reduce scope
ad4000: Added AD4000_SDI_GND and switch case for more accurate error msg
ad4000: Removed unused st->turbo_mode
ad4000: Removed old misleading comments in enum ad4000_sdi
ad4000: A few minor readability and code style nits

Link to v5: https://lore.kernel.org/linux-iio/cover.1719351923.git.marcelo.schmitt@analog.com/
Link to v4: https://lore.kernel.org/linux-iio/cover.1718749981.git.marcelo.schmitt@analog.com/
Link to v3: https://lore.kernel.org/linux-iio/cover.1717539384.git.marcelo.schmitt@analog.com/
Link to v2: https://lore.kernel.org/linux-iio/cover.1712585500.git.marcelo.schmitt@analog.com/
Link to v1: https://lore.kernel.org/linux-iio/cover.1711131830.git.marcelo.schmitt@analog.com/

Regard using spi_w8r8(), I tried it again and it doesn't work for ad4000.
Looks like the smallest transfer size for these devices is 16-bit.

Comments

Jonathan Cameron June 30, 2024, 10:47 a.m. UTC | #1
On Sat, 29 Jun 2024 16:04:40 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:

> The behavior of an SPI controller data output line (SDO or MOSI or COPI
> (Controller Output Peripheral Input) for disambiguation) is usually not
> specified when the controller is not clocking out data on SCLK edges.
> However, there do exist SPI peripherals that require specific MOSI line
> state when data is not being clocked out of the controller.
> 
> Conventional SPI controllers may set the MOSI line on SCLK edges then bring
> it low when no data is going out or leave the line the state of the last
> transfer bit. More elaborated controllers are capable to set the MOSI idle
> state according to different configurable levels and thus are more suitable
> for interfacing with demanding peripherals.
> 
> Add SPI mode bits to allow peripherals to request explicit MOSI idle state
> when needed.
> 
> When supporting a particular MOSI idle configuration, the data output line
> state is expected to remain at the configured level when the controller is
> not clocking out data. When a device that needs a specific MOSI idle state
> is identified, its driver should request the MOSI idle configuration by
> setting the proper SPI mode bit.
> 
> Acked-by: Nuno Sa <nuno.sa@analog.com>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>

I always like to see some nice ascii art. Very nice documentation.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 93f59ebb5b79..c8ba5e490850 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c

> @@ -3950,6 +3956,7 @@ int spi_setup(struct spi_device *spi)
>  	 */
>  	bad_bits = spi->mode & ~(spi->controller->mode_bits | SPI_CS_WORD |
>  				 SPI_NO_TX | SPI_NO_RX);
> +

Trivial grumpy comment.  Don't touch white space in unrelated code!

>  	ugly_bits = bad_bits &
>  		    (SPI_TX_DUAL | SPI_TX_QUAD | SPI_TX_OCTAL |
>  		     SPI_RX_DUAL | SPI_RX_QUAD | SPI_RX_OCTAL);
Jonathan Cameron June 30, 2024, 10:53 a.m. UTC | #2
On Sat, 29 Jun 2024 16:05:33 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:

> Implement MOSI idle low and MOSI idle high to better support peripherals
> that request specific MOSI behavior.
> 
> Acked-by: Nuno Sa <nuno.sa@analog.com>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
LGTM
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Marcelo Schmitt July 1, 2024, 5:30 p.m. UTC | #3
On 06/30, Jonathan Cameron wrote:
> On Sat, 29 Jun 2024 16:04:40 -0300
> Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:
> 
> > The behavior of an SPI controller data output line (SDO or MOSI or COPI
> > (Controller Output Peripheral Input) for disambiguation) is usually not
> > specified when the controller is not clocking out data on SCLK edges.
> > However, there do exist SPI peripherals that require specific MOSI line
> > state when data is not being clocked out of the controller.
> > 
> > Conventional SPI controllers may set the MOSI line on SCLK edges then bring
> > it low when no data is going out or leave the line the state of the last
> > transfer bit. More elaborated controllers are capable to set the MOSI idle
> > state according to different configurable levels and thus are more suitable
> > for interfacing with demanding peripherals.
> > 
> > Add SPI mode bits to allow peripherals to request explicit MOSI idle state
> > when needed.
> > 
> > When supporting a particular MOSI idle configuration, the data output line
> > state is expected to remain at the configured level when the controller is
> > not clocking out data. When a device that needs a specific MOSI idle state
> > is identified, its driver should request the MOSI idle configuration by
> > setting the proper SPI mode bit.
> > 
> > Acked-by: Nuno Sa <nuno.sa@analog.com>
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> 
> I always like to see some nice ascii art. Very nice documentation.
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> > index 93f59ebb5b79..c8ba5e490850 100644
> > --- a/drivers/spi/spi.c
> > +++ b/drivers/spi/spi.c
> 
> > @@ -3950,6 +3956,7 @@ int spi_setup(struct spi_device *spi)
> >  	 */
> >  	bad_bits = spi->mode & ~(spi->controller->mode_bits | SPI_CS_WORD |
> >  				 SPI_NO_TX | SPI_NO_RX);
> > +
> 
> Trivial grumpy comment.  Don't touch white space in unrelated code!

Ouf, must have slipped through after messing around with spi_setup().
Didn't intend to add that. Fine if that can be removed when applying the patch.

> 
> >  	ugly_bits = bad_bits &
> >  		    (SPI_TX_DUAL | SPI_TX_QUAD | SPI_TX_OCTAL |
> >  		     SPI_RX_DUAL | SPI_RX_QUAD | SPI_RX_OCTAL);
Jonathan Cameron July 29, 2024, 7:40 p.m. UTC | #4
On Mon, 29 Jul 2024 19:02:23 +0100
Mark Brown <broonie@kernel.org> wrote:

> On Sat, 29 Jun 2024 16:04:00 -0300, Marcelo Schmitt wrote:
> > This patch series extends the SPI bitbang, gpio, and spi-engine controllers to
> > support configurable MOSI line idle states.
> > It then introduces the ad4000 driver which uses the MOSI idle configuration to
> > provide improved support for the AD4000 series of ADCs.
> > Documentation is added describing the new extension to the SPI protocol.
> > The currently supported wiring modes for AD4000 devices were documented under
> > IIO documentation directory.
> > 
> > [...]  
> 
> Applied to
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
> 
> Thanks!
> 
> [1/7] spi: Enable controllers to extend the SPI protocol with MOSI idle configuration
>       commit: f58872f45c36ded048bccc22701b0986019c24d8
> [2/7] spi: bitbang: Implement support for MOSI idle state configuration
>       commit: 320f6693097bf89d67f9cabad24a2b911e23073f
> [3/7] spi: spi-gpio: Add support for MOSI idle state configuration
>       commit: 927d382c7efbcc2206c31fa2f672fa264c0f1d5b
> [4/7] spi: spi-axi-spi-engine: Add support for MOSI idle configuration
>       commit: a62073f4b2164028fc7c5ae45ceba10c9326cd91
Hi Mark,

Any chance of a tag + you seem to have also picked up the ADC dt binding.
Patch 5/7. dt-bindings: iio: adc: Add AD4000
which I'm assuming was not intentional.

I think I only need the definition of SPI_MOSI_IDLE_HIGH
for 5-7 to build fine.

If needed, I can use a local value for that in the driver and
we can follow up with a patch using the main define once the trees
come together upstream.

Jonathan


> 
> All being well this means that it will be integrated into the linux-next
> tree (usually sometime in the next 24 hours) and sent to Linus during
> the next merge window (or sooner if it is a bug fix), however if
> problems are discovered then the patch may be dropped or reverted.
> 
> You may get further e-mails resulting from automated or manual testing
> and review of the tree, please engage with people reporting problems and
> send followup patches addressing any issues that are reported if needed.
> 
> If any updates are required or you are submitting further changes they
> should be sent as incremental updates against current git, existing
> patches will not be replaced.
> 
> Please add any relevant lists and maintainers to the CCs when replying
> to this mail.
> 
> Thanks,
> Mark
>