linux-kernel - Re: [PATCH 1/2] staging: iio: ad7606: replace range/range

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 11 Nov 2016 15:18:37 +0100
From:   Lars-Peter Clausen <lars@...afoo.de>
To:     Eva Rachel Retuya <eraretuya@...il.com>, linux-iio@...r.kernel.org,
        devel@...verdev.usuosl.org, linux-kernel@...r.kernel.org
Cc:     Michael.Hennerich@...log.com, jic23@...nel.org, knaack.h@....de,
        pmeerw@...erw.net, gregkh@...uxfoundation.org
Subject: Re: [PATCH 1/2] staging: iio: ad7606: replace range/range_available
 with corresponding scale

On 11/11/2016 07:34 AM, Eva Rachel Retuya wrote:
> Eliminate the non-standard attribute in_voltage_range and move its
> functionality under the scale attribute. read_raw() has been taken care
> of previously so only write_raw() is handled here.
> 
> Additionally, rename the attribute in_voltage_range_available into
> in_voltage_scale_available.
> 
> Suggested-by: Lars-Peter Clausen <lars@...afoo.de>
> Signed-off-by: Eva Rachel Retuya <eraretuya@...il.com>

Hi,

Thanks for the patch. Unfortunately this is not quite this straight forward.

The scale is what you multiply the raw with to get the value in the standard
IIO unit. Range as implemented in this driver is the maximum output voltage.

To get the scale we need to look at the transfer function of the ADC [1].
The transfer function tells us that 1 LSB is 305uV for the 10V range and
152uV for the 5V range.

More specifically this is $RANGE*2/2**16 (times two since the ADC is bipolar).

Since the default unit for IIO is mV for voltages we need to multiply this
by 1000.

The other thing we need to handle is the case where the RANGE pin is not
connected to a GPIO but either hardwired to 1 or 0. Which we need to handle
somehow.

[1]
http://www.analog.com/media/en/technical-documentation/data-sheets/AD7606_7606-6_7606-4.pdf#page=23
(right bottom)

> ---
>  drivers/staging/iio/adc/ad7606.c | 56 ++++++++++++----------------------------
>  1 file changed, 16 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
> index 4531908..cceb18c 100644
> --- a/drivers/staging/iio/adc/ad7606.c
> +++ b/drivers/staging/iio/adc/ad7606.c
> @@ -161,42 +161,7 @@ static int ad7606_read_raw(struct iio_dev *indio_dev,
>  	return -EINVAL;
>  }
>  
> -static ssize_t ad7606_show_range(struct device *dev,
> -				 struct device_attribute *attr, char *buf)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct ad7606_state *st = iio_priv(indio_dev);
> -
> -	return sprintf(buf, "%u\n", st->range);
> -}
> -
> -static ssize_t ad7606_store_range(struct device *dev,
> -				  struct device_attribute *attr,
> -				  const char *buf, size_t count)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct ad7606_state *st = iio_priv(indio_dev);
> -	unsigned long lval;
> -	int ret;
> -
> -	ret = kstrtoul(buf, 10, &lval);
> -	if (ret)
> -		return ret;
> -
> -	if (!(lval == 5000 || lval == 10000))
> -		return -EINVAL;
> -
> -	mutex_lock(&indio_dev->mlock);
> -	gpiod_set_value(st->gpio_range, lval == 10000);
> -	st->range = lval;
> -	mutex_unlock(&indio_dev->mlock);
> -
> -	return count;
> -}
> -
> -static IIO_DEVICE_ATTR(in_voltage_range, S_IRUGO | S_IWUSR,
> -		       ad7606_show_range, ad7606_store_range, 0);
> -static IIO_CONST_ATTR(in_voltage_range_available, "5000 10000");
> +static IIO_CONST_ATTR(in_voltage_scale_available, "5000 10000");
>  
>  static int ad7606_oversampling_get_index(unsigned int val)
>  {
> @@ -221,6 +186,19 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
>  	int ret;
>  
>  	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		if (val2)
> +			return -EINVAL;
> +
> +		if (!(val == 5000 || val == 10000))
> +			return -EINVAL;
> +
> +		mutex_lock(&indio_dev->mlock);
> +		gpiod_set_value(st->gpio_range, val == 10000);
> +		st->range = val;
> +		mutex_unlock(&indio_dev->mlock);
> +
> +		return 0;
>  	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
>  		if (val2)
>  			return -EINVAL;
> @@ -247,8 +225,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
>  static IIO_CONST_ATTR(oversampling_ratio_available, "1 2 4 8 16 32 64");
>  
>  static struct attribute *ad7606_attributes_os_and_range[] = {
> -	&iio_dev_attr_in_voltage_range.dev_attr.attr,
> -	&iio_const_attr_in_voltage_range_available.dev_attr.attr,
> +	&iio_const_attr_in_voltage_scale_available.dev_attr.attr,
>  	&iio_const_attr_oversampling_ratio_available.dev_attr.attr,
>  	NULL,
>  };
> @@ -267,8 +244,7 @@ static const struct attribute_group ad7606_attribute_group_os = {
>  };
>  
>  static struct attribute *ad7606_attributes_range[] = {
> -	&iio_dev_attr_in_voltage_range.dev_attr.attr,
> -	&iio_const_attr_in_voltage_range_available.dev_attr.attr,
> +	&iio_const_attr_in_voltage_scale_available.dev_attr.attr,
>  	NULL,
>  };
>  
>