Re: [PATCH 2/3] iio: max31856: add support for configuring the HW averaging

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 23 Sep 2019 14:17:13 +0200
Andrea Merello <andrea.merello@gmail.com> wrote:

> This sensor can perform samples averaging in hardware, but currently the
> driver leaves this setting alone (default is no averaging).
> 
> This patch introduces a new IIO attribute that allows the user to set the
> averaging as desired (the HW supports averaging of 2, 5, 8 or 16 samples)
> 
> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>

Sounds like oversampling combined with a lower sampling frequency, so
there is standard ABI for that.

Jonathan

> ---
>  drivers/iio/temperature/max31856.c | 43 ++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/drivers/iio/temperature/max31856.c b/drivers/iio/temperature/max31856.c
> index d12613f7ba3c..8b2e0102fa5c 100644
> --- a/drivers/iio/temperature/max31856.c
> +++ b/drivers/iio/temperature/max31856.c
> @@ -24,6 +24,8 @@
>  #define MAX31856_CR0_OCFAULT       BIT(4)
>  #define MAX31856_CR0_OCFAULT_MASK  GENMASK(5, 4)
>  #define MAX31856_CR0_FILTER_50HZ   BIT(0)
> +#define MAX31856_AVERAGING_MASK    GENMASK(6, 4)
> +#define MAX31856_AVERAGING_SHIFT   4
>  #define MAX31856_TC_TYPE_MASK      GENMASK(3, 0)
>  #define MAX31856_FAULT_OVUV        BIT(1)
>  #define MAX31856_FAULT_OPEN        BIT(0)
> @@ -65,6 +67,7 @@ struct max31856_data {
>  	struct spi_device *spi;
>  	u32 thermocouple_type;
>  	bool filter_50hz;
> +	int averaging;
>  };
>  
>  static int max31856_read(struct max31856_data *data, u8 reg,
> @@ -109,6 +112,10 @@ static int max31856_init(struct max31856_data *data)
>  
>  	reg_cr1_val &= ~MAX31856_TC_TYPE_MASK;
>  	reg_cr1_val |= data->thermocouple_type;
> +
> +	reg_cr1_val &= ~MAX31856_AVERAGING_MASK;
> +	reg_cr1_val |= data->averaging << MAX31856_AVERAGING_SHIFT;
> +
>  	ret = max31856_write(data, MAX31856_CR1_REG, reg_cr1_val);
>  	if (ret)
>  		return ret;
> @@ -295,14 +302,50 @@ static ssize_t set_filter(struct device *dev,
>  	return len;
>  }
>  
> +static ssize_t show_averaging(struct device *dev,
> +			      struct device_attribute *attr,
> +			      char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct max31856_data *data = iio_priv(indio_dev);
> +
> +	return sprintf(buf, "%d\n", 1 << data->averaging);
> +}
> +
> +static ssize_t set_averaging(struct device *dev,
> +			     struct device_attribute *attr,
> +			     const char *buf,
> +			     size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct max31856_data *data = iio_priv(indio_dev);
> +	unsigned int nsamples;
> +	int shift;
> +	int ret;
> +
> +	ret = kstrtouint(buf, 10, &nsamples);
> +	if (ret)
> +		return ret;
> +
> +	shift = fls(nsamples) - 1;
> +	if (nsamples > 16 || BIT(shift) != nsamples)
> +		return -EINVAL;
> +
> +	data->averaging = shift;
> +	max31856_init(data);
> +	return len;
> +}
> +
>  static IIO_DEVICE_ATTR(fault_ovuv, 0444, show_fault_ovuv, NULL, 0);
>  static IIO_DEVICE_ATTR(fault_oc, 0444, show_fault_oc, NULL, 0);
>  static IIO_DEVICE_ATTR(filter, 0644, show_filter, set_filter, 0);
> +static IIO_DEVICE_ATTR(averaging, 0644, show_averaging, set_averaging, 0);
>  
>  static struct attribute *max31856_attributes[] = {
>  	&iio_dev_attr_fault_ovuv.dev_attr.attr,
>  	&iio_dev_attr_fault_oc.dev_attr.attr,
>  	&iio_dev_attr_filter.dev_attr.attr,
> +	&iio_dev_attr_averaging.dev_attr.attr,
>  	NULL,
>  };
>