| Message ID | 20181223045743.10716-2-jeremyfertic@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | staging: iio: adt7316: dac fixes | expand |
+ CC Shreeya who is working on the same driver. On Sat, 22 Dec 2018 21:57:40 -0700 Jeremy Fertic <jeremyfertic@gmail.com> 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@gmail.com> 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);
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);
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@gmail.com> --- drivers/staging/iio/addac/adt7316.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)