+ CC Shreeya who is working on the same driver.
On Sat, 22 Dec 2018 21:57:40 -0700
Jeremy Fertic <jeremyfertic@xxxxxxxxx> wrote:
> The value of dac_bits is used in adt7316_show_DAC() and adt7316_store_DAC(),
> and it should be either 8, 10, or 12 bits depending on the device in use. The
> driver currently only assigns a value to dac_bits in
> adt7316_store_da_high_resolution(). The purpose of the dac high resolution
> option is not to change dac resolution for normal operation. Instead, it
> is specific to an optional feature where one or two of the four dacs can
> be set to output voltage proportional to temperature. If the user chooses
> to set dac a and/or dac b to output voltage proportional to temperature,
> the da_high_resolution attribute can optionally be enabled to use 10 bit
> resolution rather than the default 8 bits. This is only available on the
> 10 and 12 bit dac devices. If the user attempts to read or write dacs a
> or b under these settings, the driver's current behaviour is to return an
> error. Dacs c and d continue to operate normally under these conditions.
> With the above in mind, remove the dac_bits assignments from this function
> since the value of dac_bits as used in the driver is not dependent on this
> dac high resolution option.
>
> Since the dac_bits assignments discussed above are currently the only ones
> in this driver, the default value of dac_bits is 0. This results in incorrect
> calculations when the dacs are read or written in adt7316_show_DAC() and
> adt7316_store_DAC(). To correct this, assign a value to dac_bits in
> adt7316_probe() to ensure correct operation as soon as the device is
> registered and available to userspace.
>
> Fixes: 35f6b6b86ede ("staging: iio: new ADT7316/7/8 and ADT7516/7/9 driver")
> Signed-off-by: Jeremy Fertic <jeremyfertic@xxxxxxxxx>
Applied to the togreg branch of iio.git and pushed out as testing for the
autobuilders to play with. The driver has been broken a long time and
is undergoing a lot of churn at the moment, so I'm not going to rush
this in even though it's a fix.
Thanks,
Jonathan
> ---
> drivers/staging/iio/addac/adt7316.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/iio/addac/adt7316.c b/drivers/staging/iio/addac/adt7316.c
> index 9db49aa186bb..e17c1cb12c94 100644
> --- a/drivers/staging/iio/addac/adt7316.c
> +++ b/drivers/staging/iio/addac/adt7316.c
> @@ -652,17 +652,10 @@ static ssize_t adt7316_store_da_high_resolution(struct device *dev,
> u8 config3;
> int ret;
>
> - chip->dac_bits = 8;
> -
> - if (buf[0] == '1') {
> + if (buf[0] == '1')
> config3 = chip->config3 | ADT7316_DA_HIGH_RESOLUTION;
> - if (chip->id == ID_ADT7316 || chip->id == ID_ADT7516)
> - chip->dac_bits = 12;
> - else if (chip->id == ID_ADT7317 || chip->id == ID_ADT7517)
> - chip->dac_bits = 10;
> - } else {
> + else
> config3 = chip->config3 & (~ADT7316_DA_HIGH_RESOLUTION);
> - }
>
> ret = chip->bus.write(chip->bus.client, ADT7316_CONFIG3, config3);
> if (ret)
> @@ -2145,6 +2138,13 @@ int adt7316_probe(struct device *dev, struct adt7316_bus *bus,
> else
> return -ENODEV;
>
> + if (chip->id == ID_ADT7316 || chip->id == ID_ADT7516)
> + chip->dac_bits = 12;
> + else if (chip->id == ID_ADT7317 || chip->id == ID_ADT7517)
> + chip->dac_bits = 10;
> + else
> + chip->dac_bits = 8;
> +
> chip->ldac_pin = devm_gpiod_get_optional(dev, "adi,ldac", GPIOD_OUT_LOW);
> if (IS_ERR(chip->ldac_pin)) {
> ret = PTR_ERR(chip->ldac_pin);
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
