On 11/11/16 14:18, Lars-Peter Clausen wrote: > 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@xxxxxxxxxx> >> Signed-off-by: Eva Rachel Retuya <eraretuya@xxxxxxxxx> > > 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. Is it just me who thought, we need a fixed GPI like a fixed regulator? Would allow this sort of fixed wiring to be simply defined. Linus, worth exploring? I doubt this will be the last case of this particular problem (not exactly unusual to hard wire control lines like these as which range makes sense is often a feature of the device). Would be a pain to have to add code to every driver to cover the fixed case. > > [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, >> }; >> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
