On 09/10/2012 09:34 AM, Lars-Peter Clausen wrote:
> Slightly rework the reference voltage handling for the ad7476 driver. Now the only
> way to specify a external reference voltage is to use the regulator API,
> previously it was possible to use either platform_data or the regulator API. The
> new way is more consistent with what other drivers do.
>
> Also do not ignore errors when requesting the regulator, since this will cope
> very poorly with e.g. deferred probing.
>
> Signed-off-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
>
> ----
> Changes since v1:
> * Fix regulator error handling
Not right yet. If the regulator_get fails you will leak the iio_device
I've fixed up, could you take a look at what I ended up with just
to be sure I haven't made mistakes. It was one of those clasic cases
where a one line change made for some nasty merging...
Anyhow, I've pushed the slightly amended versions out to iio.git togreg branch.
> ---
> drivers/staging/iio/adc/ad7476.h | 11 +------
> drivers/staging/iio/adc/ad7476_core.c | 58 +++++++++++++++------------------
> 2 files changed, 27 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7476.h b/drivers/staging/iio/adc/ad7476.h
> index c4f1150..4ed5494 100644
> --- a/drivers/staging/iio/adc/ad7476.h
> +++ b/drivers/staging/iio/adc/ad7476.h
> @@ -10,16 +10,8 @@
>
> #define RES_MASK(bits) ((1 << (bits)) - 1)
>
> -/*
> - * TODO: struct ad7476_platform_data needs to go into include/linux/iio
> - */
> -
> -struct ad7476_platform_data {
> - u16 vref_mv;
> -};
> -
> struct ad7476_chip_info {
> - u16 int_vref_mv;
> + unsigned int int_vref_uv;
> struct iio_chan_spec channel[2];
> };
>
> @@ -27,7 +19,6 @@ struct ad7476_state {
> struct spi_device *spi;
> const struct ad7476_chip_info *chip_info;
> struct regulator *reg;
> - u16 int_vref_mv;
> struct spi_transfer xfer;
> struct spi_message msg;
> /*
> diff --git a/drivers/staging/iio/adc/ad7476_core.c b/drivers/staging/iio/adc/ad7476_core.c
> index c97300b..e79a179 100644
> --- a/drivers/staging/iio/adc/ad7476_core.c
> +++ b/drivers/staging/iio/adc/ad7476_core.c
> @@ -40,7 +40,7 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
> {
> int ret;
> struct ad7476_state *st = iio_priv(indio_dev);
> - unsigned int scale_uv;
> + int scale_uv;
>
> switch (m) {
> case IIO_CHAN_INFO_RAW:
> @@ -57,10 +57,16 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
> RES_MASK(st->chip_info->channel[0].scan_type.realbits);
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_SCALE:
> - scale_uv = (st->int_vref_mv * 1000)
> - >> st->chip_info->channel[0].scan_type.realbits;
> - *val = scale_uv/1000;
> - *val2 = (scale_uv%1000)*1000;
> + if (!st->chip_info->int_vref_uv) {
> + scale_uv = regulator_get_voltage(st->reg);
> + if (scale_uv < 0)
> + return scale_uv;
> + } else {
> + scale_uv = st->chip_info->int_vref_uv;
> + }
> + scale_uv >>= chan->scan_type.realbits;
> + *val = scale_uv / 1000;
> + *val2 = (scale_uv % 1000) * 1000;
> return IIO_VAL_INT_PLUS_MICRO;
> }
> return -EINVAL;
> @@ -96,7 +102,7 @@ static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
> [ID_AD7495] = {
> .channel[0] = AD7476_CHAN(12),
> .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> - .int_vref_mv = 2500,
> + .int_vref_uv = 2500000,
> },
> };
>
> @@ -107,10 +113,9 @@ static const struct iio_info ad7476_info = {
>
> static int __devinit ad7476_probe(struct spi_device *spi)
> {
> - struct ad7476_platform_data *pdata = spi->dev.platform_data;
> struct ad7476_state *st;
> struct iio_dev *indio_dev;
> - int ret, voltage_uv = 0;
> + int ret;
>
> indio_dev = iio_device_alloc(sizeof(*st));
> if (indio_dev == NULL) {
> @@ -118,25 +123,18 @@ static int __devinit ad7476_probe(struct spi_device *spi)
> goto error_ret;
> }
> st = iio_priv(indio_dev);
> - st->reg = regulator_get(&spi->dev, "vcc");
> - if (!IS_ERR(st->reg)) {
> - ret = regulator_enable(st->reg);
> - if (ret)
> - goto error_put_reg;
> -
> - voltage_uv = regulator_get_voltage(st->reg);
> - }
> st->chip_info =
> &ad7476_chip_info_tbl[spi_get_device_id(spi)->driver_data];
>
> - if (st->chip_info->int_vref_mv)
> - st->int_vref_mv = st->chip_info->int_vref_mv;
> - else if (pdata && pdata->vref_mv)
> - st->int_vref_mv = pdata->vref_mv;
> - else if (voltage_uv)
> - st->int_vref_mv = voltage_uv / 1000;
> - else
> - dev_warn(&spi->dev, "reference voltage unspecified\n");
> + st->reg = regulator_get(&spi->dev, "vcc");
> + if (IS_ERR(st->reg)) {
> + ret = PTR_ERR(st->reg);
> + goto error_ret;
> + }
> +
> + ret = regulator_enable(st->reg);
> + if (ret)
> + goto error_put_reg;
>
> spi_set_drvdata(spi, indio_dev);
>
> @@ -169,11 +167,9 @@ static int __devinit ad7476_probe(struct spi_device *spi)
> error_ring_unregister:
> ad7476_ring_cleanup(indio_dev);
> error_disable_reg:
> - if (!IS_ERR(st->reg))
> - regulator_disable(st->reg);
> + regulator_disable(st->reg);
> error_put_reg:
> - if (!IS_ERR(st->reg))
> - regulator_put(st->reg);
> + regulator_put(st->reg);
> iio_device_free(indio_dev);
>
> error_ret:
> @@ -187,10 +183,8 @@ static int __devexit ad7476_remove(struct spi_device *spi)
>
> iio_device_unregister(indio_dev);
> ad7476_ring_cleanup(indio_dev);
> - if (!IS_ERR(st->reg)) {
> - regulator_disable(st->reg);
> - regulator_put(st->reg);
> - }
> + regulator_disable(st->reg);
> + regulator_put(st->reg);
> iio_device_free(indio_dev);
>
> return 0;
>
--
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
