Messages in this thread | | | From | Andy Shevchenko <> | Date | Mon, 8 May 2017 11:25:52 +0300 | Subject | Re: [PATCH v4] iio: adc: Add support for TI ADC108S102 and ADC128S102 |
| |
On Mon, May 8, 2017 at 10:49 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote: > This is an upstream port of an IIO driver for the TI ADC108S102 and > ADC128S102. The former can be found on the Intel Galileo Gen2 and the > Siemens SIMATIC IOT2000. For those boards, ACPI-based enumeration is > included. > > Due to the lack of regulators under ACPI, we hard-code the voltage > provided to the VA pin of the ADC to 5 V, the value used on Galileo and > IOT2000. For DT usage, the regulator "vref-supply" provides this > information. Note that DT usage has not been tested. > > Original author: Bogdan Pricop <bogdan.pricop@emutex.com> > Ported from Intel Galileo Gen2 BSP to Intel Yocto kernel: > Todor Minchev <todor@minchev.co.uk>. >
> +static int adc108s102_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long m) > +{ > + struct adc108s102_state *st = iio_priv(indio_dev); > + int ret; > + > + switch (m) { > + case IIO_CHAN_INFO_RAW: > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret) > + return ret; > + > + ret = adc108s102_scan_direct(st, chan->address); > + > + iio_device_release_direct_mode(indio_dev); > + > + if (ret < 0) > + return ret; > + > + *val = ADC108S102_RES_DATA(ret); > + > + return IIO_VAL_INT; > + case IIO_CHAN_INFO_SCALE: > + if (chan->type == IIO_VOLTAGE) { > + *val = st->va_millivolt; > + *val2 = chan->scan_type.realbits; > + return IIO_VAL_FRACTIONAL_LOG2;
> + } else {
Redundant. and I would rather go with pattern
if (chan->type != IIO_VOLTAGE) return -EINVAL;
... return IIO_VAL_FRACTIONAL_LOG2;
_Or_ use
break;
here
> + return -EINVAL; > + } > + default: > + return -EINVAL;
...and here.
> + }
...and
return -EINVAL;
in one place.
> +}
> + if (ACPI_COMPANION(&spi->dev)) { > + st->va_millivolt = ADC108S102_VA_MV_ACPI_DEFAULT; > + } else {
> + st->reg = devm_regulator_get(&spi->dev, "vref");
I'm still not satisfied with this. Why we just can't use _optional() unconditionally? If regulator framework is broken it should be fixed first then.
> + if (IS_ERR(st->reg)) > + return PTR_ERR(st->reg); > + > + ret = regulator_enable(st->reg); > + if (ret < 0) { > + dev_err(&spi->dev, "Cannot enable vref regulator\n"); > + return ret; > + } > + > + ret = regulator_get_voltage(st->reg); > + if (ret < 0) { > + dev_err(&spi->dev, "vref get voltage failed\n"); > + return ret; > + } > + > + st->va_millivolt = ret / 1000; > + }
> + if (!IS_ERR(st->reg))
I'm wondering why regulator framework does need this check.
> + regulator_disable(st->reg);
-- With Best Regards, Andy Shevchenko
| |