All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 1/8] iio: Add helper functions for enum style channel attributes
@ 2012-05-11 15:53 Lars-Peter Clausen
  2012-05-11 15:53 ` [RFC 2/8] staging:iio:dac:ad5064: Use iio_enum for powerdown modes Lars-Peter Clausen
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Lars-Peter Clausen @ 2012-05-11 15:53 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Lars-Peter Clausen

We often have the case were we do have a enum style channel attribute. These
attributes have in common that they are a list of string values which usually
map in a 1-to-1 fashion to integer values.

This patch implements some common helper code for implementing enum style
channel attributes using extended channel attributes. The helper functions take
care of converting between the string and integer values, as well providing a
function for "_available" attributes which list all available enum items.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/industrialio-core.c |   57 +++++++++++++++++++++++++++++++++++++++
 include/linux/iio/iio.h         |   32 ++++++++++++++++++++++
 2 files changed, 89 insertions(+)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index b39a587..241be03 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -288,6 +288,63 @@ static ssize_t iio_write_channel_ext_info(struct device *dev,
 			       this_attr->c, buf, len);
 }
 
+ssize_t iio_enum_available_read(struct iio_dev *indio_dev,
+	uintptr_t priv, const struct iio_chan_spec *chan, char *buf)
+{
+	const struct iio_enum *e = (const struct iio_enum *)priv;
+	char *tmp = buf;
+	unsigned int i;
+
+	if (!e->num_items)
+		return 0;
+
+	for (i = 0; i < e->num_items; ++i)
+		tmp += sprintf(tmp, "%s ", e->items[i]);
+
+	/* replace last space with a newline */
+	*(tmp - 1) = '\n';
+
+	return tmp - buf;
+}
+EXPORT_SYMBOL_GPL(iio_enum_available_read);
+
+ssize_t iio_enum_read(struct iio_dev *indio_dev,
+	uintptr_t priv, const struct iio_chan_spec *chan, char *buf)
+{
+	const struct iio_enum *e = (const struct iio_enum *)priv;
+	int i;
+
+	i = e->get(indio_dev, chan);
+	if (i < 0)
+		return i;
+	else if (i >= e->num_items)
+		return -EINVAL;
+
+	return sprintf(buf, "%s\n", e->items[i]);
+}
+EXPORT_SYMBOL_GPL(iio_enum_read);
+
+ssize_t iio_enum_write(struct iio_dev *indio_dev,
+	uintptr_t priv, const struct iio_chan_spec *chan, const char *buf,
+	size_t len)
+{
+	const struct iio_enum *e = (const struct iio_enum *)priv;
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < e->num_items; i++) {
+		if (sysfs_streq(buf, e->items[i]))
+			break;
+	}
+
+	if (i == e->num_items)
+		return -EINVAL;
+
+	ret = e->set(indio_dev, chan, i);
+	return ret ? ret : len;
+}
+EXPORT_SYMBOL_GPL(iio_enum_write);
+
 static ssize_t iio_read_channel_info(struct device *dev,
 				     struct device_attribute *attr,
 				     char *buf)
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 6fdbdb8..602f118 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -124,6 +124,38 @@ struct iio_chan_spec_ext_info {
 	uintptr_t private;
 };
 
+struct iio_enum {
+	const char * const *items;
+	unsigned int num_items;
+	int (*set)(struct iio_dev *, const struct iio_chan_spec *, unsigned int);
+	int (*get)(struct iio_dev *, const struct iio_chan_spec *);
+};
+
+ssize_t iio_enum_available_read(struct iio_dev *indio_dev,
+	uintptr_t priv, const struct iio_chan_spec *chan, char *buf);
+ssize_t iio_enum_read(struct iio_dev *indio_dev,
+	uintptr_t priv, const struct iio_chan_spec *chan, char *buf);
+ssize_t iio_enum_write(struct iio_dev *indio_dev,
+	uintptr_t priv, const struct iio_chan_spec *chan, const char *buf,
+	size_t len);
+
+#define IIO_ENUM(_name, _shared, _e) \
+{ \
+	.name = (_name), \
+	.shared = (_shared), \
+	.read = iio_enum_read, \
+	.write = iio_enum_write, \
+	.private = (uintptr_t)(_e), \
+}
+
+#define IIO_ENUM_AVAILABLE(_name, _e) \
+{ \
+	.name = (_name "_available"), \
+	.shared = true, \
+	.read = iio_enum_available_read, \
+	.private = (uintptr_t)(_e), \
+}
+
 /**
  * struct iio_chan_spec - specification of a single channel
  * @type:		What type of measurement is the channel making.
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [RFC 2/8] staging:iio:dac:ad5064: Use iio_enum for powerdown modes
  2012-05-11 15:53 [RFC 1/8] iio: Add helper functions for enum style channel attributes Lars-Peter Clausen
@ 2012-05-11 15:53 ` Lars-Peter Clausen
  2012-05-14 17:48   ` Jonathan Cameron
  2012-05-11 15:53 ` [RFC 3/8] staging:iio:dac:ad5446: " Lars-Peter Clausen
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Lars-Peter Clausen @ 2012-05-11 15:53 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Lars-Peter Clausen

This allows us to remove some boilerplate code for comparing and formatting the
enum strings.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/dac/ad5064.c |   65 ++++++++++++--------------------------
 1 file changed, 21 insertions(+), 44 deletions(-)

diff --git a/drivers/staging/iio/dac/ad5064.c b/drivers/staging/iio/dac/ad5064.c
index da0fa12..276af02 100644
--- a/drivers/staging/iio/dac/ad5064.c
+++ b/drivers/staging/iio/dac/ad5064.c
@@ -135,57 +135,42 @@ static int ad5064_sync_powerdown_mode(struct ad5064_state *st,
 	return ret;
 }
 
-static const char ad5064_powerdown_modes[][15] = {
-	[AD5064_LDAC_PWRDN_NONE]	= "",
-	[AD5064_LDAC_PWRDN_1K]		= "1kohm_to_gnd",
-	[AD5064_LDAC_PWRDN_100K]	= "100kohm_to_gnd",
-	[AD5064_LDAC_PWRDN_3STATE]	= "three_state",
+static const char * const ad5064_powerdown_modes[] = {
+	"1kohm_to_gnd",
+	"100kohm_to_gnd",
+	"three_state",
 };
 
-static ssize_t ad5064_read_powerdown_mode_available(struct iio_dev *indio_dev,
-	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
-{
-	return sprintf(buf, "%s %s %s\n", ad5064_powerdown_modes[1],
-		ad5064_powerdown_modes[2], ad5064_powerdown_modes[3]);
-}
-
-static ssize_t ad5064_read_powerdown_mode(struct iio_dev *indio_dev,
-	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
+static int ad5064_get_powerdown_mode(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan)
 {
 	struct ad5064_state *st = iio_priv(indio_dev);
 
-	return sprintf(buf, "%s\n",
-		ad5064_powerdown_modes[st->pwr_down_mode[chan->channel]]);
+	return st->pwr_down_mode[chan->channel] - 1;
 }
 
-static ssize_t ad5064_write_powerdown_mode(struct iio_dev *indio_dev,
-	uintptr_t private, const struct iio_chan_spec *chan, const char *buf,
-	size_t len)
+static int ad5064_set_powerdown_mode(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, unsigned int mode)
 {
 	struct ad5064_state *st = iio_priv(indio_dev);
-	unsigned int mode, i;
 	int ret;
 
-	mode = 0;
-
-	for (i = 1; i < ARRAY_SIZE(ad5064_powerdown_modes); ++i) {
-		if (sysfs_streq(buf, ad5064_powerdown_modes[i])) {
-			mode = i;
-			break;
-		}
-	}
-	if (mode == 0)
-		return  -EINVAL;
-
 	mutex_lock(&indio_dev->mlock);
-	st->pwr_down_mode[chan->channel] = mode;
+	st->pwr_down_mode[chan->channel] = mode + 1;
 
 	ret = ad5064_sync_powerdown_mode(st, chan->channel);
 	mutex_unlock(&indio_dev->mlock);
 
-	return ret ? ret : len;
+	return ret;
 }
 
+static const struct iio_enum ad5064_powerdown_mode_enum = {
+	.items = ad5064_powerdown_modes,
+	.num_items = ARRAY_SIZE(ad5064_powerdown_modes),
+	.get = ad5064_get_powerdown_mode,
+	.set = ad5064_set_powerdown_mode,
+};
+
 static ssize_t ad5064_read_dac_powerdown(struct iio_dev *indio_dev,
 	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
 {
@@ -285,22 +270,14 @@ static const struct iio_info ad5064_info = {
 	.driver_module = THIS_MODULE,
 };
 
-static struct iio_chan_spec_ext_info ad5064_ext_info[] = {
+static const struct iio_chan_spec_ext_info ad5064_ext_info[] = {
 	{
 		.name = "powerdown",
 		.read = ad5064_read_dac_powerdown,
 		.write = ad5064_write_dac_powerdown,
 	},
-	{
-		.name = "powerdown_mode",
-		.read = ad5064_read_powerdown_mode,
-		.write = ad5064_write_powerdown_mode,
-	},
-	{
-		.name = "powerdown_mode_available",
-		.shared = true,
-		.read = ad5064_read_powerdown_mode_available,
-	},
+	IIO_ENUM("powerdown_mode", false, &ad5064_powerdown_mode_enum),
+	IIO_ENUM_AVAILABLE("powerdown_mode", &ad5064_powerdown_mode_enum),
 	{ },
 };
 
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [RFC 3/8] staging:iio:dac:ad5446: Use iio_enum for powerdown modes
  2012-05-11 15:53 [RFC 1/8] iio: Add helper functions for enum style channel attributes Lars-Peter Clausen
  2012-05-11 15:53 ` [RFC 2/8] staging:iio:dac:ad5064: Use iio_enum for powerdown modes Lars-Peter Clausen
@ 2012-05-11 15:53 ` Lars-Peter Clausen
  2012-05-14 17:55   ` Jonathan Cameron
  2012-05-11 15:53 ` [RFC 4/8] staging:iio:dac:ad5380: Convert to extended channel attributes Lars-Peter Clausen
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Lars-Peter Clausen @ 2012-05-11 15:53 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Lars-Peter Clausen

This allows us to remove some boilerplate code for comparing and formatting the
enum strings.

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

diff --git a/drivers/staging/iio/dac/ad5446.c b/drivers/staging/iio/dac/ad5446.c
index 3e53d0c..f769b74 100644
--- a/drivers/staging/iio/dac/ad5446.c
+++ b/drivers/staging/iio/dac/ad5446.c
@@ -41,47 +41,34 @@ static int ad5660_write(struct ad5446_state *st, unsigned val)
 }
 
 static const char * const ad5446_powerdown_modes[] = {
-	"", "1kohm_to_gnd", "100kohm_to_gnd", "three_state"
+	"1kohm_to_gnd", "100kohm_to_gnd", "three_state"
 };
 
-static ssize_t ad5446_read_powerdown_mode_available(struct iio_dev *indio_dev,
-	uintptr_t private, 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,
-					    uintptr_t private,
-					    const struct iio_chan_spec *chan,
-					    const char *buf, size_t len)
+static int ad5446_set_powerdown_mode(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, unsigned int mode)
 {
 	struct ad5446_state *st = iio_priv(indio_dev);
-	int i;
-
-	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;
+	st->pwr_down_mode = mode + 1;
 
-	return len;
+	return 0;
 }
 
-static ssize_t ad5446_read_powerdown_mode(struct iio_dev *indio_dev,
-					   uintptr_t private,
-					   const struct iio_chan_spec *chan,
-					   char *buf)
+static int ad5446_get_powerdown_mode(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan)
 {
 	struct ad5446_state *st = iio_priv(indio_dev);
 
-	return sprintf(buf, "%s\n", ad5446_powerdown_modes[st->pwr_down_mode]);
+	return st->pwr_down_mode - 1;
 }
 
+static const struct iio_enum ad5446_powerdown_mode_enum = {
+	.items = ad5446_powerdown_modes,
+	.num_items = ARRAY_SIZE(ad5446_powerdown_modes),
+	.get = ad5446_get_powerdown_mode,
+	.set = ad5446_set_powerdown_mode,
+};
+
 static ssize_t ad5446_read_dac_powerdown(struct iio_dev *indio_dev,
 					   uintptr_t private,
 					   const struct iio_chan_spec *chan,
@@ -128,15 +115,9 @@ 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,
 	},
+	IIO_ENUM("powerdown_mode", false, &ad5446_powerdown_mode_enum),
+	IIO_ENUM_AVAILABLE("powerdown_mode", &ad5446_powerdown_mode_enum),
 	{ },
 };
 
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [RFC 4/8] staging:iio:dac:ad5380: Convert to extended channel attributes
  2012-05-11 15:53 [RFC 1/8] iio: Add helper functions for enum style channel attributes Lars-Peter Clausen
  2012-05-11 15:53 ` [RFC 2/8] staging:iio:dac:ad5064: Use iio_enum for powerdown modes Lars-Peter Clausen
  2012-05-11 15:53 ` [RFC 3/8] staging:iio:dac:ad5446: " Lars-Peter Clausen
@ 2012-05-11 15:53 ` Lars-Peter Clausen
  2012-05-14 18:11   ` Jonathan Cameron
  2012-05-11 15:53 ` [RFC 5/8] staging:iio:dac:ad5504: " Lars-Peter Clausen
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Lars-Peter Clausen @ 2012-05-11 15:53 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Lars-Peter Clausen

Use extended channel attributes instead of raw sysfs files for the additional
channel attributes. This allows us to remove some boilerplate code.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/dac/ad5380.c |  250 ++++++++++++++++++--------------------
 1 file changed, 116 insertions(+), 134 deletions(-)

diff --git a/drivers/staging/iio/dac/ad5380.c b/drivers/staging/iio/dac/ad5380.c
index 1baaabe..5dfb445 100644
--- a/drivers/staging/iio/dac/ad5380.c
+++ b/drivers/staging/iio/dac/ad5380.c
@@ -79,103 +79,18 @@ enum ad5380_type {
 	ID_AD5392_5,
 };
 
-#define AD5380_CHANNEL(_bits) {					\
-	.type = IIO_VOLTAGE,					\
-	.indexed = 1,						\
-	.output = 1,						\
-	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |		\
-		IIO_CHAN_INFO_SCALE_SHARED_BIT |		\
-		IIO_CHAN_INFO_CALIBSCALE_SEPARATE_BIT |		\
-		IIO_CHAN_INFO_CALIBBIAS_SEPARATE_BIT,		\
-	.scan_type = IIO_ST('u', (_bits), 16, 14 - (_bits))	\
-}
-
-static const struct ad5380_chip_info ad5380_chip_info_tbl[] = {
-	[ID_AD5380_3] = {
-		.channel_template = AD5380_CHANNEL(14),
-		.num_channels = 40,
-		.int_vref = 1250000,
-	},
-	[ID_AD5380_5] = {
-		.channel_template = AD5380_CHANNEL(14),
-		.num_channels = 40,
-		.int_vref = 2500000,
-	},
-	[ID_AD5381_3] = {
-		.channel_template = AD5380_CHANNEL(12),
-		.num_channels = 16,
-		.int_vref = 1250000,
-	},
-	[ID_AD5381_5] = {
-		.channel_template = AD5380_CHANNEL(12),
-		.num_channels = 16,
-		.int_vref = 2500000,
-	},
-	[ID_AD5382_3] = {
-		.channel_template = AD5380_CHANNEL(14),
-		.num_channels = 32,
-		.int_vref = 1250000,
-	},
-	[ID_AD5382_5] = {
-		.channel_template = AD5380_CHANNEL(14),
-		.num_channels = 32,
-		.int_vref = 2500000,
-	},
-	[ID_AD5383_3] = {
-		.channel_template = AD5380_CHANNEL(12),
-		.num_channels = 32,
-		.int_vref = 1250000,
-	},
-	[ID_AD5383_5] = {
-		.channel_template = AD5380_CHANNEL(12),
-		.num_channels = 32,
-		.int_vref = 2500000,
-	},
-	[ID_AD5390_3] = {
-		.channel_template = AD5380_CHANNEL(14),
-		.num_channels = 16,
-		.int_vref = 1250000,
-	},
-	[ID_AD5390_5] = {
-		.channel_template = AD5380_CHANNEL(14),
-		.num_channels = 16,
-		.int_vref = 2500000,
-	},
-	[ID_AD5391_3] = {
-		.channel_template = AD5380_CHANNEL(12),
-		.num_channels = 16,
-		.int_vref = 1250000,
-	},
-	[ID_AD5391_5] = {
-		.channel_template = AD5380_CHANNEL(12),
-		.num_channels = 16,
-		.int_vref = 2500000,
-	},
-	[ID_AD5392_3] = {
-		.channel_template = AD5380_CHANNEL(14),
-		.num_channels = 8,
-		.int_vref = 1250000,
-	},
-	[ID_AD5392_5] = {
-		.channel_template = AD5380_CHANNEL(14),
-		.num_channels = 8,
-		.int_vref = 2500000,
-	},
-};
-
-static ssize_t ad5380_read_dac_powerdown(struct device *dev,
-	struct device_attribute *attr, char *buf)
+static ssize_t ad5380_read_dac_powerdown(struct iio_dev *indio_dev,
+	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5380_state *st = iio_priv(indio_dev);
 
 	return sprintf(buf, "%d\n", st->pwr_down);
 }
 
-static ssize_t ad5380_write_dac_powerdown(struct device *dev,
-	struct device_attribute *attr, const char *buf, size_t len)
+static ssize_t ad5380_write_dac_powerdown(struct iio_dev *indio_dev,
+	 uintptr_t private, const struct iio_chan_spec *chan, const char *buf,
+	 size_t len)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5380_state *st = iio_priv(indio_dev);
 	bool pwr_down;
 	int ret;
@@ -198,20 +113,14 @@ static ssize_t ad5380_write_dac_powerdown(struct device *dev,
 	return ret ? ret : len;
 }
 
-static IIO_DEVICE_ATTR(out_voltage_powerdown,
-			S_IRUGO | S_IWUSR,
-			ad5380_read_dac_powerdown,
-			ad5380_write_dac_powerdown, 0);
-
-static const char ad5380_powerdown_modes[][15] = {
-	[0]	= "100kohm_to_gnd",
-	[1]	= "three_state",
+static const char * const ad5380_powerdown_modes[] = {
+	"100kohm_to_gnd",
+	"three_state",
 };
 
-static ssize_t ad5380_read_powerdown_mode(struct device *dev,
-	struct device_attribute *attr, char *buf)
+static int ad5380_get_powerdown_mode(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5380_state *st = iio_priv(indio_dev);
 	unsigned int mode;
 	int ret;
@@ -222,49 +131,27 @@ static ssize_t ad5380_read_powerdown_mode(struct device *dev,
 
 	mode = (mode >> AD5380_CTRL_PWR_DOWN_MODE_OFFSET) & 1;
 
-	return sprintf(buf, "%s\n", ad5380_powerdown_modes[mode]);
+	return mode;
 }
 
-static ssize_t ad5380_write_powerdown_mode(struct device *dev,
-	struct device_attribute *attr, const char *buf, size_t len)
+static int ad5380_set_powerdown_mode(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, unsigned int mode)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5380_state *st = iio_priv(indio_dev);
-	unsigned int i;
 	int ret;
 
-	for (i = 0; i < ARRAY_SIZE(ad5380_powerdown_modes); ++i) {
-		if (sysfs_streq(buf, ad5380_powerdown_modes[i]))
-			break;
-	}
-
-	if (i == ARRAY_SIZE(ad5380_powerdown_modes))
-		return -EINVAL;
-
 	ret = regmap_update_bits(st->regmap, AD5380_REG_SF_CTRL,
 		1 << AD5380_CTRL_PWR_DOWN_MODE_OFFSET,
-		i << AD5380_CTRL_PWR_DOWN_MODE_OFFSET);
+		mode << AD5380_CTRL_PWR_DOWN_MODE_OFFSET);
 
-	return ret ? ret : len;
+	return ret;
 }
 
-static IIO_DEVICE_ATTR(out_voltage_powerdown_mode,
-			S_IRUGO | S_IWUSR,
-			ad5380_read_powerdown_mode,
-			ad5380_write_powerdown_mode, 0);
-
-static IIO_CONST_ATTR(out_voltage_powerdown_mode_available,
-			"100kohm_to_gnd three_state");
-
-static struct attribute *ad5380_attributes[] = {
-	&iio_dev_attr_out_voltage_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 ad5380_attribute_group = {
-	.attrs = ad5380_attributes,
+static const struct iio_enum ad5380_powerdown_mode_enum = {
+	.items = ad5380_powerdown_modes,
+	.num_items = ARRAY_SIZE(ad5380_powerdown_modes),
+	.get = ad5380_get_powerdown_mode,
+	.set = ad5380_set_powerdown_mode,
 };
 
 static unsigned int ad5380_info_to_reg(struct iio_chan_spec const *chan,
@@ -352,10 +239,105 @@ static int ad5380_read_raw(struct iio_dev *indio_dev,
 static const struct iio_info ad5380_info = {
 	.read_raw = ad5380_read_raw,
 	.write_raw = ad5380_write_raw,
-	.attrs = &ad5380_attribute_group,
 	.driver_module = THIS_MODULE,
 };
 
+static struct iio_chan_spec_ext_info ad5380_ext_info[] = {
+	{
+		.name = "powerdown",
+		.read = ad5380_read_dac_powerdown,
+		.write = ad5380_write_dac_powerdown,
+	},
+	IIO_ENUM("powerdown_mode", true, &ad5380_powerdown_mode_enum),
+	IIO_ENUM_AVAILABLE("powerdown_mode", &ad5380_powerdown_mode_enum),
+	{ },
+};
+
+#define AD5380_CHANNEL(_bits) {					\
+	.type = IIO_VOLTAGE,					\
+	.indexed = 1,						\
+	.output = 1,						\
+	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |		\
+		IIO_CHAN_INFO_SCALE_SHARED_BIT |		\
+		IIO_CHAN_INFO_CALIBSCALE_SEPARATE_BIT |		\
+		IIO_CHAN_INFO_CALIBBIAS_SEPARATE_BIT,		\
+	.scan_type = IIO_ST('u', (_bits), 16, 14 - (_bits)),	\
+	.ext_info = ad5380_ext_info,				\
+}
+
+static const struct ad5380_chip_info ad5380_chip_info_tbl[] = {
+	[ID_AD5380_3] = {
+		.channel_template = AD5380_CHANNEL(14),
+		.num_channels = 40,
+		.int_vref = 1250000,
+	},
+	[ID_AD5380_5] = {
+		.channel_template = AD5380_CHANNEL(14),
+		.num_channels = 40,
+		.int_vref = 2500000,
+	},
+	[ID_AD5381_3] = {
+		.channel_template = AD5380_CHANNEL(12),
+		.num_channels = 16,
+		.int_vref = 1250000,
+	},
+	[ID_AD5381_5] = {
+		.channel_template = AD5380_CHANNEL(12),
+		.num_channels = 16,
+		.int_vref = 2500000,
+	},
+	[ID_AD5382_3] = {
+		.channel_template = AD5380_CHANNEL(14),
+		.num_channels = 32,
+		.int_vref = 1250000,
+	},
+	[ID_AD5382_5] = {
+		.channel_template = AD5380_CHANNEL(14),
+		.num_channels = 32,
+		.int_vref = 2500000,
+	},
+	[ID_AD5383_3] = {
+		.channel_template = AD5380_CHANNEL(12),
+		.num_channels = 32,
+		.int_vref = 1250000,
+	},
+	[ID_AD5383_5] = {
+		.channel_template = AD5380_CHANNEL(12),
+		.num_channels = 32,
+		.int_vref = 2500000,
+	},
+	[ID_AD5390_3] = {
+		.channel_template = AD5380_CHANNEL(14),
+		.num_channels = 16,
+		.int_vref = 1250000,
+	},
+	[ID_AD5390_5] = {
+		.channel_template = AD5380_CHANNEL(14),
+		.num_channels = 16,
+		.int_vref = 2500000,
+	},
+	[ID_AD5391_3] = {
+		.channel_template = AD5380_CHANNEL(12),
+		.num_channels = 16,
+		.int_vref = 1250000,
+	},
+	[ID_AD5391_5] = {
+		.channel_template = AD5380_CHANNEL(12),
+		.num_channels = 16,
+		.int_vref = 2500000,
+	},
+	[ID_AD5392_3] = {
+		.channel_template = AD5380_CHANNEL(14),
+		.num_channels = 8,
+		.int_vref = 1250000,
+	},
+	[ID_AD5392_5] = {
+		.channel_template = AD5380_CHANNEL(14),
+		.num_channels = 8,
+		.int_vref = 2500000,
+	},
+};
+
 static int __devinit ad5380_alloc_channels(struct iio_dev *indio_dev)
 {
 	struct ad5380_state *st = iio_priv(indio_dev);
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [RFC 5/8] staging:iio:dac:ad5504: Convert to extended channel attributes
  2012-05-11 15:53 [RFC 1/8] iio: Add helper functions for enum style channel attributes Lars-Peter Clausen
                   ` (2 preceding siblings ...)
  2012-05-11 15:53 ` [RFC 4/8] staging:iio:dac:ad5380: Convert to extended channel attributes Lars-Peter Clausen
@ 2012-05-11 15:53 ` Lars-Peter Clausen
  2012-05-14 18:14   ` Jonathan Cameron
  2012-05-11 15:53 ` [RFC 6/8] staging:iio:dac:ad5624r: " Lars-Peter Clausen
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Lars-Peter Clausen @ 2012-05-11 15:53 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Lars-Peter Clausen

Use extended channel attributes instead of raw sysfs files for the additional
channel attributes. This allows us to remove some boilerplate code.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/dac/ad5504.c |  164 ++++++++++++++------------------------
 1 file changed, 58 insertions(+), 106 deletions(-)

diff --git a/drivers/staging/iio/dac/ad5504.c b/drivers/staging/iio/dac/ad5504.c
index db84d85..07b31f4 100644
--- a/drivers/staging/iio/dac/ad5504.c
+++ b/drivers/staging/iio/dac/ad5504.c
@@ -22,24 +22,6 @@
 
 #include "ad5504.h"
 
-#define AD5504_CHANNEL(_chan) { \
-	.type = IIO_VOLTAGE, \
-	.indexed = 1, \
-	.output = 1, \
-	.channel = (_chan), \
-	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT | \
-		     IIO_CHAN_INFO_SCALE_SHARED_BIT, \
-	.address = AD5504_ADDR_DAC(_chan), \
-	.scan_type = IIO_ST('u', 12, 16, 0), \
-}
-
-static const struct iio_chan_spec ad5504_channels[] = {
-	AD5504_CHANNEL(0),
-	AD5504_CHANNEL(1),
-	AD5504_CHANNEL(2),
-	AD5504_CHANNEL(3),
-};
-
 static int ad5504_spi_write(struct spi_device *spi, u8 addr, u16 val)
 {
 	u16 tmp = cpu_to_be16(AD5504_CMD_WRITE |
@@ -122,65 +104,60 @@ static int ad5504_write_raw(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
-static ssize_t ad5504_read_powerdown_mode(struct device *dev,
-				      struct device_attribute *attr, char *buf)
+static const char * const ad5504_powerdown_modes[] = {
+	"20kohm_to_gnd",
+	"three_state",
+};
+
+static int ad5504_get_powerdown_mode(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5504_state *st = iio_priv(indio_dev);
 
-	const char mode[][14] = {"20kohm_to_gnd", "three_state"};
-
-	return sprintf(buf, "%s\n", mode[st->pwr_down_mode]);
+	return st->pwr_down_mode;
 }
 
-static ssize_t ad5504_write_powerdown_mode(struct device *dev,
-				       struct device_attribute *attr,
-				       const char *buf, size_t len)
+static int ad5504_set_powerdown_mode(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, unsigned int mode)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5504_state *st = iio_priv(indio_dev);
-	int ret;
-
-	if (sysfs_streq(buf, "20kohm_to_gnd"))
-		st->pwr_down_mode = AD5504_DAC_PWRDN_20K;
-	else if (sysfs_streq(buf, "three_state"))
-		st->pwr_down_mode = AD5504_DAC_PWRDN_3STATE;
-	else
-		ret = -EINVAL;
 
-	return ret ? ret : len;
+	st->pwr_down_mode = mode;
+	return 0;
 }
 
-static ssize_t ad5504_read_dac_powerdown(struct device *dev,
-					   struct device_attribute *attr,
-					   char *buf)
+static const struct iio_enum ad5504_powerdown_mode_enum = {
+	.items = ad5504_powerdown_modes,
+	.num_items = ARRAY_SIZE(ad5504_powerdown_modes),
+	.get = ad5504_get_powerdown_mode,
+	.set = ad5504_set_powerdown_mode,
+};
+
+static ssize_t ad5504_read_dac_powerdown(struct iio_dev *indio_dev,
+	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5504_state *st = iio_priv(indio_dev);
-	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
 
 	return sprintf(buf, "%d\n",
-			!(st->pwr_down_mask & (1 << this_attr->address)));
+			!(st->pwr_down_mask & (1 << chan->channel)));
 }
 
-static ssize_t ad5504_write_dac_powerdown(struct device *dev,
-					    struct device_attribute *attr,
-					    const char *buf, size_t len)
+static ssize_t ad5504_write_dac_powerdown(struct iio_dev *indio_dev,
+	uintptr_t private, const struct iio_chan_spec *chan, const char *buf,
+	size_t len)
 {
 	long readin;
 	int ret;
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5504_state *st = iio_priv(indio_dev);
-	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
 
 	ret = strict_strtol(buf, 10, &readin);
 	if (ret)
 		return ret;
 
 	if (readin == 0)
-		st->pwr_down_mask |= (1 << this_attr->address);
+		st->pwr_down_mask |= (1 << chan->channel);
 	else if (readin == 1)
-		st->pwr_down_mask &= ~(1 << this_attr->address);
+		st->pwr_down_mask &= ~(1 << chan->channel);
 	else
 		ret = -EINVAL;
 
@@ -194,50 +171,6 @@ static ssize_t ad5504_write_dac_powerdown(struct device *dev,
 	return ret ? ret : len;
 }
 
-static IIO_DEVICE_ATTR(out_voltage_powerdown_mode, S_IRUGO |
-			S_IWUSR, ad5504_read_powerdown_mode,
-			ad5504_write_powerdown_mode, 0);
-
-static IIO_CONST_ATTR(out_voltage_powerdown_mode_available,
-			"20kohm_to_gnd three_state");
-
-#define IIO_DEV_ATTR_DAC_POWERDOWN(_num, _show, _store, _addr)		\
-	IIO_DEVICE_ATTR(out_voltage##_num##_powerdown,			\
-			S_IRUGO | S_IWUSR, _show, _store, _addr)
-static IIO_DEV_ATTR_DAC_POWERDOWN(0, ad5504_read_dac_powerdown,
-				   ad5504_write_dac_powerdown, 0);
-static IIO_DEV_ATTR_DAC_POWERDOWN(1, ad5504_read_dac_powerdown,
-				   ad5504_write_dac_powerdown, 1);
-static IIO_DEV_ATTR_DAC_POWERDOWN(2, ad5504_read_dac_powerdown,
-				   ad5504_write_dac_powerdown, 2);
-static IIO_DEV_ATTR_DAC_POWERDOWN(3, ad5504_read_dac_powerdown,
-				   ad5504_write_dac_powerdown, 3);
-
-static struct attribute *ad5504_attributes[] = {
-	&iio_dev_attr_out_voltage0_powerdown.dev_attr.attr,
-	&iio_dev_attr_out_voltage1_powerdown.dev_attr.attr,
-	&iio_dev_attr_out_voltage2_powerdown.dev_attr.attr,
-	&iio_dev_attr_out_voltage3_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 ad5504_attribute_group = {
-	.attrs = ad5504_attributes,
-};
-
-static struct attribute *ad5501_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 ad5501_attribute_group = {
-	.attrs = ad5501_attributes,
-};
-
 static IIO_CONST_ATTR(temp0_thresh_rising_value, "110000");
 static IIO_CONST_ATTR(temp0_thresh_rising_en, "1");
 
@@ -267,17 +200,38 @@ static irqreturn_t ad5504_event_handler(int irq, void *private)
 static const struct iio_info ad5504_info = {
 	.write_raw = ad5504_write_raw,
 	.read_raw = ad5504_read_raw,
-	.attrs = &ad5504_attribute_group,
 	.event_attrs = &ad5504_ev_attribute_group,
 	.driver_module = THIS_MODULE,
 };
 
-static const struct iio_info ad5501_info = {
-	.write_raw = ad5504_write_raw,
-	.read_raw = ad5504_read_raw,
-	.attrs = &ad5501_attribute_group,
-	.event_attrs = &ad5504_ev_attribute_group,
-	.driver_module = THIS_MODULE,
+static const struct iio_chan_spec_ext_info ad5504_ext_info[] = {
+	{
+		.name = "powerdown",
+		.read = ad5504_read_dac_powerdown,
+		.write = ad5504_write_dac_powerdown,
+	},
+	IIO_ENUM("powerdown_mode", true, &ad5504_powerdown_mode_enum),
+	IIO_ENUM_AVAILABLE("powerdown_mode", &ad5504_powerdown_mode_enum),
+	{ },
+};
+
+#define AD5504_CHANNEL(_chan) { \
+	.type = IIO_VOLTAGE, \
+	.indexed = 1, \
+	.output = 1, \
+	.channel = (_chan), \
+	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT | \
+		     IIO_CHAN_INFO_SCALE_SHARED_BIT, \
+	.address = AD5504_ADDR_DAC(_chan), \
+	.scan_type = IIO_ST('u', 12, 16, 0), \
+	.ext_info = ad5504_ext_info, \
+}
+
+static const struct iio_chan_spec ad5504_channels[] = {
+	AD5504_CHANNEL(0),
+	AD5504_CHANNEL(1),
+	AD5504_CHANNEL(2),
+	AD5504_CHANNEL(3),
 };
 
 static int __devinit ad5504_probe(struct spi_device *spi)
@@ -315,13 +269,11 @@ static int __devinit ad5504_probe(struct spi_device *spi)
 	st->spi = spi;
 	indio_dev->dev.parent = &spi->dev;
 	indio_dev->name = spi_get_device_id(st->spi)->name;
-	if (spi_get_device_id(st->spi)->driver_data == ID_AD5501) {
-		indio_dev->info = &ad5501_info;
+	indio_dev->info = &ad5504_info;
+	if (spi_get_device_id(st->spi)->driver_data == ID_AD5501)
 		indio_dev->num_channels = 1;
-	} else {
-		indio_dev->info = &ad5504_info;
+	else
 		indio_dev->num_channels = 4;
-	}
 	indio_dev->channels = ad5504_channels;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [RFC 6/8] staging:iio:dac:ad5624r: Convert to extended channel attributes
  2012-05-11 15:53 [RFC 1/8] iio: Add helper functions for enum style channel attributes Lars-Peter Clausen
                   ` (3 preceding siblings ...)
  2012-05-11 15:53 ` [RFC 5/8] staging:iio:dac:ad5504: " Lars-Peter Clausen
@ 2012-05-11 15:53 ` Lars-Peter Clausen
  2012-05-14 18:16   ` Jonathan Cameron
  2012-05-11 15:53 ` [RFC 7/8] staging:iio:dac:ad5686: " Lars-Peter Clausen
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Lars-Peter Clausen @ 2012-05-11 15:53 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Lars-Peter Clausen

Use extended channel attributes instead of raw sysfs files for the additional
channel attributes. This allows us to remove some boilerplate code.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/dac/ad5624r_spi.c |  209 ++++++++++++++-------------------
 1 file changed, 91 insertions(+), 118 deletions(-)

diff --git a/drivers/staging/iio/dac/ad5624r_spi.c b/drivers/staging/iio/dac/ad5624r_spi.c
index 646b448..ef6a320 100644
--- a/drivers/staging/iio/dac/ad5624r_spi.c
+++ b/drivers/staging/iio/dac/ad5624r_spi.c
@@ -21,56 +21,6 @@
 
 #include "ad5624r.h"
 
-#define AD5624R_CHANNEL(_chan, _bits) { \
-	.type = IIO_VOLTAGE, \
-	.indexed = 1, \
-	.output = 1, \
-	.channel = (_chan), \
-	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT | \
-		     IIO_CHAN_INFO_SCALE_SHARED_BIT, \
-	.address = (_chan), \
-	.scan_type = IIO_ST('u', (_bits), 16, 16 - (_bits)), \
-}
-
-#define DECLARE_AD5624R_CHANNELS(_name, _bits) \
-	const struct iio_chan_spec _name##_channels[] = { \
-		AD5624R_CHANNEL(0, _bits), \
-		AD5624R_CHANNEL(1, _bits), \
-		AD5624R_CHANNEL(2, _bits), \
-		AD5624R_CHANNEL(3, _bits), \
-}
-
-static DECLARE_AD5624R_CHANNELS(ad5624r, 12);
-static DECLARE_AD5624R_CHANNELS(ad5644r, 14);
-static DECLARE_AD5624R_CHANNELS(ad5664r, 16);
-
-static const struct ad5624r_chip_info ad5624r_chip_info_tbl[] = {
-	[ID_AD5624R3] = {
-		.channels = ad5624r_channels,
-		.int_vref_mv = 1250,
-	},
-	[ID_AD5624R5] = {
-		.channels = ad5624r_channels,
-		.int_vref_mv = 2500,
-	},
-	[ID_AD5644R3] = {
-		.channels = ad5644r_channels,
-		.int_vref_mv = 1250,
-	},
-	[ID_AD5644R5] = {
-		.channels = ad5644r_channels,
-		.int_vref_mv = 2500,
-	},
-	[ID_AD5664R3] = {
-		.channels = ad5664r_channels,
-		.int_vref_mv = 1250,
-	},
-	[ID_AD5664R5] = {
-		.channels = ad5664r_channels,
-		.int_vref_mv = 2500,
-	},
-};
-
 static int ad5624r_spi_write(struct spi_device *spi,
 			     u8 cmd, u8 addr, u16 val, u8 len)
 {
@@ -138,67 +88,62 @@ static int ad5624r_write_raw(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
-static ssize_t ad5624r_read_powerdown_mode(struct device *dev,
-				      struct device_attribute *attr, char *buf)
+static const char * const ad5624r_powerdown_modes[] = {
+	"1kohm_to_gnd",
+	"100kohm_to_gnd",
+	"three_state"
+};
+
+static int ad5624r_get_powerdown_mode(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5624r_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 st->pwr_down_mode;
 }
 
-static ssize_t ad5624r_write_powerdown_mode(struct device *dev,
-				       struct device_attribute *attr,
-				       const char *buf, size_t len)
+static int ad5624r_set_powerdown_mode(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, unsigned int mode)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5624r_state *st = iio_priv(indio_dev);
-	int ret;
 
-	if (sysfs_streq(buf, "1kohm_to_gnd"))
-		st->pwr_down_mode = AD5624R_LDAC_PWRDN_1K;
-	else if (sysfs_streq(buf, "100kohm_to_gnd"))
-		st->pwr_down_mode = AD5624R_LDAC_PWRDN_100K;
-	else if (sysfs_streq(buf, "three_state"))
-		st->pwr_down_mode = AD5624R_LDAC_PWRDN_3STATE;
-	else
-		ret = -EINVAL;
+	st->pwr_down_mode = mode;
 
-	return ret ? ret : len;
+	return 0;
 }
 
-static ssize_t ad5624r_read_dac_powerdown(struct device *dev,
-					   struct device_attribute *attr,
-					   char *buf)
+static const struct iio_enum ad5624r_powerdown_mode_enum = {
+	.items = ad5624r_powerdown_modes,
+	.num_items = ARRAY_SIZE(ad5624r_powerdown_modes),
+	.get = ad5624r_get_powerdown_mode,
+	.set = ad5624r_set_powerdown_mode,
+};
+
+static ssize_t ad5624r_read_dac_powerdown(struct iio_dev *indio_dev,
+	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5624r_state *st = iio_priv(indio_dev);
-	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
 
 	return sprintf(buf, "%d\n",
-			!!(st->pwr_down_mask & (1 << this_attr->address)));
+			!!(st->pwr_down_mask & (1 << chan->channel)));
 }
 
-static ssize_t ad5624r_write_dac_powerdown(struct device *dev,
-					    struct device_attribute *attr,
-					    const char *buf, size_t len)
+static ssize_t ad5624r_write_dac_powerdown(struct iio_dev *indio_dev,
+	uintptr_t private, const struct iio_chan_spec *chan, const char *buf,
+	size_t len)
 {
 	long readin;
 	int ret;
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5624r_state *st = iio_priv(indio_dev);
-	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
 
 	ret = strict_strtol(buf, 10, &readin);
 	if (ret)
 		return ret;
 
 	if (readin == 1)
-		st->pwr_down_mask |= (1 << this_attr->address);
+		st->pwr_down_mask |= (1 << chan->channel);
 	else if (!readin)
-		st->pwr_down_mask &= ~(1 << this_attr->address);
+		st->pwr_down_mask &= ~(1 << chan->channel);
 	else
 		ret = -EINVAL;
 
@@ -209,47 +154,75 @@ static ssize_t ad5624r_write_dac_powerdown(struct device *dev,
 	return ret ? ret : len;
 }
 
-static IIO_DEVICE_ATTR(out_voltage_powerdown_mode, S_IRUGO |
-			S_IWUSR, ad5624r_read_powerdown_mode,
-			ad5624r_write_powerdown_mode, 0);
-
-static IIO_CONST_ATTR(out_voltage_powerdown_mode_available,
-			"1kohm_to_gnd 100kohm_to_gnd three_state");
-
-#define IIO_DEV_ATTR_DAC_POWERDOWN(_num, _show, _store, _addr)		\
-	IIO_DEVICE_ATTR(out_voltage##_num##_powerdown,			\
-			S_IRUGO | S_IWUSR, _show, _store, _addr)
-
-static IIO_DEV_ATTR_DAC_POWERDOWN(0, ad5624r_read_dac_powerdown,
-				   ad5624r_write_dac_powerdown, 0);
-static IIO_DEV_ATTR_DAC_POWERDOWN(1, ad5624r_read_dac_powerdown,
-				   ad5624r_write_dac_powerdown, 1);
-static IIO_DEV_ATTR_DAC_POWERDOWN(2, ad5624r_read_dac_powerdown,
-				   ad5624r_write_dac_powerdown, 2);
-static IIO_DEV_ATTR_DAC_POWERDOWN(3, ad5624r_read_dac_powerdown,
-				   ad5624r_write_dac_powerdown, 3);
-
-static struct attribute *ad5624r_attributes[] = {
-	&iio_dev_attr_out_voltage0_powerdown.dev_attr.attr,
-	&iio_dev_attr_out_voltage1_powerdown.dev_attr.attr,
-	&iio_dev_attr_out_voltage2_powerdown.dev_attr.attr,
-	&iio_dev_attr_out_voltage3_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 ad5624r_attribute_group = {
-	.attrs = ad5624r_attributes,
-};
-
 static const struct iio_info ad5624r_info = {
 	.write_raw = ad5624r_write_raw,
 	.read_raw = ad5624r_read_raw,
-	.attrs = &ad5624r_attribute_group,
 	.driver_module = THIS_MODULE,
 };
 
+static const struct iio_chan_spec_ext_info ad5624r_ext_info[] = {
+	{
+		.name = "powerdown",
+		.read = ad5624r_read_dac_powerdown,
+		.write = ad5624r_write_dac_powerdown,
+	},
+	IIO_ENUM("powerdown_mode", true, &ad5624r_powerdown_mode_enum),
+	IIO_ENUM_AVAILABLE("powerdown_mode", &ad5624r_powerdown_mode_enum),
+	{ },
+};
+
+#define AD5624R_CHANNEL(_chan, _bits) { \
+	.type = IIO_VOLTAGE, \
+	.indexed = 1, \
+	.output = 1, \
+	.channel = (_chan), \
+	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT | \
+		     IIO_CHAN_INFO_SCALE_SHARED_BIT, \
+	.address = (_chan), \
+	.scan_type = IIO_ST('u', (_bits), 16, 16 - (_bits)), \
+	.ext_info = ad5624r_ext_info, \
+}
+
+#define DECLARE_AD5624R_CHANNELS(_name, _bits) \
+	const struct iio_chan_spec _name##_channels[] = { \
+		AD5624R_CHANNEL(0, _bits), \
+		AD5624R_CHANNEL(1, _bits), \
+		AD5624R_CHANNEL(2, _bits), \
+		AD5624R_CHANNEL(3, _bits), \
+}
+
+static DECLARE_AD5624R_CHANNELS(ad5624r, 12);
+static DECLARE_AD5624R_CHANNELS(ad5644r, 14);
+static DECLARE_AD5624R_CHANNELS(ad5664r, 16);
+
+static const struct ad5624r_chip_info ad5624r_chip_info_tbl[] = {
+	[ID_AD5624R3] = {
+		.channels = ad5624r_channels,
+		.int_vref_mv = 1250,
+	},
+	[ID_AD5624R5] = {
+		.channels = ad5624r_channels,
+		.int_vref_mv = 2500,
+	},
+	[ID_AD5644R3] = {
+		.channels = ad5644r_channels,
+		.int_vref_mv = 1250,
+	},
+	[ID_AD5644R5] = {
+		.channels = ad5644r_channels,
+		.int_vref_mv = 2500,
+	},
+	[ID_AD5664R3] = {
+		.channels = ad5664r_channels,
+		.int_vref_mv = 1250,
+	},
+	[ID_AD5664R5] = {
+		.channels = ad5664r_channels,
+		.int_vref_mv = 2500,
+	},
+};
+
+
 static int __devinit ad5624r_probe(struct spi_device *spi)
 {
 	struct ad5624r_state *st;
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [RFC 7/8] staging:iio:dac:ad5686: Convert to extended channel attributes
  2012-05-11 15:53 [RFC 1/8] iio: Add helper functions for enum style channel attributes Lars-Peter Clausen
                   ` (4 preceding siblings ...)
  2012-05-11 15:53 ` [RFC 6/8] staging:iio:dac:ad5624r: " Lars-Peter Clausen
@ 2012-05-11 15:53 ` Lars-Peter Clausen
  2012-05-14 18:18   ` Jonathan Cameron
  2012-05-11 15:53 ` [RFC 8/8] staging:iio:dac:ad5791: " Lars-Peter Clausen
  2012-05-14 17:42 ` [RFC 1/8] iio: Add helper functions for enum style " Jonathan Cameron
  7 siblings, 1 reply; 17+ messages in thread
From: Lars-Peter Clausen @ 2012-05-11 15:53 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Lars-Peter Clausen

Use extended channel attributes instead of raw sysfs files for the additional
channel attributes. This allows us to remove some boilerplate code.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/dac/ad5686.c |  193 +++++++++++++++-----------------------
 1 file changed, 77 insertions(+), 116 deletions(-)

diff --git a/drivers/staging/iio/dac/ad5686.c b/drivers/staging/iio/dac/ad5686.c
index 46239b3..cf833c9 100644
--- a/drivers/staging/iio/dac/ad5686.c
+++ b/drivers/staging/iio/dac/ad5686.c
@@ -92,40 +92,6 @@ enum ad5686_supported_device_ids {
 	ID_AD5685,
 	ID_AD5686,
 };
-#define AD5868_CHANNEL(chan, bits, shift) {			\
-		.type = IIO_VOLTAGE,				\
-		.indexed = 1,					\
-		.output = 1,					\
-		.channel = chan,				\
-		.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |	\
-		IIO_CHAN_INFO_SCALE_SHARED_BIT,			\
-		.address = AD5686_ADDR_DAC(chan),			\
-		.scan_type = IIO_ST('u', bits, 16, shift)	\
-}
-static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
-	[ID_AD5684] = {
-		.channel[0] = AD5868_CHANNEL(0, 12, 4),
-		.channel[1] = AD5868_CHANNEL(1, 12, 4),
-		.channel[2] = AD5868_CHANNEL(2, 12, 4),
-		.channel[3] = AD5868_CHANNEL(3, 12, 4),
-		.int_vref_mv = 2500,
-	},
-	[ID_AD5685] = {
-		.channel[0] = AD5868_CHANNEL(0, 14, 2),
-		.channel[1] = AD5868_CHANNEL(1, 14, 2),
-		.channel[2] = AD5868_CHANNEL(2, 14, 2),
-		.channel[3] = AD5868_CHANNEL(3, 14, 2),
-		.int_vref_mv = 2500,
-	},
-	[ID_AD5686] = {
-		.channel[0] = AD5868_CHANNEL(0, 16, 0),
-		.channel[1] = AD5868_CHANNEL(1, 16, 0),
-		.channel[2] = AD5868_CHANNEL(2, 16, 0),
-		.channel[3] = AD5868_CHANNEL(3, 16, 0),
-		.int_vref_mv = 2500,
-	},
-};
-
 static int ad5686_spi_write(struct ad5686_state *st,
 			     u8 cmd, u8 addr, u16 val, u8 shift)
 {
@@ -169,73 +135,63 @@ static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
 	return be32_to_cpu(st->data[2].d32);
 }
 
-static ssize_t ad5686_read_powerdown_mode(struct device *dev,
-				      struct device_attribute *attr, char *buf)
+static const char * const ad5686_powerdown_modes[] = {
+	"1kohm_to_gnd",
+	"100kohm_to_gnd",
+	"three_state"
+};
+
+static int ad5686_get_powerdown_mode(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5686_state *st = iio_priv(indio_dev);
-	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
-
-	char mode[][15] = {"", "1kohm_to_gnd", "100kohm_to_gnd", "three_state"};
 
-	return sprintf(buf, "%s\n", mode[(st->pwr_down_mode >>
-					 (this_attr->address * 2)) & 0x3]);
+	return ((st->pwr_down_mode >> (chan->channel * 2)) & 0x3) - 1;
 }
 
-static ssize_t ad5686_write_powerdown_mode(struct device *dev,
-				       struct device_attribute *attr,
-				       const char *buf, size_t len)
+static int ad5686_set_powerdown_mode(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, unsigned int mode)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5686_state *st = iio_priv(indio_dev);
-	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
-	unsigned mode;
-
-	if (sysfs_streq(buf, "1kohm_to_gnd"))
-		mode = AD5686_LDAC_PWRDN_1K;
-	else if (sysfs_streq(buf, "100kohm_to_gnd"))
-		mode = AD5686_LDAC_PWRDN_100K;
-	else if (sysfs_streq(buf, "three_state"))
-		mode = AD5686_LDAC_PWRDN_3STATE;
-	else
-		return  -EINVAL;
 
-	st->pwr_down_mode &= ~(0x3 << (this_attr->address * 2));
-	st->pwr_down_mode |= (mode << (this_attr->address * 2));
+	st->pwr_down_mode &= ~(0x3 << (chan->channel * 2));
+	st->pwr_down_mode |= ((mode + 1) << (chan->channel * 2));
 
-	return len;
+	return 0;
 }
 
-static ssize_t ad5686_read_dac_powerdown(struct device *dev,
-					   struct device_attribute *attr,
-					   char *buf)
+static const struct iio_enum ad5686_powerdown_mode_enum = {
+	.items = ad5686_powerdown_modes,
+	.num_items = ARRAY_SIZE(ad5686_powerdown_modes),
+	.get = ad5686_get_powerdown_mode,
+	.set = ad5686_set_powerdown_mode,
+};
+
+static ssize_t ad5686_read_dac_powerdown(struct iio_dev *indio_dev,
+	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5686_state *st = iio_priv(indio_dev);
-	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
 
 	return sprintf(buf, "%d\n", !!(st->pwr_down_mask &
-			(0x3 << (this_attr->address * 2))));
+			(0x3 << (chan->channel * 2))));
 }
 
-static ssize_t ad5686_write_dac_powerdown(struct device *dev,
-					    struct device_attribute *attr,
-					    const char *buf, size_t len)
+static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev,
+	 uintptr_t private, const struct iio_chan_spec *chan, const char *buf,
+	 size_t len)
 {
 	bool readin;
 	int ret;
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5686_state *st = iio_priv(indio_dev);
-	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
 
 	ret = strtobool(buf, &readin);
 	if (ret)
 		return ret;
 
 	if (readin == true)
-		st->pwr_down_mask |= (0x3 << (this_attr->address * 2));
+		st->pwr_down_mask |= (0x3 << (chan->channel * 2));
 	else
-		st->pwr_down_mask &= ~(0x3 << (this_attr->address * 2));
+		st->pwr_down_mask &= ~(0x3 << (chan->channel * 2));
 
 	ret = ad5686_spi_write(st, AD5686_CMD_POWERDOWN_DAC, 0,
 			       st->pwr_down_mask & st->pwr_down_mode, 0);
@@ -243,48 +199,6 @@ static ssize_t ad5686_write_dac_powerdown(struct device *dev,
 	return ret ? ret : len;
 }
 
-static IIO_CONST_ATTR(out_voltage_powerdown_mode_available,
-			"1kohm_to_gnd 100kohm_to_gnd three_state");
-
-#define IIO_DEV_ATTR_DAC_POWERDOWN_MODE(_num)				\
-	IIO_DEVICE_ATTR(out_voltage##_num##_powerdown_mode,		\
-			S_IRUGO | S_IWUSR,				\
-			ad5686_read_powerdown_mode,			\
-			ad5686_write_powerdown_mode, _num)
-
-static IIO_DEV_ATTR_DAC_POWERDOWN_MODE(0);
-static IIO_DEV_ATTR_DAC_POWERDOWN_MODE(1);
-static IIO_DEV_ATTR_DAC_POWERDOWN_MODE(2);
-static IIO_DEV_ATTR_DAC_POWERDOWN_MODE(3);
-
-#define IIO_DEV_ATTR_DAC_POWERDOWN(_num)				\
-	IIO_DEVICE_ATTR(out_voltage##_num##_powerdown,			\
-			S_IRUGO | S_IWUSR,				\
-			ad5686_read_dac_powerdown,			\
-			ad5686_write_dac_powerdown, _num)
-
-static IIO_DEV_ATTR_DAC_POWERDOWN(0);
-static IIO_DEV_ATTR_DAC_POWERDOWN(1);
-static IIO_DEV_ATTR_DAC_POWERDOWN(2);
-static IIO_DEV_ATTR_DAC_POWERDOWN(3);
-
-static struct attribute *ad5686_attributes[] = {
-	&iio_dev_attr_out_voltage0_powerdown.dev_attr.attr,
-	&iio_dev_attr_out_voltage1_powerdown.dev_attr.attr,
-	&iio_dev_attr_out_voltage2_powerdown.dev_attr.attr,
-	&iio_dev_attr_out_voltage3_powerdown.dev_attr.attr,
-	&iio_dev_attr_out_voltage0_powerdown_mode.dev_attr.attr,
-	&iio_dev_attr_out_voltage1_powerdown_mode.dev_attr.attr,
-	&iio_dev_attr_out_voltage2_powerdown_mode.dev_attr.attr,
-	&iio_dev_attr_out_voltage3_powerdown_mode.dev_attr.attr,
-	&iio_const_attr_out_voltage_powerdown_mode_available.dev_attr.attr,
-	NULL,
-};
-
-static const struct attribute_group ad5686_attribute_group = {
-	.attrs = ad5686_attributes,
-};
-
 static int ad5686_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val,
@@ -348,10 +262,57 @@ static int ad5686_write_raw(struct iio_dev *indio_dev,
 static const struct iio_info ad5686_info = {
 	.read_raw = ad5686_read_raw,
 	.write_raw = ad5686_write_raw,
-	.attrs = &ad5686_attribute_group,
 	.driver_module = THIS_MODULE,
 };
 
+static const struct iio_chan_spec_ext_info ad5686_ext_info[] = {
+	{
+		.name = "powerdown",
+		.read = ad5686_read_dac_powerdown,
+		.write = ad5686_write_dac_powerdown,
+	},
+	IIO_ENUM("powerdown_mode", false, &ad5686_powerdown_mode_enum),
+	IIO_ENUM_AVAILABLE("powerdown_mode", &ad5686_powerdown_mode_enum),
+	{ },
+};
+
+#define AD5868_CHANNEL(chan, bits, shift) {			\
+		.type = IIO_VOLTAGE,				\
+		.indexed = 1,					\
+		.output = 1,					\
+		.channel = chan,				\
+		.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |	\
+		IIO_CHAN_INFO_SCALE_SHARED_BIT,			\
+		.address = AD5686_ADDR_DAC(chan),			\
+		.scan_type = IIO_ST('u', bits, 16, shift),	\
+		.ext_info = ad5686_ext_info,			\
+}
+
+static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
+	[ID_AD5684] = {
+		.channel[0] = AD5868_CHANNEL(0, 12, 4),
+		.channel[1] = AD5868_CHANNEL(1, 12, 4),
+		.channel[2] = AD5868_CHANNEL(2, 12, 4),
+		.channel[3] = AD5868_CHANNEL(3, 12, 4),
+		.int_vref_mv = 2500,
+	},
+	[ID_AD5685] = {
+		.channel[0] = AD5868_CHANNEL(0, 14, 2),
+		.channel[1] = AD5868_CHANNEL(1, 14, 2),
+		.channel[2] = AD5868_CHANNEL(2, 14, 2),
+		.channel[3] = AD5868_CHANNEL(3, 14, 2),
+		.int_vref_mv = 2500,
+	},
+	[ID_AD5686] = {
+		.channel[0] = AD5868_CHANNEL(0, 16, 0),
+		.channel[1] = AD5868_CHANNEL(1, 16, 0),
+		.channel[2] = AD5868_CHANNEL(2, 16, 0),
+		.channel[3] = AD5868_CHANNEL(3, 16, 0),
+		.int_vref_mv = 2500,
+	},
+};
+
+
 static int __devinit ad5686_probe(struct spi_device *spi)
 {
 	struct ad5686_state *st;
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [RFC 8/8] staging:iio:dac:ad5791: Convert to extended channel attributes
  2012-05-11 15:53 [RFC 1/8] iio: Add helper functions for enum style channel attributes Lars-Peter Clausen
                   ` (5 preceding siblings ...)
  2012-05-11 15:53 ` [RFC 7/8] staging:iio:dac:ad5686: " Lars-Peter Clausen
@ 2012-05-11 15:53 ` Lars-Peter Clausen
  2012-05-14 18:21   ` Jonathan Cameron
  2012-05-14 17:42 ` [RFC 1/8] iio: Add helper functions for enum style " Jonathan Cameron
  7 siblings, 1 reply; 17+ messages in thread
From: Lars-Peter Clausen @ 2012-05-11 15:53 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Lars-Peter Clausen

Use extended channel attributes instead of raw sysfs files for the additional
channel attributes. This allows us to remove some boilerplate code.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/dac/ad5791.c |  122 +++++++++++++++++---------------------
 1 file changed, 53 insertions(+), 69 deletions(-)

diff --git a/drivers/staging/iio/dac/ad5791.c b/drivers/staging/iio/dac/ad5791.c
index 8628964..4e955ed 100644
--- a/drivers/staging/iio/dac/ad5791.c
+++ b/drivers/staging/iio/dac/ad5791.c
@@ -72,71 +72,50 @@ static int ad5791_spi_read(struct spi_device *spi, u8 addr, u32 *val)
 	return ret;
 }
 
-#define AD5791_CHAN(bits, shift) {			\
-	.type = IIO_VOLTAGE,				\
-	.output = 1,					\
-	.indexed = 1,					\
-	.address = AD5791_ADDR_DAC0,			\
-	.channel = 0,					\
-	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |	\
-		IIO_CHAN_INFO_SCALE_SHARED_BIT |	\
-		IIO_CHAN_INFO_OFFSET_SHARED_BIT,	\
-	.scan_type = IIO_ST('u', bits, 24, shift)	\
-}
-
-static const struct iio_chan_spec ad5791_channels[] = {
-	[ID_AD5760] = AD5791_CHAN(16, 4),
-	[ID_AD5780] = AD5791_CHAN(18, 2),
-	[ID_AD5781] = AD5791_CHAN(18, 2),
-	[ID_AD5791] = AD5791_CHAN(20, 0)
+static const char * const ad5791_powerdown_modes[] = {
+	"6kohm_to_gnd",
+	"three_state",
 };
 
-static ssize_t ad5791_read_powerdown_mode(struct device *dev,
-				      struct device_attribute *attr, char *buf)
+static int ad5791_get_powerdown_mode(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5791_state *st = iio_priv(indio_dev);
 
-	const char mode[][14] = {"6kohm_to_gnd", "three_state"};
-
-	return sprintf(buf, "%s\n", mode[st->pwr_down_mode]);
+	return st->pwr_down_mode;
 }
 
-static ssize_t ad5791_write_powerdown_mode(struct device *dev,
-				       struct device_attribute *attr,
-				       const char *buf, size_t len)
+static int ad5791_set_powerdown_mode(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, unsigned int mode)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5791_state *st = iio_priv(indio_dev);
-	int ret;
 
-	if (sysfs_streq(buf, "6kohm_to_gnd"))
-		st->pwr_down_mode = AD5791_DAC_PWRDN_6K;
-	else if (sysfs_streq(buf, "three_state"))
-		st->pwr_down_mode = AD5791_DAC_PWRDN_3STATE;
-	else
-		ret = -EINVAL;
+	st->pwr_down_mode = mode;
 
-	return ret ? ret : len;
+	return 0;
 }
 
-static ssize_t ad5791_read_dac_powerdown(struct device *dev,
-					   struct device_attribute *attr,
-					   char *buf)
+static const struct iio_enum ad5791_powerdown_mode_enum = {
+	.items = ad5791_powerdown_modes,
+	.num_items = ARRAY_SIZE(ad5791_powerdown_modes),
+	.get = ad5791_get_powerdown_mode,
+	.set = ad5791_set_powerdown_mode,
+};
+
+static ssize_t ad5791_read_dac_powerdown(struct iio_dev *indio_dev,
+	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5791_state *st = iio_priv(indio_dev);
 
 	return sprintf(buf, "%d\n", st->pwr_down);
 }
 
-static ssize_t ad5791_write_dac_powerdown(struct device *dev,
-					    struct device_attribute *attr,
-					    const char *buf, size_t len)
+static ssize_t ad5791_write_dac_powerdown(struct iio_dev *indio_dev,
+	 uintptr_t private, const struct iio_chan_spec *chan, const char *buf,
+	 size_t len)
 {
 	long readin;
 	int ret;
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct ad5791_state *st = iio_priv(indio_dev);
 
 	ret = strict_strtol(buf, 10, &readin);
@@ -160,31 +139,6 @@ static ssize_t ad5791_write_dac_powerdown(struct device *dev,
 	return ret ? ret : len;
 }
 
-static IIO_DEVICE_ATTR(out_voltage_powerdown_mode, S_IRUGO |
-			S_IWUSR, ad5791_read_powerdown_mode,
-			ad5791_write_powerdown_mode, 0);
-
-static IIO_CONST_ATTR(out_voltage_powerdown_mode_available,
-			"6kohm_to_gnd three_state");
-
-#define IIO_DEV_ATTR_DAC_POWERDOWN(_num, _show, _store, _addr)		\
-	IIO_DEVICE_ATTR(out_voltage##_num##_powerdown,			\
-			S_IRUGO | S_IWUSR, _show, _store, _addr)
-
-static IIO_DEV_ATTR_DAC_POWERDOWN(0, ad5791_read_dac_powerdown,
-				   ad5791_write_dac_powerdown, 0);
-
-static struct attribute *ad5791_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 ad5791_attribute_group = {
-	.attrs = ad5791_attributes,
-};
-
 static int ad5791_get_lin_comp(unsigned int span)
 {
 	if (span <= 10000)
@@ -254,6 +208,37 @@ static int ad5791_read_raw(struct iio_dev *indio_dev,
 
 };
 
+static const struct iio_chan_spec_ext_info ad5791_ext_info[] = {
+	{
+		.name = "powerdown",
+		.shared = true,
+		.read = ad5791_read_dac_powerdown,
+		.write = ad5791_write_dac_powerdown,
+	},
+	IIO_ENUM("powerdown_mode", true, &ad5791_powerdown_mode_enum),
+	IIO_ENUM_AVAILABLE("powerdown_mode", &ad5791_powerdown_mode_enum),
+	{ },
+};
+
+#define AD5791_CHAN(bits, shift) {			\
+	.type = IIO_VOLTAGE,				\
+	.output = 1,					\
+	.indexed = 1,					\
+	.address = AD5791_ADDR_DAC0,			\
+	.channel = 0,					\
+	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |	\
+		IIO_CHAN_INFO_SCALE_SHARED_BIT |	\
+		IIO_CHAN_INFO_OFFSET_SHARED_BIT,	\
+	.scan_type = IIO_ST('u', bits, 24, shift),	\
+	.ext_info = ad5791_ext_info,			\
+}
+
+static const struct iio_chan_spec ad5791_channels[] = {
+	[ID_AD5760] = AD5791_CHAN(16, 4),
+	[ID_AD5780] = AD5791_CHAN(18, 2),
+	[ID_AD5781] = AD5791_CHAN(18, 2),
+	[ID_AD5791] = AD5791_CHAN(20, 0)
+};
 
 static int ad5791_write_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
@@ -278,7 +263,6 @@ static int ad5791_write_raw(struct iio_dev *indio_dev,
 static const struct iio_info ad5791_info = {
 	.read_raw = &ad5791_read_raw,
 	.write_raw = &ad5791_write_raw,
-	.attrs = &ad5791_attribute_group,
 	.driver_module = THIS_MODULE,
 };
 
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [RFC 1/8] iio: Add helper functions for enum style channel attributes
  2012-05-11 15:53 [RFC 1/8] iio: Add helper functions for enum style channel attributes Lars-Peter Clausen
                   ` (6 preceding siblings ...)
  2012-05-11 15:53 ` [RFC 8/8] staging:iio:dac:ad5791: " Lars-Peter Clausen
@ 2012-05-14 17:42 ` Jonathan Cameron
  2012-05-14 18:15   ` Lars-Peter Clausen
  7 siblings, 1 reply; 17+ messages in thread
From: Jonathan Cameron @ 2012-05-14 17:42 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Jonathan Cameron, linux-iio

On 05/11/2012 04:53 PM, Lars-Peter Clausen wrote:
> We often have the case were we do have a enum style channel attribute. These
> attributes have in common that they are a list of string values which usually
> map in a 1-to-1 fashion to integer values.
> 
> This patch implements some common helper code for implementing enum style
> channel attributes using extended channel attributes. The helper functions take
> care of converting between the string and integer values, as well providing a
> function for "_available" attributes which list all available enum items.
> 
General principal is good, but needs some documentation :)

Also couple of trivial points inline.

> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/iio/industrialio-core.c |   57 +++++++++++++++++++++++++++++++++++++++
>  include/linux/iio/iio.h         |   32 ++++++++++++++++++++++
>  2 files changed, 89 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index b39a587..241be03 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -288,6 +288,63 @@ static ssize_t iio_write_channel_ext_info(struct device *dev,
>  			       this_attr->c, buf, len);
>  }
>  
> +ssize_t iio_enum_available_read(struct iio_dev *indio_dev,
> +	uintptr_t priv, const struct iio_chan_spec *chan, char *buf)
> +{
> +	const struct iio_enum *e = (const struct iio_enum *)priv;
> +	char *tmp = buf;
> +	unsigned int i;
> +
> +	if (!e->num_items)
> +		return 0;
> +
Best to add some santify checking on the length of the resulting string
given it's technically unbounded.
> +	for (i = 0; i < e->num_items; ++i)
> +		tmp += sprintf(tmp, "%s ", e->items[i]);
> +
> +	/* replace last space with a newline */
> +	*(tmp - 1) = '\n';
> +
> +	return tmp - buf;
> +}
> +EXPORT_SYMBOL_GPL(iio_enum_available_read);
> +
> +ssize_t iio_enum_read(struct iio_dev *indio_dev,
> +	uintptr_t priv, const struct iio_chan_spec *chan, char *buf)
> +{
> +	const struct iio_enum *e = (const struct iio_enum *)priv;
> +	int i;
> +
> +	i = e->get(indio_dev, chan);
> +	if (i < 0)
> +		return i;
> +	else if (i >= e->num_items)
> +		return -EINVAL;
> +
> +	return sprintf(buf, "%s\n", e->items[i]);
> +}
> +EXPORT_SYMBOL_GPL(iio_enum_read);
> +
> +ssize_t iio_enum_write(struct iio_dev *indio_dev,
> +	uintptr_t priv, const struct iio_chan_spec *chan, const char *buf,
> +	size_t len)
> +{
> +	const struct iio_enum *e = (const struct iio_enum *)priv;
> +	unsigned int i;
> +	int ret;
> +
excess brackets.
> +	for (i = 0; i < e->num_items; i++) {
> +		if (sysfs_streq(buf, e->items[i]))
> +			break;
> +	}
> +
> +	if (i == e->num_items)
> +		return -EINVAL;
> +
> +	ret = e->set(indio_dev, chan, i);
> +	return ret ? ret : len;
> +}
> +EXPORT_SYMBOL_GPL(iio_enum_write);
> +
>  static ssize_t iio_read_channel_info(struct device *dev,
>  				     struct device_attribute *attr,
>  				     char *buf)
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 6fdbdb8..602f118 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -124,6 +124,38 @@ struct iio_chan_spec_ext_info {
>  	uintptr_t private;
>  };
>  
> +struct iio_enum {
> +	const char * const *items;
> +	unsigned int num_items;
> +	int (*set)(struct iio_dev *, const struct iio_chan_spec *, unsigned int);
> +	int (*get)(struct iio_dev *, const struct iio_chan_spec *);
> +};
> +
> +ssize_t iio_enum_available_read(struct iio_dev *indio_dev,
> +	uintptr_t priv, const struct iio_chan_spec *chan, char *buf);
> +ssize_t iio_enum_read(struct iio_dev *indio_dev,
> +	uintptr_t priv, const struct iio_chan_spec *chan, char *buf);
> +ssize_t iio_enum_write(struct iio_dev *indio_dev,
> +	uintptr_t priv, const struct iio_chan_spec *chan, const char *buf,
> +	size_t len);
> +
Document these two...
> +#define IIO_ENUM(_name, _shared, _e) \
> +{ \
> +	.name = (_name), \
> +	.shared = (_shared), \
> +	.read = iio_enum_read, \
> +	.write = iio_enum_write, \
> +	.private = (uintptr_t)(_e), \
> +}
> +
> +#define IIO_ENUM_AVAILABLE(_name, _e) \
> +{ \
> +	.name = (_name "_available"), \
> +	.shared = true, \
> +	.read = iio_enum_available_read, \
> +	.private = (uintptr_t)(_e), \
> +}
> +
>  /**
>   * struct iio_chan_spec - specification of a single channel
>   * @type:		What type of measurement is the channel making.

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC 2/8] staging:iio:dac:ad5064: Use iio_enum for powerdown modes
  2012-05-11 15:53 ` [RFC 2/8] staging:iio:dac:ad5064: Use iio_enum for powerdown modes Lars-Peter Clausen
@ 2012-05-14 17:48   ` Jonathan Cameron
  0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2012-05-14 17:48 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Jonathan Cameron, linux-iio

On 05/11/2012 04:53 PM, Lars-Peter Clausen wrote:
> This allows us to remove some boilerplate code for comparing and formatting the
> enum strings.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/staging/iio/dac/ad5064.c |   65 ++++++++++++--------------------------
>  1 file changed, 21 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/staging/iio/dac/ad5064.c b/drivers/staging/iio/dac/ad5064.c
> index da0fa12..276af02 100644
> --- a/drivers/staging/iio/dac/ad5064.c
> +++ b/drivers/staging/iio/dac/ad5064.c
> @@ -135,57 +135,42 @@ static int ad5064_sync_powerdown_mode(struct ad5064_state *st,
>  	return ret;
>  }
>  
> -static const char ad5064_powerdown_modes[][15] = {
> -	[AD5064_LDAC_PWRDN_NONE]	= "",
> -	[AD5064_LDAC_PWRDN_1K]		= "1kohm_to_gnd",
> -	[AD5064_LDAC_PWRDN_100K]	= "100kohm_to_gnd",
> -	[AD5064_LDAC_PWRDN_3STATE]	= "three_state",
> +static const char * const ad5064_powerdown_modes[] = {
> +	"1kohm_to_gnd",
> +	"100kohm_to_gnd",
> +	"three_state",
>  };
>  
> -static ssize_t ad5064_read_powerdown_mode_available(struct iio_dev *indio_dev,
> -	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
> -{
> -	return sprintf(buf, "%s %s %s\n", ad5064_powerdown_modes[1],
> -		ad5064_powerdown_modes[2], ad5064_powerdown_modes[3]);
> -}
> -
> -static ssize_t ad5064_read_powerdown_mode(struct iio_dev *indio_dev,
> -	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
> +static int ad5064_get_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan)
>  {
>  	struct ad5064_state *st = iio_priv(indio_dev);
>  
> -	return sprintf(buf, "%s\n",
> -		ad5064_powerdown_modes[st->pwr_down_mode[chan->channel]]);
> +	return st->pwr_down_mode[chan->channel] - 1;
>  }
>  
> -static ssize_t ad5064_write_powerdown_mode(struct iio_dev *indio_dev,
> -	uintptr_t private, const struct iio_chan_spec *chan, const char *buf,
> -	size_t len)
> +static int ad5064_set_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, unsigned int mode)
>  {
>  	struct ad5064_state *st = iio_priv(indio_dev);
> -	unsigned int mode, i;
>  	int ret;
>  
> -	mode = 0;
> -
> -	for (i = 1; i < ARRAY_SIZE(ad5064_powerdown_modes); ++i) {
> -		if (sysfs_streq(buf, ad5064_powerdown_modes[i])) {
> -			mode = i;
> -			break;
> -		}
> -	}
> -	if (mode == 0)
> -		return  -EINVAL;
> -
>  	mutex_lock(&indio_dev->mlock);
> -	st->pwr_down_mode[chan->channel] = mode;
> +	st->pwr_down_mode[chan->channel] = mode + 1;
>  
>  	ret = ad5064_sync_powerdown_mode(st, chan->channel);
>  	mutex_unlock(&indio_dev->mlock);
>  
> -	return ret ? ret : len;
> +	return ret;
>  }
>  
> +static const struct iio_enum ad5064_powerdown_mode_enum = {
> +	.items = ad5064_powerdown_modes,
> +	.num_items = ARRAY_SIZE(ad5064_powerdown_modes),
> +	.get = ad5064_get_powerdown_mode,
> +	.set = ad5064_set_powerdown_mode,
> +};
> +
>  static ssize_t ad5064_read_dac_powerdown(struct iio_dev *indio_dev,
>  	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
>  {
> @@ -285,22 +270,14 @@ static const struct iio_info ad5064_info = {
>  	.driver_module = THIS_MODULE,
>  };
>  
> -static struct iio_chan_spec_ext_info ad5064_ext_info[] = {
> +static const struct iio_chan_spec_ext_info ad5064_ext_info[] = {
>  	{
>  		.name = "powerdown",
>  		.read = ad5064_read_dac_powerdown,
>  		.write = ad5064_write_dac_powerdown,
>  	},
> -	{
> -		.name = "powerdown_mode",
> -		.read = ad5064_read_powerdown_mode,
> -		.write = ad5064_write_powerdown_mode,
> -	},
> -	{
> -		.name = "powerdown_mode_available",
> -		.shared = true,
> -		.read = ad5064_read_powerdown_mode_available,
> -	},
> +	IIO_ENUM("powerdown_mode", false, &ad5064_powerdown_mode_enum),
> +	IIO_ENUM_AVAILABLE("powerdown_mode", &ad5064_powerdown_mode_enum),
>  	{ },
>  };
>  

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC 3/8] staging:iio:dac:ad5446: Use iio_enum for powerdown modes
  2012-05-11 15:53 ` [RFC 3/8] staging:iio:dac:ad5446: " Lars-Peter Clausen
@ 2012-05-14 17:55   ` Jonathan Cameron
  0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2012-05-14 17:55 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Jonathan Cameron, linux-iio

On 05/11/2012 04:53 PM, Lars-Peter Clausen wrote:
> This allows us to remove some boilerplate code for comparing and formatting the
> enum strings.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/staging/iio/dac/ad5446.c |   53 ++++++++++++--------------------------
>  1 file changed, 17 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/staging/iio/dac/ad5446.c b/drivers/staging/iio/dac/ad5446.c
> index 3e53d0c..f769b74 100644
> --- a/drivers/staging/iio/dac/ad5446.c
> +++ b/drivers/staging/iio/dac/ad5446.c
> @@ -41,47 +41,34 @@ static int ad5660_write(struct ad5446_state *st, unsigned val)
>  }
>  
>  static const char * const ad5446_powerdown_modes[] = {
> -	"", "1kohm_to_gnd", "100kohm_to_gnd", "three_state"
> +	"1kohm_to_gnd", "100kohm_to_gnd", "three_state"
>  };
>  
> -static ssize_t ad5446_read_powerdown_mode_available(struct iio_dev *indio_dev,
> -	uintptr_t private, 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,
> -					    uintptr_t private,
> -					    const struct iio_chan_spec *chan,
> -					    const char *buf, size_t len)
> +static int ad5446_set_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, unsigned int mode)
>  {
>  	struct ad5446_state *st = iio_priv(indio_dev);
> -	int i;
> -
> -	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;
> +	st->pwr_down_mode = mode + 1;
>  
> -	return len;
> +	return 0;
>  }
>  
> -static ssize_t ad5446_read_powerdown_mode(struct iio_dev *indio_dev,
> -					   uintptr_t private,
> -					   const struct iio_chan_spec *chan,
> -					   char *buf)
> +static int ad5446_get_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan)
>  {
>  	struct ad5446_state *st = iio_priv(indio_dev);
>  
> -	return sprintf(buf, "%s\n", ad5446_powerdown_modes[st->pwr_down_mode]);
> +	return st->pwr_down_mode - 1;
>  }
>  
> +static const struct iio_enum ad5446_powerdown_mode_enum = {
> +	.items = ad5446_powerdown_modes,
> +	.num_items = ARRAY_SIZE(ad5446_powerdown_modes),
> +	.get = ad5446_get_powerdown_mode,
> +	.set = ad5446_set_powerdown_mode,
> +};
> +
>  static ssize_t ad5446_read_dac_powerdown(struct iio_dev *indio_dev,
>  					   uintptr_t private,
>  					   const struct iio_chan_spec *chan,
> @@ -128,15 +115,9 @@ 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,
>  	},
> +	IIO_ENUM("powerdown_mode", false, &ad5446_powerdown_mode_enum),
> +	IIO_ENUM_AVAILABLE("powerdown_mode", &ad5446_powerdown_mode_enum),
>  	{ },
>  };
>  

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC 4/8] staging:iio:dac:ad5380: Convert to extended channel attributes
  2012-05-11 15:53 ` [RFC 4/8] staging:iio:dac:ad5380: Convert to extended channel attributes Lars-Peter Clausen
@ 2012-05-14 18:11   ` Jonathan Cameron
  0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2012-05-14 18:11 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Jonathan Cameron, linux-iio

On 05/11/2012 04:53 PM, Lars-Peter Clausen wrote:
> Use extended channel attributes instead of raw sysfs files for the additional
> channel attributes. This allows us to remove some boilerplate code.
> 
Looks good.
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/staging/iio/dac/ad5380.c |  250 ++++++++++++++++++--------------------
>  1 file changed, 116 insertions(+), 134 deletions(-)
> 
> diff --git a/drivers/staging/iio/dac/ad5380.c b/drivers/staging/iio/dac/ad5380.c
> index 1baaabe..5dfb445 100644
> --- a/drivers/staging/iio/dac/ad5380.c
> +++ b/drivers/staging/iio/dac/ad5380.c
> @@ -79,103 +79,18 @@ enum ad5380_type {
>  	ID_AD5392_5,
>  };
>  
> -#define AD5380_CHANNEL(_bits) {					\
> -	.type = IIO_VOLTAGE,					\
> -	.indexed = 1,						\
> -	.output = 1,						\
> -	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |		\
> -		IIO_CHAN_INFO_SCALE_SHARED_BIT |		\
> -		IIO_CHAN_INFO_CALIBSCALE_SEPARATE_BIT |		\
> -		IIO_CHAN_INFO_CALIBBIAS_SEPARATE_BIT,		\
> -	.scan_type = IIO_ST('u', (_bits), 16, 14 - (_bits))	\
> -}
> -
> -static const struct ad5380_chip_info ad5380_chip_info_tbl[] = {
> -	[ID_AD5380_3] = {
> -		.channel_template = AD5380_CHANNEL(14),
> -		.num_channels = 40,
> -		.int_vref = 1250000,
> -	},
> -	[ID_AD5380_5] = {
> -		.channel_template = AD5380_CHANNEL(14),
> -		.num_channels = 40,
> -		.int_vref = 2500000,
> -	},
> -	[ID_AD5381_3] = {
> -		.channel_template = AD5380_CHANNEL(12),
> -		.num_channels = 16,
> -		.int_vref = 1250000,
> -	},
> -	[ID_AD5381_5] = {
> -		.channel_template = AD5380_CHANNEL(12),
> -		.num_channels = 16,
> -		.int_vref = 2500000,
> -	},
> -	[ID_AD5382_3] = {
> -		.channel_template = AD5380_CHANNEL(14),
> -		.num_channels = 32,
> -		.int_vref = 1250000,
> -	},
> -	[ID_AD5382_5] = {
> -		.channel_template = AD5380_CHANNEL(14),
> -		.num_channels = 32,
> -		.int_vref = 2500000,
> -	},
> -	[ID_AD5383_3] = {
> -		.channel_template = AD5380_CHANNEL(12),
> -		.num_channels = 32,
> -		.int_vref = 1250000,
> -	},
> -	[ID_AD5383_5] = {
> -		.channel_template = AD5380_CHANNEL(12),
> -		.num_channels = 32,
> -		.int_vref = 2500000,
> -	},
> -	[ID_AD5390_3] = {
> -		.channel_template = AD5380_CHANNEL(14),
> -		.num_channels = 16,
> -		.int_vref = 1250000,
> -	},
> -	[ID_AD5390_5] = {
> -		.channel_template = AD5380_CHANNEL(14),
> -		.num_channels = 16,
> -		.int_vref = 2500000,
> -	},
> -	[ID_AD5391_3] = {
> -		.channel_template = AD5380_CHANNEL(12),
> -		.num_channels = 16,
> -		.int_vref = 1250000,
> -	},
> -	[ID_AD5391_5] = {
> -		.channel_template = AD5380_CHANNEL(12),
> -		.num_channels = 16,
> -		.int_vref = 2500000,
> -	},
> -	[ID_AD5392_3] = {
> -		.channel_template = AD5380_CHANNEL(14),
> -		.num_channels = 8,
> -		.int_vref = 1250000,
> -	},
> -	[ID_AD5392_5] = {
> -		.channel_template = AD5380_CHANNEL(14),
> -		.num_channels = 8,
> -		.int_vref = 2500000,
> -	},
> -};
> -
> -static ssize_t ad5380_read_dac_powerdown(struct device *dev,
> -	struct device_attribute *attr, char *buf)
> +static ssize_t ad5380_read_dac_powerdown(struct iio_dev *indio_dev,
> +	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5380_state *st = iio_priv(indio_dev);
>  
>  	return sprintf(buf, "%d\n", st->pwr_down);
>  }
>  
> -static ssize_t ad5380_write_dac_powerdown(struct device *dev,
> -	struct device_attribute *attr, const char *buf, size_t len)
> +static ssize_t ad5380_write_dac_powerdown(struct iio_dev *indio_dev,
> +	 uintptr_t private, const struct iio_chan_spec *chan, const char *buf,
> +	 size_t len)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5380_state *st = iio_priv(indio_dev);
>  	bool pwr_down;
>  	int ret;
> @@ -198,20 +113,14 @@ static ssize_t ad5380_write_dac_powerdown(struct device *dev,
>  	return ret ? ret : len;
>  }
>  
> -static IIO_DEVICE_ATTR(out_voltage_powerdown,
> -			S_IRUGO | S_IWUSR,
> -			ad5380_read_dac_powerdown,
> -			ad5380_write_dac_powerdown, 0);
> -
> -static const char ad5380_powerdown_modes[][15] = {
> -	[0]	= "100kohm_to_gnd",
> -	[1]	= "three_state",
> +static const char * const ad5380_powerdown_modes[] = {
> +	"100kohm_to_gnd",
> +	"three_state",
>  };

>  
> -static ssize_t ad5380_read_powerdown_mode(struct device *dev,
> -	struct device_attribute *attr, char *buf)
> +static int ad5380_get_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5380_state *st = iio_priv(indio_dev);
>  	unsigned int mode;
>  	int ret;
> @@ -222,49 +131,27 @@ static ssize_t ad5380_read_powerdown_mode(struct device *dev,
>  
>  	mode = (mode >> AD5380_CTRL_PWR_DOWN_MODE_OFFSET) & 1;
>  
> -	return sprintf(buf, "%s\n", ad5380_powerdown_modes[mode]);
> +	return mode;
>  }
>  
> -static ssize_t ad5380_write_powerdown_mode(struct device *dev,
> -	struct device_attribute *attr, const char *buf, size_t len)
> +static int ad5380_set_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, unsigned int mode)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5380_state *st = iio_priv(indio_dev);
> -	unsigned int i;
>  	int ret;
>  
> -	for (i = 0; i < ARRAY_SIZE(ad5380_powerdown_modes); ++i) {
> -		if (sysfs_streq(buf, ad5380_powerdown_modes[i]))
> -			break;
> -	}
> -
> -	if (i == ARRAY_SIZE(ad5380_powerdown_modes))
> -		return -EINVAL;
> -
>  	ret = regmap_update_bits(st->regmap, AD5380_REG_SF_CTRL,
>  		1 << AD5380_CTRL_PWR_DOWN_MODE_OFFSET,
> -		i << AD5380_CTRL_PWR_DOWN_MODE_OFFSET);
> +		mode << AD5380_CTRL_PWR_DOWN_MODE_OFFSET);
>  
> -	return ret ? ret : len;
> +	return ret;
>  }
>  
> -static IIO_DEVICE_ATTR(out_voltage_powerdown_mode,
> -			S_IRUGO | S_IWUSR,
> -			ad5380_read_powerdown_mode,
> -			ad5380_write_powerdown_mode, 0);
> -
> -static IIO_CONST_ATTR(out_voltage_powerdown_mode_available,
> -			"100kohm_to_gnd three_state");
> -
> -static struct attribute *ad5380_attributes[] = {
> -	&iio_dev_attr_out_voltage_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 ad5380_attribute_group = {
> -	.attrs = ad5380_attributes,
> +static const struct iio_enum ad5380_powerdown_mode_enum = {
> +	.items = ad5380_powerdown_modes,
> +	.num_items = ARRAY_SIZE(ad5380_powerdown_modes),
> +	.get = ad5380_get_powerdown_mode,
> +	.set = ad5380_set_powerdown_mode,
>  };
>  
>  static unsigned int ad5380_info_to_reg(struct iio_chan_spec const *chan,
> @@ -352,10 +239,105 @@ static int ad5380_read_raw(struct iio_dev *indio_dev,
>  static const struct iio_info ad5380_info = {
>  	.read_raw = ad5380_read_raw,
>  	.write_raw = ad5380_write_raw,
> -	.attrs = &ad5380_attribute_group,
>  	.driver_module = THIS_MODULE,
>  };
>  
> +static struct iio_chan_spec_ext_info ad5380_ext_info[] = {
> +	{
> +		.name = "powerdown",
> +		.read = ad5380_read_dac_powerdown,
> +		.write = ad5380_write_dac_powerdown,
> +	},
> +	IIO_ENUM("powerdown_mode", true, &ad5380_powerdown_mode_enum),
> +	IIO_ENUM_AVAILABLE("powerdown_mode", &ad5380_powerdown_mode_enum),
> +	{ },
> +};
> +
> +#define AD5380_CHANNEL(_bits) {					\
> +	.type = IIO_VOLTAGE,					\
> +	.indexed = 1,						\
> +	.output = 1,						\
> +	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |		\
> +		IIO_CHAN_INFO_SCALE_SHARED_BIT |		\
> +		IIO_CHAN_INFO_CALIBSCALE_SEPARATE_BIT |		\
> +		IIO_CHAN_INFO_CALIBBIAS_SEPARATE_BIT,		\
> +	.scan_type = IIO_ST('u', (_bits), 16, 14 - (_bits)),	\
> +	.ext_info = ad5380_ext_info,				\
> +}
> +
> +static const struct ad5380_chip_info ad5380_chip_info_tbl[] = {
> +	[ID_AD5380_3] = {
> +		.channel_template = AD5380_CHANNEL(14),
> +		.num_channels = 40,
> +		.int_vref = 1250000,
> +	},
> +	[ID_AD5380_5] = {
> +		.channel_template = AD5380_CHANNEL(14),
> +		.num_channels = 40,
> +		.int_vref = 2500000,
> +	},
> +	[ID_AD5381_3] = {
> +		.channel_template = AD5380_CHANNEL(12),
> +		.num_channels = 16,
> +		.int_vref = 1250000,
> +	},
> +	[ID_AD5381_5] = {
> +		.channel_template = AD5380_CHANNEL(12),
> +		.num_channels = 16,
> +		.int_vref = 2500000,
> +	},
> +	[ID_AD5382_3] = {
> +		.channel_template = AD5380_CHANNEL(14),
> +		.num_channels = 32,
> +		.int_vref = 1250000,
> +	},
> +	[ID_AD5382_5] = {
> +		.channel_template = AD5380_CHANNEL(14),
> +		.num_channels = 32,
> +		.int_vref = 2500000,
> +	},
> +	[ID_AD5383_3] = {
> +		.channel_template = AD5380_CHANNEL(12),
> +		.num_channels = 32,
> +		.int_vref = 1250000,
> +	},
> +	[ID_AD5383_5] = {
> +		.channel_template = AD5380_CHANNEL(12),
> +		.num_channels = 32,
> +		.int_vref = 2500000,
> +	},
> +	[ID_AD5390_3] = {
> +		.channel_template = AD5380_CHANNEL(14),
> +		.num_channels = 16,
> +		.int_vref = 1250000,
> +	},
> +	[ID_AD5390_5] = {
> +		.channel_template = AD5380_CHANNEL(14),
> +		.num_channels = 16,
> +		.int_vref = 2500000,
> +	},
> +	[ID_AD5391_3] = {
> +		.channel_template = AD5380_CHANNEL(12),
> +		.num_channels = 16,
> +		.int_vref = 1250000,
> +	},
> +	[ID_AD5391_5] = {
> +		.channel_template = AD5380_CHANNEL(12),
> +		.num_channels = 16,
> +		.int_vref = 2500000,
> +	},
> +	[ID_AD5392_3] = {
> +		.channel_template = AD5380_CHANNEL(14),
> +		.num_channels = 8,
> +		.int_vref = 1250000,
> +	},
> +	[ID_AD5392_5] = {
> +		.channel_template = AD5380_CHANNEL(14),
> +		.num_channels = 8,
> +		.int_vref = 2500000,
> +	},
> +};
> +
>  static int __devinit ad5380_alloc_channels(struct iio_dev *indio_dev)
>  {
>  	struct ad5380_state *st = iio_priv(indio_dev);

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC 5/8] staging:iio:dac:ad5504: Convert to extended channel attributes
  2012-05-11 15:53 ` [RFC 5/8] staging:iio:dac:ad5504: " Lars-Peter Clausen
@ 2012-05-14 18:14   ` Jonathan Cameron
  0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2012-05-14 18:14 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Jonathan Cameron, linux-iio

On 05/11/2012 04:53 PM, Lars-Peter Clausen wrote:
> Use extended channel attributes instead of raw sysfs files for the additional
> channel attributes. This allows us to remove some boilerplate code.

One trivial point inline...
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/staging/iio/dac/ad5504.c |  164 ++++++++++++++------------------------
>  1 file changed, 58 insertions(+), 106 deletions(-)
> 
> diff --git a/drivers/staging/iio/dac/ad5504.c b/drivers/staging/iio/dac/ad5504.c
> index db84d85..07b31f4 100644
> --- a/drivers/staging/iio/dac/ad5504.c
> +++ b/drivers/staging/iio/dac/ad5504.c
> @@ -22,24 +22,6 @@
>  
>  #include "ad5504.h"
>  
> -#define AD5504_CHANNEL(_chan) { \
> -	.type = IIO_VOLTAGE, \
> -	.indexed = 1, \
> -	.output = 1, \
> -	.channel = (_chan), \
> -	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT | \
> -		     IIO_CHAN_INFO_SCALE_SHARED_BIT, \
> -	.address = AD5504_ADDR_DAC(_chan), \
> -	.scan_type = IIO_ST('u', 12, 16, 0), \
> -}
> -
> -static const struct iio_chan_spec ad5504_channels[] = {
> -	AD5504_CHANNEL(0),
> -	AD5504_CHANNEL(1),
> -	AD5504_CHANNEL(2),
> -	AD5504_CHANNEL(3),
> -};
> -
>  static int ad5504_spi_write(struct spi_device *spi, u8 addr, u16 val)
>  {
>  	u16 tmp = cpu_to_be16(AD5504_CMD_WRITE |
> @@ -122,65 +104,60 @@ static int ad5504_write_raw(struct iio_dev *indio_dev,
>  	return -EINVAL;
>  }
>  
> -static ssize_t ad5504_read_powerdown_mode(struct device *dev,
> -				      struct device_attribute *attr, char *buf)
> +static const char * const ad5504_powerdown_modes[] = {
> +	"20kohm_to_gnd",
> +	"three_state",
> +};
> +
> +static int ad5504_get_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5504_state *st = iio_priv(indio_dev);
>  
> -	const char mode[][14] = {"20kohm_to_gnd", "three_state"};
> -
> -	return sprintf(buf, "%s\n", mode[st->pwr_down_mode]);
> +	return st->pwr_down_mode;
>  }
>  
> -static ssize_t ad5504_write_powerdown_mode(struct device *dev,
> -				       struct device_attribute *attr,
> -				       const char *buf, size_t len)
> +static int ad5504_set_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, unsigned int mode)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5504_state *st = iio_priv(indio_dev);
> -	int ret;
> -
> -	if (sysfs_streq(buf, "20kohm_to_gnd"))
> -		st->pwr_down_mode = AD5504_DAC_PWRDN_20K;
> -	else if (sysfs_streq(buf, "three_state"))
> -		st->pwr_down_mode = AD5504_DAC_PWRDN_3STATE;
> -	else
> -		ret = -EINVAL;
>  
> -	return ret ? ret : len;
> +	st->pwr_down_mode = mode;
blank line here.
> +	return 0;
>  }
>  
> -static ssize_t ad5504_read_dac_powerdown(struct device *dev,
> -					   struct device_attribute *attr,
> -					   char *buf)
> +static const struct iio_enum ad5504_powerdown_mode_enum = {
> +	.items = ad5504_powerdown_modes,
> +	.num_items = ARRAY_SIZE(ad5504_powerdown_modes),
> +	.get = ad5504_get_powerdown_mode,
> +	.set = ad5504_set_powerdown_mode,
> +};
> +
> +static ssize_t ad5504_read_dac_powerdown(struct iio_dev *indio_dev,
> +	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5504_state *st = iio_priv(indio_dev);
> -	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>  
>  	return sprintf(buf, "%d\n",
> -			!(st->pwr_down_mask & (1 << this_attr->address)));
> +			!(st->pwr_down_mask & (1 << chan->channel)));
>  }
>  
> -static ssize_t ad5504_write_dac_powerdown(struct device *dev,
> -					    struct device_attribute *attr,
> -					    const char *buf, size_t len)
> +static ssize_t ad5504_write_dac_powerdown(struct iio_dev *indio_dev,
> +	uintptr_t private, const struct iio_chan_spec *chan, const char *buf,
> +	size_t len)
>  {
>  	long readin;
>  	int ret;
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5504_state *st = iio_priv(indio_dev);
> -	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>  
>  	ret = strict_strtol(buf, 10, &readin);
>  	if (ret)
>  		return ret;
>  
>  	if (readin == 0)
> -		st->pwr_down_mask |= (1 << this_attr->address);
> +		st->pwr_down_mask |= (1 << chan->channel);
>  	else if (readin == 1)
> -		st->pwr_down_mask &= ~(1 << this_attr->address);
> +		st->pwr_down_mask &= ~(1 << chan->channel);
>  	else
>  		ret = -EINVAL;
>  
> @@ -194,50 +171,6 @@ static ssize_t ad5504_write_dac_powerdown(struct device *dev,
>  	return ret ? ret : len;
>  }
>  
> -static IIO_DEVICE_ATTR(out_voltage_powerdown_mode, S_IRUGO |
> -			S_IWUSR, ad5504_read_powerdown_mode,
> -			ad5504_write_powerdown_mode, 0);
> -
> -static IIO_CONST_ATTR(out_voltage_powerdown_mode_available,
> -			"20kohm_to_gnd three_state");
> -
> -#define IIO_DEV_ATTR_DAC_POWERDOWN(_num, _show, _store, _addr)		\
> -	IIO_DEVICE_ATTR(out_voltage##_num##_powerdown,			\
> -			S_IRUGO | S_IWUSR, _show, _store, _addr)
> -static IIO_DEV_ATTR_DAC_POWERDOWN(0, ad5504_read_dac_powerdown,
> -				   ad5504_write_dac_powerdown, 0);
> -static IIO_DEV_ATTR_DAC_POWERDOWN(1, ad5504_read_dac_powerdown,
> -				   ad5504_write_dac_powerdown, 1);
> -static IIO_DEV_ATTR_DAC_POWERDOWN(2, ad5504_read_dac_powerdown,
> -				   ad5504_write_dac_powerdown, 2);
> -static IIO_DEV_ATTR_DAC_POWERDOWN(3, ad5504_read_dac_powerdown,
> -				   ad5504_write_dac_powerdown, 3);
> -
> -static struct attribute *ad5504_attributes[] = {
> -	&iio_dev_attr_out_voltage0_powerdown.dev_attr.attr,
> -	&iio_dev_attr_out_voltage1_powerdown.dev_attr.attr,
> -	&iio_dev_attr_out_voltage2_powerdown.dev_attr.attr,
> -	&iio_dev_attr_out_voltage3_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 ad5504_attribute_group = {
> -	.attrs = ad5504_attributes,
> -};
> -
> -static struct attribute *ad5501_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 ad5501_attribute_group = {
> -	.attrs = ad5501_attributes,
> -};
> -
>  static IIO_CONST_ATTR(temp0_thresh_rising_value, "110000");
>  static IIO_CONST_ATTR(temp0_thresh_rising_en, "1");
>  
> @@ -267,17 +200,38 @@ static irqreturn_t ad5504_event_handler(int irq, void *private)
>  static const struct iio_info ad5504_info = {
>  	.write_raw = ad5504_write_raw,
>  	.read_raw = ad5504_read_raw,
> -	.attrs = &ad5504_attribute_group,
>  	.event_attrs = &ad5504_ev_attribute_group,
>  	.driver_module = THIS_MODULE,
>  };
>  
> -static const struct iio_info ad5501_info = {
> -	.write_raw = ad5504_write_raw,
> -	.read_raw = ad5504_read_raw,
> -	.attrs = &ad5501_attribute_group,
> -	.event_attrs = &ad5504_ev_attribute_group,
> -	.driver_module = THIS_MODULE,
> +static const struct iio_chan_spec_ext_info ad5504_ext_info[] = {
> +	{
> +		.name = "powerdown",
> +		.read = ad5504_read_dac_powerdown,
> +		.write = ad5504_write_dac_powerdown,
> +	},
> +	IIO_ENUM("powerdown_mode", true, &ad5504_powerdown_mode_enum),
> +	IIO_ENUM_AVAILABLE("powerdown_mode", &ad5504_powerdown_mode_enum),
> +	{ },
> +};
> +
> +#define AD5504_CHANNEL(_chan) { \
> +	.type = IIO_VOLTAGE, \
> +	.indexed = 1, \
> +	.output = 1, \
> +	.channel = (_chan), \
> +	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT | \
> +		     IIO_CHAN_INFO_SCALE_SHARED_BIT, \
> +	.address = AD5504_ADDR_DAC(_chan), \
> +	.scan_type = IIO_ST('u', 12, 16, 0), \
> +	.ext_info = ad5504_ext_info, \
> +}
> +
> +static const struct iio_chan_spec ad5504_channels[] = {
> +	AD5504_CHANNEL(0),
> +	AD5504_CHANNEL(1),
> +	AD5504_CHANNEL(2),
> +	AD5504_CHANNEL(3),
>  };
>  
>  static int __devinit ad5504_probe(struct spi_device *spi)
> @@ -315,13 +269,11 @@ static int __devinit ad5504_probe(struct spi_device *spi)
>  	st->spi = spi;
>  	indio_dev->dev.parent = &spi->dev;
>  	indio_dev->name = spi_get_device_id(st->spi)->name;
> -	if (spi_get_device_id(st->spi)->driver_data == ID_AD5501) {
> -		indio_dev->info = &ad5501_info;
> +	indio_dev->info = &ad5504_info;
> +	if (spi_get_device_id(st->spi)->driver_data == ID_AD5501)
>  		indio_dev->num_channels = 1;
> -	} else {
> -		indio_dev->info = &ad5504_info;
> +	else
>  		indio_dev->num_channels = 4;
> -	}
>  	indio_dev->channels = ad5504_channels;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC 1/8] iio: Add helper functions for enum style channel attributes
  2012-05-14 17:42 ` [RFC 1/8] iio: Add helper functions for enum style " Jonathan Cameron
@ 2012-05-14 18:15   ` Lars-Peter Clausen
  0 siblings, 0 replies; 17+ messages in thread
From: Lars-Peter Clausen @ 2012-05-14 18:15 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Jonathan Cameron, linux-iio

On 05/14/2012 07:42 PM, Jonathan Cameron wrote:
> On 05/11/2012 04:53 PM, Lars-Peter Clausen wrote:
>> We often have the case were we do have a enum style channel attribute. These
>> attributes have in common that they are a list of string values which usually
>> map in a 1-to-1 fashion to integer values.
>>
>> This patch implements some common helper code for implementing enum style
>> channel attributes using extended channel attributes. The helper functions take
>> care of converting between the string and integer values, as well providing a
>> function for "_available" attributes which list all available enum items.
>>
> General principal is good, but needs some documentation :)

Oops, yes wanted to add that before sending the patches out, but well looks
like I forgot about it.

> 
> Also couple of trivial points inline.
> 
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> ---
>>  drivers/iio/industrialio-core.c |   57 +++++++++++++++++++++++++++++++++++++++
>>  include/linux/iio/iio.h         |   32 ++++++++++++++++++++++
>>  2 files changed, 89 insertions(+)
>>
>> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
>> index b39a587..241be03 100644
>> --- a/drivers/iio/industrialio-core.c
>> +++ b/drivers/iio/industrialio-core.c
>> @@ -288,6 +288,63 @@ static ssize_t iio_write_channel_ext_info(struct device *dev,
>>  			       this_attr->c, buf, len);
>>  }
>>  
>> +ssize_t iio_enum_available_read(struct iio_dev *indio_dev,
>> +	uintptr_t priv, const struct iio_chan_spec *chan, char *buf)
>> +{
>> +	const struct iio_enum *e = (const struct iio_enum *)priv;
>> +	char *tmp = buf;
>> +	unsigned int i;
>> +
>> +	if (!e->num_items)
>> +		return 0;
>> +
> Best to add some santify checking on the length of the resulting string
> given it's technically unbounded.

I guess if we hit the PAGE_SIZE limit something went seriously wrong anyway,
but yeah I'll add a check.

>> +	for (i = 0; i < e->num_items; ++i)
>> +		tmp += sprintf(tmp, "%s ", e->items[i]);
>> +
>> +	/* replace last space with a newline */
>> +	*(tmp - 1) = '\n';
>> +
>> +	return tmp - buf;
>> +}
>> +EXPORT_SYMBOL_GPL(iio_enum_available_read);
>> +
>> +ssize_t iio_enum_read(struct iio_dev *indio_dev,
>> +	uintptr_t priv, const struct iio_chan_spec *chan, char *buf)
>> +{
>> +	const struct iio_enum *e = (const struct iio_enum *)priv;
>> +	int i;
>> +
>> +	i = e->get(indio_dev, chan);
>> +	if (i < 0)
>> +		return i;
>> +	else if (i >= e->num_items)
>> +		return -EINVAL;
>> +
>> +	return sprintf(buf, "%s\n", e->items[i]);
>> +}
>> +EXPORT_SYMBOL_GPL(iio_enum_read);
>> +
>> +ssize_t iio_enum_write(struct iio_dev *indio_dev,
>> +	uintptr_t priv, const struct iio_chan_spec *chan, const char *buf,
>> +	size_t len)
>> +{
>> +	const struct iio_enum *e = (const struct iio_enum *)priv;
>> +	unsigned int i;
>> +	int ret;
>> +
> excess brackets.

While it would be legal C I think it really hurts readability to remove them.
Also CodingStyle says that they are only optional if they are enclosing only a
single statements.

>> +	for (i = 0; i < e->num_items; i++) {
>> +		if (sysfs_streq(buf, e->items[i]))
>> +			break;
>> +	}
>> +

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC 6/8] staging:iio:dac:ad5624r: Convert to extended channel attributes
  2012-05-11 15:53 ` [RFC 6/8] staging:iio:dac:ad5624r: " Lars-Peter Clausen
@ 2012-05-14 18:16   ` Jonathan Cameron
  0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2012-05-14 18:16 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Jonathan Cameron, linux-iio

On 05/11/2012 04:53 PM, Lars-Peter Clausen wrote:
> Use extended channel attributes instead of raw sysfs files for the additional
> channel attributes. This allows us to remove some boilerplate code.
> 
Another nitpick inline.
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/staging/iio/dac/ad5624r_spi.c |  209 ++++++++++++++-------------------
>  1 file changed, 91 insertions(+), 118 deletions(-)
> 
> diff --git a/drivers/staging/iio/dac/ad5624r_spi.c b/drivers/staging/iio/dac/ad5624r_spi.c
> index 646b448..ef6a320 100644
> --- a/drivers/staging/iio/dac/ad5624r_spi.c
> +++ b/drivers/staging/iio/dac/ad5624r_spi.c
> @@ -21,56 +21,6 @@
>  
>  #include "ad5624r.h"
>  
> -#define AD5624R_CHANNEL(_chan, _bits) { \
> -	.type = IIO_VOLTAGE, \
> -	.indexed = 1, \
> -	.output = 1, \
> -	.channel = (_chan), \
> -	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT | \
> -		     IIO_CHAN_INFO_SCALE_SHARED_BIT, \
> -	.address = (_chan), \
> -	.scan_type = IIO_ST('u', (_bits), 16, 16 - (_bits)), \
> -}
> -
> -#define DECLARE_AD5624R_CHANNELS(_name, _bits) \
> -	const struct iio_chan_spec _name##_channels[] = { \
> -		AD5624R_CHANNEL(0, _bits), \
> -		AD5624R_CHANNEL(1, _bits), \
> -		AD5624R_CHANNEL(2, _bits), \
> -		AD5624R_CHANNEL(3, _bits), \
> -}
> -
> -static DECLARE_AD5624R_CHANNELS(ad5624r, 12);
> -static DECLARE_AD5624R_CHANNELS(ad5644r, 14);
> -static DECLARE_AD5624R_CHANNELS(ad5664r, 16);
> -
> -static const struct ad5624r_chip_info ad5624r_chip_info_tbl[] = {
> -	[ID_AD5624R3] = {
> -		.channels = ad5624r_channels,
> -		.int_vref_mv = 1250,
> -	},
> -	[ID_AD5624R5] = {
> -		.channels = ad5624r_channels,
> -		.int_vref_mv = 2500,
> -	},
> -	[ID_AD5644R3] = {
> -		.channels = ad5644r_channels,
> -		.int_vref_mv = 1250,
> -	},
> -	[ID_AD5644R5] = {
> -		.channels = ad5644r_channels,
> -		.int_vref_mv = 2500,
> -	},
> -	[ID_AD5664R3] = {
> -		.channels = ad5664r_channels,
> -		.int_vref_mv = 1250,
> -	},
> -	[ID_AD5664R5] = {
> -		.channels = ad5664r_channels,
> -		.int_vref_mv = 2500,
> -	},
> -};
> -
>  static int ad5624r_spi_write(struct spi_device *spi,
>  			     u8 cmd, u8 addr, u16 val, u8 len)
>  {
> @@ -138,67 +88,62 @@ static int ad5624r_write_raw(struct iio_dev *indio_dev,
>  	return -EINVAL;
>  }
>  
> -static ssize_t ad5624r_read_powerdown_mode(struct device *dev,
> -				      struct device_attribute *attr, char *buf)
> +static const char * const ad5624r_powerdown_modes[] = {
> +	"1kohm_to_gnd",
> +	"100kohm_to_gnd",
> +	"three_state"
> +};
> +
> +static int ad5624r_get_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5624r_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 st->pwr_down_mode;
>  }
>  
> -static ssize_t ad5624r_write_powerdown_mode(struct device *dev,
> -				       struct device_attribute *attr,
> -				       const char *buf, size_t len)
> +static int ad5624r_set_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, unsigned int mode)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5624r_state *st = iio_priv(indio_dev);
> -	int ret;
>  
> -	if (sysfs_streq(buf, "1kohm_to_gnd"))
> -		st->pwr_down_mode = AD5624R_LDAC_PWRDN_1K;
> -	else if (sysfs_streq(buf, "100kohm_to_gnd"))
> -		st->pwr_down_mode = AD5624R_LDAC_PWRDN_100K;
> -	else if (sysfs_streq(buf, "three_state"))
> -		st->pwr_down_mode = AD5624R_LDAC_PWRDN_3STATE;
> -	else
> -		ret = -EINVAL;
> +	st->pwr_down_mode = mode;
>  
> -	return ret ? ret : len;
> +	return 0;
>  }
>  
> -static ssize_t ad5624r_read_dac_powerdown(struct device *dev,
> -					   struct device_attribute *attr,
> -					   char *buf)
> +static const struct iio_enum ad5624r_powerdown_mode_enum = {
> +	.items = ad5624r_powerdown_modes,
> +	.num_items = ARRAY_SIZE(ad5624r_powerdown_modes),
> +	.get = ad5624r_get_powerdown_mode,
> +	.set = ad5624r_set_powerdown_mode,
> +};
> +
> +static ssize_t ad5624r_read_dac_powerdown(struct iio_dev *indio_dev,
> +	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5624r_state *st = iio_priv(indio_dev);
> -	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>  
>  	return sprintf(buf, "%d\n",
> -			!!(st->pwr_down_mask & (1 << this_attr->address)));
> +			!!(st->pwr_down_mask & (1 << chan->channel)));
>  }
>  
> -static ssize_t ad5624r_write_dac_powerdown(struct device *dev,
> -					    struct device_attribute *attr,
> -					    const char *buf, size_t len)
> +static ssize_t ad5624r_write_dac_powerdown(struct iio_dev *indio_dev,
> +	uintptr_t private, const struct iio_chan_spec *chan, const char *buf,
> +	size_t len)
>  {
>  	long readin;
>  	int ret;
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5624r_state *st = iio_priv(indio_dev);
> -	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>  
>  	ret = strict_strtol(buf, 10, &readin);
>  	if (ret)
>  		return ret;
>  
>  	if (readin == 1)
> -		st->pwr_down_mask |= (1 << this_attr->address);
> +		st->pwr_down_mask |= (1 << chan->channel);
>  	else if (!readin)
> -		st->pwr_down_mask &= ~(1 << this_attr->address);
> +		st->pwr_down_mask &= ~(1 << chan->channel);
>  	else
>  		ret = -EINVAL;
>  
> @@ -209,47 +154,75 @@ static ssize_t ad5624r_write_dac_powerdown(struct device *dev,
>  	return ret ? ret : len;
>  }
>  
> -static IIO_DEVICE_ATTR(out_voltage_powerdown_mode, S_IRUGO |
> -			S_IWUSR, ad5624r_read_powerdown_mode,
> -			ad5624r_write_powerdown_mode, 0);
> -
> -static IIO_CONST_ATTR(out_voltage_powerdown_mode_available,
> -			"1kohm_to_gnd 100kohm_to_gnd three_state");
> -
> -#define IIO_DEV_ATTR_DAC_POWERDOWN(_num, _show, _store, _addr)		\
> -	IIO_DEVICE_ATTR(out_voltage##_num##_powerdown,			\
> -			S_IRUGO | S_IWUSR, _show, _store, _addr)
> -
> -static IIO_DEV_ATTR_DAC_POWERDOWN(0, ad5624r_read_dac_powerdown,
> -				   ad5624r_write_dac_powerdown, 0);
> -static IIO_DEV_ATTR_DAC_POWERDOWN(1, ad5624r_read_dac_powerdown,
> -				   ad5624r_write_dac_powerdown, 1);
> -static IIO_DEV_ATTR_DAC_POWERDOWN(2, ad5624r_read_dac_powerdown,
> -				   ad5624r_write_dac_powerdown, 2);
> -static IIO_DEV_ATTR_DAC_POWERDOWN(3, ad5624r_read_dac_powerdown,
> -				   ad5624r_write_dac_powerdown, 3);
> -
> -static struct attribute *ad5624r_attributes[] = {
> -	&iio_dev_attr_out_voltage0_powerdown.dev_attr.attr,
> -	&iio_dev_attr_out_voltage1_powerdown.dev_attr.attr,
> -	&iio_dev_attr_out_voltage2_powerdown.dev_attr.attr,
> -	&iio_dev_attr_out_voltage3_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 ad5624r_attribute_group = {
> -	.attrs = ad5624r_attributes,
> -};
> -
>  static const struct iio_info ad5624r_info = {
>  	.write_raw = ad5624r_write_raw,
>  	.read_raw = ad5624r_read_raw,
> -	.attrs = &ad5624r_attribute_group,
>  	.driver_module = THIS_MODULE,
>  };
>  
> +static const struct iio_chan_spec_ext_info ad5624r_ext_info[] = {
> +	{
> +		.name = "powerdown",
> +		.read = ad5624r_read_dac_powerdown,
> +		.write = ad5624r_write_dac_powerdown,
> +	},
> +	IIO_ENUM("powerdown_mode", true, &ad5624r_powerdown_mode_enum),
> +	IIO_ENUM_AVAILABLE("powerdown_mode", &ad5624r_powerdown_mode_enum),
> +	{ },
> +};
> +
> +#define AD5624R_CHANNEL(_chan, _bits) { \
> +	.type = IIO_VOLTAGE, \
> +	.indexed = 1, \
> +	.output = 1, \
> +	.channel = (_chan), \
> +	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT | \
> +		     IIO_CHAN_INFO_SCALE_SHARED_BIT, \
> +	.address = (_chan), \
> +	.scan_type = IIO_ST('u', (_bits), 16, 16 - (_bits)), \
> +	.ext_info = ad5624r_ext_info, \
> +}
> +
> +#define DECLARE_AD5624R_CHANNELS(_name, _bits) \
> +	const struct iio_chan_spec _name##_channels[] = { \
> +		AD5624R_CHANNEL(0, _bits), \
> +		AD5624R_CHANNEL(1, _bits), \
> +		AD5624R_CHANNEL(2, _bits), \
> +		AD5624R_CHANNEL(3, _bits), \
> +}
> +
> +static DECLARE_AD5624R_CHANNELS(ad5624r, 12);
> +static DECLARE_AD5624R_CHANNELS(ad5644r, 14);
> +static DECLARE_AD5624R_CHANNELS(ad5664r, 16);
> +
> +static const struct ad5624r_chip_info ad5624r_chip_info_tbl[] = {
> +	[ID_AD5624R3] = {
> +		.channels = ad5624r_channels,
> +		.int_vref_mv = 1250,
> +	},
> +	[ID_AD5624R5] = {
> +		.channels = ad5624r_channels,
> +		.int_vref_mv = 2500,
> +	},
> +	[ID_AD5644R3] = {
> +		.channels = ad5644r_channels,
> +		.int_vref_mv = 1250,
> +	},
> +	[ID_AD5644R5] = {
> +		.channels = ad5644r_channels,
> +		.int_vref_mv = 2500,
> +	},
> +	[ID_AD5664R3] = {
> +		.channels = ad5664r_channels,
> +		.int_vref_mv = 1250,
> +	},
> +	[ID_AD5664R5] = {
> +		.channels = ad5664r_channels,
> +		.int_vref_mv = 2500,
> +	},
> +};
> +
one too many blank lines.
> +
>  static int __devinit ad5624r_probe(struct spi_device *spi)
>  {
>  	struct ad5624r_state *st;

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC 7/8] staging:iio:dac:ad5686: Convert to extended channel attributes
  2012-05-11 15:53 ` [RFC 7/8] staging:iio:dac:ad5686: " Lars-Peter Clausen
@ 2012-05-14 18:18   ` Jonathan Cameron
  0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2012-05-14 18:18 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Jonathan Cameron, linux-iio

On 05/11/2012 04:53 PM, Lars-Peter Clausen wrote:
> Use extended channel attributes instead of raw sysfs files for the additional
> channel attributes. This allows us to remove some boilerplate code.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/staging/iio/dac/ad5686.c |  193 +++++++++++++++-----------------------
>  1 file changed, 77 insertions(+), 116 deletions(-)
> 
> diff --git a/drivers/staging/iio/dac/ad5686.c b/drivers/staging/iio/dac/ad5686.c
> index 46239b3..cf833c9 100644
> --- a/drivers/staging/iio/dac/ad5686.c
> +++ b/drivers/staging/iio/dac/ad5686.c
> @@ -92,40 +92,6 @@ enum ad5686_supported_device_ids {
>  	ID_AD5685,
>  	ID_AD5686,
>  };
> -#define AD5868_CHANNEL(chan, bits, shift) {			\
> -		.type = IIO_VOLTAGE,				\
> -		.indexed = 1,					\
> -		.output = 1,					\
> -		.channel = chan,				\
> -		.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |	\
> -		IIO_CHAN_INFO_SCALE_SHARED_BIT,			\
> -		.address = AD5686_ADDR_DAC(chan),			\
> -		.scan_type = IIO_ST('u', bits, 16, shift)	\
> -}
> -static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
> -	[ID_AD5684] = {
> -		.channel[0] = AD5868_CHANNEL(0, 12, 4),
> -		.channel[1] = AD5868_CHANNEL(1, 12, 4),
> -		.channel[2] = AD5868_CHANNEL(2, 12, 4),
> -		.channel[3] = AD5868_CHANNEL(3, 12, 4),
> -		.int_vref_mv = 2500,
> -	},
> -	[ID_AD5685] = {
> -		.channel[0] = AD5868_CHANNEL(0, 14, 2),
> -		.channel[1] = AD5868_CHANNEL(1, 14, 2),
> -		.channel[2] = AD5868_CHANNEL(2, 14, 2),
> -		.channel[3] = AD5868_CHANNEL(3, 14, 2),
> -		.int_vref_mv = 2500,
> -	},
> -	[ID_AD5686] = {
> -		.channel[0] = AD5868_CHANNEL(0, 16, 0),
> -		.channel[1] = AD5868_CHANNEL(1, 16, 0),
> -		.channel[2] = AD5868_CHANNEL(2, 16, 0),
> -		.channel[3] = AD5868_CHANNEL(3, 16, 0),
> -		.int_vref_mv = 2500,
> -	},
> -};
> -
>  static int ad5686_spi_write(struct ad5686_state *st,
>  			     u8 cmd, u8 addr, u16 val, u8 shift)
>  {
> @@ -169,73 +135,63 @@ static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
>  	return be32_to_cpu(st->data[2].d32);
>  }
>  
> -static ssize_t ad5686_read_powerdown_mode(struct device *dev,
> -				      struct device_attribute *attr, char *buf)
> +static const char * const ad5686_powerdown_modes[] = {
> +	"1kohm_to_gnd",
> +	"100kohm_to_gnd",
> +	"three_state"
> +};
> +
> +static int ad5686_get_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5686_state *st = iio_priv(indio_dev);
> -	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> -
> -	char mode[][15] = {"", "1kohm_to_gnd", "100kohm_to_gnd", "three_state"};
>  
> -	return sprintf(buf, "%s\n", mode[(st->pwr_down_mode >>
> -					 (this_attr->address * 2)) & 0x3]);
> +	return ((st->pwr_down_mode >> (chan->channel * 2)) & 0x3) - 1;
>  }
>  
> -static ssize_t ad5686_write_powerdown_mode(struct device *dev,
> -				       struct device_attribute *attr,
> -				       const char *buf, size_t len)
> +static int ad5686_set_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, unsigned int mode)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5686_state *st = iio_priv(indio_dev);
> -	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> -	unsigned mode;
> -
> -	if (sysfs_streq(buf, "1kohm_to_gnd"))
> -		mode = AD5686_LDAC_PWRDN_1K;
> -	else if (sysfs_streq(buf, "100kohm_to_gnd"))
> -		mode = AD5686_LDAC_PWRDN_100K;
> -	else if (sysfs_streq(buf, "three_state"))
> -		mode = AD5686_LDAC_PWRDN_3STATE;
> -	else
> -		return  -EINVAL;
>  
> -	st->pwr_down_mode &= ~(0x3 << (this_attr->address * 2));
> -	st->pwr_down_mode |= (mode << (this_attr->address * 2));
> +	st->pwr_down_mode &= ~(0x3 << (chan->channel * 2));
> +	st->pwr_down_mode |= ((mode + 1) << (chan->channel * 2));
>  
> -	return len;
> +	return 0;
>  }
>  
> -static ssize_t ad5686_read_dac_powerdown(struct device *dev,
> -					   struct device_attribute *attr,
> -					   char *buf)
> +static const struct iio_enum ad5686_powerdown_mode_enum = {
> +	.items = ad5686_powerdown_modes,
> +	.num_items = ARRAY_SIZE(ad5686_powerdown_modes),
> +	.get = ad5686_get_powerdown_mode,
> +	.set = ad5686_set_powerdown_mode,
> +};
> +
> +static ssize_t ad5686_read_dac_powerdown(struct iio_dev *indio_dev,
> +	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5686_state *st = iio_priv(indio_dev);
> -	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>  
>  	return sprintf(buf, "%d\n", !!(st->pwr_down_mask &
> -			(0x3 << (this_attr->address * 2))));
> +			(0x3 << (chan->channel * 2))));
>  }
>  
> -static ssize_t ad5686_write_dac_powerdown(struct device *dev,
> -					    struct device_attribute *attr,
> -					    const char *buf, size_t len)
> +static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev,
> +	 uintptr_t private, const struct iio_chan_spec *chan, const char *buf,
> +	 size_t len)
>  {
>  	bool readin;
>  	int ret;
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5686_state *st = iio_priv(indio_dev);
> -	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>  
>  	ret = strtobool(buf, &readin);
>  	if (ret)
>  		return ret;
>  
>  	if (readin == true)
> -		st->pwr_down_mask |= (0x3 << (this_attr->address * 2));
> +		st->pwr_down_mask |= (0x3 << (chan->channel * 2));
>  	else
> -		st->pwr_down_mask &= ~(0x3 << (this_attr->address * 2));
> +		st->pwr_down_mask &= ~(0x3 << (chan->channel * 2));
>  
>  	ret = ad5686_spi_write(st, AD5686_CMD_POWERDOWN_DAC, 0,
>  			       st->pwr_down_mask & st->pwr_down_mode, 0);
> @@ -243,48 +199,6 @@ static ssize_t ad5686_write_dac_powerdown(struct device *dev,
>  	return ret ? ret : len;
>  }
>  
> -static IIO_CONST_ATTR(out_voltage_powerdown_mode_available,
> -			"1kohm_to_gnd 100kohm_to_gnd three_state");
> -
> -#define IIO_DEV_ATTR_DAC_POWERDOWN_MODE(_num)				\
> -	IIO_DEVICE_ATTR(out_voltage##_num##_powerdown_mode,		\
> -			S_IRUGO | S_IWUSR,				\
> -			ad5686_read_powerdown_mode,			\
> -			ad5686_write_powerdown_mode, _num)
> -
> -static IIO_DEV_ATTR_DAC_POWERDOWN_MODE(0);
> -static IIO_DEV_ATTR_DAC_POWERDOWN_MODE(1);
> -static IIO_DEV_ATTR_DAC_POWERDOWN_MODE(2);
> -static IIO_DEV_ATTR_DAC_POWERDOWN_MODE(3);
> -
> -#define IIO_DEV_ATTR_DAC_POWERDOWN(_num)				\
> -	IIO_DEVICE_ATTR(out_voltage##_num##_powerdown,			\
> -			S_IRUGO | S_IWUSR,				\
> -			ad5686_read_dac_powerdown,			\
> -			ad5686_write_dac_powerdown, _num)
> -
> -static IIO_DEV_ATTR_DAC_POWERDOWN(0);
> -static IIO_DEV_ATTR_DAC_POWERDOWN(1);
> -static IIO_DEV_ATTR_DAC_POWERDOWN(2);
> -static IIO_DEV_ATTR_DAC_POWERDOWN(3);
> -
> -static struct attribute *ad5686_attributes[] = {
> -	&iio_dev_attr_out_voltage0_powerdown.dev_attr.attr,
> -	&iio_dev_attr_out_voltage1_powerdown.dev_attr.attr,
> -	&iio_dev_attr_out_voltage2_powerdown.dev_attr.attr,
> -	&iio_dev_attr_out_voltage3_powerdown.dev_attr.attr,
> -	&iio_dev_attr_out_voltage0_powerdown_mode.dev_attr.attr,
> -	&iio_dev_attr_out_voltage1_powerdown_mode.dev_attr.attr,
> -	&iio_dev_attr_out_voltage2_powerdown_mode.dev_attr.attr,
> -	&iio_dev_attr_out_voltage3_powerdown_mode.dev_attr.attr,
> -	&iio_const_attr_out_voltage_powerdown_mode_available.dev_attr.attr,
> -	NULL,
> -};
> -
> -static const struct attribute_group ad5686_attribute_group = {
> -	.attrs = ad5686_attributes,
> -};
> -
>  static int ad5686_read_raw(struct iio_dev *indio_dev,
>  			   struct iio_chan_spec const *chan,
>  			   int *val,
> @@ -348,10 +262,57 @@ static int ad5686_write_raw(struct iio_dev *indio_dev,
>  static const struct iio_info ad5686_info = {
>  	.read_raw = ad5686_read_raw,
>  	.write_raw = ad5686_write_raw,
> -	.attrs = &ad5686_attribute_group,
>  	.driver_module = THIS_MODULE,
>  };
>  
> +static const struct iio_chan_spec_ext_info ad5686_ext_info[] = {
> +	{
> +		.name = "powerdown",
> +		.read = ad5686_read_dac_powerdown,
> +		.write = ad5686_write_dac_powerdown,
> +	},
> +	IIO_ENUM("powerdown_mode", false, &ad5686_powerdown_mode_enum),
> +	IIO_ENUM_AVAILABLE("powerdown_mode", &ad5686_powerdown_mode_enum),
> +	{ },
> +};
> +
> +#define AD5868_CHANNEL(chan, bits, shift) {			\
> +		.type = IIO_VOLTAGE,				\
> +		.indexed = 1,					\
> +		.output = 1,					\
> +		.channel = chan,				\
> +		.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |	\
> +		IIO_CHAN_INFO_SCALE_SHARED_BIT,			\
> +		.address = AD5686_ADDR_DAC(chan),			\
> +		.scan_type = IIO_ST('u', bits, 16, shift),	\
> +		.ext_info = ad5686_ext_info,			\
> +}
> +
> +static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
> +	[ID_AD5684] = {
> +		.channel[0] = AD5868_CHANNEL(0, 12, 4),
> +		.channel[1] = AD5868_CHANNEL(1, 12, 4),
> +		.channel[2] = AD5868_CHANNEL(2, 12, 4),
> +		.channel[3] = AD5868_CHANNEL(3, 12, 4),
> +		.int_vref_mv = 2500,
> +	},
> +	[ID_AD5685] = {
> +		.channel[0] = AD5868_CHANNEL(0, 14, 2),
> +		.channel[1] = AD5868_CHANNEL(1, 14, 2),
> +		.channel[2] = AD5868_CHANNEL(2, 14, 2),
> +		.channel[3] = AD5868_CHANNEL(3, 14, 2),
> +		.int_vref_mv = 2500,
> +	},
> +	[ID_AD5686] = {
> +		.channel[0] = AD5868_CHANNEL(0, 16, 0),
> +		.channel[1] = AD5868_CHANNEL(1, 16, 0),
> +		.channel[2] = AD5868_CHANNEL(2, 16, 0),
> +		.channel[3] = AD5868_CHANNEL(3, 16, 0),
> +		.int_vref_mv = 2500,
> +	},
> +};
> +
> +
>  static int __devinit ad5686_probe(struct spi_device *spi)
>  {
>  	struct ad5686_state *st;

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [RFC 8/8] staging:iio:dac:ad5791: Convert to extended channel attributes
  2012-05-11 15:53 ` [RFC 8/8] staging:iio:dac:ad5791: " Lars-Peter Clausen
@ 2012-05-14 18:21   ` Jonathan Cameron
  0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2012-05-14 18:21 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Jonathan Cameron, linux-iio

On 05/11/2012 04:53 PM, Lars-Peter Clausen wrote:
> Use extended channel attributes instead of raw sysfs files for the additional
> channel attributes. This allows us to remove some boilerplate code.

A nice patch set.  I do wonder if we ultimately need to find common
ground for these power down commands with other subsystems.  I have a
vague feeling that the pinmux stuff will overlap with this at somepoint.
Afterall it's not uncommon to have these sorts of controls on gpio's
or other pins on a SoC.  Probably just a question of keeping an eye
on progress elsewhere for now...
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/staging/iio/dac/ad5791.c |  122 +++++++++++++++++---------------------
>  1 file changed, 53 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/staging/iio/dac/ad5791.c b/drivers/staging/iio/dac/ad5791.c
> index 8628964..4e955ed 100644
> --- a/drivers/staging/iio/dac/ad5791.c
> +++ b/drivers/staging/iio/dac/ad5791.c
> @@ -72,71 +72,50 @@ static int ad5791_spi_read(struct spi_device *spi, u8 addr, u32 *val)
>  	return ret;
>  }
>  
> -#define AD5791_CHAN(bits, shift) {			\
> -	.type = IIO_VOLTAGE,				\
> -	.output = 1,					\
> -	.indexed = 1,					\
> -	.address = AD5791_ADDR_DAC0,			\
> -	.channel = 0,					\
> -	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |	\
> -		IIO_CHAN_INFO_SCALE_SHARED_BIT |	\
> -		IIO_CHAN_INFO_OFFSET_SHARED_BIT,	\
> -	.scan_type = IIO_ST('u', bits, 24, shift)	\
> -}
> -
> -static const struct iio_chan_spec ad5791_channels[] = {
> -	[ID_AD5760] = AD5791_CHAN(16, 4),
> -	[ID_AD5780] = AD5791_CHAN(18, 2),
> -	[ID_AD5781] = AD5791_CHAN(18, 2),
> -	[ID_AD5791] = AD5791_CHAN(20, 0)
> +static const char * const ad5791_powerdown_modes[] = {
> +	"6kohm_to_gnd",
> +	"three_state",
>  };
>  
> -static ssize_t ad5791_read_powerdown_mode(struct device *dev,
> -				      struct device_attribute *attr, char *buf)
> +static int ad5791_get_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5791_state *st = iio_priv(indio_dev);
>  
> -	const char mode[][14] = {"6kohm_to_gnd", "three_state"};
> -
> -	return sprintf(buf, "%s\n", mode[st->pwr_down_mode]);
> +	return st->pwr_down_mode;
>  }
>  
> -static ssize_t ad5791_write_powerdown_mode(struct device *dev,
> -				       struct device_attribute *attr,
> -				       const char *buf, size_t len)
> +static int ad5791_set_powerdown_mode(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, unsigned int mode)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5791_state *st = iio_priv(indio_dev);
> -	int ret;
>  
> -	if (sysfs_streq(buf, "6kohm_to_gnd"))
> -		st->pwr_down_mode = AD5791_DAC_PWRDN_6K;
> -	else if (sysfs_streq(buf, "three_state"))
> -		st->pwr_down_mode = AD5791_DAC_PWRDN_3STATE;
> -	else
> -		ret = -EINVAL;
> +	st->pwr_down_mode = mode;
>  
> -	return ret ? ret : len;
> +	return 0;
>  }
>  
> -static ssize_t ad5791_read_dac_powerdown(struct device *dev,
> -					   struct device_attribute *attr,
> -					   char *buf)
> +static const struct iio_enum ad5791_powerdown_mode_enum = {
> +	.items = ad5791_powerdown_modes,
> +	.num_items = ARRAY_SIZE(ad5791_powerdown_modes),
> +	.get = ad5791_get_powerdown_mode,
> +	.set = ad5791_set_powerdown_mode,
> +};
> +
> +static ssize_t ad5791_read_dac_powerdown(struct iio_dev *indio_dev,
> +	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
>  {
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5791_state *st = iio_priv(indio_dev);
>  
>  	return sprintf(buf, "%d\n", st->pwr_down);
>  }
>  
> -static ssize_t ad5791_write_dac_powerdown(struct device *dev,
> -					    struct device_attribute *attr,
> -					    const char *buf, size_t len)
> +static ssize_t ad5791_write_dac_powerdown(struct iio_dev *indio_dev,
> +	 uintptr_t private, const struct iio_chan_spec *chan, const char *buf,
> +	 size_t len)
>  {
>  	long readin;
>  	int ret;
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct ad5791_state *st = iio_priv(indio_dev);
>  
>  	ret = strict_strtol(buf, 10, &readin);
> @@ -160,31 +139,6 @@ static ssize_t ad5791_write_dac_powerdown(struct device *dev,
>  	return ret ? ret : len;
>  }
>  
> -static IIO_DEVICE_ATTR(out_voltage_powerdown_mode, S_IRUGO |
> -			S_IWUSR, ad5791_read_powerdown_mode,
> -			ad5791_write_powerdown_mode, 0);
> -
> -static IIO_CONST_ATTR(out_voltage_powerdown_mode_available,
> -			"6kohm_to_gnd three_state");
> -
> -#define IIO_DEV_ATTR_DAC_POWERDOWN(_num, _show, _store, _addr)		\
> -	IIO_DEVICE_ATTR(out_voltage##_num##_powerdown,			\
> -			S_IRUGO | S_IWUSR, _show, _store, _addr)
> -
> -static IIO_DEV_ATTR_DAC_POWERDOWN(0, ad5791_read_dac_powerdown,
> -				   ad5791_write_dac_powerdown, 0);
> -
> -static struct attribute *ad5791_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 ad5791_attribute_group = {
> -	.attrs = ad5791_attributes,
> -};
> -
>  static int ad5791_get_lin_comp(unsigned int span)
>  {
>  	if (span <= 10000)
> @@ -254,6 +208,37 @@ static int ad5791_read_raw(struct iio_dev *indio_dev,
>  
>  };
>  
> +static const struct iio_chan_spec_ext_info ad5791_ext_info[] = {
> +	{
> +		.name = "powerdown",
> +		.shared = true,
> +		.read = ad5791_read_dac_powerdown,
> +		.write = ad5791_write_dac_powerdown,
> +	},
> +	IIO_ENUM("powerdown_mode", true, &ad5791_powerdown_mode_enum),
> +	IIO_ENUM_AVAILABLE("powerdown_mode", &ad5791_powerdown_mode_enum),
> +	{ },
> +};
> +
> +#define AD5791_CHAN(bits, shift) {			\
> +	.type = IIO_VOLTAGE,				\
> +	.output = 1,					\
> +	.indexed = 1,					\
> +	.address = AD5791_ADDR_DAC0,			\
> +	.channel = 0,					\
> +	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |	\
> +		IIO_CHAN_INFO_SCALE_SHARED_BIT |	\
> +		IIO_CHAN_INFO_OFFSET_SHARED_BIT,	\
> +	.scan_type = IIO_ST('u', bits, 24, shift),	\
> +	.ext_info = ad5791_ext_info,			\
> +}
> +
> +static const struct iio_chan_spec ad5791_channels[] = {
> +	[ID_AD5760] = AD5791_CHAN(16, 4),
> +	[ID_AD5780] = AD5791_CHAN(18, 2),
> +	[ID_AD5781] = AD5791_CHAN(18, 2),
> +	[ID_AD5791] = AD5791_CHAN(20, 0)
> +};
>  
>  static int ad5791_write_raw(struct iio_dev *indio_dev,
>  			    struct iio_chan_spec const *chan,
> @@ -278,7 +263,6 @@ static int ad5791_write_raw(struct iio_dev *indio_dev,
>  static const struct iio_info ad5791_info = {
>  	.read_raw = &ad5791_read_raw,
>  	.write_raw = &ad5791_write_raw,
> -	.attrs = &ad5791_attribute_group,
>  	.driver_module = THIS_MODULE,
>  };
>  

^ permalink raw reply	[flat|nested] 17+ messages in thread
end of thread, other threads:[~2012-05-14 18:21 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-11 15:53 [RFC 1/8] iio: Add helper functions for enum style channel attributes Lars-Peter Clausen
2012-05-11 15:53 ` [RFC 2/8] staging:iio:dac:ad5064: Use iio_enum for powerdown modes Lars-Peter Clausen
2012-05-14 17:48   ` Jonathan Cameron
2012-05-11 15:53 ` [RFC 3/8] staging:iio:dac:ad5446: " Lars-Peter Clausen
2012-05-14 17:55   ` Jonathan Cameron
2012-05-11 15:53 ` [RFC 4/8] staging:iio:dac:ad5380: Convert to extended channel attributes Lars-Peter Clausen
2012-05-14 18:11   ` Jonathan Cameron
2012-05-11 15:53 ` [RFC 5/8] staging:iio:dac:ad5504: " Lars-Peter Clausen
2012-05-14 18:14   ` Jonathan Cameron
2012-05-11 15:53 ` [RFC 6/8] staging:iio:dac:ad5624r: " Lars-Peter Clausen
2012-05-14 18:16   ` Jonathan Cameron
2012-05-11 15:53 ` [RFC 7/8] staging:iio:dac:ad5686: " Lars-Peter Clausen
2012-05-14 18:18   ` Jonathan Cameron
2012-05-11 15:53 ` [RFC 8/8] staging:iio:dac:ad5791: " Lars-Peter Clausen
2012-05-14 18:21   ` Jonathan Cameron
2012-05-14 17:42 ` [RFC 1/8] iio: Add helper functions for enum style " Jonathan Cameron
2012-05-14 18:15   ` Lars-Peter Clausen

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.