Re: [PATCH v2 3/3] iio: adc: new driver to support Linear technology's ltc2496

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 14 Nov 2019 11:51:59 +0100
Uwe Kleine-König         <u.kleine-koenig@pengutronix.de> wrote:

> This chip is similar to the LTC2497 ADC, it just uses SPI instead of I2C
> and so has a slightly different protocol. Only the actual hardware
> access is different. The spi protocol is different enough to not be able
> to map the differences via a regmap.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
As mentioned earlier, there are restrictions on buffers being dma
safe when passed to spi calls.   This more or less requires that they
never share a cacheline with other data and the only easy way to
do that is to allocate them from the heap.  It's better to do this
once rather than every time, hence the standard trick with ___cacheline_aligned
in the iio_priv structure.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/Kconfig   |  10 ++++
>  drivers/iio/adc/Makefile  |   1 +
>  drivers/iio/adc/ltc2496.c | 100 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 111 insertions(+)
>  create mode 100644 drivers/iio/adc/ltc2496.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index f0af3a42f53c..deb86f6039b3 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -492,6 +492,16 @@ config LTC2485
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called ltc2485.
>  
> +config LTC2496
> +	tristate "Linear Technology LTC2496 ADC driver"
> +	depends on SPI
> +	help
> +	  Say yes here to build support for Linear Technology LTC2496
> +	  16-Bit 8-/16-Channel Delta Sigma ADC.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called ltc2496.
> +
>  config LTC2497
>  	tristate "Linear Technology LTC2497 ADC driver"
>  	depends on I2C
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 660242c2cca7..afe2b6db4a5e 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -47,6 +47,7 @@ obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
>  obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o
>  obj-$(CONFIG_LTC2471) += ltc2471.o
>  obj-$(CONFIG_LTC2485) += ltc2485.o
> +obj-$(CONFIG_LTC2496) += ltc2496.o ltc249x.o
>  obj-$(CONFIG_LTC2497) += ltc2497.o ltc249x.o
>  obj-$(CONFIG_MAX1027) += max1027.o
>  obj-$(CONFIG_MAX11100) += max11100.o
> diff --git a/drivers/iio/adc/ltc2496.c b/drivers/iio/adc/ltc2496.c
> new file mode 100644
> index 000000000000..870526c6df1b
> --- /dev/null
> +++ b/drivers/iio/adc/ltc2496.c
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ltc2496.c - Driver for Analog Devices/Linear Technology LTC2496 ADC
> + *
> + * Based on ltc2497.c which has
> + * Copyright (C) 2017 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2.
> + *
> + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/2496fc.pdf
> + */
> +
> +#include <linux/spi/spi.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/driver.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +
> +#include "ltc249x.h"
> +
> +struct ltc2496_st {
> +	/* this must be the first member */
> +	struct ltc249x_driverdata common_ddata;
> +	struct spi_device *spi;
> +};
> +
> +static int ltc2496_result_and_measure(struct ltc249x_driverdata *ddata,
> +				      u8 address, int *val)
> +{
> +	struct ltc2496_st *st =
> +		container_of(ddata, struct ltc2496_st, common_ddata);
> +	unsigned char txbuf[3] = { LTC249X_ENABLE | address, };
> +	unsigned char rxbuf[3];

You must make sure that buffers passed directly to the SPI subsystem
are not sharing a cacheline with other elements.  Hence these need
to be on the heap.. Easiest way of doing that is to use the
___cacheline_aligned tricks at the end of a iio_priv structure
(which is appropriately placed and aligned to allow this to work).

> +	struct spi_transfer t = {
> +		.tx_buf = txbuf,
> +		.rx_buf = rxbuf,
> +		.len = sizeof(txbuf),
> +	};
> +	int ret;
> +
> +	ret = spi_sync_transfer(st->spi, &t, 1);
> +	if (ret < 0)  {
> +		dev_err(&st->spi->dev, "spi_sync_transfer failed: %pe\n",
> +			ERR_PTR(ret));
> +		return ret;
> +	}
> +
> +	if (val)
> +		*val = ((rxbuf[0] & 0x3f) << 12 | rxbuf[1] << 4 | rxbuf[2] >> 4)
> +			- (1 << 17);
> +
> +	return 0;
> +}
> +
> +static int ltc2496_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct ltc2496_st *st;
> +	struct device *dev = &spi->dev;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	spi_set_drvdata(spi, indio_dev);
> +	st->spi = spi;
> +	st->common_ddata.result_and_measure = ltc2496_result_and_measure;
> +
> +	return ltc249x_probe(dev, indio_dev);
> +}
> +
> +static int ltc2496_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +
> +	ltc249x_remove(indio_dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ltc2496_of_match[] = {
> +	{ .compatible = "lltc,ltc2496", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, ltc2496_of_match);
> +
> +static struct spi_driver ltc2496_driver = {
> +	.driver = {
> +		.name = "ltc2496",
> +		.of_match_table = of_match_ptr(ltc2496_of_match),
> +	},
> +	.probe = ltc2496_probe,
> +	.remove = ltc2496_remove,
> +};
> +module_spi_driver(ltc2496_driver);
> +
> +MODULE_AUTHOR("Uwe Kleine-König <u.kleine-könig@pengutronix.de>");
> +MODULE_DESCRIPTION("Linear Technology LTC2496 ADC driver");
> +MODULE_LICENSE("GPL v2");