Re: [PATCH v3 1/2] iio: temperature: Add support for LTC2983

[Jonathan Cameron <jic23@xxxxxxxxxx>]

On Fri, 4 Oct 2019 15:55:18 +0200
Nuno Sá <nuno.sa@xxxxxxxxxx> wrote:

> The LTC2983 is a Multi-Sensor High Accuracy Digital Temperature
> Measurement System. It measures a wide variety of temperature sensors and
> digitally outputs the result, in °C or °F, with 0.1°C accuracy and
> 0.001°C resolution. It can measure the temperature of all standard
> thermocouples (type B,E,J,K,N,S,R,T), standard 2-,3-,4-wire RTDs,
> thermistors and diodes.
> 
> Signed-off-by: Nuno Sá <nuno.sa@xxxxxxxxxx>

Two things inline that I'd missed before. Otherwise looks good.

Thanks,

Jonathan

> ---
> Changes in v2:
>  * Added some needed blank lines (for readability);
>  * Allocate iio_chan in the setup() function;
>  * Rename reset to sleep;
>  * Remove unneeded dev_dbg calls;
>  * Remove unneeded line wrapping;
>  * Remove unneeded comments;
>  * Remove extend_names. Use the standard ABI;
>  * Adapt the scales to report in millivolt and milli degrees;
>  * Adapt the of_property readings to the renaming of the properties;
>  * For custom thermistors, excitation-current cannot be set to Auto range.
> 
> Changes in v3:
>  * Use normal `devm_request_irq`;
>  * Handle and decode the new devicetree properties for sensor configuration.
> 
>  MAINTAINERS                       |    7 +
>  drivers/iio/temperature/Kconfig   |   10 +
>  drivers/iio/temperature/Makefile  |    1 +
>  drivers/iio/temperature/ltc2983.c | 1554 +++++++++++++++++++++++++++++
>  4 files changed, 1572 insertions(+)
>  create mode 100644 drivers/iio/temperature/ltc2983.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f0c03740b9fb..14a256e785ca 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9491,6 +9491,13 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/dac/ltc1660.txt
>  F:	drivers/iio/dac/ltc1660.c
>  
> +LTC2983 IIO TEMPERATURE DRIVER
> +M:	Nuno Sá <nuno.sa@xxxxxxxxxx>
> +W:	http://ez.analog.com/community/linux-device-drivers
> +L:	linux-iio@xxxxxxxxxxxxxxx
> +S:	Supported
> +F:	drivers/iio/temperature/ltc2983.c
> +
>  LTC4261 HARDWARE MONITOR DRIVER
>  M:	Guenter Roeck <linux@xxxxxxxxxxxx>
>  L:	linux-hwmon@xxxxxxxxxxxxxxx
> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
> index 737faa0901fe..04b5a67b593c 100644
> --- a/drivers/iio/temperature/Kconfig
> +++ b/drivers/iio/temperature/Kconfig
> @@ -4,6 +4,16 @@
>  #
>  menu "Temperature sensors"
>  
> +config LTC2983
> +	tristate "Analog Devices Multi-Sensor Digital Temperature Measurement System"
> +	depends on SPI

Select REGMAP_SPI needed I think.

> +	help
> +	  Say yes here to build support for the LTC2983 Multi-Sensor
> +	  high accuracy digital temperature measurement system.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called ltc2983.
> +
>  config MAXIM_THERMOCOUPLE
>  	tristate "Maxim thermocouple sensors"
>  	depends on SPI
> diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
> index baca4776ca0d..d6b850b0cf63 100644
> --- a/drivers/iio/temperature/Makefile
> +++ b/drivers/iio/temperature/Makefile
> @@ -3,6 +3,7 @@
>  # Makefile for industrial I/O temperature drivers
>  #
>  
> +obj-$(CONFIG_LTC2983) += ltc2983.o
>  obj-$(CONFIG_HID_SENSOR_TEMP) += hid-sensor-temperature.o
>  obj-$(CONFIG_MAXIM_THERMOCOUPLE) += maxim_thermocouple.o
>  obj-$(CONFIG_MAX31856) += max31856.o
> diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
> new file mode 100644
> index 000000000000..f899c1d75f8a
> --- /dev/null
> +++ b/drivers/iio/temperature/ltc2983.c
> @@ -0,0 +1,1554 @@
...

> +static int ltc2983_chan_read(struct ltc2983_data *st,
> +			const struct ltc2983_sensor *sensor, int *val)
> +{
> +	u32 start_conversion = 0;
> +	int ret;
> +	unsigned long time;
> +	__be32 temp;
> +
> +	/*
> +	 * Do not allow channel readings if device is in sleep state.
> +	 * A read/write on the spi bus would bring the device prematurely
> +	 * out of sleep.
> +	 */
> +	if (st->sleep)
> +		return -EPERM;
> +
> +	start_conversion = LTC2983_STATUS_START(true);
> +	start_conversion |= LTC2983_STATUS_CHAN_SEL(sensor->chan);
> +	dev_dbg(&st->spi->dev, "Start conversion on chan:%d, status:%02X\n",
> +		sensor->chan, start_conversion);
> +	/* start conversion */
> +	ret = regmap_write(st->regmap, LTC2983_STATUS_REG, start_conversion);
> +	if (ret)
> +		return ret;
> +
> +	reinit_completion(&st->completion);
> +	/*
> +	 * wait for conversion to complete.
> +	 * 300 ms should be more than enough to complete the conversion.
> +	 * Depending on the sensor configuration, there are 2/3 conversions
> +	 * cycles of 82ms.
> +	 */
> +	time = wait_for_completion_timeout(&st->completion,
> +					   msecs_to_jiffies(300));
> +	if (!time) {
> +		dev_warn(&st->spi->dev, "Conversion timed out\n");
> +		return -ETIMEDOUT;
> +	}
> +
> +	/* read the converted data */
> +	ret = regmap_bulk_read(st->regmap, LTC2983_CHAN_RES_ADDR(sensor->chan),
> +			       &temp, sizeof(temp));

I'd missed this before.  regmap_bulk_read can directly use the supplied buffer
for dma.  Hence it needs to be dma safe.  That means you have to have it
in it's own cacheline.  There is no way of enforcing that on the stack so
either allocate it locally from the heap, or put it at the end of the
data structure and mark it __cacheline_aligned (we make sure those structures
are also cacheline aligned and on the heap specifically to allow us to do that
trick).


> +	if (ret)
> +		return ret;
> +
> +	*val = __be32_to_cpu(temp);
> +
> +	if (!(LTC2983_RES_VALID_MASK & *val)) {
> +		dev_err(&st->spi->dev, "Invalid conversion detected\n");
> +		return -EIO;
> +	}
> +
> +	ret = sensor->fault_handler(st, *val);
> +	if (ret)
> +		return ret;
> +
> +	*val = sign_extend32((*val) & LTC2983_DATA_MASK, LTC2983_DATA_SIGN_BIT);
> +	return 0;
> +}
> +
...