From: Rodrigo Alencar <[email protected]> For single-channel parts the control register is used to enable the internal voltage reference and perform powerdown control. The reference enable bit position was ignored when writing the register at the probe function. This patch adds a control_sync() function that properly consumes the mask definitions with FIELD_PREP(). As further cleanup, the created functions and definitions are also used in ad5686_write_dac_powerdown(). Some local variables ended up being unused (so removed) and st->use_internal_vref is initialized earlier in probe.
Signed-off-by: Rodrigo Alencar <[email protected]> --- drivers/iio/dac/ad5686.c | 79 +++++++++++++++++++++++++++--------------------- drivers/iio/dac/ad5686.h | 3 +- 2 files changed, 46 insertions(+), 36 deletions(-) diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c index e67faef91164..4ca9cd5b6e38 100644 --- a/drivers/iio/dac/ad5686.c +++ b/drivers/iio/dac/ad5686.c @@ -6,11 +6,13 @@ */ #include <linux/array_size.h> +#include <linux/bitfield.h> #include <linux/err.h> #include <linux/export.h> #include <linux/module.h> #include <linux/regulator/consumer.h> #include <linux/sysfs.h> +#include <linux/wordpart.h> #include "ad5686.h" @@ -20,6 +22,24 @@ static const char * const ad5686_powerdown_modes[] = { "three_state" }; +static int ad5310_control_sync(struct ad5686_state *st) +{ + unsigned int pd_val = st->pwr_down_mask & st->pwr_down_mode; + + return st->write(st, AD5686_CMD_CONTROL_REG, 0, + FIELD_PREP(AD5310_PD_MSK, pd_val) | + FIELD_PREP(AD5310_REF_BIT_MSK, !st->use_internal_vref)); +} + +static int ad5683_control_sync(struct ad5686_state *st) +{ + unsigned int pd_val = st->pwr_down_mask & st->pwr_down_mode; + + return st->write(st, AD5686_CMD_CONTROL_REG, 0, + FIELD_PREP(AD5683_PD_MSK, pd_val) | + FIELD_PREP(AD5683_REF_BIT_MSK, !st->use_internal_vref)); +} + static int ad5686_get_powerdown_mode(struct iio_dev *indio_dev, const struct iio_chan_spec *chan) { @@ -65,8 +85,8 @@ static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev, bool readin; int ret; struct ad5686_state *st = iio_priv(indio_dev); - unsigned int val, ref_bit_msk; - u8 shift, address = 0; + unsigned int val; + u8 address = 0; ret = kstrtobool(buf, &readin); if (ret) @@ -79,30 +99,26 @@ static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev, switch (st->chip_info->regmap_type) { case AD5310_REGMAP: - shift = 9; - ref_bit_msk = AD5310_REF_BIT_MSK; + ret = ad5310_control_sync(st); break; case AD5683_REGMAP: - shift = 13; - ref_bit_msk = AD5683_REF_BIT_MSK; + ret = ad5683_control_sync(st); break; case AD5686_REGMAP: - shift = 0; - ref_bit_msk = 0; /* AD5674R/AD5679R have 16 channels and 2 powerdown registers */ - if (chan->channel > 0x7) + val = st->pwr_down_mask & st->pwr_down_mode; + if (chan->channel > 0x7) { address = 0x8; + val = upper_16_bits(val); + } else { + val = lower_16_bits(val); + } + ret = st->write(st, AD5686_CMD_POWERDOWN_DAC, address, val); break; default: return -EINVAL; } - val = ((st->pwr_down_mask & st->pwr_down_mode) << shift); - if (!st->use_internal_vref) - val |= ref_bit_msk; - - ret = st->write(st, AD5686_CMD_POWERDOWN_DAC, - address, val >> (address * 2)); return ret ? ret : len; } @@ -416,11 +432,8 @@ int ad5686_probe(struct device *dev, const char *name, ad5686_write_func write, ad5686_read_func read) { - struct ad5686_state *st; struct iio_dev *indio_dev; - unsigned int val, ref_bit_msk; - bool has_external_vref; - u8 cmd; + struct ad5686_state *st; int ret, i; indio_dev = devm_iio_device_alloc(dev, sizeof(*st)); @@ -438,8 +451,8 @@ int ad5686_probe(struct device *dev, if (ret < 0 && ret != -ENODEV) return ret; - has_external_vref = ret != -ENODEV; - st->vref_mv = has_external_vref ? ret / 1000 : st->chip_info->int_vref_mv; + st->use_internal_vref = ret == -ENODEV; + st->vref_mv = st->use_internal_vref ? st->chip_info->int_vref_mv : ret / 1000; /* Set all the power down mode for all channels to 1K pulldown */ for (i = 0; i < st->chip_info->num_channels; i++) @@ -457,29 +470,25 @@ int ad5686_probe(struct device *dev, switch (st->chip_info->regmap_type) { case AD5310_REGMAP: - cmd = AD5686_CMD_CONTROL_REG; - ref_bit_msk = AD5310_REF_BIT_MSK; - st->use_internal_vref = !has_external_vref; + ret = ad5310_control_sync(st); + if (ret) + return ret; break; case AD5683_REGMAP: - cmd = AD5686_CMD_CONTROL_REG; - ref_bit_msk = AD5683_REF_BIT_MSK; - st->use_internal_vref = !has_external_vref; + ret = ad5683_control_sync(st); + if (ret) + return ret; break; case AD5686_REGMAP: - cmd = AD5686_CMD_INTERNAL_REFER_SETUP; - ref_bit_msk = 0; + ret = st->write(st, AD5686_CMD_INTERNAL_REFER_SETUP, 0, + !st->use_internal_vref); + if (ret) + return ret; break; default: return -EINVAL; } - val = (has_external_vref | ref_bit_msk); - - ret = st->write(st, cmd, 0, !!val); - if (ret) - return ret; - return devm_iio_device_register(dev, indio_dev); } EXPORT_SYMBOL_NS_GPL(ad5686_probe, "IIO_AD5686"); diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h index 6e3552c92e4b..7004d0d1d97a 100644 --- a/drivers/iio/dac/ad5686.h +++ b/drivers/iio/dac/ad5686.h @@ -44,8 +44,9 @@ #define AD5686_CMD_READBACK_ENABLE_V2 0x5 #define AD5310_REF_BIT_MSK BIT(8) +#define AD5310_PD_MSK GENMASK(10, 9) #define AD5683_REF_BIT_MSK BIT(12) - +#define AD5683_PD_MSK GENMASK(14, 13) enum ad5686_regmap_type { AD5310_REGMAP, -- 2.43.0
