[PATCH 1/8] staging:iio:dac:ad5446: Do not exit powerdown when writing a sample

[PATCH 1/8] staging:iio:dac:ad5446: Do not exit powerdown when writing a sample

[Lars-Peter Clausen]

Both the powerdown mode bits and the sample value are stored in the same
register, so writing a sample while the device is powered down will clear the
power down bits. To avoid this only update the cached value when the device is
powered down.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/dac/ad5446.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/iio/dac/ad5446.c b/drivers/staging/iio/dac/ad5446.c
index ec6968b..0cfe445 100644
--- a/drivers/staging/iio/dac/ad5446.c
+++ b/drivers/staging/iio/dac/ad5446.c
@@ -282,7 +282,8 @@ static int ad5446_write_raw(struct iio_dev *indio_dev,
 		val <<= chan->scan_type.shift;
 		mutex_lock(&indio_dev->mlock);
 		st->cached_val = val;
-		st->chip_info->store_sample(st, val);
+		if (!st->pwr_down)
+			st->chip_info->store_sample(st, val);
 		ret = spi_sync(st->spi, &st->msg);
 		mutex_unlock(&indio_dev->mlock);
 		break;
-- 
1.7.9.5

[PATCH 2/8] staging:iio:dac:ad5446: Remove unused struct field

[Lars-Peter Clausen]

Remove the unused "poll_work" field from the ad5446_state struct.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/dac/ad5446.h |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/iio/dac/ad5446.h b/drivers/staging/iio/dac/ad5446.h
index 4ea3476..72ad07f 100644
--- a/drivers/staging/iio/dac/ad5446.h
+++ b/drivers/staging/iio/dac/ad5446.h
@@ -34,7 +34,6 @@
  * @spi:		spi_device
  * @chip_info:		chip model specific constants, available modes etc
  * @reg:		supply regulator
- * @poll_work:		bottom half of polling interrupt handler
  * @vref_mv:		actual reference voltage used
  * @xfer:		default spi transfer
  * @msg:		default spi message
@@ -45,7 +44,6 @@ struct ad5446_state {
 	struct spi_device		*spi;
 	const struct ad5446_chip_info	*chip_info;
 	struct regulator		*reg;
-	struct work_struct		poll_work;
 	unsigned short			vref_mv;
 	unsigned			cached_val;
 	unsigned			pwr_down_mode;
-- 
1.7.9.5

[PATCH 3/8] staging:iio:dac:ad5446: Do not check for individual chip ids in probe

[Lars-Peter Clausen]

Use the chip_info's int_vref_mv field to decide whether a certain chip has a
internal reference or not. There is no need to check for individual chip ids.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/dac/ad5446.c |   20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/iio/dac/ad5446.c b/drivers/staging/iio/dac/ad5446.c
index 0cfe445..9d9a452 100644
--- a/drivers/staging/iio/dac/ad5446.c
+++ b/drivers/staging/iio/dac/ad5446.c
@@ -355,22 +355,12 @@ static int __devinit ad5446_probe(struct spi_device *spi)
 	spi_message_init(&st->msg);
 	spi_message_add_tail(&st->xfer, &st->msg);
 
-	switch (spi_get_device_id(spi)->driver_data) {
-	case ID_AD5620_2500:
-	case ID_AD5620_1250:
-	case ID_AD5640_2500:
-	case ID_AD5640_1250:
-	case ID_AD5660_2500:
-	case ID_AD5660_1250:
+	if (st->chip_info->int_vref_mv)
 		st->vref_mv = st->chip_info->int_vref_mv;
-		break;
-	default:
-		if (voltage_uv)
-			st->vref_mv = voltage_uv / 1000;
-		else
-			dev_warn(&spi->dev,
-				 "reference voltage unspecified\n");
-	}
+	else if (voltage_uv)
+		st->vref_mv = voltage_uv / 1000;
+	else
+		dev_warn(&spi->dev, "reference voltage unspecified\n");
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
-- 
1.7.9.5

[PATCH 4/8] staging:iio:dac:ad5446: Remove duplicated chip_info entries

[Lars-Peter Clausen]

There are three identical chip_info entries. Remove two of them and use the id
of the remaining entry for all three device table entries.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/dac/ad5446.c |   12 ++----------
 drivers/staging/iio/dac/ad5446.h |    2 --
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/iio/dac/ad5446.c b/drivers/staging/iio/dac/ad5446.c
index 9d9a452..693485a 100644
--- a/drivers/staging/iio/dac/ad5446.c
+++ b/drivers/staging/iio/dac/ad5446.c
@@ -176,14 +176,6 @@ static const struct ad5446_chip_info ad5446_chip_info_tbl[] = {
 		.channel = AD5446_CHANNEL(16, 16, 0),
 		.store_sample = ad5542_store_sample,
 	},
-	[ID_AD5542A] = {
-		.channel = AD5446_CHANNEL(16, 16, 0),
-		.store_sample = ad5542_store_sample,
-	},
-	[ID_AD5543] = {
-		.channel = AD5446_CHANNEL(16, 16, 0),
-		.store_sample = ad5542_store_sample,
-	},
 	[ID_AD5512A] = {
 		.channel = AD5446_CHANNEL(12, 16, 4),
 		.store_sample = ad5542_store_sample,
@@ -400,8 +392,8 @@ static const struct spi_device_id ad5446_id[] = {
 	{"ad5446", ID_AD5446},
 	{"ad5512a", ID_AD5512A},
 	{"ad5541a", ID_AD5541A},
-	{"ad5542a", ID_AD5542A},
-	{"ad5543", ID_AD5543},
+	{"ad5542a", ID_AD5541A}, /* ad5541a and ad5542a are compatible */
+	{"ad5543", ID_AD5541A}, /* ad5541a and ad5543 are compatible */
 	{"ad5553", ID_AD5553},
 	{"ad5601", ID_AD5601},
 	{"ad5611", ID_AD5611},
diff --git a/drivers/staging/iio/dac/ad5446.h b/drivers/staging/iio/dac/ad5446.h
index 72ad07f..264df1e 100644
--- a/drivers/staging/iio/dac/ad5446.h
+++ b/drivers/staging/iio/dac/ad5446.h
@@ -83,8 +83,6 @@ enum ad5446_supported_device_ids {
 	ID_AD5444,
 	ID_AD5446,
 	ID_AD5541A,
-	ID_AD5542A,
-	ID_AD5543,
 	ID_AD5512A,
 	ID_AD5553,
 	ID_AD5601,
-- 
1.7.9.5

[PATCH 5/8] staging:iio:dac:ad5446: Remove duplicated write sample functions

[Lars-Peter Clausen]

AD5620_LOAD and AD5446_LOAD are both 0, so all these three functions are
identical and we can replace them with only one.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/dac/ad5446.c |   30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/iio/dac/ad5446.c b/drivers/staging/iio/dac/ad5446.c
index 693485a..6b8f341 100644
--- a/drivers/staging/iio/dac/ad5446.c
+++ b/drivers/staging/iio/dac/ad5446.c
@@ -26,19 +26,9 @@
 
 static void ad5446_store_sample(struct ad5446_state *st, unsigned val)
 {
-	st->data.d16 = cpu_to_be16(AD5446_LOAD | val);
-}
-
-static void ad5542_store_sample(struct ad5446_state *st, unsigned val)
-{
 	st->data.d16 = cpu_to_be16(val);
 }
 
-static void ad5620_store_sample(struct ad5446_state *st, unsigned val)
-{
-	st->data.d16 = cpu_to_be16(AD5620_LOAD | val);
-}
-
 static void ad5660_store_sample(struct ad5446_state *st, unsigned val)
 {
 	val |= AD5660_LOAD;
@@ -174,53 +164,53 @@ static const struct ad5446_chip_info ad5446_chip_info_tbl[] = {
 	},
 	[ID_AD5541A] = {
 		.channel = AD5446_CHANNEL(16, 16, 0),
-		.store_sample = ad5542_store_sample,
+		.store_sample = ad5446_store_sample,
 	},
 	[ID_AD5512A] = {
 		.channel = AD5446_CHANNEL(12, 16, 4),
-		.store_sample = ad5542_store_sample,
+		.store_sample = ad5446_store_sample,
 	},
 	[ID_AD5553] = {
 		.channel = AD5446_CHANNEL(14, 16, 0),
-		.store_sample = ad5542_store_sample,
+		.store_sample = ad5446_store_sample,
 	},
 	[ID_AD5601] = {
 		.channel = AD5446_CHANNEL(8, 16, 6),
-		.store_sample = ad5542_store_sample,
+		.store_sample = ad5446_store_sample,
 		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5611] = {
 		.channel = AD5446_CHANNEL(10, 16, 4),
-		.store_sample = ad5542_store_sample,
+		.store_sample = ad5446_store_sample,
 		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5621] = {
 		.channel = AD5446_CHANNEL(12, 16, 2),
-		.store_sample = ad5542_store_sample,
+		.store_sample = ad5446_store_sample,
 		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5620_2500] = {
 		.channel = AD5446_CHANNEL(12, 16, 2),
 		.int_vref_mv = 2500,
-		.store_sample = ad5620_store_sample,
+		.store_sample = ad5446_store_sample,
 		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5620_1250] = {
 		.channel = AD5446_CHANNEL(12, 16, 2),
 		.int_vref_mv = 1250,
-		.store_sample = ad5620_store_sample,
+		.store_sample = ad5446_store_sample,
 		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5640_2500] = {
 		.channel = AD5446_CHANNEL(14, 16, 0),
 		.int_vref_mv = 2500,
-		.store_sample = ad5620_store_sample,
+		.store_sample = ad5446_store_sample,
 		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5640_1250] = {
 		.channel = AD5446_CHANNEL(14, 16, 0),
 		.int_vref_mv = 1250,
-		.store_sample = ad5620_store_sample,
+		.store_sample = ad5446_store_sample,
 		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5660_2500] = {
-- 
1.7.9.5

[PATCH 6/8] staging:iio:dac:ad5446: Convert to extended channel attributes

[Lars-Peter Clausen]

Use extended channel attributes instead of raw sysfs files for the additional
channel attributes.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/dac/ad5446.c |  141 +++++++++++++++++++-------------------
 1 file changed, 70 insertions(+), 71 deletions(-)

diff --git a/drivers/staging/iio/dac/ad5446.c b/drivers/staging/iio/dac/ad5446.c
index 6b8f341..c767024 100644
--- a/drivers/staging/iio/dac/ad5446.c
+++ b/drivers/staging/iio/dac/ad5446.c
@@ -51,64 +51,69 @@ static void ad5660_store_pwr_down(struct ad5446_state *st, unsigned mode)
 	st->data.d24[2] = val & 0xFF;
 }
 
-static ssize_t ad5446_write_powerdown_mode(struct device *dev,
-				       struct device_attribute *attr,
-				       const char *buf, size_t len)
+static const char * const ad5446_powerdown_modes[] = {
+	"", "1kohm_to_gnd", "100kohm_to_gnd", "three_state"
+};
+
+static ssize_t ad5446_read_powerdown_mode_available(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, char *buf)
+{
+	return sprintf(buf, "%s %s %s\n", ad5446_powerdown_modes[1],
+		ad5446_powerdown_modes[2], ad5446_powerdown_modes[3]);
+}
+
+static ssize_t ad5446_write_powerdown_mode(struct iio_dev *indio_dev,
+					    const struct iio_chan_spec *chan,
+					    const char *buf, size_t len)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5446_state *st = iio_priv(indio_dev);
+	int i;
 
-	if (sysfs_streq(buf, "1kohm_to_gnd"))
-		st->pwr_down_mode = MODE_PWRDWN_1k;
-	else if (sysfs_streq(buf, "100kohm_to_gnd"))
-		st->pwr_down_mode = MODE_PWRDWN_100k;
-	else if (sysfs_streq(buf, "three_state"))
-		st->pwr_down_mode = MODE_PWRDWN_TRISTATE;
-	else
+	for (i = 1; i < ARRAY_SIZE(ad5446_powerdown_modes); i++) {
+		if (sysfs_streq(buf, ad5446_powerdown_modes[i])) {
+			st->pwr_down_mode = i;
+			break;
+		}
+	}
+
+	if (i == ARRAY_SIZE(ad5446_powerdown_modes))
 		return -EINVAL;
 
 	return len;
 }
 
-static ssize_t ad5446_read_powerdown_mode(struct device *dev,
-				      struct device_attribute *attr, char *buf)
+static ssize_t ad5446_read_powerdown_mode(struct iio_dev *indio_dev,
+					   const struct iio_chan_spec *chan,
+					   char *buf)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5446_state *st = iio_priv(indio_dev);
 
-	char mode[][15] = {"", "1kohm_to_gnd", "100kohm_to_gnd", "three_state"};
-
-	return sprintf(buf, "%s\n", mode[st->pwr_down_mode]);
+	return sprintf(buf, "%s\n", ad5446_powerdown_modes[st->pwr_down_mode]);
 }
 
-static ssize_t ad5446_read_dac_powerdown(struct device *dev,
-					   struct device_attribute *attr,
+static ssize_t ad5446_read_dac_powerdown(struct iio_dev *indio_dev,
+					   const struct iio_chan_spec *chan,
 					   char *buf)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5446_state *st = iio_priv(indio_dev);
 
 	return sprintf(buf, "%d\n", st->pwr_down);
 }
 
-static ssize_t ad5446_write_dac_powerdown(struct device *dev,
-					    struct device_attribute *attr,
+static ssize_t ad5446_write_dac_powerdown(struct iio_dev *indio_dev,
+					    const struct iio_chan_spec *chan,
 					    const char *buf, size_t len)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5446_state *st = iio_priv(indio_dev);
-	unsigned long readin;
+	bool powerdown;
 	int ret;
 
-	ret = strict_strtol(buf, 10, &readin);
+	ret = strtobool(buf, &powerdown);
 	if (ret)
 		return ret;
 
-	if (readin > 1)
-		ret = -EINVAL;
-
 	mutex_lock(&indio_dev->mlock);
-	st->pwr_down = readin;
+	st->pwr_down = powerdown;
 
 	if (st->pwr_down)
 		st->chip_info->store_pwr_down(st, st->pwr_down_mode);
@@ -121,38 +126,42 @@ static ssize_t ad5446_write_dac_powerdown(struct device *dev,
 	return ret ? ret : len;
 }
 
-static IIO_DEVICE_ATTR(out_voltage_powerdown_mode, S_IRUGO | S_IWUSR,
-			ad5446_read_powerdown_mode,
-			ad5446_write_powerdown_mode, 0);
-
-static IIO_CONST_ATTR(out_voltage_powerdown_mode_available,
-			"1kohm_to_gnd 100kohm_to_gnd three_state");
-
-static IIO_DEVICE_ATTR(out_voltage0_powerdown, S_IRUGO | S_IWUSR,
-			ad5446_read_dac_powerdown,
-			ad5446_write_dac_powerdown, 0);
-
-static struct attribute *ad5446_attributes[] = {
-	&iio_dev_attr_out_voltage0_powerdown.dev_attr.attr,
-	&iio_dev_attr_out_voltage_powerdown_mode.dev_attr.attr,
-	&iio_const_attr_out_voltage_powerdown_mode_available.dev_attr.attr,
-	NULL,
-};
-
-static const struct attribute_group ad5446_attribute_group = {
-	.attrs = ad5446_attributes,
+static const struct iio_chan_spec_ext_info ad5064_ext_info_powerdown[] = {
+	{
+		.name = "powerdown",
+		.read = ad5446_read_dac_powerdown,
+		.write = ad5446_write_dac_powerdown,
+	},
+	{
+		.name = "powerdown_mode",
+		.read = ad5446_read_powerdown_mode,
+		.write = ad5446_write_powerdown_mode,
+	},
+	{
+		.name = "powerdown_mode_available",
+		.shared = true,
+		.read = ad5446_read_powerdown_mode_available,
+	},
+	{ },
 };
 
-#define AD5446_CHANNEL(bits, storage, shift) { \
+#define _AD5446_CHANNEL(bits, storage, shift, ext) { \
 	.type = IIO_VOLTAGE, \
 	.indexed = 1, \
 	.output = 1, \
 	.channel = 0, \
 	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT | \
 	IIO_CHAN_INFO_SCALE_SHARED_BIT,	\
-	.scan_type = IIO_ST('u', (bits), (storage), (shift)) \
+	.scan_type = IIO_ST('u', (bits), (storage), (shift)), \
+	.ext_info = (ext), \
 }
 
+#define AD5446_CHANNEL(bits, storage, shift) \
+	_AD5446_CHANNEL(bits, storage, shift, NULL)
+
+#define AD5446_CHANNEL_POWERDOWN(bits, storage, shift) \
+	_AD5446_CHANNEL(bits, storage, shift, ad5064_ext_info_powerdown)
+
 static const struct ad5446_chip_info ad5446_chip_info_tbl[] = {
 	[ID_AD5444] = {
 		.channel = AD5446_CHANNEL(12, 16, 2),
@@ -175,52 +184,52 @@ static const struct ad5446_chip_info ad5446_chip_info_tbl[] = {
 		.store_sample = ad5446_store_sample,
 	},
 	[ID_AD5601] = {
-		.channel = AD5446_CHANNEL(8, 16, 6),
+		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
 		.store_sample = ad5446_store_sample,
 		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5611] = {
-		.channel = AD5446_CHANNEL(10, 16, 4),
+		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
 		.store_sample = ad5446_store_sample,
 		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5621] = {
-		.channel = AD5446_CHANNEL(12, 16, 2),
+		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
 		.store_sample = ad5446_store_sample,
 		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5620_2500] = {
-		.channel = AD5446_CHANNEL(12, 16, 2),
+		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
 		.int_vref_mv = 2500,
 		.store_sample = ad5446_store_sample,
 		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5620_1250] = {
-		.channel = AD5446_CHANNEL(12, 16, 2),
+		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
 		.int_vref_mv = 1250,
 		.store_sample = ad5446_store_sample,
 		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5640_2500] = {
-		.channel = AD5446_CHANNEL(14, 16, 0),
+		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
 		.int_vref_mv = 2500,
 		.store_sample = ad5446_store_sample,
 		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5640_1250] = {
-		.channel = AD5446_CHANNEL(14, 16, 0),
+		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
 		.int_vref_mv = 1250,
 		.store_sample = ad5446_store_sample,
 		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5660_2500] = {
-		.channel = AD5446_CHANNEL(16, 16, 0),
+		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
 		.int_vref_mv = 2500,
 		.store_sample = ad5660_store_sample,
 		.store_pwr_down = ad5660_store_pwr_down,
 	},
 	[ID_AD5660_1250] = {
-		.channel = AD5446_CHANNEL(16, 16, 0),
+		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
 		.int_vref_mv = 1250,
 		.store_sample = ad5660_store_sample,
 		.store_pwr_down = ad5660_store_pwr_down,
@@ -279,13 +288,6 @@ static int ad5446_write_raw(struct iio_dev *indio_dev,
 static const struct iio_info ad5446_info = {
 	.read_raw = ad5446_read_raw,
 	.write_raw = ad5446_write_raw,
-	.attrs = &ad5446_attribute_group,
-	.driver_module = THIS_MODULE,
-};
-
-static const struct iio_info ad5446_info_no_pwr_down = {
-	.read_raw = ad5446_read_raw,
-	.write_raw = ad5446_write_raw,
 	.driver_module = THIS_MODULE,
 };
 
@@ -321,10 +323,7 @@ static int __devinit ad5446_probe(struct spi_device *spi)
 	/* Establish that the iio_dev is a child of the spi device */
 	indio_dev->dev.parent = &spi->dev;
 	indio_dev->name = spi_get_device_id(spi)->name;
-	if (st->chip_info->store_pwr_down)
-		indio_dev->info = &ad5446_info;
-	else
-		indio_dev->info = &ad5446_info_no_pwr_down;
+	indio_dev->info = &ad5446_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = &st->chip_info->channel;
 	indio_dev->num_channels = 1;
-- 
1.7.9.5

[PATCH 7/8] staging:iio:dac:ad5446: Consolidate store_sample and store_pwr_down functions

[Lars-Peter Clausen]

The devices supported by this drivers only have a single shift register, which
contains both the power down mode and the output sample. So writing the power
down mode and the output sample can be done by the same function. The two power
down bits are always placed ontop of the msb of the output sample, so we can
easily calculate their position by adding the channels shift to the channels
realbits.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/dac/ad5446.c |   38 ++++++++++----------------------------
 1 file changed, 10 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/iio/dac/ad5446.c b/drivers/staging/iio/dac/ad5446.c
index c767024..90461b2 100644
--- a/drivers/staging/iio/dac/ad5446.c
+++ b/drivers/staging/iio/dac/ad5446.c
@@ -31,21 +31,6 @@ static void ad5446_store_sample(struct ad5446_state *st, unsigned val)
 
 static void ad5660_store_sample(struct ad5446_state *st, unsigned val)
 {
-	val |= AD5660_LOAD;
-	st->data.d24[0] = (val >> 16) & 0xFF;
-	st->data.d24[1] = (val >> 8) & 0xFF;
-	st->data.d24[2] = val & 0xFF;
-}
-
-static void ad5620_store_pwr_down(struct ad5446_state *st, unsigned mode)
-{
-	st->data.d16 = cpu_to_be16(mode << 14);
-}
-
-static void ad5660_store_pwr_down(struct ad5446_state *st, unsigned mode)
-{
-	unsigned val = mode << 16;
-
 	st->data.d24[0] = (val >> 16) & 0xFF;
 	st->data.d24[1] = (val >> 8) & 0xFF;
 	st->data.d24[2] = val & 0xFF;
@@ -105,6 +90,8 @@ static ssize_t ad5446_write_dac_powerdown(struct iio_dev *indio_dev,
 					    const char *buf, size_t len)
 {
 	struct ad5446_state *st = iio_priv(indio_dev);
+	unsigned int shift;
+	unsigned int val;
 	bool powerdown;
 	int ret;
 
@@ -115,10 +102,14 @@ static ssize_t ad5446_write_dac_powerdown(struct iio_dev *indio_dev,
 	mutex_lock(&indio_dev->mlock);
 	st->pwr_down = powerdown;
 
-	if (st->pwr_down)
-		st->chip_info->store_pwr_down(st, st->pwr_down_mode);
-	else
-		st->chip_info->store_sample(st, st->cached_val);
+	if (st->pwr_down) {
+		shift = chan->scan_type.realbits + chan->scan_type.shift;
+		val = st->pwr_down_mode << shift;
+	} else {
+		val = st->cached_val;
+	}
+
+	st->chip_info->store_sample(st, val);
 
 	ret = spi_sync(st->spi, &st->msg);
 	mutex_unlock(&indio_dev->mlock);
@@ -186,53 +177,44 @@ static const struct ad5446_chip_info ad5446_chip_info_tbl[] = {
 	[ID_AD5601] = {
 		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
 		.store_sample = ad5446_store_sample,
-		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5611] = {
 		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
 		.store_sample = ad5446_store_sample,
-		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5621] = {
 		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
 		.store_sample = ad5446_store_sample,
-		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5620_2500] = {
 		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
 		.int_vref_mv = 2500,
 		.store_sample = ad5446_store_sample,
-		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5620_1250] = {
 		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
 		.int_vref_mv = 1250,
 		.store_sample = ad5446_store_sample,
-		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5640_2500] = {
 		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
 		.int_vref_mv = 2500,
 		.store_sample = ad5446_store_sample,
-		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5640_1250] = {
 		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
 		.int_vref_mv = 1250,
 		.store_sample = ad5446_store_sample,
-		.store_pwr_down = ad5620_store_pwr_down,
 	},
 	[ID_AD5660_2500] = {
 		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
 		.int_vref_mv = 2500,
 		.store_sample = ad5660_store_sample,
-		.store_pwr_down = ad5660_store_pwr_down,
 	},
 	[ID_AD5660_1250] = {
 		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
 		.int_vref_mv = 1250,
 		.store_sample = ad5660_store_sample,
-		.store_pwr_down = ad5660_store_pwr_down,
 	},
 };
 
-- 
1.7.9.5

[PATCH 8/8] staging:iio:dac:ad5446: Add support for the AD5662

[Lars-Peter Clausen]

The AD5662 is compatible to the AD5660, but uses an external reference instead
of an internal.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/dac/Kconfig  |    2 +-
 drivers/staging/iio/dac/ad5446.c |    5 +++++
 drivers/staging/iio/dac/ad5446.h |    1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/iio/dac/Kconfig b/drivers/staging/iio/dac/Kconfig
index a57803a..1a2be98 100644
--- a/drivers/staging/iio/dac/Kconfig
+++ b/drivers/staging/iio/dac/Kconfig
@@ -56,7 +56,7 @@ config AD5624R_SPI
 	  AD5664R converters (DAC). This driver uses the common SPI interface.
 
 config AD5446
-	tristate "Analog Devices AD5444/6, AD5620/40/60 and AD5542A/12A DAC SPI driver"
+	tristate "Analog Devices AD5444/6, AD5620/40/60/62 and AD5542A/12A DAC SPI driver"
 	depends on SPI
 	help
 	  Say yes here to build support for Analog Devices AD5444, AD5446,
diff --git a/drivers/staging/iio/dac/ad5446.c b/drivers/staging/iio/dac/ad5446.c
index 90461b2..bcb849b 100644
--- a/drivers/staging/iio/dac/ad5446.c
+++ b/drivers/staging/iio/dac/ad5446.c
@@ -216,6 +216,10 @@ static const struct ad5446_chip_info ad5446_chip_info_tbl[] = {
 		.int_vref_mv = 1250,
 		.store_sample = ad5660_store_sample,
 	},
+	[ID_AD5662] = {
+		.channel = AD5446_CHANNEL(16, 16, 0),
+		.store_sample = ad5660_store_sample,
+	},
 };
 
 static int ad5446_read_raw(struct iio_dev *indio_dev,
@@ -375,6 +379,7 @@ static const struct spi_device_id ad5446_id[] = {
 	{"ad5640-1250", ID_AD5640_1250},
 	{"ad5660-2500", ID_AD5660_2500},
 	{"ad5660-1250", ID_AD5660_1250},
+	{"ad5662", ID_AD5662},
 	{}
 };
 MODULE_DEVICE_TABLE(spi, ad5446_id);
diff --git a/drivers/staging/iio/dac/ad5446.h b/drivers/staging/iio/dac/ad5446.h
index 264df1e..071ce39 100644
--- a/drivers/staging/iio/dac/ad5446.h
+++ b/drivers/staging/iio/dac/ad5446.h
@@ -94,6 +94,7 @@ enum ad5446_supported_device_ids {
 	ID_AD5640_1250,
 	ID_AD5660_2500,
 	ID_AD5660_1250,
+	ID_AD5662,
 };
 
 #endif /* IIO_DAC_AD5446_H_ */
-- 
1.7.9.5

Re: [PATCH 1/8] staging:iio:dac:ad5446: Do not exit powerdown when writing a sample

[Jonathan Cameron]

On 04/23/2012 06:51 PM, Lars-Peter Clausen wrote:
> Both the powerdown mode bits and the sample value are stored in the same
> register, so writing a sample while the device is powered down will clear the
> power down bits. To avoid this only update the cached value when the device is
> powered down.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/staging/iio/dac/ad5446.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/dac/ad5446.c b/drivers/staging/iio/dac/ad5446.c
> index ec6968b..0cfe445 100644
> --- a/drivers/staging/iio/dac/ad5446.c
> +++ b/drivers/staging/iio/dac/ad5446.c
> @@ -282,7 +282,8 @@ static int ad5446_write_raw(struct iio_dev *indio_dev,
>  		val <<= chan->scan_type.shift;
>  		mutex_lock(&indio_dev->mlock);
>  		st->cached_val = val;
> -		st->chip_info->store_sample(st, val);
> +		if (!st->pwr_down)
> +			st->chip_info->store_sample(st, val);
>  		ret = spi_sync(st->spi, &st->msg);
>  		mutex_unlock(&indio_dev->mlock);
>  		break;

Re: [PATCH 2/8] staging:iio:dac:ad5446: Remove unused struct field

[Jonathan Cameron]

On 04/23/2012 06:51 PM, Lars-Peter Clausen wrote:
> Remove the unused "poll_work" field from the ad5446_state struct.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/staging/iio/dac/ad5446.h |    2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/dac/ad5446.h b/drivers/staging/iio/dac/ad5446.h
> index 4ea3476..72ad07f 100644
> --- a/drivers/staging/iio/dac/ad5446.h
> +++ b/drivers/staging/iio/dac/ad5446.h
> @@ -34,7 +34,6 @@
>   * @spi:		spi_device
>   * @chip_info:		chip model specific constants, available modes etc
>   * @reg:		supply regulator
> - * @poll_work:		bottom half of polling interrupt handler
>   * @vref_mv:		actual reference voltage used
>   * @xfer:		default spi transfer
>   * @msg:		default spi message
> @@ -45,7 +44,6 @@ struct ad5446_state {
>  	struct spi_device		*spi;
>  	const struct ad5446_chip_info	*chip_info;
>  	struct regulator		*reg;
> -	struct work_struct		poll_work;
>  	unsigned short			vref_mv;
>  	unsigned			cached_val;
>  	unsigned			pwr_down_mode;

Re: [PATCH 3/8] staging:iio:dac:ad5446: Do not check for individual chip ids in probe

[Jonathan Cameron]

On 04/23/2012 06:51 PM, Lars-Peter Clausen wrote:
> Use the chip_info's int_vref_mv field to decide whether a certain chip has a
> internal reference or not. There is no need to check for individual chip ids.
> 
That does indeed do the job.
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/staging/iio/dac/ad5446.c |   20 +++++---------------
>  1 file changed, 5 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/staging/iio/dac/ad5446.c b/drivers/staging/iio/dac/ad5446.c
> index 0cfe445..9d9a452 100644
> --- a/drivers/staging/iio/dac/ad5446.c
> +++ b/drivers/staging/iio/dac/ad5446.c
> @@ -355,22 +355,12 @@ static int __devinit ad5446_probe(struct spi_device *spi)
>  	spi_message_init(&st->msg);
>  	spi_message_add_tail(&st->xfer, &st->msg);
>  
> -	switch (spi_get_device_id(spi)->driver_data) {
> -	case ID_AD5620_2500:
> -	case ID_AD5620_1250:
> -	case ID_AD5640_2500:
> -	case ID_AD5640_1250:
> -	case ID_AD5660_2500:
> -	case ID_AD5660_1250:
> +	if (st->chip_info->int_vref_mv)
>  		st->vref_mv = st->chip_info->int_vref_mv;
> -		break;
> -	default:
> -		if (voltage_uv)
> -			st->vref_mv = voltage_uv / 1000;
> -		else
> -			dev_warn(&spi->dev,
> -				 "reference voltage unspecified\n");
> -	}
> +	else if (voltage_uv)
> +		st->vref_mv = voltage_uv / 1000;
> +	else
> +		dev_warn(&spi->dev, "reference voltage unspecified\n");
>  
>  	ret = iio_device_register(indio_dev);
>  	if (ret)

Re: [PATCH 4/8] staging:iio:dac:ad5446: Remove duplicated chip_info entries

[Jonathan Cameron]

On 04/23/2012 06:51 PM, Lars-Peter Clausen wrote:
> There are three identical chip_info entries. Remove two of them and use the id
> of the remaining entry for all three device table entries.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/staging/iio/dac/ad5446.c |   12 ++----------
>  drivers/staging/iio/dac/ad5446.h |    2 --
>  2 files changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/iio/dac/ad5446.c b/drivers/staging/iio/dac/ad5446.c
> index 9d9a452..693485a 100644
> --- a/drivers/staging/iio/dac/ad5446.c
> +++ b/drivers/staging/iio/dac/ad5446.c
> @@ -176,14 +176,6 @@ static const struct ad5446_chip_info ad5446_chip_info_tbl[] = {
>  		.channel = AD5446_CHANNEL(16, 16, 0),
>  		.store_sample = ad5542_store_sample,
>  	},
> -	[ID_AD5542A] = {
> -		.channel = AD5446_CHANNEL(16, 16, 0),
> -		.store_sample = ad5542_store_sample,
> -	},
> -	[ID_AD5543] = {
> -		.channel = AD5446_CHANNEL(16, 16, 0),
> -		.store_sample = ad5542_store_sample,
> -	},
>  	[ID_AD5512A] = {
>  		.channel = AD5446_CHANNEL(12, 16, 4),
>  		.store_sample = ad5542_store_sample,
> @@ -400,8 +392,8 @@ static const struct spi_device_id ad5446_id[] = {
>  	{"ad5446", ID_AD5446},
>  	{"ad5512a", ID_AD5512A},
>  	{"ad5541a", ID_AD5541A},
> -	{"ad5542a", ID_AD5542A},
> -	{"ad5543", ID_AD5543},
> +	{"ad5542a", ID_AD5541A}, /* ad5541a and ad5542a are compatible */
> +	{"ad5543", ID_AD5541A}, /* ad5541a and ad5543 are compatible */
>  	{"ad5553", ID_AD5553},
>  	{"ad5601", ID_AD5601},
>  	{"ad5611", ID_AD5611},
> diff --git a/drivers/staging/iio/dac/ad5446.h b/drivers/staging/iio/dac/ad5446.h
> index 72ad07f..264df1e 100644
> --- a/drivers/staging/iio/dac/ad5446.h
> +++ b/drivers/staging/iio/dac/ad5446.h
> @@ -83,8 +83,6 @@ enum ad5446_supported_device_ids {
>  	ID_AD5444,
>  	ID_AD5446,
>  	ID_AD5541A,
> -	ID_AD5542A,
> -	ID_AD5543,
>  	ID_AD5512A,
>  	ID_AD5553,
>  	ID_AD5601,

Re: [PATCH 5/8] staging:iio:dac:ad5446: Remove duplicated write sample functions

[Jonathan Cameron]

On 04/23/2012 06:51 PM, Lars-Peter Clausen wrote:
> AD5620_LOAD and AD5446_LOAD are both 0, so all these three functions are
> identical and we can replace them with only one.
Nice spot.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/staging/iio/dac/ad5446.c |   30 ++++++++++--------------------
>  1 file changed, 10 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/staging/iio/dac/ad5446.c b/drivers/staging/iio/dac/ad5446.c
> index 693485a..6b8f341 100644
> --- a/drivers/staging/iio/dac/ad5446.c
> +++ b/drivers/staging/iio/dac/ad5446.c
> @@ -26,19 +26,9 @@
>  
>  static void ad5446_store_sample(struct ad5446_state *st, unsigned val)
>  {
> -	st->data.d16 = cpu_to_be16(AD5446_LOAD | val);
> -}
> -
> -static void ad5542_store_sample(struct ad5446_state *st, unsigned val)
> -{
>  	st->data.d16 = cpu_to_be16(val);
>  }
>  
> -static void ad5620_store_sample(struct ad5446_state *st, unsigned val)
> -{
> -	st->data.d16 = cpu_to_be16(AD5620_LOAD | val);
> -}
> -
>  static void ad5660_store_sample(struct ad5446_state *st, unsigned val)
>  {
>  	val |= AD5660_LOAD;
> @@ -174,53 +164,53 @@ static const struct ad5446_chip_info ad5446_chip_info_tbl[] = {
>  	},
>  	[ID_AD5541A] = {
>  		.channel = AD5446_CHANNEL(16, 16, 0),
> -		.store_sample = ad5542_store_sample,
> +		.store_sample = ad5446_store_sample,
>  	},
>  	[ID_AD5512A] = {
>  		.channel = AD5446_CHANNEL(12, 16, 4),
> -		.store_sample = ad5542_store_sample,
> +		.store_sample = ad5446_store_sample,
>  	},
>  	[ID_AD5553] = {
>  		.channel = AD5446_CHANNEL(14, 16, 0),
> -		.store_sample = ad5542_store_sample,
> +		.store_sample = ad5446_store_sample,
>  	},
>  	[ID_AD5601] = {
>  		.channel = AD5446_CHANNEL(8, 16, 6),
> -		.store_sample = ad5542_store_sample,
> +		.store_sample = ad5446_store_sample,
>  		.store_pwr_down = ad5620_store_pwr_down,
>  	},
>  	[ID_AD5611] = {
>  		.channel = AD5446_CHANNEL(10, 16, 4),
> -		.store_sample = ad5542_store_sample,
> +		.store_sample = ad5446_store_sample,
>  		.store_pwr_down = ad5620_store_pwr_down,
>  	},
>  	[ID_AD5621] = {
>  		.channel = AD5446_CHANNEL(12, 16, 2),
> -		.store_sample = ad5542_store_sample,
> +		.store_sample = ad5446_store_sample,
>  		.store_pwr_down = ad5620_store_pwr_down,
>  	},
>  	[ID_AD5620_2500] = {
>  		.channel = AD5446_CHANNEL(12, 16, 2),
>  		.int_vref_mv = 2500,
> -		.store_sample = ad5620_store_sample,
> +		.store_sample = ad5446_store_sample,
>  		.store_pwr_down = ad5620_store_pwr_down,
>  	},
>  	[ID_AD5620_1250] = {
>  		.channel = AD5446_CHANNEL(12, 16, 2),
>  		.int_vref_mv = 1250,
> -		.store_sample = ad5620_store_sample,
> +		.store_sample = ad5446_store_sample,
>  		.store_pwr_down = ad5620_store_pwr_down,
>  	},
>  	[ID_AD5640_2500] = {
>  		.channel = AD5446_CHANNEL(14, 16, 0),
>  		.int_vref_mv = 2500,
> -		.store_sample = ad5620_store_sample,
> +		.store_sample = ad5446_store_sample,
>  		.store_pwr_down = ad5620_store_pwr_down,
>  	},
>  	[ID_AD5640_1250] = {
>  		.channel = AD5446_CHANNEL(14, 16, 0),
>  		.int_vref_mv = 1250,
> -		.store_sample = ad5620_store_sample,
> +		.store_sample = ad5446_store_sample,
>  		.store_pwr_down = ad5620_store_pwr_down,
>  	},
>  	[ID_AD5660_2500] = {

Re: [PATCH 6/8] staging:iio:dac:ad5446: Convert to extended channel attributes

[Jonathan Cameron]

On 04/23/2012 06:51 PM, Lars-Peter Clausen wrote:
> Use extended channel attributes instead of raw sysfs files for the additional
> channel attributes.
One formatting suggestion.  I am a little unconvinced that we gain much
from using the extended channel attributes here, but otherwise fine.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/staging/iio/dac/ad5446.c |  141 +++++++++++++++++++-------------------
>  1 file changed, 70 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/staging/iio/dac/ad5446.c b/drivers/staging/iio/dac/ad5446.c
> index 6b8f341..c767024 100644
> --- a/drivers/staging/iio/dac/ad5446.c
> +++ b/drivers/staging/iio/dac/ad5446.c
> @@ -51,64 +51,69 @@ static void ad5660_store_pwr_down(struct ad5446_state *st, unsigned mode)
>  	st->data.d24[2] = val & 0xFF;
>  }
>  
> -static ssize_t ad5446_write_powerdown_mode(struct device *dev,
> -				       struct device_attribute *attr,
> -				       const char *buf, size_t len)
> +static const char * const ad5446_powerdown_modes[] = {
> +	"", "1kohm_to_gnd", "100kohm_to_gnd", "three_state"
> +};
> +
> +static ssize_t ad5446_read_powerdown_mode_available(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, char *buf)
> +{
> +	return sprintf(buf, "%s %s %s\n", ad5446_powerdown_modes[1],
> +		ad5446_powerdown_modes[2], ad5446_powerdown_modes[3]);
> +}
> +
> +static ssize_t ad5446_write_powerdown_mode(struct iio_dev *indio_dev,
> +					    const struct iio_chan_spec *chan,
> +					    const char *buf, size_t len)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5446_state *st = iio_priv(indio_dev);
> +	int i;
>  
> -	if (sysfs_streq(buf, "1kohm_to_gnd"))
> -		st->pwr_down_mode = MODE_PWRDWN_1k;
> -	else if (sysfs_streq(buf, "100kohm_to_gnd"))
> -		st->pwr_down_mode = MODE_PWRDWN_100k;
> -	else if (sysfs_streq(buf, "three_state"))
> -		st->pwr_down_mode = MODE_PWRDWN_TRISTATE;
> -	else
> +	for (i = 1; i < ARRAY_SIZE(ad5446_powerdown_modes); i++) {
> +		if (sysfs_streq(buf, ad5446_powerdown_modes[i])) {
> +			st->pwr_down_mode = i;
> +			break;
> +		}
> +	}
> +
> +	if (i == ARRAY_SIZE(ad5446_powerdown_modes))
>  		return -EINVAL;
>  
>  	return len;
>  }
>  
> -static ssize_t ad5446_read_powerdown_mode(struct device *dev,
> -				      struct device_attribute *attr, char *buf)
> +static ssize_t ad5446_read_powerdown_mode(struct iio_dev *indio_dev,
> +					   const struct iio_chan_spec *chan,
> +					   char *buf)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5446_state *st = iio_priv(indio_dev);
>  
> -	char mode[][15] = {"", "1kohm_to_gnd", "100kohm_to_gnd", "three_state"};
> -
> -	return sprintf(buf, "%s\n", mode[st->pwr_down_mode]);
> +	return sprintf(buf, "%s\n", ad5446_powerdown_modes[st->pwr_down_mode]);
>  }
>  
> -static ssize_t ad5446_read_dac_powerdown(struct device *dev,
> -					   struct device_attribute *attr,
> +static ssize_t ad5446_read_dac_powerdown(struct iio_dev *indio_dev,
> +					   const struct iio_chan_spec *chan,
>  					   char *buf)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5446_state *st = iio_priv(indio_dev);
>  
>  	return sprintf(buf, "%d\n", st->pwr_down);
>  }
>  
> -static ssize_t ad5446_write_dac_powerdown(struct device *dev,
> -					    struct device_attribute *attr,
> +static ssize_t ad5446_write_dac_powerdown(struct iio_dev *indio_dev,
> +					    const struct iio_chan_spec *chan,
>  					    const char *buf, size_t len)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5446_state *st = iio_priv(indio_dev);
> -	unsigned long readin;
> +	bool powerdown;
>  	int ret;
>  
> -	ret = strict_strtol(buf, 10, &readin);
> +	ret = strtobool(buf, &powerdown);
>  	if (ret)
>  		return ret;
>  
> -	if (readin > 1)
> -		ret = -EINVAL;
> -
>  	mutex_lock(&indio_dev->mlock);
> -	st->pwr_down = readin;
> +	st->pwr_down = powerdown;
>  
>  	if (st->pwr_down)
>  		st->chip_info->store_pwr_down(st, st->pwr_down_mode);
> @@ -121,38 +126,42 @@ static ssize_t ad5446_write_dac_powerdown(struct device *dev,
>  	return ret ? ret : len;
>  }
>  
> -static IIO_DEVICE_ATTR(out_voltage_powerdown_mode, S_IRUGO | S_IWUSR,
> -			ad5446_read_powerdown_mode,
> -			ad5446_write_powerdown_mode, 0);
> -
> -static IIO_CONST_ATTR(out_voltage_powerdown_mode_available,
> -			"1kohm_to_gnd 100kohm_to_gnd three_state");
> -
> -static IIO_DEVICE_ATTR(out_voltage0_powerdown, S_IRUGO | S_IWUSR,
> -			ad5446_read_dac_powerdown,
> -			ad5446_write_dac_powerdown, 0);
> -
> -static struct attribute *ad5446_attributes[] = {
> -	&iio_dev_attr_out_voltage0_powerdown.dev_attr.attr,
> -	&iio_dev_attr_out_voltage_powerdown_mode.dev_attr.attr,
> -	&iio_const_attr_out_voltage_powerdown_mode_available.dev_attr.attr,
> -	NULL,
> -};
> -
> -static const struct attribute_group ad5446_attribute_group = {
> -	.attrs = ad5446_attributes,
> +static const struct iio_chan_spec_ext_info ad5064_ext_info_powerdown[] = {
> +	{
> +		.name = "powerdown",
> +		.read = ad5446_read_dac_powerdown,
> +		.write = ad5446_write_dac_powerdown,
> +	},
}, { is preferred I think
> +	{
> +		.name = "powerdown_mode",
> +		.read = ad5446_read_powerdown_mode,
> +		.write = ad5446_write_powerdown_mode,
> +	},
> +	{
> +		.name = "powerdown_mode_available",
> +		.shared = true,
> +		.read = ad5446_read_powerdown_mode_available,
> +	},
> +	{ },
>  };
>  
> -#define AD5446_CHANNEL(bits, storage, shift) { \
> +#define _AD5446_CHANNEL(bits, storage, shift, ext) { \
>  	.type = IIO_VOLTAGE, \
>  	.indexed = 1, \
>  	.output = 1, \
>  	.channel = 0, \
>  	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT | \
>  	IIO_CHAN_INFO_SCALE_SHARED_BIT,	\
> -	.scan_type = IIO_ST('u', (bits), (storage), (shift)) \
> +	.scan_type = IIO_ST('u', (bits), (storage), (shift)), \
> +	.ext_info = (ext), \
>  }
>  
> +#define AD5446_CHANNEL(bits, storage, shift) \
> +	_AD5446_CHANNEL(bits, storage, shift, NULL)
> +
> +#define AD5446_CHANNEL_POWERDOWN(bits, storage, shift) \
> +	_AD5446_CHANNEL(bits, storage, shift, ad5064_ext_info_powerdown)
> +
>  static const struct ad5446_chip_info ad5446_chip_info_tbl[] = {
>  	[ID_AD5444] = {
>  		.channel = AD5446_CHANNEL(12, 16, 2),
> @@ -175,52 +184,52 @@ static const struct ad5446_chip_info ad5446_chip_info_tbl[] = {
>  		.store_sample = ad5446_store_sample,
>  	},
>  	[ID_AD5601] = {
> -		.channel = AD5446_CHANNEL(8, 16, 6),
> +		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
>  		.store_sample = ad5446_store_sample,
>  		.store_pwr_down = ad5620_store_pwr_down,
>  	},
>  	[ID_AD5611] = {
> -		.channel = AD5446_CHANNEL(10, 16, 4),
> +		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
>  		.store_sample = ad5446_store_sample,
>  		.store_pwr_down = ad5620_store_pwr_down,
>  	},
>  	[ID_AD5621] = {
> -		.channel = AD5446_CHANNEL(12, 16, 2),
> +		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>  		.store_sample = ad5446_store_sample,
>  		.store_pwr_down = ad5620_store_pwr_down,
>  	},
>  	[ID_AD5620_2500] = {
> -		.channel = AD5446_CHANNEL(12, 16, 2),
> +		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>  		.int_vref_mv = 2500,
>  		.store_sample = ad5446_store_sample,
>  		.store_pwr_down = ad5620_store_pwr_down,
>  	},
>  	[ID_AD5620_1250] = {
> -		.channel = AD5446_CHANNEL(12, 16, 2),
> +		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>  		.int_vref_mv = 1250,
>  		.store_sample = ad5446_store_sample,
>  		.store_pwr_down = ad5620_store_pwr_down,
>  	},
>  	[ID_AD5640_2500] = {
> -		.channel = AD5446_CHANNEL(14, 16, 0),
> +		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
>  		.int_vref_mv = 2500,
>  		.store_sample = ad5446_store_sample,
>  		.store_pwr_down = ad5620_store_pwr_down,
>  	},
>  	[ID_AD5640_1250] = {
> -		.channel = AD5446_CHANNEL(14, 16, 0),
> +		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
>  		.int_vref_mv = 1250,
>  		.store_sample = ad5446_store_sample,
>  		.store_pwr_down = ad5620_store_pwr_down,
>  	},
>  	[ID_AD5660_2500] = {
> -		.channel = AD5446_CHANNEL(16, 16, 0),
> +		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>  		.int_vref_mv = 2500,
>  		.store_sample = ad5660_store_sample,
>  		.store_pwr_down = ad5660_store_pwr_down,
>  	},
>  	[ID_AD5660_1250] = {
> -		.channel = AD5446_CHANNEL(16, 16, 0),
> +		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>  		.int_vref_mv = 1250,
>  		.store_sample = ad5660_store_sample,
>  		.store_pwr_down = ad5660_store_pwr_down,
> @@ -279,13 +288,6 @@ static int ad5446_write_raw(struct iio_dev *indio_dev,
>  static const struct iio_info ad5446_info = {
>  	.read_raw = ad5446_read_raw,
>  	.write_raw = ad5446_write_raw,
> -	.attrs = &ad5446_attribute_group,
> -	.driver_module = THIS_MODULE,
> -};
> -
> -static const struct iio_info ad5446_info_no_pwr_down = {
> -	.read_raw = ad5446_read_raw,
> -	.write_raw = ad5446_write_raw,
>  	.driver_module = THIS_MODULE,
>  };
>  
> @@ -321,10 +323,7 @@ static int __devinit ad5446_probe(struct spi_device *spi)
>  	/* Establish that the iio_dev is a child of the spi device */
>  	indio_dev->dev.parent = &spi->dev;
>  	indio_dev->name = spi_get_device_id(spi)->name;
> -	if (st->chip_info->store_pwr_down)
> -		indio_dev->info = &ad5446_info;
> -	else
> -		indio_dev->info = &ad5446_info_no_pwr_down;
> +	indio_dev->info = &ad5446_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->channels = &st->chip_info->channel;
>  	indio_dev->num_channels = 1;

Re: [PATCH 7/8] staging:iio:dac:ad5446: Consolidate store_sample and store_pwr_down functions

[Jonathan Cameron]

On 4/23/2012 6:51 PM, Lars-Peter Clausen wrote:
> The devices supported by this drivers only have a single shift register, which
> contains both the power down mode and the output sample. So writing the power
> down mode and the output sample can be done by the same function. The two power
> down bits are always placed ontop of the msb of the output sample, so we can
> easily calculate their position by adding the channels shift to the channels
> realbits.
I wonder if the change in logic here slightly hides what is going on.
Perhaps simply renaming store_sample to something that indicates it's 
dual purpose
will make things clearer in the resulting code?

Also does this mean you can kill of store_pwr_down in the structure?
Given nothing sets it this should be fine!

>
> Signed-off-by: Lars-Peter Clausen<lars@metafoo.de>
> ---
>   drivers/staging/iio/dac/ad5446.c |   38 ++++++++++----------------------------
>   1 file changed, 10 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/staging/iio/dac/ad5446.c b/drivers/staging/iio/dac/ad5446.c
> index c767024..90461b2 100644
> --- a/drivers/staging/iio/dac/ad5446.c
> +++ b/drivers/staging/iio/dac/ad5446.c
> @@ -31,21 +31,6 @@ static void ad5446_store_sample(struct ad5446_state *st, unsigned val)
>
>   static void ad5660_store_sample(struct ad5446_state *st, unsigned val)
>   {
> -	val |= AD5660_LOAD;
> -	st->data.d24[0] = (val>>  16)&  0xFF;
> -	st->data.d24[1] = (val>>  8)&  0xFF;
> -	st->data.d24[2] = val&  0xFF;
> -}
> -
> -static void ad5620_store_pwr_down(struct ad5446_state *st, unsigned mode)
> -{
> -	st->data.d16 = cpu_to_be16(mode<<  14);
> -}
> -
> -static void ad5660_store_pwr_down(struct ad5446_state *st, unsigned mode)
> -{
> -	unsigned val = mode<<  16;
> -
>   	st->data.d24[0] = (val>>  16)&  0xFF;
>   	st->data.d24[1] = (val>>  8)&  0xFF;
>   	st->data.d24[2] = val&  0xFF;
> @@ -105,6 +90,8 @@ static ssize_t ad5446_write_dac_powerdown(struct iio_dev *indio_dev,
>   					    const char *buf, size_t len)
>   {
>   	struct ad5446_state *st = iio_priv(indio_dev);
> +	unsigned int shift;
> +	unsigned int val;
>   	bool powerdown;
>   	int ret;
>
> @@ -115,10 +102,14 @@ static ssize_t ad5446_write_dac_powerdown(struct iio_dev *indio_dev,
>   	mutex_lock(&indio_dev->mlock);
>   	st->pwr_down = powerdown;
>
> -	if (st->pwr_down)
> -		st->chip_info->store_pwr_down(st, st->pwr_down_mode);
> -	else
> -		st->chip_info->store_sample(st, st->cached_val);
> +	if (st->pwr_down) {
> +		shift = chan->scan_type.realbits + chan->scan_type.shift;
> +		val = st->pwr_down_mode<<  shift;
> +	} else {
> +		val = st->cached_val;
> +	}
> +
> +	st->chip_info->store_sample(st, val);
>
>   	ret = spi_sync(st->spi,&st->msg);
>   	mutex_unlock(&indio_dev->mlock);
> @@ -186,53 +177,44 @@ static const struct ad5446_chip_info ad5446_chip_info_tbl[] = {
>   	[ID_AD5601] = {
>   		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
>   		.store_sample = ad5446_store_sample,
> -		.store_pwr_down = ad5620_store_pwr_down,
>   	},
>   	[ID_AD5611] = {
>   		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
>   		.store_sample = ad5446_store_sample,
> -		.store_pwr_down = ad5620_store_pwr_down,
>   	},
>   	[ID_AD5621] = {
>   		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>   		.store_sample = ad5446_store_sample,
> -		.store_pwr_down = ad5620_store_pwr_down,
>   	},
>   	[ID_AD5620_2500] = {
>   		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>   		.int_vref_mv = 2500,
>   		.store_sample = ad5446_store_sample,
> -		.store_pwr_down = ad5620_store_pwr_down,
>   	},
>   	[ID_AD5620_1250] = {
>   		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>   		.int_vref_mv = 1250,
>   		.store_sample = ad5446_store_sample,
> -		.store_pwr_down = ad5620_store_pwr_down,
>   	},
>   	[ID_AD5640_2500] = {
>   		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
>   		.int_vref_mv = 2500,
>   		.store_sample = ad5446_store_sample,
> -		.store_pwr_down = ad5620_store_pwr_down,
>   	},
>   	[ID_AD5640_1250] = {
>   		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
>   		.int_vref_mv = 1250,
>   		.store_sample = ad5446_store_sample,
> -		.store_pwr_down = ad5620_store_pwr_down,
>   	},
>   	[ID_AD5660_2500] = {
>   		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>   		.int_vref_mv = 2500,
>   		.store_sample = ad5660_store_sample,
> -		.store_pwr_down = ad5660_store_pwr_down,
>   	},
>   	[ID_AD5660_1250] = {
>   		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>   		.int_vref_mv = 1250,
>   		.store_sample = ad5660_store_sample,
> -		.store_pwr_down = ad5660_store_pwr_down,
>   	},
>   };
>

Re: [PATCH 8/8] staging:iio:dac:ad5446: Add support for the AD5662

[Jonathan Cameron]

On 4/23/2012 6:51 PM, Lars-Peter Clausen wrote:
> The AD5662 is compatible to the AD5660, but uses an external reference instead
> of an internal.
Ah, I wondered why you were going though this driver with such a fine 
toothed comb!
>
> Signed-off-by: Lars-Peter Clausen<lars@metafoo.de>
> ---
>   drivers/staging/iio/dac/Kconfig  |    2 +-
>   drivers/staging/iio/dac/ad5446.c |    5 +++++
>   drivers/staging/iio/dac/ad5446.h |    1 +
>   3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/iio/dac/Kconfig b/drivers/staging/iio/dac/Kconfig
> index a57803a..1a2be98 100644
> --- a/drivers/staging/iio/dac/Kconfig
> +++ b/drivers/staging/iio/dac/Kconfig
> @@ -56,7 +56,7 @@ config AD5624R_SPI
>   	  AD5664R converters (DAC). This driver uses the common SPI interface.
>
>   config AD5446
> -	tristate "Analog Devices AD5444/6, AD5620/40/60 and AD5542A/12A DAC SPI driver"
> +	tristate "Analog Devices AD5444/6, AD5620/40/60/62 and AD5542A/12A DAC SPI driver"
>   	depends on SPI
>   	help
>   	  Say yes here to build support for Analog Devices AD5444, AD5446,
The odd bit is it is already in the description... hmm. There is quite a 
disconnect between parts in the two
lists... Don't suppose you'd mind verifying this and cleaning up.  At 
somepoint the list of parts in here
is going to get two long.  Maybe it is already time to go to "AD5444 and 
similar SPI DACs"
and rely on the help to tell people what it supports.
> diff --git a/drivers/staging/iio/dac/ad5446.c b/drivers/staging/iio/dac/ad5446.c
> index 90461b2..bcb849b 100644
> --- a/drivers/staging/iio/dac/ad5446.c
> +++ b/drivers/staging/iio/dac/ad5446.c
> @@ -216,6 +216,10 @@ static const struct ad5446_chip_info ad5446_chip_info_tbl[] = {
>   		.int_vref_mv = 1250,
>   		.store_sample = ad5660_store_sample,
>   	},
> +	[ID_AD5662] = {
> +		.channel = AD5446_CHANNEL(16, 16, 0),
> +		.store_sample = ad5660_store_sample,
> +	},
>   };
>
>   static int ad5446_read_raw(struct iio_dev *indio_dev,
> @@ -375,6 +379,7 @@ static const struct spi_device_id ad5446_id[] = {
>   	{"ad5640-1250", ID_AD5640_1250},
>   	{"ad5660-2500", ID_AD5660_2500},
>   	{"ad5660-1250", ID_AD5660_1250},
> +	{"ad5662", ID_AD5662},
>   	{}
>   };
>   MODULE_DEVICE_TABLE(spi, ad5446_id);
> diff --git a/drivers/staging/iio/dac/ad5446.h b/drivers/staging/iio/dac/ad5446.h
> index 264df1e..071ce39 100644
> --- a/drivers/staging/iio/dac/ad5446.h
> +++ b/drivers/staging/iio/dac/ad5446.h
> @@ -94,6 +94,7 @@ enum ad5446_supported_device_ids {
>   	ID_AD5640_1250,
>   	ID_AD5660_2500,
>   	ID_AD5660_1250,
> +	ID_AD5662,
>   };
>
>   #endif /* IIO_DAC_AD5446_H_ */

Re: [PATCH 7/8] staging:iio:dac:ad5446: Consolidate store_sample and store_pwr_down functions

[Jonathan Cameron]

On 4/24/2012 10:11 AM, Lars-Peter Clausen wrote:
> On 04/24/2012 10:06 AM, Jonathan Cameron wrote:
>> On 4/23/2012 6:51 PM, Lars-Peter Clausen wrote:
>>> The devices supported by this drivers only have a single shift register,
>>> which
>>> contains both the power down mode and the output sample. So writing the power
>>> down mode and the output sample can be done by the same function. The two
>>> power
>>> down bits are always placed ontop of the msb of the output sample, so we can
>>> easily calculate their position by adding the channels shift to the channels
>>> realbits.
>> I wonder if the change in logic here slightly hides what is going on.
>> Perhaps simply renaming store_sample to something that indicates it's dual
>> purpose
>> will make things clearer in the resulting code?
> I had initially renamed it to simply 'write', but well reverted it again. Do
> you think renaming it to 'write' will work?
yeah.
>
>> Also does this mean you can kill of store_pwr_down in the structure?
>> Given nothing sets it this should be fine!
>>
> Uhm, yes, that was actually the plan, looks like I missed it though. Good catch.
>
> Thanks,
> - Lars
>
>>> Signed-off-by: Lars-Peter Clausen<lars@metafoo.de>
>>> ---
>>>    drivers/staging/iio/dac/ad5446.c |   38
>>> ++++++++++----------------------------
>>>    1 file changed, 10 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/drivers/staging/iio/dac/ad5446.c
>>> b/drivers/staging/iio/dac/ad5446.c
>>> index c767024..90461b2 100644
>>> --- a/drivers/staging/iio/dac/ad5446.c
>>> +++ b/drivers/staging/iio/dac/ad5446.c
>>> @@ -31,21 +31,6 @@ static void ad5446_store_sample(struct ad5446_state
>>> *st, unsigned val)
>>>
>>>    static void ad5660_store_sample(struct ad5446_state *st, unsigned val)
>>>    {
>>> -    val |= AD5660_LOAD;
>>> -    st->data.d24[0] = (val>>   16)&   0xFF;
>>> -    st->data.d24[1] = (val>>   8)&   0xFF;
>>> -    st->data.d24[2] = val&   0xFF;
>>> -}
>>> -
>>> -static void ad5620_store_pwr_down(struct ad5446_state *st, unsigned mode)
>>> -{
>>> -    st->data.d16 = cpu_to_be16(mode<<   14);
>>> -}
>>> -
>>> -static void ad5660_store_pwr_down(struct ad5446_state *st, unsigned mode)
>>> -{
>>> -    unsigned val = mode<<   16;
>>> -
>>>        st->data.d24[0] = (val>>   16)&   0xFF;
>>>        st->data.d24[1] = (val>>   8)&   0xFF;
>>>        st->data.d24[2] = val&   0xFF;
>>> @@ -105,6 +90,8 @@ static ssize_t ad5446_write_dac_powerdown(struct
>>> iio_dev *indio_dev,
>>>                            const char *buf, size_t len)
>>>    {
>>>        struct ad5446_state *st = iio_priv(indio_dev);
>>> +    unsigned int shift;
>>> +    unsigned int val;
>>>        bool powerdown;
>>>        int ret;
>>>
>>> @@ -115,10 +102,14 @@ static ssize_t ad5446_write_dac_powerdown(struct
>>> iio_dev *indio_dev,
>>>        mutex_lock(&indio_dev->mlock);
>>>        st->pwr_down = powerdown;
>>>
>>> -    if (st->pwr_down)
>>> -        st->chip_info->store_pwr_down(st, st->pwr_down_mode);
>>> -    else
>>> -        st->chip_info->store_sample(st, st->cached_val);
>>> +    if (st->pwr_down) {
>>> +        shift = chan->scan_type.realbits + chan->scan_type.shift;
>>> +        val = st->pwr_down_mode<<   shift;
>>> +    } else {
>>> +        val = st->cached_val;
>>> +    }
>>> +
>>> +    st->chip_info->store_sample(st, val);
>>>
>>>        ret = spi_sync(st->spi,&st->msg);
>>>        mutex_unlock(&indio_dev->mlock);
>>> @@ -186,53 +177,44 @@ static const struct ad5446_chip_info
>>> ad5446_chip_info_tbl[] = {
>>>        [ID_AD5601] = {
>>>            .channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
>>>            .store_sample = ad5446_store_sample,
>>> -        .store_pwr_down = ad5620_store_pwr_down,
>>>        },
>>>        [ID_AD5611] = {
>>>            .channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
>>>            .store_sample = ad5446_store_sample,
>>> -        .store_pwr_down = ad5620_store_pwr_down,
>>>        },
>>>        [ID_AD5621] = {
>>>            .channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>>>            .store_sample = ad5446_store_sample,
>>> -        .store_pwr_down = ad5620_store_pwr_down,
>>>        },
>>>        [ID_AD5620_2500] = {
>>>            .channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>>>            .int_vref_mv = 2500,
>>>            .store_sample = ad5446_store_sample,
>>> -        .store_pwr_down = ad5620_store_pwr_down,
>>>        },
>>>        [ID_AD5620_1250] = {
>>>            .channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>>>            .int_vref_mv = 1250,
>>>            .store_sample = ad5446_store_sample,
>>> -        .store_pwr_down = ad5620_store_pwr_down,
>>>        },
>>>        [ID_AD5640_2500] = {
>>>            .channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
>>>            .int_vref_mv = 2500,
>>>            .store_sample = ad5446_store_sample,
>>> -        .store_pwr_down = ad5620_store_pwr_down,
>>>        },
>>>        [ID_AD5640_1250] = {
>>>            .channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
>>>            .int_vref_mv = 1250,
>>>            .store_sample = ad5446_store_sample,
>>> -        .store_pwr_down = ad5620_store_pwr_down,
>>>        },
>>>        [ID_AD5660_2500] = {
>>>            .channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>>>            .int_vref_mv = 2500,
>>>            .store_sample = ad5660_store_sample,
>>> -        .store_pwr_down = ad5660_store_pwr_down,
>>>        },
>>>        [ID_AD5660_1250] = {
>>>            .channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>>>            .int_vref_mv = 1250,
>>>            .store_sample = ad5660_store_sample,
>>> -        .store_pwr_down = ad5660_store_pwr_down,
>>>        },
>>>    };
>>>

Re: [PATCH 7/8] staging:iio:dac:ad5446: Consolidate store_sample and store_pwr_down functions

[Lars-Peter Clausen]

On 04/24/2012 10:06 AM, Jonathan Cameron wrote:
> On 4/23/2012 6:51 PM, Lars-Peter Clausen wrote:
>> The devices supported by this drivers only have a single shift register,
>> which
>> contains both the power down mode and the output sample. So writing the power
>> down mode and the output sample can be done by the same function. The two
>> power
>> down bits are always placed ontop of the msb of the output sample, so we can
>> easily calculate their position by adding the channels shift to the channels
>> realbits.
> I wonder if the change in logic here slightly hides what is going on.
> Perhaps simply renaming store_sample to something that indicates it's dual
> purpose
> will make things clearer in the resulting code?

I had initially renamed it to simply 'write', but well reverted it again. Do
you think renaming it to 'write' will work?

> 
> Also does this mean you can kill of store_pwr_down in the structure?
> Given nothing sets it this should be fine!
> 

Uhm, yes, that was actually the plan, looks like I missed it though. Good catch.

Thanks,
- Lars

>>
>> Signed-off-by: Lars-Peter Clausen<lars@metafoo.de>
>> ---
>>   drivers/staging/iio/dac/ad5446.c |   38
>> ++++++++++----------------------------
>>   1 file changed, 10 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/staging/iio/dac/ad5446.c
>> b/drivers/staging/iio/dac/ad5446.c
>> index c767024..90461b2 100644
>> --- a/drivers/staging/iio/dac/ad5446.c
>> +++ b/drivers/staging/iio/dac/ad5446.c
>> @@ -31,21 +31,6 @@ static void ad5446_store_sample(struct ad5446_state
>> *st, unsigned val)
>>
>>   static void ad5660_store_sample(struct ad5446_state *st, unsigned val)
>>   {
>> -    val |= AD5660_LOAD;
>> -    st->data.d24[0] = (val>>  16)&  0xFF;
>> -    st->data.d24[1] = (val>>  8)&  0xFF;
>> -    st->data.d24[2] = val&  0xFF;
>> -}
>> -
>> -static void ad5620_store_pwr_down(struct ad5446_state *st, unsigned mode)
>> -{
>> -    st->data.d16 = cpu_to_be16(mode<<  14);
>> -}
>> -
>> -static void ad5660_store_pwr_down(struct ad5446_state *st, unsigned mode)
>> -{
>> -    unsigned val = mode<<  16;
>> -
>>       st->data.d24[0] = (val>>  16)&  0xFF;
>>       st->data.d24[1] = (val>>  8)&  0xFF;
>>       st->data.d24[2] = val&  0xFF;
>> @@ -105,6 +90,8 @@ static ssize_t ad5446_write_dac_powerdown(struct
>> iio_dev *indio_dev,
>>                           const char *buf, size_t len)
>>   {
>>       struct ad5446_state *st = iio_priv(indio_dev);
>> +    unsigned int shift;
>> +    unsigned int val;
>>       bool powerdown;
>>       int ret;
>>
>> @@ -115,10 +102,14 @@ static ssize_t ad5446_write_dac_powerdown(struct
>> iio_dev *indio_dev,
>>       mutex_lock(&indio_dev->mlock);
>>       st->pwr_down = powerdown;
>>
>> -    if (st->pwr_down)
>> -        st->chip_info->store_pwr_down(st, st->pwr_down_mode);
>> -    else
>> -        st->chip_info->store_sample(st, st->cached_val);
>> +    if (st->pwr_down) {
>> +        shift = chan->scan_type.realbits + chan->scan_type.shift;
>> +        val = st->pwr_down_mode<<  shift;
>> +    } else {
>> +        val = st->cached_val;
>> +    }
>> +
>> +    st->chip_info->store_sample(st, val);
>>
>>       ret = spi_sync(st->spi,&st->msg);
>>       mutex_unlock(&indio_dev->mlock);
>> @@ -186,53 +177,44 @@ static const struct ad5446_chip_info
>> ad5446_chip_info_tbl[] = {
>>       [ID_AD5601] = {
>>           .channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
>>           .store_sample = ad5446_store_sample,
>> -        .store_pwr_down = ad5620_store_pwr_down,
>>       },
>>       [ID_AD5611] = {
>>           .channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
>>           .store_sample = ad5446_store_sample,
>> -        .store_pwr_down = ad5620_store_pwr_down,
>>       },
>>       [ID_AD5621] = {
>>           .channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>>           .store_sample = ad5446_store_sample,
>> -        .store_pwr_down = ad5620_store_pwr_down,
>>       },
>>       [ID_AD5620_2500] = {
>>           .channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>>           .int_vref_mv = 2500,
>>           .store_sample = ad5446_store_sample,
>> -        .store_pwr_down = ad5620_store_pwr_down,
>>       },
>>       [ID_AD5620_1250] = {
>>           .channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
>>           .int_vref_mv = 1250,
>>           .store_sample = ad5446_store_sample,
>> -        .store_pwr_down = ad5620_store_pwr_down,
>>       },
>>       [ID_AD5640_2500] = {
>>           .channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
>>           .int_vref_mv = 2500,
>>           .store_sample = ad5446_store_sample,
>> -        .store_pwr_down = ad5620_store_pwr_down,
>>       },
>>       [ID_AD5640_1250] = {
>>           .channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
>>           .int_vref_mv = 1250,
>>           .store_sample = ad5446_store_sample,
>> -        .store_pwr_down = ad5620_store_pwr_down,
>>       },
>>       [ID_AD5660_2500] = {
>>           .channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>>           .int_vref_mv = 2500,
>>           .store_sample = ad5660_store_sample,
>> -        .store_pwr_down = ad5660_store_pwr_down,
>>       },
>>       [ID_AD5660_1250] = {
>>           .channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
>>           .int_vref_mv = 1250,
>>           .store_sample = ad5660_store_sample,
>> -        .store_pwr_down = ad5660_store_pwr_down,
>>       },
>>   };
>>