* [PATCH 1/3] iio:ad5064: Fix address of the second channel for ad5065/ad5045/ad5025
@ 2013-03-01 13:07 Lars-Peter Clausen
2013-03-01 13:07 ` [PATCH 2/3] iio:ad5064: Fix off by one in DAC value range check Lars-Peter Clausen
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Lars-Peter Clausen @ 2013-03-01 13:07 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, Lars-Peter Clausen
The ad5065, ad5045, ad5025 use address '3' for the second channel, so they need
their own channel spec.
Note that ad5064_sync_powerdown_mode() also needs to be slightly updated since
it was relying on the fact that chan->address always equaled chan->channel.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/iio/dac/ad5064.c | 49 ++++++++++++++++++++++++++++--------------------
1 file changed, 29 insertions(+), 20 deletions(-)
diff --git a/drivers/iio/dac/ad5064.c b/drivers/iio/dac/ad5064.c
index eb281a2..e19b73f 100644
--- a/drivers/iio/dac/ad5064.c
+++ b/drivers/iio/dac/ad5064.c
@@ -27,7 +27,6 @@
#define AD5064_ADDR(x) ((x) << 20)
#define AD5064_CMD(x) ((x) << 24)
-#define AD5064_ADDR_DAC(chan) (chan)
#define AD5064_ADDR_ALL_DAC 0xF
#define AD5064_CMD_WRITE_INPUT_N 0x0
@@ -131,15 +130,15 @@ static int ad5064_write(struct ad5064_state *st, unsigned int cmd,
}
static int ad5064_sync_powerdown_mode(struct ad5064_state *st,
- unsigned int channel)
+ const struct iio_chan_spec *chan)
{
unsigned int val;
int ret;
- val = (0x1 << channel);
+ val = (0x1 << chan->address);
- if (st->pwr_down[channel])
- val |= st->pwr_down_mode[channel] << 8;
+ if (st->pwr_down[chan->channel])
+ val |= st->pwr_down_mode[chan->channel] << 8;
ret = ad5064_write(st, AD5064_CMD_POWERDOWN_DAC, 0, val, 0);
@@ -169,7 +168,7 @@ static int ad5064_set_powerdown_mode(struct iio_dev *indio_dev,
mutex_lock(&indio_dev->mlock);
st->pwr_down_mode[chan->channel] = mode + 1;
- ret = ad5064_sync_powerdown_mode(st, chan->channel);
+ ret = ad5064_sync_powerdown_mode(st, chan);
mutex_unlock(&indio_dev->mlock);
return ret;
@@ -205,7 +204,7 @@ static ssize_t ad5064_write_dac_powerdown(struct iio_dev *indio_dev,
mutex_lock(&indio_dev->mlock);
st->pwr_down[chan->channel] = pwr_down;
- ret = ad5064_sync_powerdown_mode(st, chan->channel);
+ ret = ad5064_sync_powerdown_mode(st, chan);
mutex_unlock(&indio_dev->mlock);
return ret ? ret : len;
}
@@ -292,34 +291,44 @@ static const struct iio_chan_spec_ext_info ad5064_ext_info[] = {
{ },
};
-#define AD5064_CHANNEL(chan, bits) { \
+#define AD5064_CHANNEL(chan, addr, bits) { \
.type = IIO_VOLTAGE, \
.indexed = 1, \
.output = 1, \
.channel = (chan), \
.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT | \
IIO_CHAN_INFO_SCALE_SEPARATE_BIT, \
- .address = AD5064_ADDR_DAC(chan), \
+ .address = addr, \
.scan_type = IIO_ST('u', (bits), 16, 20 - (bits)), \
.ext_info = ad5064_ext_info, \
}
#define DECLARE_AD5064_CHANNELS(name, bits) \
const struct iio_chan_spec name[] = { \
- AD5064_CHANNEL(0, bits), \
- AD5064_CHANNEL(1, bits), \
- AD5064_CHANNEL(2, bits), \
- AD5064_CHANNEL(3, bits), \
- AD5064_CHANNEL(4, bits), \
- AD5064_CHANNEL(5, bits), \
- AD5064_CHANNEL(6, bits), \
- AD5064_CHANNEL(7, bits), \
+ AD5064_CHANNEL(0, 0, bits), \
+ AD5064_CHANNEL(1, 1, bits), \
+ AD5064_CHANNEL(2, 2, bits), \
+ AD5064_CHANNEL(3, 3, bits), \
+ AD5064_CHANNEL(4, 4, bits), \
+ AD5064_CHANNEL(5, 5, bits), \
+ AD5064_CHANNEL(6, 6, bits), \
+ AD5064_CHANNEL(7, 7, bits), \
+}
+
+#define DECLARE_AD5065_CHANNELS(name, bits) \
+const struct iio_chan_spec name[] = { \
+ AD5064_CHANNEL(0, 0, bits), \
+ AD5064_CHANNEL(1, 3, bits), \
}
static DECLARE_AD5064_CHANNELS(ad5024_channels, 12);
static DECLARE_AD5064_CHANNELS(ad5044_channels, 14);
static DECLARE_AD5064_CHANNELS(ad5064_channels, 16);
+static DECLARE_AD5065_CHANNELS(ad5025_channels, 12);
+static DECLARE_AD5065_CHANNELS(ad5045_channels, 14);
+static DECLARE_AD5065_CHANNELS(ad5065_channels, 16);
+
static const struct ad5064_chip_info ad5064_chip_info_tbl[] = {
[ID_AD5024] = {
.shared_vref = false,
@@ -328,7 +337,7 @@ static const struct ad5064_chip_info ad5064_chip_info_tbl[] = {
},
[ID_AD5025] = {
.shared_vref = false,
- .channels = ad5024_channels,
+ .channels = ad5025_channels,
.num_channels = 2,
},
[ID_AD5044] = {
@@ -338,7 +347,7 @@ static const struct ad5064_chip_info ad5064_chip_info_tbl[] = {
},
[ID_AD5045] = {
.shared_vref = false,
- .channels = ad5044_channels,
+ .channels = ad5045_channels,
.num_channels = 2,
},
[ID_AD5064] = {
@@ -353,7 +362,7 @@ static const struct ad5064_chip_info ad5064_chip_info_tbl[] = {
},
[ID_AD5065] = {
.shared_vref = false,
- .channels = ad5064_channels,
+ .channels = ad5065_channels,
.num_channels = 2,
},
[ID_AD5628_1] = {
--
1.8.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] iio:ad5064: Fix off by one in DAC value range check
2013-03-01 13:07 [PATCH 1/3] iio:ad5064: Fix address of the second channel for ad5065/ad5045/ad5025 Lars-Peter Clausen
@ 2013-03-01 13:07 ` Lars-Peter Clausen
2013-03-02 16:24 ` Jonathan Cameron
2013-03-01 13:07 ` [PATCH 3/3] iio:ad5064: Initialize register cache correctly Lars-Peter Clausen
2013-03-02 16:23 ` [PATCH 1/3] iio:ad5064: Fix address of the second channel for ad5065/ad5045/ad5025 Jonathan Cameron
2 siblings, 1 reply; 6+ messages in thread
From: Lars-Peter Clausen @ 2013-03-01 13:07 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, Lars-Peter Clausen
The DAC value range check allows values one larger than the maximum value, which
effectively results in setting the DAC value to 0. This patch fixes the issue.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/iio/dac/ad5064.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/dac/ad5064.c b/drivers/iio/dac/ad5064.c
index e19b73f..a491976 100644
--- a/drivers/iio/dac/ad5064.c
+++ b/drivers/iio/dac/ad5064.c
@@ -257,7 +257,7 @@ static int ad5064_write_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_RAW:
- if (val > (1 << chan->scan_type.realbits) || val < 0)
+ if (val >= (1 << chan->scan_type.realbits) || val < 0)
return -EINVAL;
mutex_lock(&indio_dev->mlock);
--
1.8.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] iio:ad5064: Initialize register cache correctly
2013-03-01 13:07 [PATCH 1/3] iio:ad5064: Fix address of the second channel for ad5065/ad5045/ad5025 Lars-Peter Clausen
2013-03-01 13:07 ` [PATCH 2/3] iio:ad5064: Fix off by one in DAC value range check Lars-Peter Clausen
@ 2013-03-01 13:07 ` Lars-Peter Clausen
2013-03-02 16:25 ` Jonathan Cameron
2013-03-02 16:23 ` [PATCH 1/3] iio:ad5064: Fix address of the second channel for ad5065/ad5045/ad5025 Jonathan Cameron
2 siblings, 1 reply; 6+ messages in thread
From: Lars-Peter Clausen @ 2013-03-01 13:07 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, Lars-Peter Clausen
Initialize the register cache to the proper mid-scale value based on the
resolution of the device.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/iio/dac/ad5064.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/dac/ad5064.c b/drivers/iio/dac/ad5064.c
index a491976..0c8d03a 100644
--- a/drivers/iio/dac/ad5064.c
+++ b/drivers/iio/dac/ad5064.c
@@ -438,6 +438,7 @@ static int __devinit ad5064_probe(struct device *dev, enum ad5064_type type,
{
struct iio_dev *indio_dev;
struct ad5064_state *st;
+ unsigned int midscale;
unsigned int i;
int ret;
@@ -474,11 +475,6 @@ static int __devinit ad5064_probe(struct device *dev, enum ad5064_type type,
goto error_free_reg;
}
- for (i = 0; i < st->chip_info->num_channels; ++i) {
- st->pwr_down_mode[i] = AD5064_LDAC_PWRDN_1K;
- st->dac_cache[i] = 0x8000;
- }
-
indio_dev->dev.parent = dev;
indio_dev->name = name;
indio_dev->info = &ad5064_info;
@@ -486,6 +482,13 @@ static int __devinit ad5064_probe(struct device *dev, enum ad5064_type type,
indio_dev->channels = st->chip_info->channels;
indio_dev->num_channels = st->chip_info->num_channels;
+ midscale = (1 << indio_dev->channels[0].scan_type.realbits) / 2;
+
+ for (i = 0; i < st->chip_info->num_channels; ++i) {
+ st->pwr_down_mode[i] = AD5064_LDAC_PWRDN_1K;
+ st->dac_cache[i] = midscale;
+ }
+
ret = iio_device_register(indio_dev);
if (ret)
goto error_disable_reg;
--
1.8.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] iio:ad5064: Fix address of the second channel for ad5065/ad5045/ad5025
2013-03-01 13:07 [PATCH 1/3] iio:ad5064: Fix address of the second channel for ad5065/ad5045/ad5025 Lars-Peter Clausen
2013-03-01 13:07 ` [PATCH 2/3] iio:ad5064: Fix off by one in DAC value range check Lars-Peter Clausen
2013-03-01 13:07 ` [PATCH 3/3] iio:ad5064: Initialize register cache correctly Lars-Peter Clausen
@ 2013-03-02 16:23 ` Jonathan Cameron
2 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2013-03-02 16:23 UTC (permalink / raw)
To: Lars-Peter Clausen; +Cc: linux-iio
On 03/01/2013 01:07 PM, Lars-Peter Clausen wrote:
> The ad5065, ad5045, ad5025 use address '3' for the second channel, so they need
> their own channel spec.
Obviously! (another candidate for weird hardware designer of the day :)
>
> Note that ad5064_sync_powerdown_mode() also needs to be slightly updated since
> it was relying on the fact that chan->address always equaled chan->channel.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
applied to fixes-togreg branch of iio.git
> ---
> drivers/iio/dac/ad5064.c | 49 ++++++++++++++++++++++++++++--------------------
> 1 file changed, 29 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/iio/dac/ad5064.c b/drivers/iio/dac/ad5064.c
> index eb281a2..e19b73f 100644
> --- a/drivers/iio/dac/ad5064.c
> +++ b/drivers/iio/dac/ad5064.c
> @@ -27,7 +27,6 @@
> #define AD5064_ADDR(x) ((x) << 20)
> #define AD5064_CMD(x) ((x) << 24)
>
> -#define AD5064_ADDR_DAC(chan) (chan)
> #define AD5064_ADDR_ALL_DAC 0xF
>
> #define AD5064_CMD_WRITE_INPUT_N 0x0
> @@ -131,15 +130,15 @@ static int ad5064_write(struct ad5064_state *st, unsigned int cmd,
> }
>
> static int ad5064_sync_powerdown_mode(struct ad5064_state *st,
> - unsigned int channel)
> + const struct iio_chan_spec *chan)
> {
> unsigned int val;
> int ret;
>
> - val = (0x1 << channel);
> + val = (0x1 << chan->address);
>
> - if (st->pwr_down[channel])
> - val |= st->pwr_down_mode[channel] << 8;
> + if (st->pwr_down[chan->channel])
> + val |= st->pwr_down_mode[chan->channel] << 8;
>
> ret = ad5064_write(st, AD5064_CMD_POWERDOWN_DAC, 0, val, 0);
>
> @@ -169,7 +168,7 @@ static int ad5064_set_powerdown_mode(struct iio_dev *indio_dev,
> mutex_lock(&indio_dev->mlock);
> st->pwr_down_mode[chan->channel] = mode + 1;
>
> - ret = ad5064_sync_powerdown_mode(st, chan->channel);
> + ret = ad5064_sync_powerdown_mode(st, chan);
> mutex_unlock(&indio_dev->mlock);
>
> return ret;
> @@ -205,7 +204,7 @@ static ssize_t ad5064_write_dac_powerdown(struct iio_dev *indio_dev,
> mutex_lock(&indio_dev->mlock);
> st->pwr_down[chan->channel] = pwr_down;
>
> - ret = ad5064_sync_powerdown_mode(st, chan->channel);
> + ret = ad5064_sync_powerdown_mode(st, chan);
> mutex_unlock(&indio_dev->mlock);
> return ret ? ret : len;
> }
> @@ -292,34 +291,44 @@ static const struct iio_chan_spec_ext_info ad5064_ext_info[] = {
> { },
> };
>
> -#define AD5064_CHANNEL(chan, bits) { \
> +#define AD5064_CHANNEL(chan, addr, bits) { \
> .type = IIO_VOLTAGE, \
> .indexed = 1, \
> .output = 1, \
> .channel = (chan), \
> .info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT | \
> IIO_CHAN_INFO_SCALE_SEPARATE_BIT, \
> - .address = AD5064_ADDR_DAC(chan), \
> + .address = addr, \
> .scan_type = IIO_ST('u', (bits), 16, 20 - (bits)), \
> .ext_info = ad5064_ext_info, \
> }
>
> #define DECLARE_AD5064_CHANNELS(name, bits) \
> const struct iio_chan_spec name[] = { \
> - AD5064_CHANNEL(0, bits), \
> - AD5064_CHANNEL(1, bits), \
> - AD5064_CHANNEL(2, bits), \
> - AD5064_CHANNEL(3, bits), \
> - AD5064_CHANNEL(4, bits), \
> - AD5064_CHANNEL(5, bits), \
> - AD5064_CHANNEL(6, bits), \
> - AD5064_CHANNEL(7, bits), \
> + AD5064_CHANNEL(0, 0, bits), \
> + AD5064_CHANNEL(1, 1, bits), \
> + AD5064_CHANNEL(2, 2, bits), \
> + AD5064_CHANNEL(3, 3, bits), \
> + AD5064_CHANNEL(4, 4, bits), \
> + AD5064_CHANNEL(5, 5, bits), \
> + AD5064_CHANNEL(6, 6, bits), \
> + AD5064_CHANNEL(7, 7, bits), \
> +}
> +
> +#define DECLARE_AD5065_CHANNELS(name, bits) \
> +const struct iio_chan_spec name[] = { \
> + AD5064_CHANNEL(0, 0, bits), \
> + AD5064_CHANNEL(1, 3, bits), \
> }
>
> static DECLARE_AD5064_CHANNELS(ad5024_channels, 12);
> static DECLARE_AD5064_CHANNELS(ad5044_channels, 14);
> static DECLARE_AD5064_CHANNELS(ad5064_channels, 16);
>
> +static DECLARE_AD5065_CHANNELS(ad5025_channels, 12);
> +static DECLARE_AD5065_CHANNELS(ad5045_channels, 14);
> +static DECLARE_AD5065_CHANNELS(ad5065_channels, 16);
> +
> static const struct ad5064_chip_info ad5064_chip_info_tbl[] = {
> [ID_AD5024] = {
> .shared_vref = false,
> @@ -328,7 +337,7 @@ static const struct ad5064_chip_info ad5064_chip_info_tbl[] = {
> },
> [ID_AD5025] = {
> .shared_vref = false,
> - .channels = ad5024_channels,
> + .channels = ad5025_channels,
> .num_channels = 2,
> },
> [ID_AD5044] = {
> @@ -338,7 +347,7 @@ static const struct ad5064_chip_info ad5064_chip_info_tbl[] = {
> },
> [ID_AD5045] = {
> .shared_vref = false,
> - .channels = ad5044_channels,
> + .channels = ad5045_channels,
> .num_channels = 2,
> },
> [ID_AD5064] = {
> @@ -353,7 +362,7 @@ static const struct ad5064_chip_info ad5064_chip_info_tbl[] = {
> },
> [ID_AD5065] = {
> .shared_vref = false,
> - .channels = ad5064_channels,
> + .channels = ad5065_channels,
> .num_channels = 2,
> },
> [ID_AD5628_1] = {
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] iio:ad5064: Fix off by one in DAC value range check
2013-03-01 13:07 ` [PATCH 2/3] iio:ad5064: Fix off by one in DAC value range check Lars-Peter Clausen
@ 2013-03-02 16:24 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2013-03-02 16:24 UTC (permalink / raw)
To: Lars-Peter Clausen; +Cc: linux-iio
On 03/01/2013 01:07 PM, Lars-Peter Clausen wrote:
> The DAC value range check allows values one larger than the maximum value, which
> effectively results in setting the DAC value to 0. This patch fixes the issue.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
applied to fixes-togreg branch of iio.git
> ---
> drivers/iio/dac/ad5064.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/dac/ad5064.c b/drivers/iio/dac/ad5064.c
> index e19b73f..a491976 100644
> --- a/drivers/iio/dac/ad5064.c
> +++ b/drivers/iio/dac/ad5064.c
> @@ -257,7 +257,7 @@ static int ad5064_write_raw(struct iio_dev *indio_dev,
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> - if (val > (1 << chan->scan_type.realbits) || val < 0)
> + if (val >= (1 << chan->scan_type.realbits) || val < 0)
> return -EINVAL;
>
> mutex_lock(&indio_dev->mlock);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] iio:ad5064: Initialize register cache correctly
2013-03-01 13:07 ` [PATCH 3/3] iio:ad5064: Initialize register cache correctly Lars-Peter Clausen
@ 2013-03-02 16:25 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2013-03-02 16:25 UTC (permalink / raw)
To: Lars-Peter Clausen; +Cc: linux-iio
On 03/01/2013 01:07 PM, Lars-Peter Clausen wrote:
> Initialize the register cache to the proper mid-scale value based on the
> resolution of the device.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
applied to togreg branch of iio.git
Thanks
> ---
> drivers/iio/dac/ad5064.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/dac/ad5064.c b/drivers/iio/dac/ad5064.c
> index a491976..0c8d03a 100644
> --- a/drivers/iio/dac/ad5064.c
> +++ b/drivers/iio/dac/ad5064.c
> @@ -438,6 +438,7 @@ static int __devinit ad5064_probe(struct device *dev, enum ad5064_type type,
> {
> struct iio_dev *indio_dev;
> struct ad5064_state *st;
> + unsigned int midscale;
> unsigned int i;
> int ret;
>
> @@ -474,11 +475,6 @@ static int __devinit ad5064_probe(struct device *dev, enum ad5064_type type,
> goto error_free_reg;
> }
>
> - for (i = 0; i < st->chip_info->num_channels; ++i) {
> - st->pwr_down_mode[i] = AD5064_LDAC_PWRDN_1K;
> - st->dac_cache[i] = 0x8000;
> - }
> -
> indio_dev->dev.parent = dev;
> indio_dev->name = name;
> indio_dev->info = &ad5064_info;
> @@ -486,6 +482,13 @@ static int __devinit ad5064_probe(struct device *dev, enum ad5064_type type,
> indio_dev->channels = st->chip_info->channels;
> indio_dev->num_channels = st->chip_info->num_channels;
>
> + midscale = (1 << indio_dev->channels[0].scan_type.realbits) / 2;
> +
> + for (i = 0; i < st->chip_info->num_channels; ++i) {
> + st->pwr_down_mode[i] = AD5064_LDAC_PWRDN_1K;
> + st->dac_cache[i] = midscale;
> + }
> +
> ret = iio_device_register(indio_dev);
> if (ret)
> goto error_disable_reg;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-03-02 16:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-01 13:07 [PATCH 1/3] iio:ad5064: Fix address of the second channel for ad5065/ad5045/ad5025 Lars-Peter Clausen
2013-03-01 13:07 ` [PATCH 2/3] iio:ad5064: Fix off by one in DAC value range check Lars-Peter Clausen
2013-03-02 16:24 ` Jonathan Cameron
2013-03-01 13:07 ` [PATCH 3/3] iio:ad5064: Initialize register cache correctly Lars-Peter Clausen
2013-03-02 16:25 ` Jonathan Cameron
2013-03-02 16:23 ` [PATCH 1/3] iio:ad5064: Fix address of the second channel for ad5065/ad5045/ad5025 Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.