[PATCH 0/5] Remove use of isvisible for attr groups that are coppied.

[PATCH 0/5] Remove use of isvisible for attr groups that are coppied.

[Jonathan Cameron]

Hi Greg,

These are all fixes.

These patches were originally part of a series that squished the attribute
groups entirely, but whilst everyone was happy with these, the final patch
was dropped.

Anyhow, the eventual attribute groups that get registered are mostly
built from the iio_chan_spec structures. These groups are just used
to pass in bits the core doesn't handle.  This made it it rather
difficult to ensure that the is_visible function was consistent
(as it would be applied to a much larger set than apparent if we
coppied it on as is) and in the vast majority of cases its use was
trivial anyway.

If a good reason for using is_visible comes up, then the right option will be
to first verifying if the element comes from these attribute
groups that are passed in before calling the is_visible function provided
in these groups.  Anyhow, not too bad, but not currently necessary.

Even if we do put this in, these cases are easier to read (and in somecases
shorter) when not using it.

Jonathan

Jonathan Cameron (5):
  staging:iio:adc:ad7192 unwind use of is_visible for attribute group.
  staging:iio:dds:ad9834 unwind use of is_visible for attrs.
  staging:iio:dac:ad5446 unwind use of is_visible for attrs.
  staging:iio:adc:ad7606 unwind use of is_visible for attrs.
  staging:iio:adc:adt7310/7410 sticking plaster fix for broken event
    attrs.

 drivers/staging/iio/adc/ad7192.c      |   43 ++++++++++-------
 drivers/staging/iio/adc/ad7606_core.c |   83 +++++++++++++++++++++------------
 drivers/staging/iio/adc/adt7310.c     |   21 ++-------
 drivers/staging/iio/adc/adt7410.c     |   21 ++-------
 drivers/staging/iio/dac/ad5446.c      |   33 ++++---------
 drivers/staging/iio/dds/ad9834.c      |   53 +++++++++++++--------
 6 files changed, 128 insertions(+), 126 deletions(-)

-- 
1.7.8.4

[PATCH 1/5] staging:iio:adc:ad7192 unwind use of is_visible for attribute group.

[Jonathan Cameron]

It saves a couple of lines of code but reduces simplicity of code.
I generally wish to discourage use of is_visible throughout IIO.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7192.c |   43 ++++++++++++++++++++++---------------
 1 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index 45f4504..d8abc09 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -824,25 +824,20 @@ static struct attribute *ad7192_attributes[] = {
 	NULL
 };
 
-static umode_t ad7192_attr_is_visible(struct kobject *kobj,
-				     struct attribute *attr, int n)
-{
-	struct device *dev = container_of(kobj, struct device, kobj);
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
-	struct ad7192_state *st = iio_priv(indio_dev);
-
-	umode_t mode = attr->mode;
-
-	if ((st->devid != ID_AD7195) &&
-		(attr == &iio_dev_attr_ac_excitation_en.dev_attr.attr))
-		mode = 0;
-
-	return mode;
-}
-
 static const struct attribute_group ad7192_attribute_group = {
 	.attrs = ad7192_attributes,
-	.is_visible = ad7192_attr_is_visible,
+};
+
+static struct attribute *ad7195_attributes[] = {
+	&iio_dev_attr_sampling_frequency.dev_attr.attr,
+	&iio_dev_attr_in_v_m_v_scale_available.dev_attr.attr,
+	&iio_dev_attr_in_voltage_scale_available.dev_attr.attr,
+	&iio_dev_attr_bridge_switch_en.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group ad7195_attribute_group = {
+	.attrs = ad7195_attributes,
 };
 
 static int ad7192_read_raw(struct iio_dev *indio_dev,
@@ -972,6 +967,15 @@ static const struct iio_info ad7192_info = {
 	.driver_module = THIS_MODULE,
 };
 
+static const struct iio_info ad7195_info = {
+	.read_raw = &ad7192_read_raw,
+	.write_raw = &ad7192_write_raw,
+	.write_raw_get_fmt = &ad7192_write_raw_get_fmt,
+	.attrs = &ad7195_attribute_group,
+	.validate_trigger = ad7192_validate_trigger,
+	.driver_module = THIS_MODULE,
+};
+
 #define AD7192_CHAN_DIFF(_chan, _chan2, _name, _address, _si)		\
 	{ .type = IIO_VOLTAGE,						\
 	  .differential = 1,						\
@@ -1064,7 +1068,10 @@ static int __devinit ad7192_probe(struct spi_device *spi)
 	indio_dev->channels = ad7192_channels;
 	indio_dev->num_channels = ARRAY_SIZE(ad7192_channels);
 	indio_dev->available_scan_masks = st->available_scan_masks;
-	indio_dev->info = &ad7192_info;
+	if (st->devid == ID_AD7195)
+		indio_dev->info = &ad7195_info;
+	else
+		indio_dev->info = &ad7192_info;
 
 	for (i = 0; i < indio_dev->num_channels; i++)
 		st->available_scan_masks[i] = (1 << i) | (1 <<
-- 
1.7.8.4

[PATCH 2/5] staging:iio:dds:ad9834 unwind use of is_visible for attrs.

[Jonathan Cameron]

Trivial usecase in which just having two different attr
groups covers all options.  Slightly more code, but a simpler
to follow result.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/dds/ad9834.c |   53 +++++++++++++++++++++++---------------
 1 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/iio/dds/ad9834.c b/drivers/staging/iio/dds/ad9834.c
index 5e67104..38a2de0 100644
--- a/drivers/staging/iio/dds/ad9834.c
+++ b/drivers/staging/iio/dds/ad9834.c
@@ -281,29 +281,27 @@ static struct attribute *ad9834_attributes[] = {
 	NULL,
 };
 
-static umode_t ad9834_attr_is_visible(struct kobject *kobj,
-				     struct attribute *attr, int n)
-{
-	struct device *dev = container_of(kobj, struct device, kobj);
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
-	struct ad9834_state *st = iio_priv(indio_dev);
-
-	umode_t mode = attr->mode;
-
-	if (((st->devid == ID_AD9833) || (st->devid == ID_AD9837)) &&
-		((attr == &iio_dev_attr_dds0_out1_enable.dev_attr.attr) ||
-		(attr == &iio_dev_attr_dds0_out1_wavetype.dev_attr.attr) ||
-		(attr ==
-		&iio_dev_attr_dds0_out1_wavetype_available.dev_attr.attr) ||
-		(attr == &iio_dev_attr_dds0_pincontrol_en.dev_attr.attr)))
-		mode = 0;
-
-	return mode;
-}
+static struct attribute *ad9833_attributes[] = {
+	&iio_dev_attr_dds0_freq0.dev_attr.attr,
+	&iio_dev_attr_dds0_freq1.dev_attr.attr,
+	&iio_const_attr_dds0_freq_scale.dev_attr.attr,
+	&iio_dev_attr_dds0_phase0.dev_attr.attr,
+	&iio_dev_attr_dds0_phase1.dev_attr.attr,
+	&iio_const_attr_dds0_phase_scale.dev_attr.attr,
+	&iio_dev_attr_dds0_freqsymbol.dev_attr.attr,
+	&iio_dev_attr_dds0_phasesymbol.dev_attr.attr,
+	&iio_dev_attr_dds0_out_enable.dev_attr.attr,
+	&iio_dev_attr_dds0_out0_wavetype.dev_attr.attr,
+	&iio_dev_attr_dds0_out0_wavetype_available.dev_attr.attr,
+	NULL,
+};
 
 static const struct attribute_group ad9834_attribute_group = {
 	.attrs = ad9834_attributes,
-	.is_visible = ad9834_attr_is_visible,
+};
+
+static const struct attribute_group ad9833_attribute_group = {
+	.attrs = ad9833_attributes,
 };
 
 static const struct iio_info ad9834_info = {
@@ -311,6 +309,11 @@ static const struct iio_info ad9834_info = {
 	.driver_module = THIS_MODULE,
 };
 
+static const struct iio_info ad9833_info = {
+	.attrs = &ad9833_attribute_group,
+	.driver_module = THIS_MODULE,
+};
+
 static int __devinit ad9834_probe(struct spi_device *spi)
 {
 	struct ad9834_platform_data *pdata = spi->dev.platform_data;
@@ -344,7 +347,15 @@ static int __devinit ad9834_probe(struct spi_device *spi)
 	st->reg = reg;
 	indio_dev->dev.parent = &spi->dev;
 	indio_dev->name = spi_get_device_id(spi)->name;
-	indio_dev->info = &ad9834_info;
+	switch (st->devid) {
+	case ID_AD9833:
+	case ID_AD9837:
+		indio_dev->info = &ad9833_info;
+		break;
+	default:
+		indio_dev->info = &ad9834_info;
+		break;
+	}
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
 	/* Setup default messages */
-- 
1.7.8.4

[PATCH 3/5] staging:iio:dac:ad5446 unwind use of is_visible for attrs.

[Jonathan Cameron]

Trivial case where no attributes are valid for some parts. Better
handled using two iio_info structures and selecting the right one
at probe time.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/dac/ad5446.c |   33 ++++++++++-----------------------
 1 files changed, 10 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/iio/dac/ad5446.c b/drivers/staging/iio/dac/ad5446.c
index 693e748..e439bae 100644
--- a/drivers/staging/iio/dac/ad5446.c
+++ b/drivers/staging/iio/dac/ad5446.c
@@ -149,30 +149,8 @@ static struct attribute *ad5446_attributes[] = {
 	NULL,
 };
 
-static umode_t ad5446_attr_is_visible(struct kobject *kobj,
-				     struct attribute *attr, int n)
-{
-	struct device *dev = container_of(kobj, struct device, kobj);
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
-	struct ad5446_state *st = iio_priv(indio_dev);
-
-	umode_t mode = attr->mode;
-
-	if (!st->chip_info->store_pwr_down &&
-		(attr == &iio_dev_attr_out_voltage0_powerdown.dev_attr.attr ||
-		attr == &iio_dev_attr_out_voltage_powerdown_mode.
-		 dev_attr.attr ||
-		attr ==
-		&iio_const_attr_out_voltage_powerdown_mode_available.
-		 dev_attr.attr))
-		mode = 0;
-
-	return mode;
-}
-
 static const struct attribute_group ad5446_attribute_group = {
 	.attrs = ad5446_attributes,
-	.is_visible = ad5446_attr_is_visible,
 };
 
 #define AD5446_CHANNEL(bits, storage, shift) { \
@@ -321,6 +299,12 @@ static const struct iio_info ad5446_info = {
 	.driver_module = THIS_MODULE,
 };
 
+static const struct iio_info ad5446_info_no_pwr_down = {
+	.read_raw = ad5446_read_raw,
+	.write_raw = ad5446_write_raw,
+	.driver_module = THIS_MODULE,
+};
+
 static int __devinit ad5446_probe(struct spi_device *spi)
 {
 	struct ad5446_state *st;
@@ -353,7 +337,10 @@ static int __devinit ad5446_probe(struct spi_device *spi)
 	/* Estabilish that the iio_dev is a child of the spi device */
 	indio_dev->dev.parent = &spi->dev;
 	indio_dev->name = spi_get_device_id(spi)->name;
-	indio_dev->info = &ad5446_info;
+	if (st->chip_info->store_pwr_down)
+		indio_dev->info = &ad5446_info;
+	else
+		indio_dev->info = &ad5446_info_no_pwr_down;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = &st->chip_info->channel;
 	indio_dev->num_channels = 1;
-- 
1.7.8.4

[PATCH 4/5] staging:iio:adc:ad7606 unwind use of is_visible for attrs.

[Jonathan Cameron]

This is the most controversial of this set of is_visible removals.
There are two conditions controlling availability of attrs resulting
in 4 different attribute groups.

Still for a few more lines things are clearer to read to my mind.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/ad7606_core.c |   83 +++++++++++++++++++++------------
 1 files changed, 53 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
index ddb7ef9..97e8d3d 100644
--- a/drivers/staging/iio/adc/ad7606_core.c
+++ b/drivers/staging/iio/adc/ad7606_core.c
@@ -197,7 +197,7 @@ static IIO_DEVICE_ATTR(oversampling_ratio, S_IRUGO | S_IWUSR,
 		       ad7606_store_oversampling_ratio, 0);
 static IIO_CONST_ATTR(oversampling_ratio_available, "0 2 4 8 16 32 64");
 
-static struct attribute *ad7606_attributes[] = {
+static struct attribute *ad7606_attributes_os_and_range[] = {
 	&iio_dev_attr_in_voltage_range.dev_attr.attr,
 	&iio_const_attr_in_voltage_range_available.dev_attr.attr,
 	&iio_dev_attr_oversampling_ratio.dev_attr.attr,
@@ -205,34 +205,28 @@ static struct attribute *ad7606_attributes[] = {
 	NULL,
 };
 
-static umode_t ad7606_attr_is_visible(struct kobject *kobj,
-				     struct attribute *attr, int n)
-{
-	struct device *dev = container_of(kobj, struct device, kobj);
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
-	struct ad7606_state *st = iio_priv(indio_dev);
+static const struct attribute_group ad7606_attribute_group_os_and_range = {
+	.attrs = ad7606_attributes_os_and_range,
+};
 
-	umode_t mode = attr->mode;
-
-	if (!(gpio_is_valid(st->pdata->gpio_os0) &&
-	      gpio_is_valid(st->pdata->gpio_os1) &&
-	      gpio_is_valid(st->pdata->gpio_os2)) &&
-	    (attr == &iio_dev_attr_oversampling_ratio.dev_attr.attr ||
-	     attr ==
-	     &iio_const_attr_oversampling_ratio_available.dev_attr.attr))
-		mode = 0;
-	else if (!gpio_is_valid(st->pdata->gpio_range) &&
-		 (attr == &iio_dev_attr_in_voltage_range.dev_attr.attr ||
-		  attr ==
-		  &iio_const_attr_in_voltage_range_available.dev_attr.attr))
-		mode = 0;
-
-	return mode;
-}
+static struct attribute *ad7606_attributes_os[] = {
+	&iio_dev_attr_oversampling_ratio.dev_attr.attr,
+	&iio_const_attr_oversampling_ratio_available.dev_attr.attr,
+	NULL,
+};
 
-static const struct attribute_group ad7606_attribute_group = {
-	.attrs = ad7606_attributes,
-	.is_visible = ad7606_attr_is_visible,
+static const struct attribute_group ad7606_attribute_group_os = {
+	.attrs = ad7606_attributes_os,
+};
+
+static struct attribute *ad7606_attributes_range[] = {
+	&iio_dev_attr_in_voltage_range.dev_attr.attr,
+	&iio_const_attr_in_voltage_range_available.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group ad7606_attribute_group_range = {
+	.attrs = ad7606_attributes_range,
 };
 
 #define AD7606_CHANNEL(num)				\
@@ -435,10 +429,27 @@ static irqreturn_t ad7606_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 };
 
-static const struct iio_info ad7606_info = {
+static const struct iio_info ad7606_info_no_os_or_range = {
 	.driver_module = THIS_MODULE,
 	.read_raw = &ad7606_read_raw,
-	.attrs = &ad7606_attribute_group,
+};
+
+static const struct iio_info ad7606_info_os_and_range = {
+	.driver_module = THIS_MODULE,
+	.read_raw = &ad7606_read_raw,
+	.attrs = &ad7606_attribute_group_os_and_range,
+};
+
+static const struct iio_info ad7606_info_os = {
+	.driver_module = THIS_MODULE,
+	.read_raw = &ad7606_read_raw,
+	.attrs = &ad7606_attribute_group_os,
+};
+
+static const struct iio_info ad7606_info_range = {
+	.driver_module = THIS_MODULE,
+	.read_raw = &ad7606_read_raw,
+	.attrs = &ad7606_attribute_group_range,
 };
 
 struct iio_dev *ad7606_probe(struct device *dev, int irq,
@@ -483,7 +494,19 @@ struct iio_dev *ad7606_probe(struct device *dev, int irq,
 	st->chip_info = &ad7606_chip_info_tbl[id];
 
 	indio_dev->dev.parent = dev;
-	indio_dev->info = &ad7606_info;
+	if (gpio_is_valid(st->pdata->gpio_os0) &&
+	    gpio_is_valid(st->pdata->gpio_os1) &&
+	    gpio_is_valid(st->pdata->gpio_os2)) {
+		if (gpio_is_valid(st->pdata->gpio_range))
+			indio_dev->info = &ad7606_info_os_and_range;
+		else
+			indio_dev->info = &ad7606_info_os;
+	} else {
+		if (gpio_is_valid(st->pdata->gpio_range))
+			indio_dev->info = &ad7606_info_range;
+		else
+			indio_dev->info = &ad7606_info_no_os_or_range;
+	}
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->name = st->chip_info->name;
 	indio_dev->channels = st->chip_info->channels;
-- 
1.7.8.4

[PATCH 5/5] staging:iio:adc:adt7310/7410 sticking plaster fix for broken event attrs.

[Jonathan Cameron]

Neither of these drivers has ever been anywhere near the iio abi.
Probably as a result of this the fact they had two event groups
each was not picked up when we restricted IIO to having only
1 event line per device (as part of the chrdev merge set).

As such these definitely didn't work before.  This patch squishes
the only element from the 'comparator' event line that isn't in the
'interrupt' one into it and kills off the 'comparator' one.

Ultimately both of these drivers belong in hwmon not IIO and are just
waiting here because I don't want to kill off a driver that may
prove useful to someone.  (Ultimately I will ask Greg to scrap
these two if no one steps up to deal with them.)

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/adc/adt7310.c |   21 ++++-----------------
 drivers/staging/iio/adc/adt7410.c |   21 ++++-----------------
 2 files changed, 8 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/iio/adc/adt7310.c b/drivers/staging/iio/adc/adt7310.c
index eec2f32..caf57c1 100644
--- a/drivers/staging/iio/adc/adt7310.c
+++ b/drivers/staging/iio/adc/adt7310.c
@@ -725,32 +725,19 @@ static struct attribute *adt7310_event_int_attributes[] = {
 	&iio_dev_attr_fault_queue.dev_attr.attr,
 	&iio_dev_attr_t_alarm_high.dev_attr.attr,
 	&iio_dev_attr_t_alarm_low.dev_attr.attr,
-	&iio_dev_attr_t_hyst.dev_attr.attr,
-	NULL,
-};
-
-static struct attribute *adt7310_event_ct_attributes[] = {
-	&iio_dev_attr_event_mode.dev_attr.attr,
-	&iio_dev_attr_available_event_modes.dev_attr.attr,
-	&iio_dev_attr_fault_queue.dev_attr.attr,
 	&iio_dev_attr_t_crit.dev_attr.attr,
 	&iio_dev_attr_t_hyst.dev_attr.attr,
 	NULL,
 };
 
-static struct attribute_group adt7310_event_attribute_group[ADT7310_IRQS] = {
-	{
-		.attrs = adt7310_event_int_attributes,
-		.name = "events",
-	}, {
-		.attrs = adt7310_event_ct_attributes,
-		.name = "events",
-	}
+static struct attribute_group adt7310_event_attribute_group = {
+	.attrs = adt7310_event_int_attributes,
+	.name = "events",
 };
 
 static const struct iio_info adt7310_info = {
 	.attrs = &adt7310_attribute_group,
-	.event_attrs = adt7310_event_attribute_group,
+	.event_attrs = &adt7310_event_attribute_group,
 	.driver_module = THIS_MODULE,
 };
 
diff --git a/drivers/staging/iio/adc/adt7410.c b/drivers/staging/iio/adc/adt7410.c
index c62248c..dff3e8c 100644
--- a/drivers/staging/iio/adc/adt7410.c
+++ b/drivers/staging/iio/adc/adt7410.c
@@ -693,32 +693,19 @@ static struct attribute *adt7410_event_int_attributes[] = {
 	&iio_dev_attr_fault_queue.dev_attr.attr,
 	&iio_dev_attr_t_alarm_high.dev_attr.attr,
 	&iio_dev_attr_t_alarm_low.dev_attr.attr,
-	&iio_dev_attr_t_hyst.dev_attr.attr,
-	NULL,
-};
-
-static struct attribute *adt7410_event_ct_attributes[] = {
-	&iio_dev_attr_event_mode.dev_attr.attr,
-	&iio_dev_attr_available_event_modes.dev_attr.attr,
-	&iio_dev_attr_fault_queue.dev_attr.attr,
 	&iio_dev_attr_t_crit.dev_attr.attr,
 	&iio_dev_attr_t_hyst.dev_attr.attr,
 	NULL,
 };
 
-static struct attribute_group adt7410_event_attribute_group[ADT7410_IRQS] = {
-	{
-		.attrs = adt7410_event_int_attributes,
-		.name = "events",
-	}, {
-		.attrs = adt7410_event_ct_attributes,
-		.name = "events",
-	}
+static struct attribute_group adt7410_event_attribute_group = {
+	.attrs = adt7410_event_int_attributes,
+	.name = "events",
 };
 
 static const struct iio_info adt7410_info = {
 	.attrs = &adt7410_attribute_group,
-	.event_attrs = adt7410_event_attribute_group,
+	.event_attrs = &adt7410_event_attribute_group,
 	.driver_module = THIS_MODULE,
 };
 
-- 
1.7.8.4