[PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405

[PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405

[Andrew F. Davis]

Hello all,

I will be posting a driver for the AFE4405 soon and in preparation
for this I have made some changes to the existing drivers to better
align them with the new part. Some of these changes are trivial, others
change the sysfs entries, I understand these are considered ABIs and
changes to even testing ABIs should not be made lightly, but my hope is
that these changes will more accurately reflect the devices intended use
and improve functionality as the first users begin to appear.

Thanks,
Andrew

Andrew F. Davis (13):
  iio: health/afe440x: Fix kernel-doc format
  iio: health/afe440x: Remove of_match_ptr and ifdefs
  iio: health/afe440x: Remove unneeded initializers
  iio: health/afe440x: Always use separate gain values
  iio: health/afe440x: Fix scan_index assignment
  iio: health/afe440x: Remove unneeded offset handling
  iio: health/afe4404: Remove LED3 input channel
  iio: health/afe440x: Remove channel names
  iio: health/afe440x: Use regmap fields
  iio: health/afe440x: Make gain settings a modifier for the stages
  iio: health/afe440x: Match LED currents to stages
  iio: health/afe440x: Remove unused definitions
  iio: health/afe4404: ENSEPGAIN is part of CONTROL2 register

 .../ABI/testing/sysfs-bus-iio-health-afe440x       |  95 +++----
 drivers/iio/health/afe4403.c                       | 299 ++++++++------------
 drivers/iio/health/afe4404.c                       | 308 +++++++++------------
 drivers/iio/health/afe440x.h                       |  48 +---
 4 files changed, 295 insertions(+), 455 deletions(-)
 rewrite Documentation/ABI/testing/sysfs-bus-iio-health-afe440x (72%)

-- 
2.8.1

[PATCH 01/13] iio: health/afe440x: Fix kernel-doc format

[Andrew F. Davis]

Fix kernel-doc formatting for structs, and while we are making little
fixes, clarify the module description and update the copywrite.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 drivers/iio/health/afe4403.c | 18 +++++++++---------
 drivers/iio/health/afe4404.c | 16 ++++++++--------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
index 88e43f8..2094113 100644
--- a/drivers/iio/health/afe4403.c
+++ b/drivers/iio/health/afe4403.c
@@ -1,7 +1,7 @@
 /*
  * AFE4403 Heart Rate Monitors and Low-Cost Pulse Oximeters
  *
- * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
+ * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/
  *	Andrew F. Davis <afd@ti.com>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -103,13 +103,13 @@
 #define AFE4403_TIAGAIN_RES_NONE	0x7
 
 /**
- * struct afe4403_data
- * @dev - Device structure
- * @spi - SPI device handle
- * @regmap - Register map of the device
- * @regulator - Pointer to the regulator for the IC
- * @trig - IIO trigger for this device
- * @irq - ADC_RDY line interrupt number
+ * struct afe4403_data - AFE4403 device instance data
+ * @dev: Device structure
+ * @spi: SPI device handle
+ * @regmap: Register map of the device
+ * @regulator: Pointer to the regulator for the IC
+ * @trig: IIO trigger for this device
+ * @irq: ADC_RDY line interrupt number
  */
 struct afe4403_data {
 	struct device *dev;
@@ -704,5 +704,5 @@ static struct spi_driver afe4403_spi_driver = {
 module_spi_driver(afe4403_spi_driver);
 
 MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>");
-MODULE_DESCRIPTION("TI AFE4403 Heart Rate and Pulse Oximeter");
+MODULE_DESCRIPTION("TI AFE4403 Heart Rate Monitor and Pulse Oximeter AFE");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
index 5096a46..7127d03 100644
--- a/drivers/iio/health/afe4404.c
+++ b/drivers/iio/health/afe4404.c
@@ -1,7 +1,7 @@
 /*
  * AFE4404 Heart Rate Monitors and Low-Cost Pulse Oximeters
  *
- * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
+ * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/
  *	Andrew F. Davis <afd@ti.com>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -106,12 +106,12 @@
 #define AFE4404_TIA_GAIN_RES_2_M	0x7
 
 /**
- * struct afe4404_data
- * @dev - Device structure
- * @regmap - Register map of the device
- * @regulator - Pointer to the regulator for the IC
- * @trig - IIO trigger for this device
- * @irq - ADC_RDY line interrupt number
+ * struct afe4404_data - AFE4404 device instance data
+ * @dev: Device structure
+ * @regmap: Register map of the device
+ * @regulator: Pointer to the regulator for the IC
+ * @trig: IIO trigger for this device
+ * @irq: ADC_RDY line interrupt number
  */
 struct afe4404_data {
 	struct device *dev;
@@ -675,5 +675,5 @@ static struct i2c_driver afe4404_i2c_driver = {
 module_i2c_driver(afe4404_i2c_driver);
 
 MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>");
-MODULE_DESCRIPTION("TI AFE4404 Heart Rate and Pulse Oximeter");
+MODULE_DESCRIPTION("TI AFE4404 Heart Rate Monitor and Pulse Oximeter AFE");
 MODULE_LICENSE("GPL v2");
-- 
2.8.1

[PATCH 02/13] iio: health/afe440x: Remove of_match_ptr and ifdefs

[Andrew F. Davis]

The drivers DT tables are not built-in when OF is not enabled, this does
not save us enough to justify ugly ifdefs. Clean this up.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 drivers/iio/health/afe4403.c | 4 +---
 drivers/iio/health/afe4404.c | 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
index 2094113..e3700cf 100644
--- a/drivers/iio/health/afe4403.c
+++ b/drivers/iio/health/afe4403.c
@@ -498,13 +498,11 @@ static const struct regmap_config afe4403_regmap_config = {
 	.volatile_table = &afe4403_volatile_table,
 };
 
-#ifdef CONFIG_OF
 static const struct of_device_id afe4403_of_match[] = {
 	{ .compatible = "ti,afe4403", },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, afe4403_of_match);
-#endif
 
 static int __maybe_unused afe4403_suspend(struct device *dev)
 {
@@ -694,7 +692,7 @@ MODULE_DEVICE_TABLE(spi, afe4403_ids);
 static struct spi_driver afe4403_spi_driver = {
 	.driver = {
 		.name = AFE4403_DRIVER_NAME,
-		.of_match_table = of_match_ptr(afe4403_of_match),
+		.of_match_table = afe4403_of_match,
 		.pm = &afe4403_pm_ops,
 	},
 	.probe = afe4403_probe,
diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
index 7127d03..783b9b4 100644
--- a/drivers/iio/health/afe4404.c
+++ b/drivers/iio/health/afe4404.c
@@ -469,13 +469,11 @@ static const struct regmap_config afe4404_regmap_config = {
 	.volatile_table = &afe4404_volatile_table,
 };
 
-#ifdef CONFIG_OF
 static const struct of_device_id afe4404_of_match[] = {
 	{ .compatible = "ti,afe4404", },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, afe4404_of_match);
-#endif
 
 static int __maybe_unused afe4404_suspend(struct device *dev)
 {
@@ -665,7 +663,7 @@ MODULE_DEVICE_TABLE(i2c, afe4404_ids);
 static struct i2c_driver afe4404_i2c_driver = {
 	.driver = {
 		.name = AFE4404_DRIVER_NAME,
-		.of_match_table = of_match_ptr(afe4404_of_match),
+		.of_match_table = afe4404_of_match,
 		.pm = &afe4404_pm_ops,
 	},
 	.probe = afe4404_probe,
-- 
2.8.1

[PATCH 03/13] iio: health/afe440x: Remove unneeded initializers

[Andrew F. Davis]

The drivers set some register values during initialization that can be
set at runtime, these defaults were used in testing but are not
necessary, remove these.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 drivers/iio/health/afe4403.c | 7 +------
 drivers/iio/health/afe4404.c | 4 ----
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
index e3700cf..5484785 100644
--- a/drivers/iio/health/afe4403.c
+++ b/drivers/iio/health/afe4403.c
@@ -472,12 +472,7 @@ static const struct iio_trigger_ops afe4403_trigger_ops = {
 
 static const struct reg_sequence afe4403_reg_sequences[] = {
 	AFE4403_TIMING_PAIRS,
-	{ AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN | 0x000007},
-	{ AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES_1_M },
-	{ AFE440X_LEDCNTRL, (0x14 << AFE440X_LEDCNTRL_LED1_SHIFT) |
-			    (0x14 << AFE440X_LEDCNTRL_LED2_SHIFT) },
-	{ AFE440X_CONTROL2, AFE440X_CONTROL2_TX_REF_050 <<
-			    AFE440X_CONTROL2_TX_REF_SHIFT },
+	{ AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN },
 };
 
 static const struct regmap_range afe4403_yes_ranges[] = {
diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
index 783b9b4..2d4c522 100644
--- a/drivers/iio/health/afe4404.c
+++ b/drivers/iio/health/afe4404.c
@@ -443,10 +443,6 @@ static const struct iio_trigger_ops afe4404_trigger_ops = {
 static const struct reg_sequence afe4404_reg_sequences[] = {
 	AFE4404_TIMING_PAIRS,
 	{ AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN },
-	{ AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES_50_K },
-	{ AFE440X_LEDCNTRL, (0xf << AFE4404_LEDCNTRL_ILED1_SHIFT) |
-			    (0x3 << AFE4404_LEDCNTRL_ILED2_SHIFT) |
-			    (0x3 << AFE4404_LEDCNTRL_ILED3_SHIFT) },
 	{ AFE440X_CONTROL2, AFE440X_CONTROL3_OSC_ENABLE	},
 };
 
-- 
2.8.1

[PATCH 04/13] iio: health/afe440x: Always use separate gain values

[Andrew F. Davis]

Locking the two gain stages to the same setting adds no value for us,
so initialize them as unlocked and remove the sysfs for unlocking them.
This also allows us to greatly simplify showing and setting the gain
registers.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 .../ABI/testing/sysfs-bus-iio-health-afe440x       |  9 ----
 drivers/iio/health/afe4403.c                       | 60 ++++++----------------
 drivers/iio/health/afe4404.c                       | 60 ++++++----------------
 drivers/iio/health/afe440x.h                       | 15 ++----
 4 files changed, 37 insertions(+), 107 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
index 3740f25..b19053a 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
+++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
@@ -8,15 +8,6 @@ Description:
 		Transimpedance Amplifier. Y is 1 for Rf1 and Cf1, Y is 2 for
 		Rf2 and Cf2 values.
 
-What:		/sys/bus/iio/devices/iio:deviceX/tia_separate_en
-Date:		December 2015
-KernelVersion:
-Contact:	Andrew F. Davis <afd@ti.com>
-Description:
-		Enable or disable separate settings for the TransImpedance
-		Amplifier above, when disabled both values are set by the
-		first channel.
-
 What:		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_raw
 		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_ambient_raw
 Date:		December 2015
diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
index 5484785..bcff528 100644
--- a/drivers/iio/health/afe4403.c
+++ b/drivers/iio/health/afe4403.c
@@ -180,9 +180,9 @@ static ssize_t afe440x_show_register(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct afe4403_data *afe = iio_priv(indio_dev);
 	struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr);
-	unsigned int reg_val, type;
+	unsigned int reg_val;
 	int vals[2];
-	int ret, val_len;
+	int ret;
 
 	ret = regmap_read(afe->regmap, afe440x_attr->reg, &reg_val);
 	if (ret)
@@ -191,27 +191,13 @@ static ssize_t afe440x_show_register(struct device *dev,
 	reg_val &= afe440x_attr->mask;
 	reg_val >>= afe440x_attr->shift;
 
-	switch (afe440x_attr->type) {
-	case SIMPLE:
-		type = IIO_VAL_INT;
-		val_len = 1;
-		vals[0] = reg_val;
-		break;
-	case RESISTANCE:
-	case CAPACITANCE:
-		type = IIO_VAL_INT_PLUS_MICRO;
-		val_len = 2;
-		if (reg_val < afe440x_attr->table_size) {
-			vals[0] = afe440x_attr->val_table[reg_val].integer;
-			vals[1] = afe440x_attr->val_table[reg_val].fract;
-			break;
-		}
+	if (reg_val >= afe440x_attr->table_size)
 		return -EINVAL;
-	default:
-		return -EINVAL;
-	}
 
-	return iio_format_value(buf, type, val_len, vals);
+	vals[0] = afe440x_attr->val_table[reg_val].integer;
+	vals[1] = afe440x_attr->val_table[reg_val].fract;
+
+	return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals);
 }
 
 static ssize_t afe440x_store_register(struct device *dev,
@@ -227,22 +213,12 @@ static ssize_t afe440x_store_register(struct device *dev,
 	if (ret)
 		return ret;
 
-	switch (afe440x_attr->type) {
-	case SIMPLE:
-		val = integer;
-		break;
-	case RESISTANCE:
-	case CAPACITANCE:
-		for (val = 0; val < afe440x_attr->table_size; val++)
-			if (afe440x_attr->val_table[val].integer == integer &&
-			    afe440x_attr->val_table[val].fract == fract)
-				break;
-		if (val == afe440x_attr->table_size)
-			return -EINVAL;
-		break;
-	default:
+	for (val = 0; val < afe440x_attr->table_size; val++)
+		if (afe440x_attr->val_table[val].integer == integer &&
+		    afe440x_attr->val_table[val].fract == fract)
+			break;
+	if (val == afe440x_attr->table_size)
 		return -EINVAL;
-	}
 
 	ret = regmap_update_bits(afe->regmap, afe440x_attr->reg,
 				 afe440x_attr->mask,
@@ -253,16 +229,13 @@ static ssize_t afe440x_store_register(struct device *dev,
 	return count;
 }
 
-static AFE440X_ATTR(tia_separate_en, AFE4403_TIAGAIN, AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0);
-
-static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table, ARRAY_SIZE(afe4403_res_table));
-static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, CAPACITANCE, afe4403_cap_table, ARRAY_SIZE(afe4403_cap_table));
+static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, afe4403_res_table);
+static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, afe4403_cap_table);
 
-static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table, ARRAY_SIZE(afe4403_res_table));
-static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, CAPACITANCE, afe4403_cap_table, ARRAY_SIZE(afe4403_cap_table));
+static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_res_table);
+static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_cap_table);
 
 static struct attribute *afe440x_attributes[] = {
-	&afe440x_attr_tia_separate_en.dev_attr.attr,
 	&afe440x_attr_tia_resistance1.dev_attr.attr,
 	&afe440x_attr_tia_capacitance1.dev_attr.attr,
 	&afe440x_attr_tia_resistance2.dev_attr.attr,
@@ -473,6 +446,7 @@ static const struct iio_trigger_ops afe4403_trigger_ops = {
 static const struct reg_sequence afe4403_reg_sequences[] = {
 	AFE4403_TIMING_PAIRS,
 	{ AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN },
+	{ AFE4403_TIAGAIN, AFE440X_TIAGAIN_ENSEPGAIN },
 };
 
 static const struct regmap_range afe4403_yes_ranges[] = {
diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
index 2d4c522..b9c1666 100644
--- a/drivers/iio/health/afe4404.c
+++ b/drivers/iio/health/afe4404.c
@@ -193,9 +193,9 @@ static ssize_t afe440x_show_register(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct afe4404_data *afe = iio_priv(indio_dev);
 	struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr);
-	unsigned int reg_val, type;
+	unsigned int reg_val;
 	int vals[2];
-	int ret, val_len;
+	int ret;
 
 	ret = regmap_read(afe->regmap, afe440x_attr->reg, &reg_val);
 	if (ret)
@@ -204,27 +204,13 @@ static ssize_t afe440x_show_register(struct device *dev,
 	reg_val &= afe440x_attr->mask;
 	reg_val >>= afe440x_attr->shift;
 
-	switch (afe440x_attr->type) {
-	case SIMPLE:
-		type = IIO_VAL_INT;
-		val_len = 1;
-		vals[0] = reg_val;
-		break;
-	case RESISTANCE:
-	case CAPACITANCE:
-		type = IIO_VAL_INT_PLUS_MICRO;
-		val_len = 2;
-		if (reg_val < afe440x_attr->table_size) {
-			vals[0] = afe440x_attr->val_table[reg_val].integer;
-			vals[1] = afe440x_attr->val_table[reg_val].fract;
-			break;
-		}
+	if (reg_val >= afe440x_attr->table_size)
 		return -EINVAL;
-	default:
-		return -EINVAL;
-	}
 
-	return iio_format_value(buf, type, val_len, vals);
+	vals[0] = afe440x_attr->val_table[reg_val].integer;
+	vals[1] = afe440x_attr->val_table[reg_val].fract;
+
+	return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals);
 }
 
 static ssize_t afe440x_store_register(struct device *dev,
@@ -240,22 +226,12 @@ static ssize_t afe440x_store_register(struct device *dev,
 	if (ret)
 		return ret;
 
-	switch (afe440x_attr->type) {
-	case SIMPLE:
-		val = integer;
-		break;
-	case RESISTANCE:
-	case CAPACITANCE:
-		for (val = 0; val < afe440x_attr->table_size; val++)
-			if (afe440x_attr->val_table[val].integer == integer &&
-			    afe440x_attr->val_table[val].fract == fract)
-				break;
-		if (val == afe440x_attr->table_size)
-			return -EINVAL;
-		break;
-	default:
+	for (val = 0; val < afe440x_attr->table_size; val++)
+		if (afe440x_attr->val_table[val].integer == integer &&
+		    afe440x_attr->val_table[val].fract == fract)
+			break;
+	if (val == afe440x_attr->table_size)
 		return -EINVAL;
-	}
 
 	ret = regmap_update_bits(afe->regmap, afe440x_attr->reg,
 				 afe440x_attr->mask,
@@ -266,16 +242,13 @@ static ssize_t afe440x_store_register(struct device *dev,
 	return count;
 }
 
-static AFE440X_ATTR(tia_separate_en, AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0);
-
-static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, RESISTANCE, afe4404_res_table, ARRAY_SIZE(afe4404_res_table));
-static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, CAPACITANCE, afe4404_cap_table, ARRAY_SIZE(afe4404_cap_table));
+static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, afe4404_res_table);
+static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, afe4404_cap_table);
 
-static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, RESISTANCE, afe4404_res_table, ARRAY_SIZE(afe4404_res_table));
-static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, CAPACITANCE, afe4404_cap_table, ARRAY_SIZE(afe4404_cap_table));
+static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, afe4404_res_table);
+static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, afe4404_cap_table);
 
 static struct attribute *afe440x_attributes[] = {
-	&afe440x_attr_tia_separate_en.dev_attr.attr,
 	&afe440x_attr_tia_resistance1.dev_attr.attr,
 	&afe440x_attr_tia_capacitance1.dev_attr.attr,
 	&afe440x_attr_tia_resistance2.dev_attr.attr,
@@ -443,6 +416,7 @@ static const struct iio_trigger_ops afe4404_trigger_ops = {
 static const struct reg_sequence afe4404_reg_sequences[] = {
 	AFE4404_TIMING_PAIRS,
 	{ AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN },
+	{ AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN },
 	{ AFE440X_CONTROL2, AFE440X_CONTROL3_OSC_ENABLE	},
 };
 
diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h
index c671ab7..544bbab 100644
--- a/drivers/iio/health/afe440x.h
+++ b/drivers/iio/health/afe440x.h
@@ -71,8 +71,7 @@
 #define AFE440X_CONTROL1_TIMEREN	BIT(8)
 
 /* TIAGAIN register fields */
-#define AFE440X_TIAGAIN_ENSEPGAIN_MASK	BIT(15)
-#define AFE440X_TIAGAIN_ENSEPGAIN_SHIFT	15
+#define AFE440X_TIAGAIN_ENSEPGAIN	BIT(15)
 
 /* CONTROL2 register fields */
 #define AFE440X_CONTROL2_PDN_AFE	BIT(0)
@@ -133,12 +132,6 @@ struct afe440x_reg_info {
 		.output = true,					\
 	}
 
-enum afe440x_reg_type {
-	SIMPLE,
-	RESISTANCE,
-	CAPACITANCE,
-};
-
 struct afe440x_val_table {
 	int integer;
 	int fract;
@@ -167,7 +160,6 @@ struct afe440x_attr {
 	unsigned int reg;
 	unsigned int shift;
 	unsigned int mask;
-	enum afe440x_reg_type type;
 	const struct afe440x_val_table *val_table;
 	unsigned int table_size;
 };
@@ -175,7 +167,7 @@ struct afe440x_attr {
 #define to_afe440x_attr(_dev_attr)				\
 	container_of(_dev_attr, struct afe440x_attr, dev_attr)
 
-#define AFE440X_ATTR(_name, _reg, _field, _type, _table, _size)	\
+#define AFE440X_ATTR(_name, _reg, _field, _table)		\
 	struct afe440x_attr afe440x_attr_##_name = {		\
 		.dev_attr = __ATTR(_name, (S_IRUGO | S_IWUSR),	\
 				   afe440x_show_register,	\
@@ -183,9 +175,8 @@ struct afe440x_attr {
 		.reg = _reg,					\
 		.shift = _field ## _SHIFT,			\
 		.mask = _field ## _MASK,			\
-		.type = _type,					\
 		.val_table = _table,				\
-		.table_size = _size,				\
+		.table_size = ARRAY_SIZE(_table),		\
 	}
 
 #endif /* _AFE440X_H */
-- 
2.8.1

[PATCH 05/13] iio: health/afe440x: Fix scan_index assignment

[Andrew F. Davis]

The LED channels are not scannable and so scan_index should
be negative, fix this here.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 drivers/iio/health/afe440x.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h
index 544bbab..583d071 100644
--- a/drivers/iio/health/afe440x.h
+++ b/drivers/iio/health/afe440x.h
@@ -125,7 +125,7 @@ struct afe440x_reg_info {
 		.type = IIO_CURRENT,				\
 		.channel = _index,				\
 		.address = _index,				\
-		.scan_index = _index,				\
+		.scan_index = -1,				\
 		.extend_name = _name,				\
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
 			BIT(IIO_CHAN_INFO_SCALE),		\
-- 
2.8.1

[PATCH 06/13] iio: health/afe440x: Remove unneeded offset handling

[Andrew F. Davis]

No channel in the afe4403 driver has IIO_CHAN_INFO_OFFSET set so
remove the handlers for this.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 drivers/iio/health/afe4403.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
index bcff528..cac6090 100644
--- a/drivers/iio/health/afe4403.c
+++ b/drivers/iio/health/afe4403.c
@@ -293,14 +293,6 @@ static int afe4403_read_raw(struct iio_dev *indio_dev,
 			if (ret)
 				return ret;
 			return IIO_VAL_INT;
-		case IIO_CHAN_INFO_OFFSET:
-			ret = regmap_read(afe->regmap, reg_info.offreg,
-					  val);
-			if (ret)
-				return ret;
-			*val &= reg_info.mask;
-			*val >>= reg_info.shift;
-			return IIO_VAL_INT;
 		}
 		break;
 	case IIO_CURRENT:
@@ -333,15 +325,6 @@ static int afe4403_write_raw(struct iio_dev *indio_dev,
 	const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address];
 
 	switch (chan->type) {
-	case IIO_INTENSITY:
-		switch (mask) {
-		case IIO_CHAN_INFO_OFFSET:
-			return regmap_update_bits(afe->regmap,
-				reg_info.offreg,
-				reg_info.mask,
-				(val << reg_info.shift));
-		}
-		break;
 	case IIO_CURRENT:
 		switch (mask) {
 		case IIO_CHAN_INFO_RAW:
-- 
2.8.1

[PATCH 07/13] iio: health/afe4404: Remove LED3 input channel

[Andrew F. Davis]

Input channel LED3 is only an alias for stage ALED2, this virtual channel
does nothing for us, remove this channel.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 drivers/iio/health/afe4404.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
index b9c1666..2edb7d7 100644
--- a/drivers/iio/health/afe4404.c
+++ b/drivers/iio/health/afe4404.c
@@ -126,7 +126,6 @@ enum afe4404_chan_id {
 	ALED1,
 	LED2,
 	ALED2,
-	LED3,
 	LED1_ALED1,
 	LED2_ALED2,
 	ILED1,
@@ -139,7 +138,6 @@ static const struct afe440x_reg_info afe4404_reg_info[] = {
 	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED1),
 	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED2),
 	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED2),
-	[LED3] = AFE440X_REG_INFO(AFE440X_ALED2VAL, 0, NULL),
 	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
 	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
 	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED1),
@@ -153,7 +151,6 @@ static const struct iio_chan_spec afe4404_channels[] = {
 	AFE440X_INTENSITY_CHAN(ALED1, "led1_ambient", BIT(IIO_CHAN_INFO_OFFSET)),
 	AFE440X_INTENSITY_CHAN(LED2, "led2", BIT(IIO_CHAN_INFO_OFFSET)),
 	AFE440X_INTENSITY_CHAN(ALED2, "led2_ambient", BIT(IIO_CHAN_INFO_OFFSET)),
-	AFE440X_INTENSITY_CHAN(LED3, "led3", BIT(IIO_CHAN_INFO_OFFSET)),
 	AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0),
 	AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0),
 	/* LED current */
-- 
2.8.1

[PATCH 08/13] iio: health/afe440x: Remove channel names

[Andrew F. Davis]

These AFEs have 4 ADC mesuring stages (called LED2, ALED2, LED1, and
ALED1 in the datasheet), we map these as channels, these stages can serve
different purposes depending on the application. For instance the AFE4404
has an additional LED (LED3), this LED can be timed to be active during
stage 2 (or anystage, but the datasheet describes this case and the name
of the stage reflects this use). This ability is used further in upcoming
parts that tie the front-end gain and the LED timings together. For these
reasons we remove explicit naming the channels.

Without channel names it is best that the index numbers are in order to
match the stage number, reorder the channel numbers.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 .../ABI/testing/sysfs-bus-iio-health-afe440x       | 39 ++++++++++------------
 drivers/iio/health/afe4403.c                       | 28 ++++++++--------
 drivers/iio/health/afe4404.c                       | 30 ++++++++---------
 drivers/iio/health/afe440x.h                       |  8 ++---
 4 files changed, 51 insertions(+), 54 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
index b19053a..a067073 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
+++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
@@ -8,38 +8,35 @@ Description:
 		Transimpedance Amplifier. Y is 1 for Rf1 and Cf1, Y is 2 for
 		Rf2 and Cf2 values.
 
-What:		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_raw
-		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_ambient_raw
-Date:		December 2015
+What:		/sys/bus/iio/devices/iio:deviceX/in_intensityY_raw
+Date:		May 2016
 KernelVersion:
 Contact:	Andrew F. Davis <afd@ti.com>
 Description:
 		Get measured values from the ADC for these stages. Y is the
-		specific LED number. The values are expressed in 24-bit twos
-		complement.
-
-What:		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY-ledY_ambient_raw
-Date:		December 2015
-KernelVersion:
-Contact:	Andrew F. Davis <afd@ti.com>
-Description:
-		Get differential values from the ADC for these stages. Y is the
-		specific LED number. The values are expressed in 24-bit twos
-		complement for the specified LEDs.
+		specific stage number corresponding to datasheet stage names
+		as follows:
+		1 -> LED2
+		2 -> ALED2/LED3
+		3 -> LED1
+		4 -> ALED1/LED4
+		Note that channels 5 and 6 represent LED2-ALED2 and LED1-ALED1
+		respectively which simply helper channels containing the
+		calculated difference in the value of stage 1 - 2 and 3 - 4.
+		The values are expressed in 24-bit twos complement.
 
-What:		/sys/bus/iio/devices/iio:deviceX/out_current_ledY_offset
-		/sys/bus/iio/devices/iio:deviceX/out_current_ledY_ambient_offset
-Date:		December 2015
+What:		/sys/bus/iio/devices/iio:deviceX/in_intensityY_offset
+Date:		May 2016
 KernelVersion:
 Contact:	Andrew F. Davis <afd@ti.com>
 Description:
 		Get and set the offset cancellation DAC setting for these
 		stages. The values are expressed in 5-bit sign-magnitude.
 
-What:		/sys/bus/iio/devices/iio:deviceX/out_current_ledY_raw
-Date:		December 2015
+What:		/sys/bus/iio/devices/iio:deviceX/out_currentY_raw
+Date:		May 2016
 KernelVersion:
 Contact:	Andrew F. Davis <afd@ti.com>
 Description:
-		Get and set the LED current for the specified LED. Y is the
-		specific LED number.
+		Get and set the LED current for the specified LED active during
+		this stage. Y is the specific stage number.
diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
index cac6090..4a58064 100644
--- a/drivers/iio/health/afe4403.c
+++ b/drivers/iio/health/afe4403.c
@@ -121,38 +121,38 @@ struct afe4403_data {
 };
 
 enum afe4403_chan_id {
+	LED2 = 1,
+	ALED2,
 	LED1,
 	ALED1,
-	LED2,
-	ALED2,
-	LED1_ALED1,
 	LED2_ALED2,
+	LED1_ALED1,
 	ILED1,
 	ILED2,
 };
 
 static const struct afe440x_reg_info afe4403_reg_info[] = {
-	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, 0, NULL),
-	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, 0, NULL),
 	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, 0, NULL),
 	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, 0, NULL),
-	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
+	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, 0, NULL),
+	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, 0, NULL),
 	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
+	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
 	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED1),
 	[ILED2] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED2),
 };
 
 static const struct iio_chan_spec afe4403_channels[] = {
 	/* ADC values */
-	AFE440X_INTENSITY_CHAN(LED1, "led1", 0),
-	AFE440X_INTENSITY_CHAN(ALED1, "led1_ambient", 0),
-	AFE440X_INTENSITY_CHAN(LED2, "led2", 0),
-	AFE440X_INTENSITY_CHAN(ALED2, "led2_ambient", 0),
-	AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0),
-	AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0),
+	AFE440X_INTENSITY_CHAN(LED2, 0),
+	AFE440X_INTENSITY_CHAN(ALED2, 0),
+	AFE440X_INTENSITY_CHAN(LED1, 0),
+	AFE440X_INTENSITY_CHAN(ALED1, 0),
+	AFE440X_INTENSITY_CHAN(LED2_ALED2, 0),
+	AFE440X_INTENSITY_CHAN(LED1_ALED1, 0),
 	/* LED current */
-	AFE440X_CURRENT_CHAN(ILED1, "led1"),
-	AFE440X_CURRENT_CHAN(ILED2, "led2"),
+	AFE440X_CURRENT_CHAN(ILED1),
+	AFE440X_CURRENT_CHAN(ILED2),
 };
 
 static const struct afe440x_val_table afe4403_res_table[] = {
diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
index 2edb7d7..7806a45 100644
--- a/drivers/iio/health/afe4404.c
+++ b/drivers/iio/health/afe4404.c
@@ -122,24 +122,24 @@ struct afe4404_data {
 };
 
 enum afe4404_chan_id {
+	LED2 = 1,
+	ALED2,
 	LED1,
 	ALED1,
-	LED2,
-	ALED2,
-	LED1_ALED1,
 	LED2_ALED2,
+	LED1_ALED1,
 	ILED1,
 	ILED2,
 	ILED3,
 };
 
 static const struct afe440x_reg_info afe4404_reg_info[] = {
-	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED1),
-	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED1),
 	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED2),
 	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED2),
-	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
+	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED1),
+	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED1),
 	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
+	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
 	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED1),
 	[ILED2] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED2),
 	[ILED3] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED3),
@@ -147,16 +147,16 @@ static const struct afe440x_reg_info afe4404_reg_info[] = {
 
 static const struct iio_chan_spec afe4404_channels[] = {
 	/* ADC values */
-	AFE440X_INTENSITY_CHAN(LED1, "led1", BIT(IIO_CHAN_INFO_OFFSET)),
-	AFE440X_INTENSITY_CHAN(ALED1, "led1_ambient", BIT(IIO_CHAN_INFO_OFFSET)),
-	AFE440X_INTENSITY_CHAN(LED2, "led2", BIT(IIO_CHAN_INFO_OFFSET)),
-	AFE440X_INTENSITY_CHAN(ALED2, "led2_ambient", BIT(IIO_CHAN_INFO_OFFSET)),
-	AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0),
-	AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0),
+	AFE440X_INTENSITY_CHAN(LED2, BIT(IIO_CHAN_INFO_OFFSET)),
+	AFE440X_INTENSITY_CHAN(ALED2, BIT(IIO_CHAN_INFO_OFFSET)),
+	AFE440X_INTENSITY_CHAN(LED1, BIT(IIO_CHAN_INFO_OFFSET)),
+	AFE440X_INTENSITY_CHAN(ALED1, BIT(IIO_CHAN_INFO_OFFSET)),
+	AFE440X_INTENSITY_CHAN(LED2_ALED2, 0),
+	AFE440X_INTENSITY_CHAN(LED1_ALED1, 0),
 	/* LED current */
-	AFE440X_CURRENT_CHAN(ILED1, "led1"),
-	AFE440X_CURRENT_CHAN(ILED2, "led2"),
-	AFE440X_CURRENT_CHAN(ILED3, "led3"),
+	AFE440X_CURRENT_CHAN(ILED1),
+	AFE440X_CURRENT_CHAN(ILED2),
+	AFE440X_CURRENT_CHAN(ILED3),
 };
 
 static const struct afe440x_val_table afe4404_res_table[] = {
diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h
index 583d071..713972f 100644
--- a/drivers/iio/health/afe440x.h
+++ b/drivers/iio/health/afe440x.h
@@ -103,7 +103,7 @@ struct afe440x_reg_info {
 		.mask = _sm ## _MASK,				\
 	}
 
-#define AFE440X_INTENSITY_CHAN(_index, _name, _mask)		\
+#define AFE440X_INTENSITY_CHAN(_index, _mask)			\
 	{							\
 		.type = IIO_INTENSITY,				\
 		.channel = _index,				\
@@ -115,20 +115,20 @@ struct afe440x_reg_info {
 				.storagebits = 32,		\
 				.endianness = IIO_CPU,		\
 		},						\
-		.extend_name = _name,				\
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
 			_mask,					\
+		.indexed = true,				\
 	}
 
-#define AFE440X_CURRENT_CHAN(_index, _name)			\
+#define AFE440X_CURRENT_CHAN(_index)				\
 	{							\
 		.type = IIO_CURRENT,				\
 		.channel = _index,				\
 		.address = _index,				\
 		.scan_index = -1,				\
-		.extend_name = _name,				\
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
 			BIT(IIO_CHAN_INFO_SCALE),		\
+		.indexed = true,				\
 		.output = true,					\
 	}
 
-- 
2.8.1

[PATCH 09/13] iio: health/afe440x: Use regmap fields

[Andrew F. Davis]

These drivers can use regmap fields to access fields in registers, this
allows us to remove some macros/defines and simplify code, do this here.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 drivers/iio/health/afe4403.c | 118 ++++++++++++++++----------------
 drivers/iio/health/afe4404.c | 156 ++++++++++++++++++++++---------------------
 drivers/iio/health/afe440x.h |  25 +------
 3 files changed, 144 insertions(+), 155 deletions(-)

diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
index 4a58064..1950155 100644
--- a/drivers/iio/health/afe4403.c
+++ b/drivers/iio/health/afe4403.c
@@ -39,32 +39,6 @@
 #define AFE4403_TIAGAIN			0x20
 #define AFE4403_TIA_AMB_GAIN		0x21
 
-/* AFE4403 GAIN register fields */
-#define AFE4403_TIAGAIN_RES_MASK	GENMASK(2, 0)
-#define AFE4403_TIAGAIN_RES_SHIFT	0
-#define AFE4403_TIAGAIN_CAP_MASK	GENMASK(7, 3)
-#define AFE4403_TIAGAIN_CAP_SHIFT	3
-
-/* AFE4403 LEDCNTRL register fields */
-#define AFE440X_LEDCNTRL_LED1_MASK		GENMASK(15, 8)
-#define AFE440X_LEDCNTRL_LED1_SHIFT		8
-#define AFE440X_LEDCNTRL_LED2_MASK		GENMASK(7, 0)
-#define AFE440X_LEDCNTRL_LED2_SHIFT		0
-#define AFE440X_LEDCNTRL_LED_RANGE_MASK		GENMASK(17, 16)
-#define AFE440X_LEDCNTRL_LED_RANGE_SHIFT	16
-
-/* AFE4403 CONTROL2 register fields */
-#define AFE440X_CONTROL2_PWR_DWN_TX	BIT(2)
-#define AFE440X_CONTROL2_EN_SLOW_DIAG	BIT(8)
-#define AFE440X_CONTROL2_DIAG_OUT_TRI	BIT(10)
-#define AFE440X_CONTROL2_TX_BRDG_MOD	BIT(11)
-#define AFE440X_CONTROL2_TX_REF_MASK	GENMASK(18, 17)
-#define AFE440X_CONTROL2_TX_REF_SHIFT	17
-
-/* AFE4404 NULL fields */
-#define NULL_MASK	0
-#define NULL_SHIFT	0
-
 /* AFE4403 LEDCNTRL values */
 #define AFE440X_LEDCNTRL_RANGE_TX_HALF	0x1
 #define AFE440X_LEDCNTRL_RANGE_TX_FULL	0x2
@@ -102,11 +76,35 @@
 #define AFE4403_TIAGAIN_RES_1_M		0x6
 #define AFE4403_TIAGAIN_RES_NONE	0x7
 
+enum afe4403_fields {
+	/* Gains */
+	F_RF_LED1, F_CF_LED1,
+	F_RF_LED, F_CF_LED,
+
+	/* LED Current */
+	F_ILED1, F_ILED2,
+
+	/* sentinel */
+	F_MAX_FIELDS
+};
+
+static const struct reg_field afe4403_reg_fields[] = {
+	/* Gains */
+	[F_RF_LED1]	= REG_FIELD(AFE4403_TIAGAIN, 0, 2),
+	[F_CF_LED1]	= REG_FIELD(AFE4403_TIAGAIN, 3, 7),
+	[F_RF_LED]	= REG_FIELD(AFE4403_TIA_AMB_GAIN, 0, 2),
+	[F_CF_LED]	= REG_FIELD(AFE4403_TIA_AMB_GAIN, 3, 7),
+	/* LED Current */
+	[F_ILED1]	= REG_FIELD(AFE440X_LEDCNTRL, 0, 7),
+	[F_ILED2]	= REG_FIELD(AFE440X_LEDCNTRL, 8, 15),
+};
+
 /**
  * struct afe4403_data - AFE4403 device instance data
  * @dev: Device structure
  * @spi: SPI device handle
  * @regmap: Register map of the device
+ * @fields: Register fields of the device
  * @regulator: Pointer to the regulator for the IC
  * @trig: IIO trigger for this device
  * @irq: ADC_RDY line interrupt number
@@ -115,6 +113,7 @@ struct afe4403_data {
 	struct device *dev;
 	struct spi_device *spi;
 	struct regmap *regmap;
+	struct regmap_field *fields[F_MAX_FIELDS];
 	struct regulator *regulator;
 	struct iio_trigger *trig;
 	int irq;
@@ -131,15 +130,18 @@ enum afe4403_chan_id {
 	ILED2,
 };
 
-static const struct afe440x_reg_info afe4403_reg_info[] = {
-	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, 0, NULL),
-	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, 0, NULL),
-	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, 0, NULL),
-	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, 0, NULL),
-	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
-	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
-	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED1),
-	[ILED2] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED2),
+static const unsigned int afe4403_channel_values[] = {
+	[LED2] = AFE440X_LED2VAL,
+	[ALED2] = AFE440X_ALED2VAL,
+	[LED1] = AFE440X_LED1VAL,
+	[ALED1] = AFE440X_ALED1VAL,
+	[LED2_ALED2] = AFE440X_LED2_ALED2VAL,
+	[LED1_ALED1] = AFE440X_LED1_ALED1VAL,
+};
+
+static const unsigned int afe4403_channel_leds[] = {
+	[ILED1] = F_ILED1,
+	[ILED2] = F_ILED2,
 };
 
 static const struct iio_chan_spec afe4403_channels[] = {
@@ -184,13 +186,10 @@ static ssize_t afe440x_show_register(struct device *dev,
 	int vals[2];
 	int ret;
 
-	ret = regmap_read(afe->regmap, afe440x_attr->reg, &reg_val);
+	ret = regmap_field_read(afe->fields[afe440x_attr->field], &reg_val);
 	if (ret)
 		return ret;
 
-	reg_val &= afe440x_attr->mask;
-	reg_val >>= afe440x_attr->shift;
-
 	if (reg_val >= afe440x_attr->table_size)
 		return -EINVAL;
 
@@ -220,20 +219,18 @@ static ssize_t afe440x_store_register(struct device *dev,
 	if (val == afe440x_attr->table_size)
 		return -EINVAL;
 
-	ret = regmap_update_bits(afe->regmap, afe440x_attr->reg,
-				 afe440x_attr->mask,
-				 (val << afe440x_attr->shift));
+	ret = regmap_field_write(afe->fields[afe440x_attr->field], val);
 	if (ret)
 		return ret;
 
 	return count;
 }
 
-static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, afe4403_res_table);
-static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, afe4403_cap_table);
+static AFE440X_ATTR(tia_resistance1, F_RF_LED1, afe4403_res_table);
+static AFE440X_ATTR(tia_capacitance1, F_CF_LED1, afe4403_cap_table);
 
-static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_res_table);
-static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_cap_table);
+static AFE440X_ATTR(tia_resistance2, F_RF_LED, afe4403_res_table);
+static AFE440X_ATTR(tia_capacitance2, F_CF_LED, afe4403_cap_table);
 
 static struct attribute *afe440x_attributes[] = {
 	&afe440x_attr_tia_resistance1.dev_attr.attr,
@@ -282,14 +279,15 @@ static int afe4403_read_raw(struct iio_dev *indio_dev,
 			    int *val, int *val2, long mask)
 {
 	struct afe4403_data *afe = iio_priv(indio_dev);
-	const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address];
+	unsigned int reg = afe4403_channel_values[chan->address];
+	unsigned int field = afe4403_channel_leds[chan->address];
 	int ret;
 
 	switch (chan->type) {
 	case IIO_INTENSITY:
 		switch (mask) {
 		case IIO_CHAN_INFO_RAW:
-			ret = afe4403_read(afe, reg_info.reg, val);
+			ret = afe4403_read(afe, reg, val);
 			if (ret)
 				return ret;
 			return IIO_VAL_INT;
@@ -298,11 +296,9 @@ static int afe4403_read_raw(struct iio_dev *indio_dev,
 	case IIO_CURRENT:
 		switch (mask) {
 		case IIO_CHAN_INFO_RAW:
-			ret = regmap_read(afe->regmap, reg_info.reg, val);
+			ret = regmap_field_read(afe->fields[field], val);
 			if (ret)
 				return ret;
-			*val &= reg_info.mask;
-			*val >>= reg_info.shift;
 			return IIO_VAL_INT;
 		case IIO_CHAN_INFO_SCALE:
 			*val = 0;
@@ -322,16 +318,13 @@ static int afe4403_write_raw(struct iio_dev *indio_dev,
 			     int val, int val2, long mask)
 {
 	struct afe4403_data *afe = iio_priv(indio_dev);
-	const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address];
+	unsigned int field = afe4403_channel_leds[chan->address];
 
 	switch (chan->type) {
 	case IIO_CURRENT:
 		switch (mask) {
 		case IIO_CHAN_INFO_RAW:
-			return regmap_update_bits(afe->regmap,
-				reg_info.reg,
-				reg_info.mask,
-				(val << reg_info.shift));
+			return regmap_field_write(afe->fields[field], val);
 		}
 		break;
 	default:
@@ -366,7 +359,7 @@ static irqreturn_t afe4403_trigger_handler(int irq, void *private)
 	for_each_set_bit(bit, indio_dev->active_scan_mask,
 			 indio_dev->masklength) {
 		ret = spi_write_then_read(afe->spi,
-					  &afe4403_reg_info[bit].reg, 1,
+					  &afe4403_channel_values[bit], 1,
 					  rx, 3);
 		if (ret)
 			goto err;
@@ -503,7 +496,7 @@ static int afe4403_probe(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev;
 	struct afe4403_data *afe;
-	int ret;
+	int i, ret;
 
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*afe));
 	if (!indio_dev)
@@ -522,6 +515,15 @@ static int afe4403_probe(struct spi_device *spi)
 		return PTR_ERR(afe->regmap);
 	}
 
+	for (i = 0; i < F_MAX_FIELDS; i++) {
+		afe->fields[i] = devm_regmap_field_alloc(afe->dev, afe->regmap,
+							 afe4403_reg_fields[i]);
+		if (IS_ERR(afe->fields[i])) {
+			dev_err(afe->dev, "Unable to allocate regmap fields\n");
+			return PTR_ERR(afe->fields[i]);
+		}
+	}
+
 	afe->regulator = devm_regulator_get(afe->dev, "tx_sup");
 	if (IS_ERR(afe->regulator)) {
 		dev_err(afe->dev, "Unable to get regulator\n");
diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
index 7806a45..0d1af4a 100644
--- a/drivers/iio/health/afe4404.c
+++ b/drivers/iio/health/afe4404.c
@@ -48,43 +48,9 @@
 #define AFE4404_AVG_LED2_ALED2VAL	0x3f
 #define AFE4404_AVG_LED1_ALED1VAL	0x40
 
-/* AFE4404 GAIN register fields */
-#define AFE4404_TIA_GAIN_RES_MASK	GENMASK(2, 0)
-#define AFE4404_TIA_GAIN_RES_SHIFT	0
-#define AFE4404_TIA_GAIN_CAP_MASK	GENMASK(5, 3)
-#define AFE4404_TIA_GAIN_CAP_SHIFT	3
-
-/* AFE4404 LEDCNTRL register fields */
-#define AFE4404_LEDCNTRL_ILED1_MASK	GENMASK(5, 0)
-#define AFE4404_LEDCNTRL_ILED1_SHIFT	0
-#define AFE4404_LEDCNTRL_ILED2_MASK	GENMASK(11, 6)
-#define AFE4404_LEDCNTRL_ILED2_SHIFT	6
-#define AFE4404_LEDCNTRL_ILED3_MASK	GENMASK(17, 12)
-#define AFE4404_LEDCNTRL_ILED3_SHIFT	12
-
-/* AFE4404 CONTROL2 register fields */
-#define AFE440X_CONTROL2_ILED_2X_MASK	BIT(17)
-#define AFE440X_CONTROL2_ILED_2X_SHIFT	17
-
 /* AFE4404 CONTROL3 register fields */
 #define AFE440X_CONTROL3_OSC_ENABLE	BIT(9)
 
-/* AFE4404 OFFDAC register current fields */
-#define AFE4404_OFFDAC_CURR_LED1_MASK	GENMASK(9, 5)
-#define AFE4404_OFFDAC_CURR_LED1_SHIFT	5
-#define AFE4404_OFFDAC_CURR_LED2_MASK	GENMASK(19, 15)
-#define AFE4404_OFFDAC_CURR_LED2_SHIFT	15
-#define AFE4404_OFFDAC_CURR_LED3_MASK	GENMASK(4, 0)
-#define AFE4404_OFFDAC_CURR_LED3_SHIFT	0
-#define AFE4404_OFFDAC_CURR_ALED1_MASK	GENMASK(14, 10)
-#define AFE4404_OFFDAC_CURR_ALED1_SHIFT	10
-#define AFE4404_OFFDAC_CURR_ALED2_MASK	GENMASK(4, 0)
-#define AFE4404_OFFDAC_CURR_ALED2_SHIFT	0
-
-/* AFE4404 NULL fields */
-#define NULL_MASK	0
-#define NULL_SHIFT	0
-
 /* AFE4404 TIA_GAIN_CAP values */
 #define AFE4404_TIA_GAIN_CAP_5_P	0x0
 #define AFE4404_TIA_GAIN_CAP_2_5_P	0x1
@@ -105,10 +71,43 @@
 #define AFE4404_TIA_GAIN_RES_1_M	0x6
 #define AFE4404_TIA_GAIN_RES_2_M	0x7
 
+enum afe4404_fields {
+	/* Gains */
+	F_TIA_GAIN_SEP, F_TIA_CF_SEP,
+	F_TIA_GAIN, TIA_CF,
+
+	/* LED Current */
+	F_ILED1, F_ILED2, F_ILED3,
+
+	/* Offset DAC */
+	F_OFFDAC_AMB2, F_OFFDAC_LED1, F_OFFDAC_AMB1, F_OFFDAC_LED2,
+
+	/* sentinel */
+	F_MAX_FIELDS
+};
+
+static const struct reg_field afe4404_reg_fields[] = {
+	/* Gains */
+	[F_TIA_GAIN_SEP]	= REG_FIELD(AFE4404_TIA_GAIN_SEP, 0, 2),
+	[F_TIA_CF_SEP]		= REG_FIELD(AFE4404_TIA_GAIN_SEP, 3, 5),
+	[F_TIA_GAIN]		= REG_FIELD(AFE4404_TIA_GAIN, 0, 2),
+	[TIA_CF]		= REG_FIELD(AFE4404_TIA_GAIN, 3, 5),
+	/* LED Current */
+	[F_ILED1]		= REG_FIELD(AFE440X_LEDCNTRL, 0, 5),
+	[F_ILED2]		= REG_FIELD(AFE440X_LEDCNTRL, 6, 11),
+	[F_ILED3]		= REG_FIELD(AFE440X_LEDCNTRL, 12, 17),
+	/* Offset DAC */
+	[F_OFFDAC_AMB2]		= REG_FIELD(AFE4404_OFFDAC, 0, 4),
+	[F_OFFDAC_LED1]		= REG_FIELD(AFE4404_OFFDAC, 5, 9),
+	[F_OFFDAC_AMB1]		= REG_FIELD(AFE4404_OFFDAC, 10, 14),
+	[F_OFFDAC_LED2]		= REG_FIELD(AFE4404_OFFDAC, 15, 19),
+};
+
 /**
  * struct afe4404_data - AFE4404 device instance data
  * @dev: Device structure
  * @regmap: Register map of the device
+ * @fields: Register fields of the device
  * @regulator: Pointer to the regulator for the IC
  * @trig: IIO trigger for this device
  * @irq: ADC_RDY line interrupt number
@@ -116,6 +115,7 @@
 struct afe4404_data {
 	struct device *dev;
 	struct regmap *regmap;
+	struct regmap_field *fields[F_MAX_FIELDS];
 	struct regulator *regulator;
 	struct iio_trigger *trig;
 	int irq;
@@ -133,16 +133,26 @@ enum afe4404_chan_id {
 	ILED3,
 };
 
-static const struct afe440x_reg_info afe4404_reg_info[] = {
-	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED2),
-	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED2),
-	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED1),
-	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED1),
-	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
-	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
-	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED1),
-	[ILED2] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED2),
-	[ILED3] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED3),
+static const unsigned int afe4404_channel_values[] = {
+	[LED2] = AFE440X_LED2VAL,
+	[ALED2] = AFE440X_ALED2VAL,
+	[LED1] = AFE440X_LED1VAL,
+	[ALED1] = AFE440X_ALED1VAL,
+	[LED2_ALED2] = AFE440X_LED2_ALED2VAL,
+	[LED1_ALED1] = AFE440X_LED1_ALED1VAL,
+};
+
+static const unsigned int afe4404_channel_leds[] = {
+	[ILED1] = F_ILED1,
+	[ILED2] = F_ILED2,
+	[ILED3] = F_ILED3,
+};
+
+static const unsigned int afe4404_channel_offdacs[] = {
+	[LED2] = F_OFFDAC_LED2,
+	[ALED2] = F_OFFDAC_AMB2,
+	[LED1] = F_OFFDAC_LED1,
+	[ALED1] = F_OFFDAC_AMB1,
 };
 
 static const struct iio_chan_spec afe4404_channels[] = {
@@ -194,13 +204,10 @@ static ssize_t afe440x_show_register(struct device *dev,
 	int vals[2];
 	int ret;
 
-	ret = regmap_read(afe->regmap, afe440x_attr->reg, &reg_val);
+	ret = regmap_field_read(afe->fields[afe440x_attr->field], &reg_val);
 	if (ret)
 		return ret;
 
-	reg_val &= afe440x_attr->mask;
-	reg_val >>= afe440x_attr->shift;
-
 	if (reg_val >= afe440x_attr->table_size)
 		return -EINVAL;
 
@@ -230,20 +237,18 @@ static ssize_t afe440x_store_register(struct device *dev,
 	if (val == afe440x_attr->table_size)
 		return -EINVAL;
 
-	ret = regmap_update_bits(afe->regmap, afe440x_attr->reg,
-				 afe440x_attr->mask,
-				 (val << afe440x_attr->shift));
+	ret = regmap_field_write(afe->fields[afe440x_attr->field], val);
 	if (ret)
 		return ret;
 
 	return count;
 }
 
-static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, afe4404_res_table);
-static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, afe4404_cap_table);
+static AFE440X_ATTR(tia_resistance1, F_TIA_GAIN, afe4404_res_table);
+static AFE440X_ATTR(tia_capacitance1, TIA_CF, afe4404_cap_table);
 
-static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, afe4404_res_table);
-static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, afe4404_cap_table);
+static AFE440X_ATTR(tia_resistance2, F_TIA_GAIN_SEP, afe4404_res_table);
+static AFE440X_ATTR(tia_capacitance2, F_TIA_CF_SEP, afe4404_cap_table);
 
 static struct attribute *afe440x_attributes[] = {
 	&afe440x_attr_tia_resistance1.dev_attr.attr,
@@ -264,35 +269,32 @@ static int afe4404_read_raw(struct iio_dev *indio_dev,
 			    int *val, int *val2, long mask)
 {
 	struct afe4404_data *afe = iio_priv(indio_dev);
-	const struct afe440x_reg_info reg_info = afe4404_reg_info[chan->address];
+	unsigned int value_reg = afe4404_channel_values[chan->address];
+	unsigned int led_field = afe4404_channel_leds[chan->address];
+	unsigned int offdac_field = afe4404_channel_offdacs[chan->address];
 	int ret;
 
 	switch (chan->type) {
 	case IIO_INTENSITY:
 		switch (mask) {
 		case IIO_CHAN_INFO_RAW:
-			ret = regmap_read(afe->regmap, reg_info.reg, val);
+			ret = regmap_read(afe->regmap, value_reg, val);
 			if (ret)
 				return ret;
 			return IIO_VAL_INT;
 		case IIO_CHAN_INFO_OFFSET:
-			ret = regmap_read(afe->regmap, reg_info.offreg,
-					  val);
+			ret = regmap_field_read(afe->fields[offdac_field], val);
 			if (ret)
 				return ret;
-			*val &= reg_info.mask;
-			*val >>= reg_info.shift;
 			return IIO_VAL_INT;
 		}
 		break;
 	case IIO_CURRENT:
 		switch (mask) {
 		case IIO_CHAN_INFO_RAW:
-			ret = regmap_read(afe->regmap, reg_info.reg, val);
+			ret = regmap_field_read(afe->fields[led_field], val);
 			if (ret)
 				return ret;
-			*val &= reg_info.mask;
-			*val >>= reg_info.shift;
 			return IIO_VAL_INT;
 		case IIO_CHAN_INFO_SCALE:
 			*val = 0;
@@ -312,25 +314,20 @@ static int afe4404_write_raw(struct iio_dev *indio_dev,
 			     int val, int val2, long mask)
 {
 	struct afe4404_data *afe = iio_priv(indio_dev);
-	const struct afe440x_reg_info reg_info = afe4404_reg_info[chan->address];
+	unsigned int led_field = afe4404_channel_leds[chan->address];
+	unsigned int offdac_field = afe4404_channel_offdacs[chan->address];
 
 	switch (chan->type) {
 	case IIO_INTENSITY:
 		switch (mask) {
 		case IIO_CHAN_INFO_OFFSET:
-			return regmap_update_bits(afe->regmap,
-				reg_info.offreg,
-				reg_info.mask,
-				(val << reg_info.shift));
+			return regmap_field_write(afe->fields[offdac_field], val);
 		}
 		break;
 	case IIO_CURRENT:
 		switch (mask) {
 		case IIO_CHAN_INFO_RAW:
-			return regmap_update_bits(afe->regmap,
-				reg_info.reg,
-				reg_info.mask,
-				(val << reg_info.shift));
+			return regmap_field_write(afe->fields[led_field], val);
 		}
 		break;
 	default:
@@ -357,7 +354,7 @@ static irqreturn_t afe4404_trigger_handler(int irq, void *private)
 
 	for_each_set_bit(bit, indio_dev->active_scan_mask,
 			 indio_dev->masklength) {
-		ret = regmap_read(afe->regmap, afe4404_reg_info[bit].reg,
+		ret = regmap_read(afe->regmap, afe4404_channel_values[bit],
 				  &buffer[i++]);
 		if (ret)
 			goto err;
@@ -490,7 +487,7 @@ static int afe4404_probe(struct i2c_client *client,
 {
 	struct iio_dev *indio_dev;
 	struct afe4404_data *afe;
-	int ret;
+	int i, ret;
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*afe));
 	if (!indio_dev)
@@ -508,6 +505,15 @@ static int afe4404_probe(struct i2c_client *client,
 		return PTR_ERR(afe->regmap);
 	}
 
+	for (i = 0; i < F_MAX_FIELDS; i++) {
+		afe->fields[i] = devm_regmap_field_alloc(afe->dev, afe->regmap,
+							 afe4404_reg_fields[i]);
+		if (IS_ERR(afe->fields[i])) {
+			dev_err(afe->dev, "Unable to allocate regmap fields\n");
+			return PTR_ERR(afe->fields[i]);
+		}
+	}
+
 	afe->regulator = devm_regulator_get(afe->dev, "tx_sup");
 	if (IS_ERR(afe->regulator)) {
 		dev_err(afe->dev, "Unable to get regulator\n");
diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h
index 713972f..1a0f247 100644
--- a/drivers/iio/health/afe440x.h
+++ b/drivers/iio/health/afe440x.h
@@ -88,21 +88,6 @@
 #define AFE440X_CONTROL0_WRITE		0x0
 #define AFE440X_CONTROL0_READ		0x1
 
-struct afe440x_reg_info {
-	unsigned int reg;
-	unsigned int offreg;
-	unsigned int shift;
-	unsigned int mask;
-};
-
-#define AFE440X_REG_INFO(_reg, _offreg, _sm)			\
-	{							\
-		.reg = _reg,					\
-		.offreg = _offreg,				\
-		.shift = _sm ## _SHIFT,				\
-		.mask = _sm ## _MASK,				\
-	}
-
 #define AFE440X_INTENSITY_CHAN(_index, _mask)			\
 	{							\
 		.type = IIO_INTENSITY,				\
@@ -157,9 +142,7 @@ static DEVICE_ATTR_RO(_name)
 
 struct afe440x_attr {
 	struct device_attribute dev_attr;
-	unsigned int reg;
-	unsigned int shift;
-	unsigned int mask;
+	unsigned int field;
 	const struct afe440x_val_table *val_table;
 	unsigned int table_size;
 };
@@ -167,14 +150,12 @@ struct afe440x_attr {
 #define to_afe440x_attr(_dev_attr)				\
 	container_of(_dev_attr, struct afe440x_attr, dev_attr)
 
-#define AFE440X_ATTR(_name, _reg, _field, _table)		\
+#define AFE440X_ATTR(_name, _field, _table)			\
 	struct afe440x_attr afe440x_attr_##_name = {		\
 		.dev_attr = __ATTR(_name, (S_IRUGO | S_IWUSR),	\
 				   afe440x_show_register,	\
 				   afe440x_store_register),	\
-		.reg = _reg,					\
-		.shift = _field ## _SHIFT,			\
-		.mask = _field ## _MASK,			\
+		.field = _field,				\
 		.val_table = _table,				\
 		.table_size = ARRAY_SIZE(_table),		\
 	}
-- 
2.8.1

[PATCH 10/13] iio: health/afe440x: Make gain settings a modifier for the stages

[Andrew F. Davis]

Currently the TIA gain settings are exported to userspace as sysfs
entries that do not clearly represent their internal relation to the
sampling stages. The gain settings are enabled on a per-stage basis,
this can be seen in figure 24 of the current AFE4404 datasheet.
These gain settings should therefore be tied to the channels that are
read during these stages. Make this change here.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 .../ABI/testing/sysfs-bus-iio-health-afe440x       | 19 ++++++------
 drivers/iio/health/afe4403.c                       | 34 ++++++++++++++--------
 drivers/iio/health/afe4404.c                       | 34 ++++++++++++++--------
 3 files changed, 53 insertions(+), 34 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
index a067073..6adba90 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
+++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
@@ -1,13 +1,3 @@
-What:		/sys/bus/iio/devices/iio:deviceX/tia_resistanceY
-		/sys/bus/iio/devices/iio:deviceX/tia_capacitanceY
-Date:		December 2015
-KernelVersion:
-Contact:	Andrew F. Davis <afd@ti.com>
-Description:
-		Get and set the resistance and the capacitance settings for the
-		Transimpedance Amplifier. Y is 1 for Rf1 and Cf1, Y is 2 for
-		Rf2 and Cf2 values.
-
 What:		/sys/bus/iio/devices/iio:deviceX/in_intensityY_raw
 Date:		May 2016
 KernelVersion:
@@ -33,6 +23,15 @@ Description:
 		Get and set the offset cancellation DAC setting for these
 		stages. The values are expressed in 5-bit sign-magnitude.
 
+What:		/sys/bus/iio/devices/iio:deviceX/in_intensityY_resistance
+What:		/sys/bus/iio/devices/iio:deviceX/in_intensityY_capacitance
+Date:		May 2016
+KernelVersion:
+Contact:	Andrew F. Davis <afd@ti.com>
+Description:
+		Get and set the resistance and the capacitance settings for the
+		Transimpedance Amplifier during the associated stage.
+
 What:		/sys/bus/iio/devices/iio:deviceX/out_currentY_raw
 Date:		May 2016
 KernelVersion:
diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
index 1950155..610631b 100644
--- a/drivers/iio/health/afe4403.c
+++ b/drivers/iio/health/afe4403.c
@@ -161,7 +161,7 @@ static const struct afe440x_val_table afe4403_res_table[] = {
 	{ 500000 }, { 250000 }, { 100000 }, { 50000 },
 	{ 25000 }, { 10000 }, { 1000000 }, { 0 },
 };
-AFE440X_TABLE_ATTR(tia_resistance_available, afe4403_res_table);
+AFE440X_TABLE_ATTR(in_intensity_resistance_available, afe4403_res_table);
 
 static const struct afe440x_val_table afe4403_cap_table[] = {
 	{ 0, 5000 }, { 0, 10000 }, { 0, 20000 }, { 0, 25000 },
@@ -173,7 +173,7 @@ static const struct afe440x_val_table afe4403_cap_table[] = {
 	{ 0, 205000 }, { 0, 210000 }, { 0, 220000 }, { 0, 225000 },
 	{ 0, 230000 }, { 0, 235000 }, { 0, 245000 }, { 0, 250000 },
 };
-AFE440X_TABLE_ATTR(tia_capacitance_available, afe4403_cap_table);
+AFE440X_TABLE_ATTR(in_intensity_capacitance_available, afe4403_cap_table);
 
 static ssize_t afe440x_show_register(struct device *dev,
 				     struct device_attribute *attr,
@@ -226,19 +226,29 @@ static ssize_t afe440x_store_register(struct device *dev,
 	return count;
 }
 
-static AFE440X_ATTR(tia_resistance1, F_RF_LED1, afe4403_res_table);
-static AFE440X_ATTR(tia_capacitance1, F_CF_LED1, afe4403_cap_table);
+static AFE440X_ATTR(in_intensity1_resistance, F_RF_LED, afe4403_res_table);
+static AFE440X_ATTR(in_intensity1_capacitance, F_CF_LED, afe4403_cap_table);
 
-static AFE440X_ATTR(tia_resistance2, F_RF_LED, afe4403_res_table);
-static AFE440X_ATTR(tia_capacitance2, F_CF_LED, afe4403_cap_table);
+static AFE440X_ATTR(in_intensity2_resistance, F_RF_LED, afe4403_res_table);
+static AFE440X_ATTR(in_intensity2_capacitance, F_CF_LED, afe4403_cap_table);
+
+static AFE440X_ATTR(in_intensity3_resistance, F_RF_LED1, afe4403_res_table);
+static AFE440X_ATTR(in_intensity3_capacitance, F_CF_LED1, afe4403_cap_table);
+
+static AFE440X_ATTR(in_intensity4_resistance, F_RF_LED1, afe4403_res_table);
+static AFE440X_ATTR(in_intensity4_capacitance, F_CF_LED1, afe4403_cap_table);
 
 static struct attribute *afe440x_attributes[] = {
-	&afe440x_attr_tia_resistance1.dev_attr.attr,
-	&afe440x_attr_tia_capacitance1.dev_attr.attr,
-	&afe440x_attr_tia_resistance2.dev_attr.attr,
-	&afe440x_attr_tia_capacitance2.dev_attr.attr,
-	&dev_attr_tia_resistance_available.attr,
-	&dev_attr_tia_capacitance_available.attr,
+	&dev_attr_in_intensity_resistance_available.attr,
+	&dev_attr_in_intensity_capacitance_available.attr,
+	&afe440x_attr_in_intensity1_resistance.dev_attr.attr,
+	&afe440x_attr_in_intensity1_capacitance.dev_attr.attr,
+	&afe440x_attr_in_intensity2_resistance.dev_attr.attr,
+	&afe440x_attr_in_intensity2_capacitance.dev_attr.attr,
+	&afe440x_attr_in_intensity3_resistance.dev_attr.attr,
+	&afe440x_attr_in_intensity3_capacitance.dev_attr.attr,
+	&afe440x_attr_in_intensity4_resistance.dev_attr.attr,
+	&afe440x_attr_in_intensity4_capacitance.dev_attr.attr,
 	NULL
 };
 
diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
index 0d1af4a..69116cd 100644
--- a/drivers/iio/health/afe4404.c
+++ b/drivers/iio/health/afe4404.c
@@ -179,7 +179,7 @@ static const struct afe440x_val_table afe4404_res_table[] = {
 	{ .integer = 1000000, .fract = 0 },
 	{ .integer = 2000000, .fract = 0 },
 };
-AFE440X_TABLE_ATTR(tia_resistance_available, afe4404_res_table);
+AFE440X_TABLE_ATTR(in_intensity_resistance_available, afe4404_res_table);
 
 static const struct afe440x_val_table afe4404_cap_table[] = {
 	{ .integer = 0, .fract = 5000 },
@@ -191,7 +191,7 @@ static const struct afe440x_val_table afe4404_cap_table[] = {
 	{ .integer = 0, .fract = 25000 },
 	{ .integer = 0, .fract = 22500 },
 };
-AFE440X_TABLE_ATTR(tia_capacitance_available, afe4404_cap_table);
+AFE440X_TABLE_ATTR(in_intensity_capacitance_available, afe4404_cap_table);
 
 static ssize_t afe440x_show_register(struct device *dev,
 				     struct device_attribute *attr,
@@ -244,19 +244,29 @@ static ssize_t afe440x_store_register(struct device *dev,
 	return count;
 }
 
-static AFE440X_ATTR(tia_resistance1, F_TIA_GAIN, afe4404_res_table);
-static AFE440X_ATTR(tia_capacitance1, TIA_CF, afe4404_cap_table);
+static AFE440X_ATTR(in_intensity1_resistance, F_TIA_GAIN_SEP, afe4404_res_table);
+static AFE440X_ATTR(in_intensity1_capacitance, F_TIA_CF_SEP, afe4404_cap_table);
 
-static AFE440X_ATTR(tia_resistance2, F_TIA_GAIN_SEP, afe4404_res_table);
-static AFE440X_ATTR(tia_capacitance2, F_TIA_CF_SEP, afe4404_cap_table);
+static AFE440X_ATTR(in_intensity2_resistance, F_TIA_GAIN_SEP, afe4404_res_table);
+static AFE440X_ATTR(in_intensity2_capacitance, F_TIA_CF_SEP, afe4404_cap_table);
+
+static AFE440X_ATTR(in_intensity3_resistance, F_TIA_GAIN, afe4404_res_table);
+static AFE440X_ATTR(in_intensity3_capacitance, TIA_CF, afe4404_cap_table);
+
+static AFE440X_ATTR(in_intensity4_resistance, F_TIA_GAIN, afe4404_res_table);
+static AFE440X_ATTR(in_intensity4_capacitance, TIA_CF, afe4404_cap_table);
 
 static struct attribute *afe440x_attributes[] = {
-	&afe440x_attr_tia_resistance1.dev_attr.attr,
-	&afe440x_attr_tia_capacitance1.dev_attr.attr,
-	&afe440x_attr_tia_resistance2.dev_attr.attr,
-	&afe440x_attr_tia_capacitance2.dev_attr.attr,
-	&dev_attr_tia_resistance_available.attr,
-	&dev_attr_tia_capacitance_available.attr,
+	&dev_attr_in_intensity_resistance_available.attr,
+	&dev_attr_in_intensity_capacitance_available.attr,
+	&afe440x_attr_in_intensity1_resistance.dev_attr.attr,
+	&afe440x_attr_in_intensity1_capacitance.dev_attr.attr,
+	&afe440x_attr_in_intensity2_resistance.dev_attr.attr,
+	&afe440x_attr_in_intensity2_capacitance.dev_attr.attr,
+	&afe440x_attr_in_intensity3_resistance.dev_attr.attr,
+	&afe440x_attr_in_intensity3_capacitance.dev_attr.attr,
+	&afe440x_attr_in_intensity4_resistance.dev_attr.attr,
+	&afe440x_attr_in_intensity4_capacitance.dev_attr.attr,
 	NULL
 };
 
-- 
2.8.1

[PATCH 11/13] iio: health/afe440x: Match LED currents to stages

[Andrew F. Davis]

The current channel number for the LEDs should match the stage
number that they are active during, fix this here.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 drivers/iio/health/afe4403.c | 10 ++++------
 drivers/iio/health/afe4404.c | 15 ++++++---------
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
index 610631b..059d521 100644
--- a/drivers/iio/health/afe4403.c
+++ b/drivers/iio/health/afe4403.c
@@ -126,8 +126,6 @@ enum afe4403_chan_id {
 	ALED1,
 	LED2_ALED2,
 	LED1_ALED1,
-	ILED1,
-	ILED2,
 };
 
 static const unsigned int afe4403_channel_values[] = {
@@ -140,8 +138,8 @@ static const unsigned int afe4403_channel_values[] = {
 };
 
 static const unsigned int afe4403_channel_leds[] = {
-	[ILED1] = F_ILED1,
-	[ILED2] = F_ILED2,
+	[LED2] = F_ILED2,
+	[LED1] = F_ILED1,
 };
 
 static const struct iio_chan_spec afe4403_channels[] = {
@@ -153,8 +151,8 @@ static const struct iio_chan_spec afe4403_channels[] = {
 	AFE440X_INTENSITY_CHAN(LED2_ALED2, 0),
 	AFE440X_INTENSITY_CHAN(LED1_ALED1, 0),
 	/* LED current */
-	AFE440X_CURRENT_CHAN(ILED1),
-	AFE440X_CURRENT_CHAN(ILED2),
+	AFE440X_CURRENT_CHAN(LED2),
+	AFE440X_CURRENT_CHAN(LED1),
 };
 
 static const struct afe440x_val_table afe4403_res_table[] = {
diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
index 69116cd..aa8770b 100644
--- a/drivers/iio/health/afe4404.c
+++ b/drivers/iio/health/afe4404.c
@@ -128,9 +128,6 @@ enum afe4404_chan_id {
 	ALED1,
 	LED2_ALED2,
 	LED1_ALED1,
-	ILED1,
-	ILED2,
-	ILED3,
 };
 
 static const unsigned int afe4404_channel_values[] = {
@@ -143,9 +140,9 @@ static const unsigned int afe4404_channel_values[] = {
 };
 
 static const unsigned int afe4404_channel_leds[] = {
-	[ILED1] = F_ILED1,
-	[ILED2] = F_ILED2,
-	[ILED3] = F_ILED3,
+	[LED2] = F_ILED2,
+	[ALED2] = F_ILED3,
+	[LED1] = F_ILED1,
 };
 
 static const unsigned int afe4404_channel_offdacs[] = {
@@ -164,9 +161,9 @@ static const struct iio_chan_spec afe4404_channels[] = {
 	AFE440X_INTENSITY_CHAN(LED2_ALED2, 0),
 	AFE440X_INTENSITY_CHAN(LED1_ALED1, 0),
 	/* LED current */
-	AFE440X_CURRENT_CHAN(ILED1),
-	AFE440X_CURRENT_CHAN(ILED2),
-	AFE440X_CURRENT_CHAN(ILED3),
+	AFE440X_CURRENT_CHAN(LED2),
+	AFE440X_CURRENT_CHAN(ALED2),
+	AFE440X_CURRENT_CHAN(LED1),
 };
 
 static const struct afe440x_val_table afe4404_res_table[] = {
-- 
2.8.1

[PATCH 12/13] iio: health/afe440x: Remove unused definitions

[Andrew F. Davis]

These definitions are not currently used and if the functionality
they represent is needed the values should be added back to a table
for easy userspace use.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 drivers/iio/health/afe4403.c | 37 -------------------------------------
 drivers/iio/health/afe4404.c | 20 --------------------
 2 files changed, 57 deletions(-)

diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
index 059d521..9a08146 100644
--- a/drivers/iio/health/afe4403.c
+++ b/drivers/iio/health/afe4403.c
@@ -39,43 +39,6 @@
 #define AFE4403_TIAGAIN			0x20
 #define AFE4403_TIA_AMB_GAIN		0x21
 
-/* AFE4403 LEDCNTRL values */
-#define AFE440X_LEDCNTRL_RANGE_TX_HALF	0x1
-#define AFE440X_LEDCNTRL_RANGE_TX_FULL	0x2
-#define AFE440X_LEDCNTRL_RANGE_TX_OFF	0x3
-
-/* AFE4403 CONTROL2 values */
-#define AFE440X_CONTROL2_TX_REF_025	0x0
-#define AFE440X_CONTROL2_TX_REF_050	0x1
-#define AFE440X_CONTROL2_TX_REF_100	0x2
-#define AFE440X_CONTROL2_TX_REF_075	0x3
-
-/* AFE4403 CONTROL3 values */
-#define AFE440X_CONTROL3_CLK_DIV_2	0x0
-#define AFE440X_CONTROL3_CLK_DIV_4	0x2
-#define AFE440X_CONTROL3_CLK_DIV_6	0x3
-#define AFE440X_CONTROL3_CLK_DIV_8	0x4
-#define AFE440X_CONTROL3_CLK_DIV_12	0x5
-#define AFE440X_CONTROL3_CLK_DIV_1	0x7
-
-/* AFE4403 TIAGAIN_CAP values */
-#define AFE4403_TIAGAIN_CAP_5_P		0x0
-#define AFE4403_TIAGAIN_CAP_10_P	0x1
-#define AFE4403_TIAGAIN_CAP_20_P	0x2
-#define AFE4403_TIAGAIN_CAP_30_P	0x3
-#define AFE4403_TIAGAIN_CAP_55_P	0x8
-#define AFE4403_TIAGAIN_CAP_155_P	0x10
-
-/* AFE4403 TIAGAIN_RES values */
-#define AFE4403_TIAGAIN_RES_500_K	0x0
-#define AFE4403_TIAGAIN_RES_250_K	0x1
-#define AFE4403_TIAGAIN_RES_100_K	0x2
-#define AFE4403_TIAGAIN_RES_50_K	0x3
-#define AFE4403_TIAGAIN_RES_25_K	0x4
-#define AFE4403_TIAGAIN_RES_10_K	0x5
-#define AFE4403_TIAGAIN_RES_1_M		0x6
-#define AFE4403_TIAGAIN_RES_NONE	0x7
-
 enum afe4403_fields {
 	/* Gains */
 	F_RF_LED1, F_CF_LED1,
diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
index aa8770b..3a8131d 100644
--- a/drivers/iio/health/afe4404.c
+++ b/drivers/iio/health/afe4404.c
@@ -51,26 +51,6 @@
 /* AFE4404 CONTROL3 register fields */
 #define AFE440X_CONTROL3_OSC_ENABLE	BIT(9)
 
-/* AFE4404 TIA_GAIN_CAP values */
-#define AFE4404_TIA_GAIN_CAP_5_P	0x0
-#define AFE4404_TIA_GAIN_CAP_2_5_P	0x1
-#define AFE4404_TIA_GAIN_CAP_10_P	0x2
-#define AFE4404_TIA_GAIN_CAP_7_5_P	0x3
-#define AFE4404_TIA_GAIN_CAP_20_P	0x4
-#define AFE4404_TIA_GAIN_CAP_17_5_P	0x5
-#define AFE4404_TIA_GAIN_CAP_25_P	0x6
-#define AFE4404_TIA_GAIN_CAP_22_5_P	0x7
-
-/* AFE4404 TIA_GAIN_RES values */
-#define AFE4404_TIA_GAIN_RES_500_K	0x0
-#define AFE4404_TIA_GAIN_RES_250_K	0x1
-#define AFE4404_TIA_GAIN_RES_100_K	0x2
-#define AFE4404_TIA_GAIN_RES_50_K	0x3
-#define AFE4404_TIA_GAIN_RES_25_K	0x4
-#define AFE4404_TIA_GAIN_RES_10_K	0x5
-#define AFE4404_TIA_GAIN_RES_1_M	0x6
-#define AFE4404_TIA_GAIN_RES_2_M	0x7
-
 enum afe4404_fields {
 	/* Gains */
 	F_TIA_GAIN_SEP, F_TIA_CF_SEP,
-- 
2.8.1

[PATCH 13/13] iio: health/afe4404: ENSEPGAIN is part of CONTROL2 register

[Andrew F. Davis]

Rename this definition, no functional changes.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 drivers/iio/health/afe4404.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
index 3a8131d..4526640 100644
--- a/drivers/iio/health/afe4404.c
+++ b/drivers/iio/health/afe4404.c
@@ -48,8 +48,8 @@
 #define AFE4404_AVG_LED2_ALED2VAL	0x3f
 #define AFE4404_AVG_LED1_ALED1VAL	0x40
 
-/* AFE4404 CONTROL3 register fields */
-#define AFE440X_CONTROL3_OSC_ENABLE	BIT(9)
+/* AFE4404 CONTROL2 register fields */
+#define AFE440X_CONTROL2_OSC_ENABLE	BIT(9)
 
 enum afe4404_fields {
 	/* Gains */
@@ -398,7 +398,7 @@ static const struct reg_sequence afe4404_reg_sequences[] = {
 	AFE4404_TIMING_PAIRS,
 	{ AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN },
 	{ AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN },
-	{ AFE440X_CONTROL2, AFE440X_CONTROL3_OSC_ENABLE	},
+	{ AFE440X_CONTROL2, AFE440X_CONTROL2_OSC_ENABLE	},
 };
 
 static const struct regmap_range afe4404_yes_ranges[] = {
-- 
2.8.1

Re: [PATCH 01/13] iio: health/afe440x: Fix kernel-doc format

[Jonathan Cameron]

On 01/05/16 21:36, Andrew F. Davis wrote:
> Fix kernel-doc formatting for structs, and while we are making little
> fixes, clarify the module description and update the copywrite.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
Applied to the togreg branch of iio.git - to be pushed out in a day or two
as testing.

Thanks,

Jonathan
> ---
>  drivers/iio/health/afe4403.c | 18 +++++++++---------
>  drivers/iio/health/afe4404.c | 16 ++++++++--------
>  2 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
> index 88e43f8..2094113 100644
> --- a/drivers/iio/health/afe4403.c
> +++ b/drivers/iio/health/afe4403.c
> @@ -1,7 +1,7 @@
>  /*
>   * AFE4403 Heart Rate Monitors and Low-Cost Pulse Oximeters
>   *
> - * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
> + * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/
>   *	Andrew F. Davis <afd@ti.com>
>   *
>   * This program is free software; you can redistribute it and/or modify
> @@ -103,13 +103,13 @@
>  #define AFE4403_TIAGAIN_RES_NONE	0x7
>  
>  /**
> - * struct afe4403_data
> - * @dev - Device structure
> - * @spi - SPI device handle
> - * @regmap - Register map of the device
> - * @regulator - Pointer to the regulator for the IC
> - * @trig - IIO trigger for this device
> - * @irq - ADC_RDY line interrupt number
> + * struct afe4403_data - AFE4403 device instance data
> + * @dev: Device structure
> + * @spi: SPI device handle
> + * @regmap: Register map of the device
> + * @regulator: Pointer to the regulator for the IC
> + * @trig: IIO trigger for this device
> + * @irq: ADC_RDY line interrupt number
>   */
>  struct afe4403_data {
>  	struct device *dev;
> @@ -704,5 +704,5 @@ static struct spi_driver afe4403_spi_driver = {
>  module_spi_driver(afe4403_spi_driver);
>  
>  MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>");
> -MODULE_DESCRIPTION("TI AFE4403 Heart Rate and Pulse Oximeter");
> +MODULE_DESCRIPTION("TI AFE4403 Heart Rate Monitor and Pulse Oximeter AFE");
>  MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
> index 5096a46..7127d03 100644
> --- a/drivers/iio/health/afe4404.c
> +++ b/drivers/iio/health/afe4404.c
> @@ -1,7 +1,7 @@
>  /*
>   * AFE4404 Heart Rate Monitors and Low-Cost Pulse Oximeters
>   *
> - * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
> + * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/
>   *	Andrew F. Davis <afd@ti.com>
>   *
>   * This program is free software; you can redistribute it and/or modify
> @@ -106,12 +106,12 @@
>  #define AFE4404_TIA_GAIN_RES_2_M	0x7
>  
>  /**
> - * struct afe4404_data
> - * @dev - Device structure
> - * @regmap - Register map of the device
> - * @regulator - Pointer to the regulator for the IC
> - * @trig - IIO trigger for this device
> - * @irq - ADC_RDY line interrupt number
> + * struct afe4404_data - AFE4404 device instance data
> + * @dev: Device structure
> + * @regmap: Register map of the device
> + * @regulator: Pointer to the regulator for the IC
> + * @trig: IIO trigger for this device
> + * @irq: ADC_RDY line interrupt number
>   */
>  struct afe4404_data {
>  	struct device *dev;
> @@ -675,5 +675,5 @@ static struct i2c_driver afe4404_i2c_driver = {
>  module_i2c_driver(afe4404_i2c_driver);
>  
>  MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>");
> -MODULE_DESCRIPTION("TI AFE4404 Heart Rate and Pulse Oximeter");
> +MODULE_DESCRIPTION("TI AFE4404 Heart Rate Monitor and Pulse Oximeter AFE");
>  MODULE_LICENSE("GPL v2");
> 

Re: [PATCH 02/13] iio: health/afe440x: Remove of_match_ptr and ifdefs

[Jonathan Cameron]

On 01/05/16 21:36, Andrew F. Davis wrote:
> The drivers DT tables are not built-in when OF is not enabled, this does
> not save us enough to justify ugly ifdefs. Clean this up.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
Can't argue with that.

Applied.

Jonathan
> ---
>  drivers/iio/health/afe4403.c | 4 +---
>  drivers/iio/health/afe4404.c | 4 +---
>  2 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
> index 2094113..e3700cf 100644
> --- a/drivers/iio/health/afe4403.c
> +++ b/drivers/iio/health/afe4403.c
> @@ -498,13 +498,11 @@ static const struct regmap_config afe4403_regmap_config = {
>  	.volatile_table = &afe4403_volatile_table,
>  };
>  
> -#ifdef CONFIG_OF
>  static const struct of_device_id afe4403_of_match[] = {
>  	{ .compatible = "ti,afe4403", },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, afe4403_of_match);
> -#endif
>  
>  static int __maybe_unused afe4403_suspend(struct device *dev)
>  {
> @@ -694,7 +692,7 @@ MODULE_DEVICE_TABLE(spi, afe4403_ids);
>  static struct spi_driver afe4403_spi_driver = {
>  	.driver = {
>  		.name = AFE4403_DRIVER_NAME,
> -		.of_match_table = of_match_ptr(afe4403_of_match),
> +		.of_match_table = afe4403_of_match,
>  		.pm = &afe4403_pm_ops,
>  	},
>  	.probe = afe4403_probe,
> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
> index 7127d03..783b9b4 100644
> --- a/drivers/iio/health/afe4404.c
> +++ b/drivers/iio/health/afe4404.c
> @@ -469,13 +469,11 @@ static const struct regmap_config afe4404_regmap_config = {
>  	.volatile_table = &afe4404_volatile_table,
>  };
>  
> -#ifdef CONFIG_OF
>  static const struct of_device_id afe4404_of_match[] = {
>  	{ .compatible = "ti,afe4404", },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, afe4404_of_match);
> -#endif
>  
>  static int __maybe_unused afe4404_suspend(struct device *dev)
>  {
> @@ -665,7 +663,7 @@ MODULE_DEVICE_TABLE(i2c, afe4404_ids);
>  static struct i2c_driver afe4404_i2c_driver = {
>  	.driver = {
>  		.name = AFE4404_DRIVER_NAME,
> -		.of_match_table = of_match_ptr(afe4404_of_match),
> +		.of_match_table = afe4404_of_match,
>  		.pm = &afe4404_pm_ops,
>  	},
>  	.probe = afe4404_probe,
> 

Re: [PATCH 03/13] iio: health/afe440x: Remove unneeded initializers

[Jonathan Cameron]

On 01/05/16 21:36, Andrew F. Davis wrote:
> The drivers set some register values during initialization that can be
> set at runtime, these defaults were used in testing but are not
> necessary, remove these.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
Fair enough - applied.
> ---
>  drivers/iio/health/afe4403.c | 7 +------
>  drivers/iio/health/afe4404.c | 4 ----
>  2 files changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
> index e3700cf..5484785 100644
> --- a/drivers/iio/health/afe4403.c
> +++ b/drivers/iio/health/afe4403.c
> @@ -472,12 +472,7 @@ static const struct iio_trigger_ops afe4403_trigger_ops = {
>  
>  static const struct reg_sequence afe4403_reg_sequences[] = {
>  	AFE4403_TIMING_PAIRS,
> -	{ AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN | 0x000007},
> -	{ AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES_1_M },
> -	{ AFE440X_LEDCNTRL, (0x14 << AFE440X_LEDCNTRL_LED1_SHIFT) |
> -			    (0x14 << AFE440X_LEDCNTRL_LED2_SHIFT) },
> -	{ AFE440X_CONTROL2, AFE440X_CONTROL2_TX_REF_050 <<
> -			    AFE440X_CONTROL2_TX_REF_SHIFT },
> +	{ AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN },
>  };
>  
>  static const struct regmap_range afe4403_yes_ranges[] = {
> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
> index 783b9b4..2d4c522 100644
> --- a/drivers/iio/health/afe4404.c
> +++ b/drivers/iio/health/afe4404.c
> @@ -443,10 +443,6 @@ static const struct iio_trigger_ops afe4404_trigger_ops = {
>  static const struct reg_sequence afe4404_reg_sequences[] = {
>  	AFE4404_TIMING_PAIRS,
>  	{ AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN },
> -	{ AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES_50_K },
> -	{ AFE440X_LEDCNTRL, (0xf << AFE4404_LEDCNTRL_ILED1_SHIFT) |
> -			    (0x3 << AFE4404_LEDCNTRL_ILED2_SHIFT) |
> -			    (0x3 << AFE4404_LEDCNTRL_ILED3_SHIFT) },
>  	{ AFE440X_CONTROL2, AFE440X_CONTROL3_OSC_ENABLE	},
>  };
>  
> 

Re: [PATCH 04/13] iio: health/afe440x: Always use separate gain values

[Jonathan Cameron]

On 01/05/16 21:36, Andrew F. Davis wrote:
> Locking the two gain stages to the same setting adds no value for us,
> so initialize them as unlocked and remove the sysfs for unlocking them.
> This also allows us to greatly simplify showing and setting the gain
> registers.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
Hmm. ABI change but as you said it's an improvement.

Honestly I doubt anyone is using this device without also using userspace that
you are providing so lets apply this an cross our fingers that no one minds.

Applied.
> ---
>  .../ABI/testing/sysfs-bus-iio-health-afe440x       |  9 ----
>  drivers/iio/health/afe4403.c                       | 60 ++++++----------------
>  drivers/iio/health/afe4404.c                       | 60 ++++++----------------
>  drivers/iio/health/afe440x.h                       | 15 ++----
>  4 files changed, 37 insertions(+), 107 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
> index 3740f25..b19053a 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
> @@ -8,15 +8,6 @@ Description:
>  		Transimpedance Amplifier. Y is 1 for Rf1 and Cf1, Y is 2 for
>  		Rf2 and Cf2 values.
>  
> -What:		/sys/bus/iio/devices/iio:deviceX/tia_separate_en
> -Date:		December 2015
> -KernelVersion:
> -Contact:	Andrew F. Davis <afd@ti.com>
> -Description:
> -		Enable or disable separate settings for the TransImpedance
> -		Amplifier above, when disabled both values are set by the
> -		first channel.
> -
>  What:		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_raw
>  		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_ambient_raw
>  Date:		December 2015
> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
> index 5484785..bcff528 100644
> --- a/drivers/iio/health/afe4403.c
> +++ b/drivers/iio/health/afe4403.c
> @@ -180,9 +180,9 @@ static ssize_t afe440x_show_register(struct device *dev,
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct afe4403_data *afe = iio_priv(indio_dev);
>  	struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr);
> -	unsigned int reg_val, type;
> +	unsigned int reg_val;
>  	int vals[2];
> -	int ret, val_len;
> +	int ret;
>  
>  	ret = regmap_read(afe->regmap, afe440x_attr->reg, &reg_val);
>  	if (ret)
> @@ -191,27 +191,13 @@ static ssize_t afe440x_show_register(struct device *dev,
>  	reg_val &= afe440x_attr->mask;
>  	reg_val >>= afe440x_attr->shift;
>  
> -	switch (afe440x_attr->type) {
> -	case SIMPLE:
> -		type = IIO_VAL_INT;
> -		val_len = 1;
> -		vals[0] = reg_val;
> -		break;
> -	case RESISTANCE:
> -	case CAPACITANCE:
> -		type = IIO_VAL_INT_PLUS_MICRO;
> -		val_len = 2;
> -		if (reg_val < afe440x_attr->table_size) {
> -			vals[0] = afe440x_attr->val_table[reg_val].integer;
> -			vals[1] = afe440x_attr->val_table[reg_val].fract;
> -			break;
> -		}
> +	if (reg_val >= afe440x_attr->table_size)
>  		return -EINVAL;
> -	default:
> -		return -EINVAL;
> -	}
>  
> -	return iio_format_value(buf, type, val_len, vals);
> +	vals[0] = afe440x_attr->val_table[reg_val].integer;
> +	vals[1] = afe440x_attr->val_table[reg_val].fract;
> +
> +	return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals);
>  }
>  
>  static ssize_t afe440x_store_register(struct device *dev,
> @@ -227,22 +213,12 @@ static ssize_t afe440x_store_register(struct device *dev,
>  	if (ret)
>  		return ret;
>  
> -	switch (afe440x_attr->type) {
> -	case SIMPLE:
> -		val = integer;
> -		break;
> -	case RESISTANCE:
> -	case CAPACITANCE:
> -		for (val = 0; val < afe440x_attr->table_size; val++)
> -			if (afe440x_attr->val_table[val].integer == integer &&
> -			    afe440x_attr->val_table[val].fract == fract)
> -				break;
> -		if (val == afe440x_attr->table_size)
> -			return -EINVAL;
> -		break;
> -	default:
> +	for (val = 0; val < afe440x_attr->table_size; val++)
> +		if (afe440x_attr->val_table[val].integer == integer &&
> +		    afe440x_attr->val_table[val].fract == fract)
> +			break;
> +	if (val == afe440x_attr->table_size)
>  		return -EINVAL;
> -	}
>  
>  	ret = regmap_update_bits(afe->regmap, afe440x_attr->reg,
>  				 afe440x_attr->mask,
> @@ -253,16 +229,13 @@ static ssize_t afe440x_store_register(struct device *dev,
>  	return count;
>  }
>  
> -static AFE440X_ATTR(tia_separate_en, AFE4403_TIAGAIN, AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0);
> -
> -static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table, ARRAY_SIZE(afe4403_res_table));
> -static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, CAPACITANCE, afe4403_cap_table, ARRAY_SIZE(afe4403_cap_table));
> +static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, afe4403_res_table);
> +static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, afe4403_cap_table);
>  
> -static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table, ARRAY_SIZE(afe4403_res_table));
> -static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, CAPACITANCE, afe4403_cap_table, ARRAY_SIZE(afe4403_cap_table));
> +static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_res_table);
> +static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_cap_table);
>  
>  static struct attribute *afe440x_attributes[] = {
> -	&afe440x_attr_tia_separate_en.dev_attr.attr,
>  	&afe440x_attr_tia_resistance1.dev_attr.attr,
>  	&afe440x_attr_tia_capacitance1.dev_attr.attr,
>  	&afe440x_attr_tia_resistance2.dev_attr.attr,
> @@ -473,6 +446,7 @@ static const struct iio_trigger_ops afe4403_trigger_ops = {
>  static const struct reg_sequence afe4403_reg_sequences[] = {
>  	AFE4403_TIMING_PAIRS,
>  	{ AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN },
> +	{ AFE4403_TIAGAIN, AFE440X_TIAGAIN_ENSEPGAIN },
>  };
>  
>  static const struct regmap_range afe4403_yes_ranges[] = {
> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
> index 2d4c522..b9c1666 100644
> --- a/drivers/iio/health/afe4404.c
> +++ b/drivers/iio/health/afe4404.c
> @@ -193,9 +193,9 @@ static ssize_t afe440x_show_register(struct device *dev,
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct afe4404_data *afe = iio_priv(indio_dev);
>  	struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr);
> -	unsigned int reg_val, type;
> +	unsigned int reg_val;
>  	int vals[2];
> -	int ret, val_len;
> +	int ret;
>  
>  	ret = regmap_read(afe->regmap, afe440x_attr->reg, &reg_val);
>  	if (ret)
> @@ -204,27 +204,13 @@ static ssize_t afe440x_show_register(struct device *dev,
>  	reg_val &= afe440x_attr->mask;
>  	reg_val >>= afe440x_attr->shift;
>  
> -	switch (afe440x_attr->type) {
> -	case SIMPLE:
> -		type = IIO_VAL_INT;
> -		val_len = 1;
> -		vals[0] = reg_val;
> -		break;
> -	case RESISTANCE:
> -	case CAPACITANCE:
> -		type = IIO_VAL_INT_PLUS_MICRO;
> -		val_len = 2;
> -		if (reg_val < afe440x_attr->table_size) {
> -			vals[0] = afe440x_attr->val_table[reg_val].integer;
> -			vals[1] = afe440x_attr->val_table[reg_val].fract;
> -			break;
> -		}
> +	if (reg_val >= afe440x_attr->table_size)
>  		return -EINVAL;
> -	default:
> -		return -EINVAL;
> -	}
>  
> -	return iio_format_value(buf, type, val_len, vals);
> +	vals[0] = afe440x_attr->val_table[reg_val].integer;
> +	vals[1] = afe440x_attr->val_table[reg_val].fract;
> +
> +	return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals);
>  }
>  
>  static ssize_t afe440x_store_register(struct device *dev,
> @@ -240,22 +226,12 @@ static ssize_t afe440x_store_register(struct device *dev,
>  	if (ret)
>  		return ret;
>  
> -	switch (afe440x_attr->type) {
> -	case SIMPLE:
> -		val = integer;
> -		break;
> -	case RESISTANCE:
> -	case CAPACITANCE:
> -		for (val = 0; val < afe440x_attr->table_size; val++)
> -			if (afe440x_attr->val_table[val].integer == integer &&
> -			    afe440x_attr->val_table[val].fract == fract)
> -				break;
> -		if (val == afe440x_attr->table_size)
> -			return -EINVAL;
> -		break;
> -	default:
> +	for (val = 0; val < afe440x_attr->table_size; val++)
> +		if (afe440x_attr->val_table[val].integer == integer &&
> +		    afe440x_attr->val_table[val].fract == fract)
> +			break;
> +	if (val == afe440x_attr->table_size)
>  		return -EINVAL;
> -	}
>  
>  	ret = regmap_update_bits(afe->regmap, afe440x_attr->reg,
>  				 afe440x_attr->mask,
> @@ -266,16 +242,13 @@ static ssize_t afe440x_store_register(struct device *dev,
>  	return count;
>  }
>  
> -static AFE440X_ATTR(tia_separate_en, AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0);
> -
> -static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, RESISTANCE, afe4404_res_table, ARRAY_SIZE(afe4404_res_table));
> -static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, CAPACITANCE, afe4404_cap_table, ARRAY_SIZE(afe4404_cap_table));
> +static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, afe4404_res_table);
> +static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, afe4404_cap_table);
>  
> -static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, RESISTANCE, afe4404_res_table, ARRAY_SIZE(afe4404_res_table));
> -static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, CAPACITANCE, afe4404_cap_table, ARRAY_SIZE(afe4404_cap_table));
> +static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, afe4404_res_table);
> +static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, afe4404_cap_table);
>  
>  static struct attribute *afe440x_attributes[] = {
> -	&afe440x_attr_tia_separate_en.dev_attr.attr,
>  	&afe440x_attr_tia_resistance1.dev_attr.attr,
>  	&afe440x_attr_tia_capacitance1.dev_attr.attr,
>  	&afe440x_attr_tia_resistance2.dev_attr.attr,
> @@ -443,6 +416,7 @@ static const struct iio_trigger_ops afe4404_trigger_ops = {
>  static const struct reg_sequence afe4404_reg_sequences[] = {
>  	AFE4404_TIMING_PAIRS,
>  	{ AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN },
> +	{ AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN },
>  	{ AFE440X_CONTROL2, AFE440X_CONTROL3_OSC_ENABLE	},
>  };
>  
> diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h
> index c671ab7..544bbab 100644
> --- a/drivers/iio/health/afe440x.h
> +++ b/drivers/iio/health/afe440x.h
> @@ -71,8 +71,7 @@
>  #define AFE440X_CONTROL1_TIMEREN	BIT(8)
>  
>  /* TIAGAIN register fields */
> -#define AFE440X_TIAGAIN_ENSEPGAIN_MASK	BIT(15)
> -#define AFE440X_TIAGAIN_ENSEPGAIN_SHIFT	15
> +#define AFE440X_TIAGAIN_ENSEPGAIN	BIT(15)
>  
>  /* CONTROL2 register fields */
>  #define AFE440X_CONTROL2_PDN_AFE	BIT(0)
> @@ -133,12 +132,6 @@ struct afe440x_reg_info {
>  		.output = true,					\
>  	}
>  
> -enum afe440x_reg_type {
> -	SIMPLE,
> -	RESISTANCE,
> -	CAPACITANCE,
> -};
> -
>  struct afe440x_val_table {
>  	int integer;
>  	int fract;
> @@ -167,7 +160,6 @@ struct afe440x_attr {
>  	unsigned int reg;
>  	unsigned int shift;
>  	unsigned int mask;
> -	enum afe440x_reg_type type;
>  	const struct afe440x_val_table *val_table;
>  	unsigned int table_size;
>  };
> @@ -175,7 +167,7 @@ struct afe440x_attr {
>  #define to_afe440x_attr(_dev_attr)				\
>  	container_of(_dev_attr, struct afe440x_attr, dev_attr)
>  
> -#define AFE440X_ATTR(_name, _reg, _field, _type, _table, _size)	\
> +#define AFE440X_ATTR(_name, _reg, _field, _table)		\
>  	struct afe440x_attr afe440x_attr_##_name = {		\
>  		.dev_attr = __ATTR(_name, (S_IRUGO | S_IWUSR),	\
>  				   afe440x_show_register,	\
> @@ -183,9 +175,8 @@ struct afe440x_attr {
>  		.reg = _reg,					\
>  		.shift = _field ## _SHIFT,			\
>  		.mask = _field ## _MASK,			\
> -		.type = _type,					\
>  		.val_table = _table,				\
> -		.table_size = _size,				\
> +		.table_size = ARRAY_SIZE(_table),		\
>  	}
>  
>  #endif /* _AFE440X_H */
> 

Re: [PATCH 05/13] iio: health/afe440x: Fix scan_index assignment

[Jonathan Cameron]

On 01/05/16 21:36, Andrew F. Davis wrote:
> The LED channels are not scannable and so scan_index should
> be negative, fix this here.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
Good point.  I've applied this to the togreg branch, but if
you want me to push this one out as a fix it definitely qualifies.

Jonathan
> ---
>  drivers/iio/health/afe440x.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h
> index 544bbab..583d071 100644
> --- a/drivers/iio/health/afe440x.h
> +++ b/drivers/iio/health/afe440x.h
> @@ -125,7 +125,7 @@ struct afe440x_reg_info {
>  		.type = IIO_CURRENT,				\
>  		.channel = _index,				\
>  		.address = _index,				\
> -		.scan_index = _index,				\
> +		.scan_index = -1,				\
>  		.extend_name = _name,				\
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
>  			BIT(IIO_CHAN_INFO_SCALE),		\
> 

Re: [PATCH 06/13] iio: health/afe440x: Remove unneeded offset handling

[Jonathan Cameron]

On 01/05/16 21:36, Andrew F. Davis wrote:
> No channel in the afe4403 driver has IIO_CHAN_INFO_OFFSET set so
> remove the handlers for this.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
Oops :)

Applied.
> ---
>  drivers/iio/health/afe4403.c | 17 -----------------
>  1 file changed, 17 deletions(-)
> 
> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
> index bcff528..cac6090 100644
> --- a/drivers/iio/health/afe4403.c
> +++ b/drivers/iio/health/afe4403.c
> @@ -293,14 +293,6 @@ static int afe4403_read_raw(struct iio_dev *indio_dev,
>  			if (ret)
>  				return ret;
>  			return IIO_VAL_INT;
> -		case IIO_CHAN_INFO_OFFSET:
> -			ret = regmap_read(afe->regmap, reg_info.offreg,
> -					  val);
> -			if (ret)
> -				return ret;
> -			*val &= reg_info.mask;
> -			*val >>= reg_info.shift;
> -			return IIO_VAL_INT;
>  		}
>  		break;
>  	case IIO_CURRENT:
> @@ -333,15 +325,6 @@ static int afe4403_write_raw(struct iio_dev *indio_dev,
>  	const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address];
>  
>  	switch (chan->type) {
> -	case IIO_INTENSITY:
> -		switch (mask) {
> -		case IIO_CHAN_INFO_OFFSET:
> -			return regmap_update_bits(afe->regmap,
> -				reg_info.offreg,
> -				reg_info.mask,
> -				(val << reg_info.shift));
> -		}
> -		break;
>  	case IIO_CURRENT:
>  		switch (mask) {
>  		case IIO_CHAN_INFO_RAW:
> 

Re: [PATCH 07/13] iio: health/afe4404: Remove LED3 input channel

[Jonathan Cameron]

On 01/05/16 21:36, Andrew F. Davis wrote:
> Input channel LED3 is only an alias for stage ALED2, this virtual channel
> does nothing for us, remove this channel.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
Again, abi change, but I doubt anyone will notice *fingers crossed* ;)

Jonathan
> ---
>  drivers/iio/health/afe4404.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
> index b9c1666..2edb7d7 100644
> --- a/drivers/iio/health/afe4404.c
> +++ b/drivers/iio/health/afe4404.c
> @@ -126,7 +126,6 @@ enum afe4404_chan_id {
>  	ALED1,
>  	LED2,
>  	ALED2,
> -	LED3,
>  	LED1_ALED1,
>  	LED2_ALED2,
>  	ILED1,
> @@ -139,7 +138,6 @@ static const struct afe440x_reg_info afe4404_reg_info[] = {
>  	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED1),
>  	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED2),
>  	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED2),
> -	[LED3] = AFE440X_REG_INFO(AFE440X_ALED2VAL, 0, NULL),
>  	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
>  	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
>  	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED1),
> @@ -153,7 +151,6 @@ static const struct iio_chan_spec afe4404_channels[] = {
>  	AFE440X_INTENSITY_CHAN(ALED1, "led1_ambient", BIT(IIO_CHAN_INFO_OFFSET)),
>  	AFE440X_INTENSITY_CHAN(LED2, "led2", BIT(IIO_CHAN_INFO_OFFSET)),
>  	AFE440X_INTENSITY_CHAN(ALED2, "led2_ambient", BIT(IIO_CHAN_INFO_OFFSET)),
> -	AFE440X_INTENSITY_CHAN(LED3, "led3", BIT(IIO_CHAN_INFO_OFFSET)),
>  	AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0),
>  	AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0),
>  	/* LED current */
> 

Re: [PATCH 08/13] iio: health/afe440x: Remove channel names

[Jonathan Cameron]

On 01/05/16 21:36, Andrew F. Davis wrote:
> These AFEs have 4 ADC mesuring stages (called LED2, ALED2, LED1, and
> ALED1 in the datasheet), we map these as channels, these stages can serve
> different purposes depending on the application. For instance the AFE4404
> has an additional LED (LED3), this LED can be timed to be active during
> stage 2 (or anystage, but the datasheet describes this case and the name
> of the stage reflects this use). This ability is used further in upcoming
> parts that tie the front-end gain and the LED timings together. For these
> reasons we remove explicit naming the channels.
> 
> Without channel names it is best that the index numbers are in order to
> match the stage number, reorder the channel numbers.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
Youch - this is rather major surgery.

Hmm. Given how non standard an ABI we ended up with in the first place
again, let us cross our fingers that all the userspace for this is
effectively under your control currently.

Getting a little more nervous about this - but worth the risk I think
for the simpler result.

Applied

Jonathan
> ---
>  .../ABI/testing/sysfs-bus-iio-health-afe440x       | 39 ++++++++++------------
>  drivers/iio/health/afe4403.c                       | 28 ++++++++--------
>  drivers/iio/health/afe4404.c                       | 30 ++++++++---------
>  drivers/iio/health/afe440x.h                       |  8 ++---
>  4 files changed, 51 insertions(+), 54 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
> index b19053a..a067073 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
> @@ -8,38 +8,35 @@ Description:
>  		Transimpedance Amplifier. Y is 1 for Rf1 and Cf1, Y is 2 for
>  		Rf2 and Cf2 values.
>  
> -What:		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_raw
> -		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_ambient_raw
> -Date:		December 2015
> +What:		/sys/bus/iio/devices/iio:deviceX/in_intensityY_raw
> +Date:		May 2016
>  KernelVersion:
>  Contact:	Andrew F. Davis <afd@ti.com>
>  Description:
>  		Get measured values from the ADC for these stages. Y is the
> -		specific LED number. The values are expressed in 24-bit twos
> -		complement.
> -
> -What:		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY-ledY_ambient_raw
> -Date:		December 2015
> -KernelVersion:
> -Contact:	Andrew F. Davis <afd@ti.com>
> -Description:
> -		Get differential values from the ADC for these stages. Y is the
> -		specific LED number. The values are expressed in 24-bit twos
> -		complement for the specified LEDs.
> +		specific stage number corresponding to datasheet stage names
> +		as follows:
> +		1 -> LED2
> +		2 -> ALED2/LED3
> +		3 -> LED1
> +		4 -> ALED1/LED4
> +		Note that channels 5 and 6 represent LED2-ALED2 and LED1-ALED1
> +		respectively which simply helper channels containing the
> +		calculated difference in the value of stage 1 - 2 and 3 - 4.
> +		The values are expressed in 24-bit twos complement.
>  
> -What:		/sys/bus/iio/devices/iio:deviceX/out_current_ledY_offset
> -		/sys/bus/iio/devices/iio:deviceX/out_current_ledY_ambient_offset
> -Date:		December 2015
> +What:		/sys/bus/iio/devices/iio:deviceX/in_intensityY_offset
> +Date:		May 2016
>  KernelVersion:
>  Contact:	Andrew F. Davis <afd@ti.com>
>  Description:
>  		Get and set the offset cancellation DAC setting for these
>  		stages. The values are expressed in 5-bit sign-magnitude.
>  
> -What:		/sys/bus/iio/devices/iio:deviceX/out_current_ledY_raw
> -Date:		December 2015
> +What:		/sys/bus/iio/devices/iio:deviceX/out_currentY_raw
> +Date:		May 2016
>  KernelVersion:
>  Contact:	Andrew F. Davis <afd@ti.com>
>  Description:
> -		Get and set the LED current for the specified LED. Y is the
> -		specific LED number.
> +		Get and set the LED current for the specified LED active during
> +		this stage. Y is the specific stage number.
> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
> index cac6090..4a58064 100644
> --- a/drivers/iio/health/afe4403.c
> +++ b/drivers/iio/health/afe4403.c
> @@ -121,38 +121,38 @@ struct afe4403_data {
>  };
>  
>  enum afe4403_chan_id {
> +	LED2 = 1,
> +	ALED2,
>  	LED1,
>  	ALED1,
> -	LED2,
> -	ALED2,
> -	LED1_ALED1,
>  	LED2_ALED2,
> +	LED1_ALED1,
>  	ILED1,
>  	ILED2,
>  };
>  
>  static const struct afe440x_reg_info afe4403_reg_info[] = {
> -	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, 0, NULL),
> -	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, 0, NULL),
>  	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, 0, NULL),
>  	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, 0, NULL),
> -	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
> +	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, 0, NULL),
> +	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, 0, NULL),
>  	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
> +	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
>  	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED1),
>  	[ILED2] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED2),
>  };
>  
>  static const struct iio_chan_spec afe4403_channels[] = {
>  	/* ADC values */
> -	AFE440X_INTENSITY_CHAN(LED1, "led1", 0),
> -	AFE440X_INTENSITY_CHAN(ALED1, "led1_ambient", 0),
> -	AFE440X_INTENSITY_CHAN(LED2, "led2", 0),
> -	AFE440X_INTENSITY_CHAN(ALED2, "led2_ambient", 0),
> -	AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0),
> -	AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0),
> +	AFE440X_INTENSITY_CHAN(LED2, 0),
> +	AFE440X_INTENSITY_CHAN(ALED2, 0),
> +	AFE440X_INTENSITY_CHAN(LED1, 0),
> +	AFE440X_INTENSITY_CHAN(ALED1, 0),
> +	AFE440X_INTENSITY_CHAN(LED2_ALED2, 0),
> +	AFE440X_INTENSITY_CHAN(LED1_ALED1, 0),
>  	/* LED current */
> -	AFE440X_CURRENT_CHAN(ILED1, "led1"),
> -	AFE440X_CURRENT_CHAN(ILED2, "led2"),
> +	AFE440X_CURRENT_CHAN(ILED1),
> +	AFE440X_CURRENT_CHAN(ILED2),
>  };
>  
>  static const struct afe440x_val_table afe4403_res_table[] = {
> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
> index 2edb7d7..7806a45 100644
> --- a/drivers/iio/health/afe4404.c
> +++ b/drivers/iio/health/afe4404.c
> @@ -122,24 +122,24 @@ struct afe4404_data {
>  };
>  
>  enum afe4404_chan_id {
> +	LED2 = 1,
> +	ALED2,
>  	LED1,
>  	ALED1,
> -	LED2,
> -	ALED2,
> -	LED1_ALED1,
>  	LED2_ALED2,
> +	LED1_ALED1,
>  	ILED1,
>  	ILED2,
>  	ILED3,
>  };
>  
>  static const struct afe440x_reg_info afe4404_reg_info[] = {
> -	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED1),
> -	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED1),
>  	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED2),
>  	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED2),
> -	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
> +	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED1),
> +	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED1),
>  	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
> +	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
>  	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED1),
>  	[ILED2] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED2),
>  	[ILED3] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED3),
> @@ -147,16 +147,16 @@ static const struct afe440x_reg_info afe4404_reg_info[] = {
>  
>  static const struct iio_chan_spec afe4404_channels[] = {
>  	/* ADC values */
> -	AFE440X_INTENSITY_CHAN(LED1, "led1", BIT(IIO_CHAN_INFO_OFFSET)),
> -	AFE440X_INTENSITY_CHAN(ALED1, "led1_ambient", BIT(IIO_CHAN_INFO_OFFSET)),
> -	AFE440X_INTENSITY_CHAN(LED2, "led2", BIT(IIO_CHAN_INFO_OFFSET)),
> -	AFE440X_INTENSITY_CHAN(ALED2, "led2_ambient", BIT(IIO_CHAN_INFO_OFFSET)),
> -	AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0),
> -	AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0),
> +	AFE440X_INTENSITY_CHAN(LED2, BIT(IIO_CHAN_INFO_OFFSET)),
> +	AFE440X_INTENSITY_CHAN(ALED2, BIT(IIO_CHAN_INFO_OFFSET)),
> +	AFE440X_INTENSITY_CHAN(LED1, BIT(IIO_CHAN_INFO_OFFSET)),
> +	AFE440X_INTENSITY_CHAN(ALED1, BIT(IIO_CHAN_INFO_OFFSET)),
> +	AFE440X_INTENSITY_CHAN(LED2_ALED2, 0),
> +	AFE440X_INTENSITY_CHAN(LED1_ALED1, 0),
>  	/* LED current */
> -	AFE440X_CURRENT_CHAN(ILED1, "led1"),
> -	AFE440X_CURRENT_CHAN(ILED2, "led2"),
> -	AFE440X_CURRENT_CHAN(ILED3, "led3"),
> +	AFE440X_CURRENT_CHAN(ILED1),
> +	AFE440X_CURRENT_CHAN(ILED2),
> +	AFE440X_CURRENT_CHAN(ILED3),
>  };
>  
>  static const struct afe440x_val_table afe4404_res_table[] = {
> diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h
> index 583d071..713972f 100644
> --- a/drivers/iio/health/afe440x.h
> +++ b/drivers/iio/health/afe440x.h
> @@ -103,7 +103,7 @@ struct afe440x_reg_info {
>  		.mask = _sm ## _MASK,				\
>  	}
>  
> -#define AFE440X_INTENSITY_CHAN(_index, _name, _mask)		\
> +#define AFE440X_INTENSITY_CHAN(_index, _mask)			\
>  	{							\
>  		.type = IIO_INTENSITY,				\
>  		.channel = _index,				\
> @@ -115,20 +115,20 @@ struct afe440x_reg_info {
>  				.storagebits = 32,		\
>  				.endianness = IIO_CPU,		\
>  		},						\
> -		.extend_name = _name,				\
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
>  			_mask,					\
> +		.indexed = true,				\
>  	}
>  
> -#define AFE440X_CURRENT_CHAN(_index, _name)			\
> +#define AFE440X_CURRENT_CHAN(_index)				\
>  	{							\
>  		.type = IIO_CURRENT,				\
>  		.channel = _index,				\
>  		.address = _index,				\
>  		.scan_index = -1,				\
> -		.extend_name = _name,				\
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
>  			BIT(IIO_CHAN_INFO_SCALE),		\
> +		.indexed = true,				\
>  		.output = true,					\
>  	}
>  
> 

Re: [PATCH 09/13] iio: health/afe440x: Use regmap fields

[Jonathan Cameron]

On 01/05/16 21:36, Andrew F. Davis wrote:
> These drivers can use regmap fields to access fields in registers, this
> allows us to remove some macros/defines and simplify code, do this here.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
This could almost be a poster boy for why regmap field stuff is good ;)
So often it ends up more marginal than this.

Applied.

Jonathan
> ---
>  drivers/iio/health/afe4403.c | 118 ++++++++++++++++----------------
>  drivers/iio/health/afe4404.c | 156 ++++++++++++++++++++++---------------------
>  drivers/iio/health/afe440x.h |  25 +------
>  3 files changed, 144 insertions(+), 155 deletions(-)
> 
> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
> index 4a58064..1950155 100644
> --- a/drivers/iio/health/afe4403.c
> +++ b/drivers/iio/health/afe4403.c
> @@ -39,32 +39,6 @@
>  #define AFE4403_TIAGAIN			0x20
>  #define AFE4403_TIA_AMB_GAIN		0x21
>  
> -/* AFE4403 GAIN register fields */
> -#define AFE4403_TIAGAIN_RES_MASK	GENMASK(2, 0)
> -#define AFE4403_TIAGAIN_RES_SHIFT	0
> -#define AFE4403_TIAGAIN_CAP_MASK	GENMASK(7, 3)
> -#define AFE4403_TIAGAIN_CAP_SHIFT	3
> -
> -/* AFE4403 LEDCNTRL register fields */
> -#define AFE440X_LEDCNTRL_LED1_MASK		GENMASK(15, 8)
> -#define AFE440X_LEDCNTRL_LED1_SHIFT		8
> -#define AFE440X_LEDCNTRL_LED2_MASK		GENMASK(7, 0)
> -#define AFE440X_LEDCNTRL_LED2_SHIFT		0
> -#define AFE440X_LEDCNTRL_LED_RANGE_MASK		GENMASK(17, 16)
> -#define AFE440X_LEDCNTRL_LED_RANGE_SHIFT	16
> -
> -/* AFE4403 CONTROL2 register fields */
> -#define AFE440X_CONTROL2_PWR_DWN_TX	BIT(2)
> -#define AFE440X_CONTROL2_EN_SLOW_DIAG	BIT(8)
> -#define AFE440X_CONTROL2_DIAG_OUT_TRI	BIT(10)
> -#define AFE440X_CONTROL2_TX_BRDG_MOD	BIT(11)
> -#define AFE440X_CONTROL2_TX_REF_MASK	GENMASK(18, 17)
> -#define AFE440X_CONTROL2_TX_REF_SHIFT	17
> -
> -/* AFE4404 NULL fields */
> -#define NULL_MASK	0
> -#define NULL_SHIFT	0
> -
>  /* AFE4403 LEDCNTRL values */
>  #define AFE440X_LEDCNTRL_RANGE_TX_HALF	0x1
>  #define AFE440X_LEDCNTRL_RANGE_TX_FULL	0x2
> @@ -102,11 +76,35 @@
>  #define AFE4403_TIAGAIN_RES_1_M		0x6
>  #define AFE4403_TIAGAIN_RES_NONE	0x7
>  
> +enum afe4403_fields {
> +	/* Gains */
> +	F_RF_LED1, F_CF_LED1,
> +	F_RF_LED, F_CF_LED,
> +
> +	/* LED Current */
> +	F_ILED1, F_ILED2,
> +
> +	/* sentinel */
> +	F_MAX_FIELDS
> +};
> +
> +static const struct reg_field afe4403_reg_fields[] = {
> +	/* Gains */
> +	[F_RF_LED1]	= REG_FIELD(AFE4403_TIAGAIN, 0, 2),
> +	[F_CF_LED1]	= REG_FIELD(AFE4403_TIAGAIN, 3, 7),
> +	[F_RF_LED]	= REG_FIELD(AFE4403_TIA_AMB_GAIN, 0, 2),
> +	[F_CF_LED]	= REG_FIELD(AFE4403_TIA_AMB_GAIN, 3, 7),
> +	/* LED Current */
> +	[F_ILED1]	= REG_FIELD(AFE440X_LEDCNTRL, 0, 7),
> +	[F_ILED2]	= REG_FIELD(AFE440X_LEDCNTRL, 8, 15),
> +};
> +
>  /**
>   * struct afe4403_data - AFE4403 device instance data
>   * @dev: Device structure
>   * @spi: SPI device handle
>   * @regmap: Register map of the device
> + * @fields: Register fields of the device
>   * @regulator: Pointer to the regulator for the IC
>   * @trig: IIO trigger for this device
>   * @irq: ADC_RDY line interrupt number
> @@ -115,6 +113,7 @@ struct afe4403_data {
>  	struct device *dev;
>  	struct spi_device *spi;
>  	struct regmap *regmap;
> +	struct regmap_field *fields[F_MAX_FIELDS];
>  	struct regulator *regulator;
>  	struct iio_trigger *trig;
>  	int irq;
> @@ -131,15 +130,18 @@ enum afe4403_chan_id {
>  	ILED2,
>  };
>  
> -static const struct afe440x_reg_info afe4403_reg_info[] = {
> -	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, 0, NULL),
> -	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, 0, NULL),
> -	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, 0, NULL),
> -	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, 0, NULL),
> -	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
> -	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
> -	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED1),
> -	[ILED2] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED2),
> +static const unsigned int afe4403_channel_values[] = {
> +	[LED2] = AFE440X_LED2VAL,
> +	[ALED2] = AFE440X_ALED2VAL,
> +	[LED1] = AFE440X_LED1VAL,
> +	[ALED1] = AFE440X_ALED1VAL,
> +	[LED2_ALED2] = AFE440X_LED2_ALED2VAL,
> +	[LED1_ALED1] = AFE440X_LED1_ALED1VAL,
> +};
> +
> +static const unsigned int afe4403_channel_leds[] = {
> +	[ILED1] = F_ILED1,
> +	[ILED2] = F_ILED2,
>  };
>  
>  static const struct iio_chan_spec afe4403_channels[] = {
> @@ -184,13 +186,10 @@ static ssize_t afe440x_show_register(struct device *dev,
>  	int vals[2];
>  	int ret;
>  
> -	ret = regmap_read(afe->regmap, afe440x_attr->reg, &reg_val);
> +	ret = regmap_field_read(afe->fields[afe440x_attr->field], &reg_val);
>  	if (ret)
>  		return ret;
>  
> -	reg_val &= afe440x_attr->mask;
> -	reg_val >>= afe440x_attr->shift;
> -
>  	if (reg_val >= afe440x_attr->table_size)
>  		return -EINVAL;
>  
> @@ -220,20 +219,18 @@ static ssize_t afe440x_store_register(struct device *dev,
>  	if (val == afe440x_attr->table_size)
>  		return -EINVAL;
>  
> -	ret = regmap_update_bits(afe->regmap, afe440x_attr->reg,
> -				 afe440x_attr->mask,
> -				 (val << afe440x_attr->shift));
> +	ret = regmap_field_write(afe->fields[afe440x_attr->field], val);
>  	if (ret)
>  		return ret;
>  
>  	return count;
>  }
>  
> -static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, afe4403_res_table);
> -static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, afe4403_cap_table);
> +static AFE440X_ATTR(tia_resistance1, F_RF_LED1, afe4403_res_table);
> +static AFE440X_ATTR(tia_capacitance1, F_CF_LED1, afe4403_cap_table);
>  
> -static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_res_table);
> -static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_cap_table);
> +static AFE440X_ATTR(tia_resistance2, F_RF_LED, afe4403_res_table);
> +static AFE440X_ATTR(tia_capacitance2, F_CF_LED, afe4403_cap_table);
>  
>  static struct attribute *afe440x_attributes[] = {
>  	&afe440x_attr_tia_resistance1.dev_attr.attr,
> @@ -282,14 +279,15 @@ static int afe4403_read_raw(struct iio_dev *indio_dev,
>  			    int *val, int *val2, long mask)
>  {
>  	struct afe4403_data *afe = iio_priv(indio_dev);
> -	const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address];
> +	unsigned int reg = afe4403_channel_values[chan->address];
> +	unsigned int field = afe4403_channel_leds[chan->address];
>  	int ret;
>  
>  	switch (chan->type) {
>  	case IIO_INTENSITY:
>  		switch (mask) {
>  		case IIO_CHAN_INFO_RAW:
> -			ret = afe4403_read(afe, reg_info.reg, val);
> +			ret = afe4403_read(afe, reg, val);
>  			if (ret)
>  				return ret;
>  			return IIO_VAL_INT;
> @@ -298,11 +296,9 @@ static int afe4403_read_raw(struct iio_dev *indio_dev,
>  	case IIO_CURRENT:
>  		switch (mask) {
>  		case IIO_CHAN_INFO_RAW:
> -			ret = regmap_read(afe->regmap, reg_info.reg, val);
> +			ret = regmap_field_read(afe->fields[field], val);
>  			if (ret)
>  				return ret;
> -			*val &= reg_info.mask;
> -			*val >>= reg_info.shift;
>  			return IIO_VAL_INT;
>  		case IIO_CHAN_INFO_SCALE:
>  			*val = 0;
> @@ -322,16 +318,13 @@ static int afe4403_write_raw(struct iio_dev *indio_dev,
>  			     int val, int val2, long mask)
>  {
>  	struct afe4403_data *afe = iio_priv(indio_dev);
> -	const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address];
> +	unsigned int field = afe4403_channel_leds[chan->address];
>  
>  	switch (chan->type) {
>  	case IIO_CURRENT:
>  		switch (mask) {
>  		case IIO_CHAN_INFO_RAW:
> -			return regmap_update_bits(afe->regmap,
> -				reg_info.reg,
> -				reg_info.mask,
> -				(val << reg_info.shift));
> +			return regmap_field_write(afe->fields[field], val);
>  		}
>  		break;
>  	default:
> @@ -366,7 +359,7 @@ static irqreturn_t afe4403_trigger_handler(int irq, void *private)
>  	for_each_set_bit(bit, indio_dev->active_scan_mask,
>  			 indio_dev->masklength) {
>  		ret = spi_write_then_read(afe->spi,
> -					  &afe4403_reg_info[bit].reg, 1,
> +					  &afe4403_channel_values[bit], 1,
>  					  rx, 3);
>  		if (ret)
>  			goto err;
> @@ -503,7 +496,7 @@ static int afe4403_probe(struct spi_device *spi)
>  {
>  	struct iio_dev *indio_dev;
>  	struct afe4403_data *afe;
> -	int ret;
> +	int i, ret;
>  
>  	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*afe));
>  	if (!indio_dev)
> @@ -522,6 +515,15 @@ static int afe4403_probe(struct spi_device *spi)
>  		return PTR_ERR(afe->regmap);
>  	}
>  
> +	for (i = 0; i < F_MAX_FIELDS; i++) {
> +		afe->fields[i] = devm_regmap_field_alloc(afe->dev, afe->regmap,
> +							 afe4403_reg_fields[i]);
> +		if (IS_ERR(afe->fields[i])) {
> +			dev_err(afe->dev, "Unable to allocate regmap fields\n");
> +			return PTR_ERR(afe->fields[i]);
> +		}
> +	}
> +
>  	afe->regulator = devm_regulator_get(afe->dev, "tx_sup");
>  	if (IS_ERR(afe->regulator)) {
>  		dev_err(afe->dev, "Unable to get regulator\n");
> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
> index 7806a45..0d1af4a 100644
> --- a/drivers/iio/health/afe4404.c
> +++ b/drivers/iio/health/afe4404.c
> @@ -48,43 +48,9 @@
>  #define AFE4404_AVG_LED2_ALED2VAL	0x3f
>  #define AFE4404_AVG_LED1_ALED1VAL	0x40
>  
> -/* AFE4404 GAIN register fields */
> -#define AFE4404_TIA_GAIN_RES_MASK	GENMASK(2, 0)
> -#define AFE4404_TIA_GAIN_RES_SHIFT	0
> -#define AFE4404_TIA_GAIN_CAP_MASK	GENMASK(5, 3)
> -#define AFE4404_TIA_GAIN_CAP_SHIFT	3
> -
> -/* AFE4404 LEDCNTRL register fields */
> -#define AFE4404_LEDCNTRL_ILED1_MASK	GENMASK(5, 0)
> -#define AFE4404_LEDCNTRL_ILED1_SHIFT	0
> -#define AFE4404_LEDCNTRL_ILED2_MASK	GENMASK(11, 6)
> -#define AFE4404_LEDCNTRL_ILED2_SHIFT	6
> -#define AFE4404_LEDCNTRL_ILED3_MASK	GENMASK(17, 12)
> -#define AFE4404_LEDCNTRL_ILED3_SHIFT	12
> -
> -/* AFE4404 CONTROL2 register fields */
> -#define AFE440X_CONTROL2_ILED_2X_MASK	BIT(17)
> -#define AFE440X_CONTROL2_ILED_2X_SHIFT	17
> -
>  /* AFE4404 CONTROL3 register fields */
>  #define AFE440X_CONTROL3_OSC_ENABLE	BIT(9)
>  
> -/* AFE4404 OFFDAC register current fields */
> -#define AFE4404_OFFDAC_CURR_LED1_MASK	GENMASK(9, 5)
> -#define AFE4404_OFFDAC_CURR_LED1_SHIFT	5
> -#define AFE4404_OFFDAC_CURR_LED2_MASK	GENMASK(19, 15)
> -#define AFE4404_OFFDAC_CURR_LED2_SHIFT	15
> -#define AFE4404_OFFDAC_CURR_LED3_MASK	GENMASK(4, 0)
> -#define AFE4404_OFFDAC_CURR_LED3_SHIFT	0
> -#define AFE4404_OFFDAC_CURR_ALED1_MASK	GENMASK(14, 10)
> -#define AFE4404_OFFDAC_CURR_ALED1_SHIFT	10
> -#define AFE4404_OFFDAC_CURR_ALED2_MASK	GENMASK(4, 0)
> -#define AFE4404_OFFDAC_CURR_ALED2_SHIFT	0
> -
> -/* AFE4404 NULL fields */
> -#define NULL_MASK	0
> -#define NULL_SHIFT	0
> -
>  /* AFE4404 TIA_GAIN_CAP values */
>  #define AFE4404_TIA_GAIN_CAP_5_P	0x0
>  #define AFE4404_TIA_GAIN_CAP_2_5_P	0x1
> @@ -105,10 +71,43 @@
>  #define AFE4404_TIA_GAIN_RES_1_M	0x6
>  #define AFE4404_TIA_GAIN_RES_2_M	0x7
>  
> +enum afe4404_fields {
> +	/* Gains */
> +	F_TIA_GAIN_SEP, F_TIA_CF_SEP,
> +	F_TIA_GAIN, TIA_CF,
> +
> +	/* LED Current */
> +	F_ILED1, F_ILED2, F_ILED3,
> +
> +	/* Offset DAC */
> +	F_OFFDAC_AMB2, F_OFFDAC_LED1, F_OFFDAC_AMB1, F_OFFDAC_LED2,
> +
> +	/* sentinel */
> +	F_MAX_FIELDS
> +};
> +
> +static const struct reg_field afe4404_reg_fields[] = {
> +	/* Gains */
> +	[F_TIA_GAIN_SEP]	= REG_FIELD(AFE4404_TIA_GAIN_SEP, 0, 2),
> +	[F_TIA_CF_SEP]		= REG_FIELD(AFE4404_TIA_GAIN_SEP, 3, 5),
> +	[F_TIA_GAIN]		= REG_FIELD(AFE4404_TIA_GAIN, 0, 2),
> +	[TIA_CF]		= REG_FIELD(AFE4404_TIA_GAIN, 3, 5),
> +	/* LED Current */
> +	[F_ILED1]		= REG_FIELD(AFE440X_LEDCNTRL, 0, 5),
> +	[F_ILED2]		= REG_FIELD(AFE440X_LEDCNTRL, 6, 11),
> +	[F_ILED3]		= REG_FIELD(AFE440X_LEDCNTRL, 12, 17),
> +	/* Offset DAC */
> +	[F_OFFDAC_AMB2]		= REG_FIELD(AFE4404_OFFDAC, 0, 4),
> +	[F_OFFDAC_LED1]		= REG_FIELD(AFE4404_OFFDAC, 5, 9),
> +	[F_OFFDAC_AMB1]		= REG_FIELD(AFE4404_OFFDAC, 10, 14),
> +	[F_OFFDAC_LED2]		= REG_FIELD(AFE4404_OFFDAC, 15, 19),
> +};
> +
>  /**
>   * struct afe4404_data - AFE4404 device instance data
>   * @dev: Device structure
>   * @regmap: Register map of the device
> + * @fields: Register fields of the device
>   * @regulator: Pointer to the regulator for the IC
>   * @trig: IIO trigger for this device
>   * @irq: ADC_RDY line interrupt number
> @@ -116,6 +115,7 @@
>  struct afe4404_data {
>  	struct device *dev;
>  	struct regmap *regmap;
> +	struct regmap_field *fields[F_MAX_FIELDS];
>  	struct regulator *regulator;
>  	struct iio_trigger *trig;
>  	int irq;
> @@ -133,16 +133,26 @@ enum afe4404_chan_id {
>  	ILED3,
>  };
>  
> -static const struct afe440x_reg_info afe4404_reg_info[] = {
> -	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED2),
> -	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED2),
> -	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED1),
> -	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED1),
> -	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
> -	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
> -	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED1),
> -	[ILED2] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED2),
> -	[ILED3] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED3),
> +static const unsigned int afe4404_channel_values[] = {
> +	[LED2] = AFE440X_LED2VAL,
> +	[ALED2] = AFE440X_ALED2VAL,
> +	[LED1] = AFE440X_LED1VAL,
> +	[ALED1] = AFE440X_ALED1VAL,
> +	[LED2_ALED2] = AFE440X_LED2_ALED2VAL,
> +	[LED1_ALED1] = AFE440X_LED1_ALED1VAL,
> +};
> +
> +static const unsigned int afe4404_channel_leds[] = {
> +	[ILED1] = F_ILED1,
> +	[ILED2] = F_ILED2,
> +	[ILED3] = F_ILED3,
> +};
> +
> +static const unsigned int afe4404_channel_offdacs[] = {
> +	[LED2] = F_OFFDAC_LED2,
> +	[ALED2] = F_OFFDAC_AMB2,
> +	[LED1] = F_OFFDAC_LED1,
> +	[ALED1] = F_OFFDAC_AMB1,
>  };
>  
>  static const struct iio_chan_spec afe4404_channels[] = {
> @@ -194,13 +204,10 @@ static ssize_t afe440x_show_register(struct device *dev,
>  	int vals[2];
>  	int ret;
>  
> -	ret = regmap_read(afe->regmap, afe440x_attr->reg, &reg_val);
> +	ret = regmap_field_read(afe->fields[afe440x_attr->field], &reg_val);
>  	if (ret)
>  		return ret;
>  
> -	reg_val &= afe440x_attr->mask;
> -	reg_val >>= afe440x_attr->shift;
> -
>  	if (reg_val >= afe440x_attr->table_size)
>  		return -EINVAL;
>  
> @@ -230,20 +237,18 @@ static ssize_t afe440x_store_register(struct device *dev,
>  	if (val == afe440x_attr->table_size)
>  		return -EINVAL;
>  
> -	ret = regmap_update_bits(afe->regmap, afe440x_attr->reg,
> -				 afe440x_attr->mask,
> -				 (val << afe440x_attr->shift));
> +	ret = regmap_field_write(afe->fields[afe440x_attr->field], val);
>  	if (ret)
>  		return ret;
>  
>  	return count;
>  }
>  
> -static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, afe4404_res_table);
> -static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, afe4404_cap_table);
> +static AFE440X_ATTR(tia_resistance1, F_TIA_GAIN, afe4404_res_table);
> +static AFE440X_ATTR(tia_capacitance1, TIA_CF, afe4404_cap_table);
>  
> -static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, afe4404_res_table);
> -static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, afe4404_cap_table);
> +static AFE440X_ATTR(tia_resistance2, F_TIA_GAIN_SEP, afe4404_res_table);
> +static AFE440X_ATTR(tia_capacitance2, F_TIA_CF_SEP, afe4404_cap_table);
>  
>  static struct attribute *afe440x_attributes[] = {
>  	&afe440x_attr_tia_resistance1.dev_attr.attr,
> @@ -264,35 +269,32 @@ static int afe4404_read_raw(struct iio_dev *indio_dev,
>  			    int *val, int *val2, long mask)
>  {
>  	struct afe4404_data *afe = iio_priv(indio_dev);
> -	const struct afe440x_reg_info reg_info = afe4404_reg_info[chan->address];
> +	unsigned int value_reg = afe4404_channel_values[chan->address];
> +	unsigned int led_field = afe4404_channel_leds[chan->address];
> +	unsigned int offdac_field = afe4404_channel_offdacs[chan->address];
>  	int ret;
>  
>  	switch (chan->type) {
>  	case IIO_INTENSITY:
>  		switch (mask) {
>  		case IIO_CHAN_INFO_RAW:
> -			ret = regmap_read(afe->regmap, reg_info.reg, val);
> +			ret = regmap_read(afe->regmap, value_reg, val);
>  			if (ret)
>  				return ret;
>  			return IIO_VAL_INT;
>  		case IIO_CHAN_INFO_OFFSET:
> -			ret = regmap_read(afe->regmap, reg_info.offreg,
> -					  val);
> +			ret = regmap_field_read(afe->fields[offdac_field], val);
>  			if (ret)
>  				return ret;
> -			*val &= reg_info.mask;
> -			*val >>= reg_info.shift;
>  			return IIO_VAL_INT;
>  		}
>  		break;
>  	case IIO_CURRENT:
>  		switch (mask) {
>  		case IIO_CHAN_INFO_RAW:
> -			ret = regmap_read(afe->regmap, reg_info.reg, val);
> +			ret = regmap_field_read(afe->fields[led_field], val);
>  			if (ret)
>  				return ret;
> -			*val &= reg_info.mask;
> -			*val >>= reg_info.shift;
>  			return IIO_VAL_INT;
>  		case IIO_CHAN_INFO_SCALE:
>  			*val = 0;
> @@ -312,25 +314,20 @@ static int afe4404_write_raw(struct iio_dev *indio_dev,
>  			     int val, int val2, long mask)
>  {
>  	struct afe4404_data *afe = iio_priv(indio_dev);
> -	const struct afe440x_reg_info reg_info = afe4404_reg_info[chan->address];
> +	unsigned int led_field = afe4404_channel_leds[chan->address];
> +	unsigned int offdac_field = afe4404_channel_offdacs[chan->address];
>  
>  	switch (chan->type) {
>  	case IIO_INTENSITY:
>  		switch (mask) {
>  		case IIO_CHAN_INFO_OFFSET:
> -			return regmap_update_bits(afe->regmap,
> -				reg_info.offreg,
> -				reg_info.mask,
> -				(val << reg_info.shift));
> +			return regmap_field_write(afe->fields[offdac_field], val);
>  		}
>  		break;
>  	case IIO_CURRENT:
>  		switch (mask) {
>  		case IIO_CHAN_INFO_RAW:
> -			return regmap_update_bits(afe->regmap,
> -				reg_info.reg,
> -				reg_info.mask,
> -				(val << reg_info.shift));
> +			return regmap_field_write(afe->fields[led_field], val);
>  		}
>  		break;
>  	default:
> @@ -357,7 +354,7 @@ static irqreturn_t afe4404_trigger_handler(int irq, void *private)
>  
>  	for_each_set_bit(bit, indio_dev->active_scan_mask,
>  			 indio_dev->masklength) {
> -		ret = regmap_read(afe->regmap, afe4404_reg_info[bit].reg,
> +		ret = regmap_read(afe->regmap, afe4404_channel_values[bit],
>  				  &buffer[i++]);
>  		if (ret)
>  			goto err;
> @@ -490,7 +487,7 @@ static int afe4404_probe(struct i2c_client *client,
>  {
>  	struct iio_dev *indio_dev;
>  	struct afe4404_data *afe;
> -	int ret;
> +	int i, ret;
>  
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*afe));
>  	if (!indio_dev)
> @@ -508,6 +505,15 @@ static int afe4404_probe(struct i2c_client *client,
>  		return PTR_ERR(afe->regmap);
>  	}
>  
> +	for (i = 0; i < F_MAX_FIELDS; i++) {
> +		afe->fields[i] = devm_regmap_field_alloc(afe->dev, afe->regmap,
> +							 afe4404_reg_fields[i]);
> +		if (IS_ERR(afe->fields[i])) {
> +			dev_err(afe->dev, "Unable to allocate regmap fields\n");
> +			return PTR_ERR(afe->fields[i]);
> +		}
> +	}
> +
>  	afe->regulator = devm_regulator_get(afe->dev, "tx_sup");
>  	if (IS_ERR(afe->regulator)) {
>  		dev_err(afe->dev, "Unable to get regulator\n");
> diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h
> index 713972f..1a0f247 100644
> --- a/drivers/iio/health/afe440x.h
> +++ b/drivers/iio/health/afe440x.h
> @@ -88,21 +88,6 @@
>  #define AFE440X_CONTROL0_WRITE		0x0
>  #define AFE440X_CONTROL0_READ		0x1
>  
> -struct afe440x_reg_info {
> -	unsigned int reg;
> -	unsigned int offreg;
> -	unsigned int shift;
> -	unsigned int mask;
> -};
> -
> -#define AFE440X_REG_INFO(_reg, _offreg, _sm)			\
> -	{							\
> -		.reg = _reg,					\
> -		.offreg = _offreg,				\
> -		.shift = _sm ## _SHIFT,				\
> -		.mask = _sm ## _MASK,				\
> -	}
> -
>  #define AFE440X_INTENSITY_CHAN(_index, _mask)			\
>  	{							\
>  		.type = IIO_INTENSITY,				\
> @@ -157,9 +142,7 @@ static DEVICE_ATTR_RO(_name)
>  
>  struct afe440x_attr {
>  	struct device_attribute dev_attr;
> -	unsigned int reg;
> -	unsigned int shift;
> -	unsigned int mask;
> +	unsigned int field;
>  	const struct afe440x_val_table *val_table;
>  	unsigned int table_size;
>  };
> @@ -167,14 +150,12 @@ struct afe440x_attr {
>  #define to_afe440x_attr(_dev_attr)				\
>  	container_of(_dev_attr, struct afe440x_attr, dev_attr)
>  
> -#define AFE440X_ATTR(_name, _reg, _field, _table)		\
> +#define AFE440X_ATTR(_name, _field, _table)			\
>  	struct afe440x_attr afe440x_attr_##_name = {		\
>  		.dev_attr = __ATTR(_name, (S_IRUGO | S_IWUSR),	\
>  				   afe440x_show_register,	\
>  				   afe440x_store_register),	\
> -		.reg = _reg,					\
> -		.shift = _field ## _SHIFT,			\
> -		.mask = _field ## _MASK,			\
> +		.field = _field,				\
>  		.val_table = _table,				\
>  		.table_size = ARRAY_SIZE(_table),		\
>  	}
> 

Re: [PATCH 10/13] iio: health/afe440x: Make gain settings a modifier for the stages

[Jonathan Cameron]

On 01/05/16 21:37, Andrew F. Davis wrote:
> Currently the TIA gain settings are exported to userspace as sysfs
> entries that do not clearly represent their internal relation to the
> sampling stages. The gain settings are enabled on a per-stage basis,
> this can be seen in figure 24 of the current AFE4404 datasheet.
> These gain settings should therefore be tied to the channels that are
> read during these stages. Make this change here.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
I'd not understood this right originally.  Good to clear this up!

Applied.  I'll add a reply to the cover letter later explaining why
I am risking these ABI changes, whilst bouncing back much smaller ones.

Jonathan
> ---
>  .../ABI/testing/sysfs-bus-iio-health-afe440x       | 19 ++++++------
>  drivers/iio/health/afe4403.c                       | 34 ++++++++++++++--------
>  drivers/iio/health/afe4404.c                       | 34 ++++++++++++++--------
>  3 files changed, 53 insertions(+), 34 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
> index a067073..6adba90 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
> @@ -1,13 +1,3 @@
> -What:		/sys/bus/iio/devices/iio:deviceX/tia_resistanceY
> -		/sys/bus/iio/devices/iio:deviceX/tia_capacitanceY
> -Date:		December 2015
> -KernelVersion:
> -Contact:	Andrew F. Davis <afd@ti.com>
> -Description:
> -		Get and set the resistance and the capacitance settings for the
> -		Transimpedance Amplifier. Y is 1 for Rf1 and Cf1, Y is 2 for
> -		Rf2 and Cf2 values.
> -
>  What:		/sys/bus/iio/devices/iio:deviceX/in_intensityY_raw
>  Date:		May 2016
>  KernelVersion:
> @@ -33,6 +23,15 @@ Description:
>  		Get and set the offset cancellation DAC setting for these
>  		stages. The values are expressed in 5-bit sign-magnitude.
>  
> +What:		/sys/bus/iio/devices/iio:deviceX/in_intensityY_resistance
> +What:		/sys/bus/iio/devices/iio:deviceX/in_intensityY_capacitance
> +Date:		May 2016
> +KernelVersion:
> +Contact:	Andrew F. Davis <afd@ti.com>
> +Description:
> +		Get and set the resistance and the capacitance settings for the
> +		Transimpedance Amplifier during the associated stage.
> +
>  What:		/sys/bus/iio/devices/iio:deviceX/out_currentY_raw
>  Date:		May 2016
>  KernelVersion:
> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
> index 1950155..610631b 100644
> --- a/drivers/iio/health/afe4403.c
> +++ b/drivers/iio/health/afe4403.c
> @@ -161,7 +161,7 @@ static const struct afe440x_val_table afe4403_res_table[] = {
>  	{ 500000 }, { 250000 }, { 100000 }, { 50000 },
>  	{ 25000 }, { 10000 }, { 1000000 }, { 0 },
>  };
> -AFE440X_TABLE_ATTR(tia_resistance_available, afe4403_res_table);
> +AFE440X_TABLE_ATTR(in_intensity_resistance_available, afe4403_res_table);
>  
>  static const struct afe440x_val_table afe4403_cap_table[] = {
>  	{ 0, 5000 }, { 0, 10000 }, { 0, 20000 }, { 0, 25000 },
> @@ -173,7 +173,7 @@ static const struct afe440x_val_table afe4403_cap_table[] = {
>  	{ 0, 205000 }, { 0, 210000 }, { 0, 220000 }, { 0, 225000 },
>  	{ 0, 230000 }, { 0, 235000 }, { 0, 245000 }, { 0, 250000 },
>  };
> -AFE440X_TABLE_ATTR(tia_capacitance_available, afe4403_cap_table);
> +AFE440X_TABLE_ATTR(in_intensity_capacitance_available, afe4403_cap_table);
>  
>  static ssize_t afe440x_show_register(struct device *dev,
>  				     struct device_attribute *attr,
> @@ -226,19 +226,29 @@ static ssize_t afe440x_store_register(struct device *dev,
>  	return count;
>  }
>  
> -static AFE440X_ATTR(tia_resistance1, F_RF_LED1, afe4403_res_table);
> -static AFE440X_ATTR(tia_capacitance1, F_CF_LED1, afe4403_cap_table);
> +static AFE440X_ATTR(in_intensity1_resistance, F_RF_LED, afe4403_res_table);
> +static AFE440X_ATTR(in_intensity1_capacitance, F_CF_LED, afe4403_cap_table);
>  
> -static AFE440X_ATTR(tia_resistance2, F_RF_LED, afe4403_res_table);
> -static AFE440X_ATTR(tia_capacitance2, F_CF_LED, afe4403_cap_table);
> +static AFE440X_ATTR(in_intensity2_resistance, F_RF_LED, afe4403_res_table);
> +static AFE440X_ATTR(in_intensity2_capacitance, F_CF_LED, afe4403_cap_table);
> +
> +static AFE440X_ATTR(in_intensity3_resistance, F_RF_LED1, afe4403_res_table);
> +static AFE440X_ATTR(in_intensity3_capacitance, F_CF_LED1, afe4403_cap_table);
> +
> +static AFE440X_ATTR(in_intensity4_resistance, F_RF_LED1, afe4403_res_table);
> +static AFE440X_ATTR(in_intensity4_capacitance, F_CF_LED1, afe4403_cap_table);
>  
>  static struct attribute *afe440x_attributes[] = {
> -	&afe440x_attr_tia_resistance1.dev_attr.attr,
> -	&afe440x_attr_tia_capacitance1.dev_attr.attr,
> -	&afe440x_attr_tia_resistance2.dev_attr.attr,
> -	&afe440x_attr_tia_capacitance2.dev_attr.attr,
> -	&dev_attr_tia_resistance_available.attr,
> -	&dev_attr_tia_capacitance_available.attr,
> +	&dev_attr_in_intensity_resistance_available.attr,
> +	&dev_attr_in_intensity_capacitance_available.attr,
> +	&afe440x_attr_in_intensity1_resistance.dev_attr.attr,
> +	&afe440x_attr_in_intensity1_capacitance.dev_attr.attr,
> +	&afe440x_attr_in_intensity2_resistance.dev_attr.attr,
> +	&afe440x_attr_in_intensity2_capacitance.dev_attr.attr,
> +	&afe440x_attr_in_intensity3_resistance.dev_attr.attr,
> +	&afe440x_attr_in_intensity3_capacitance.dev_attr.attr,
> +	&afe440x_attr_in_intensity4_resistance.dev_attr.attr,
> +	&afe440x_attr_in_intensity4_capacitance.dev_attr.attr,
>  	NULL
>  };
>  
> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
> index 0d1af4a..69116cd 100644
> --- a/drivers/iio/health/afe4404.c
> +++ b/drivers/iio/health/afe4404.c
> @@ -179,7 +179,7 @@ static const struct afe440x_val_table afe4404_res_table[] = {
>  	{ .integer = 1000000, .fract = 0 },
>  	{ .integer = 2000000, .fract = 0 },
>  };
> -AFE440X_TABLE_ATTR(tia_resistance_available, afe4404_res_table);
> +AFE440X_TABLE_ATTR(in_intensity_resistance_available, afe4404_res_table);
>  
>  static const struct afe440x_val_table afe4404_cap_table[] = {
>  	{ .integer = 0, .fract = 5000 },
> @@ -191,7 +191,7 @@ static const struct afe440x_val_table afe4404_cap_table[] = {
>  	{ .integer = 0, .fract = 25000 },
>  	{ .integer = 0, .fract = 22500 },
>  };
> -AFE440X_TABLE_ATTR(tia_capacitance_available, afe4404_cap_table);
> +AFE440X_TABLE_ATTR(in_intensity_capacitance_available, afe4404_cap_table);
>  
>  static ssize_t afe440x_show_register(struct device *dev,
>  				     struct device_attribute *attr,
> @@ -244,19 +244,29 @@ static ssize_t afe440x_store_register(struct device *dev,
>  	return count;
>  }
>  
> -static AFE440X_ATTR(tia_resistance1, F_TIA_GAIN, afe4404_res_table);
> -static AFE440X_ATTR(tia_capacitance1, TIA_CF, afe4404_cap_table);
> +static AFE440X_ATTR(in_intensity1_resistance, F_TIA_GAIN_SEP, afe4404_res_table);
> +static AFE440X_ATTR(in_intensity1_capacitance, F_TIA_CF_SEP, afe4404_cap_table);
>  
> -static AFE440X_ATTR(tia_resistance2, F_TIA_GAIN_SEP, afe4404_res_table);
> -static AFE440X_ATTR(tia_capacitance2, F_TIA_CF_SEP, afe4404_cap_table);
> +static AFE440X_ATTR(in_intensity2_resistance, F_TIA_GAIN_SEP, afe4404_res_table);
> +static AFE440X_ATTR(in_intensity2_capacitance, F_TIA_CF_SEP, afe4404_cap_table);
> +
> +static AFE440X_ATTR(in_intensity3_resistance, F_TIA_GAIN, afe4404_res_table);
> +static AFE440X_ATTR(in_intensity3_capacitance, TIA_CF, afe4404_cap_table);
> +
> +static AFE440X_ATTR(in_intensity4_resistance, F_TIA_GAIN, afe4404_res_table);
> +static AFE440X_ATTR(in_intensity4_capacitance, TIA_CF, afe4404_cap_table);
>  
>  static struct attribute *afe440x_attributes[] = {
> -	&afe440x_attr_tia_resistance1.dev_attr.attr,
> -	&afe440x_attr_tia_capacitance1.dev_attr.attr,
> -	&afe440x_attr_tia_resistance2.dev_attr.attr,
> -	&afe440x_attr_tia_capacitance2.dev_attr.attr,
> -	&dev_attr_tia_resistance_available.attr,
> -	&dev_attr_tia_capacitance_available.attr,
> +	&dev_attr_in_intensity_resistance_available.attr,
> +	&dev_attr_in_intensity_capacitance_available.attr,
> +	&afe440x_attr_in_intensity1_resistance.dev_attr.attr,
> +	&afe440x_attr_in_intensity1_capacitance.dev_attr.attr,
> +	&afe440x_attr_in_intensity2_resistance.dev_attr.attr,
> +	&afe440x_attr_in_intensity2_capacitance.dev_attr.attr,
> +	&afe440x_attr_in_intensity3_resistance.dev_attr.attr,
> +	&afe440x_attr_in_intensity3_capacitance.dev_attr.attr,
> +	&afe440x_attr_in_intensity4_resistance.dev_attr.attr,
> +	&afe440x_attr_in_intensity4_capacitance.dev_attr.attr,
>  	NULL
>  };
>  
> 

Re: [PATCH 11/13] iio: health/afe440x: Match LED currents to stages

[Jonathan Cameron]

On 01/05/16 21:37, Andrew F. Davis wrote:
> The current channel number for the LEDs should match the stage
> number that they are active during, fix this here.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
Applied.
> ---
>  drivers/iio/health/afe4403.c | 10 ++++------
>  drivers/iio/health/afe4404.c | 15 ++++++---------
>  2 files changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
> index 610631b..059d521 100644
> --- a/drivers/iio/health/afe4403.c
> +++ b/drivers/iio/health/afe4403.c
> @@ -126,8 +126,6 @@ enum afe4403_chan_id {
>  	ALED1,
>  	LED2_ALED2,
>  	LED1_ALED1,
> -	ILED1,
> -	ILED2,
>  };
>  
>  static const unsigned int afe4403_channel_values[] = {
> @@ -140,8 +138,8 @@ static const unsigned int afe4403_channel_values[] = {
>  };
>  
>  static const unsigned int afe4403_channel_leds[] = {
> -	[ILED1] = F_ILED1,
> -	[ILED2] = F_ILED2,
> +	[LED2] = F_ILED2,
> +	[LED1] = F_ILED1,
>  };
>  
>  static const struct iio_chan_spec afe4403_channels[] = {
> @@ -153,8 +151,8 @@ static const struct iio_chan_spec afe4403_channels[] = {
>  	AFE440X_INTENSITY_CHAN(LED2_ALED2, 0),
>  	AFE440X_INTENSITY_CHAN(LED1_ALED1, 0),
>  	/* LED current */
> -	AFE440X_CURRENT_CHAN(ILED1),
> -	AFE440X_CURRENT_CHAN(ILED2),
> +	AFE440X_CURRENT_CHAN(LED2),
> +	AFE440X_CURRENT_CHAN(LED1),
>  };
>  
>  static const struct afe440x_val_table afe4403_res_table[] = {
> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
> index 69116cd..aa8770b 100644
> --- a/drivers/iio/health/afe4404.c
> +++ b/drivers/iio/health/afe4404.c
> @@ -128,9 +128,6 @@ enum afe4404_chan_id {
>  	ALED1,
>  	LED2_ALED2,
>  	LED1_ALED1,
> -	ILED1,
> -	ILED2,
> -	ILED3,
>  };
>  
>  static const unsigned int afe4404_channel_values[] = {
> @@ -143,9 +140,9 @@ static const unsigned int afe4404_channel_values[] = {
>  };
>  
>  static const unsigned int afe4404_channel_leds[] = {
> -	[ILED1] = F_ILED1,
> -	[ILED2] = F_ILED2,
> -	[ILED3] = F_ILED3,
> +	[LED2] = F_ILED2,
> +	[ALED2] = F_ILED3,
> +	[LED1] = F_ILED1,
>  };
>  
>  static const unsigned int afe4404_channel_offdacs[] = {
> @@ -164,9 +161,9 @@ static const struct iio_chan_spec afe4404_channels[] = {
>  	AFE440X_INTENSITY_CHAN(LED2_ALED2, 0),
>  	AFE440X_INTENSITY_CHAN(LED1_ALED1, 0),
>  	/* LED current */
> -	AFE440X_CURRENT_CHAN(ILED1),
> -	AFE440X_CURRENT_CHAN(ILED2),
> -	AFE440X_CURRENT_CHAN(ILED3),
> +	AFE440X_CURRENT_CHAN(LED2),
> +	AFE440X_CURRENT_CHAN(ALED2),
> +	AFE440X_CURRENT_CHAN(LED1),
>  };
>  
>  static const struct afe440x_val_table afe4404_res_table[] = {
> 

Re: [PATCH 12/13] iio: health/afe440x: Remove unused definitions

[Jonathan Cameron]

On 01/05/16 21:37, Andrew F. Davis wrote:
> These definitions are not currently used and if the functionality
> they represent is needed the values should be added back to a table
> for easy userspace use.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
Applied.
> ---
>  drivers/iio/health/afe4403.c | 37 -------------------------------------
>  drivers/iio/health/afe4404.c | 20 --------------------
>  2 files changed, 57 deletions(-)
> 
> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
> index 059d521..9a08146 100644
> --- a/drivers/iio/health/afe4403.c
> +++ b/drivers/iio/health/afe4403.c
> @@ -39,43 +39,6 @@
>  #define AFE4403_TIAGAIN			0x20
>  #define AFE4403_TIA_AMB_GAIN		0x21
>  
> -/* AFE4403 LEDCNTRL values */
> -#define AFE440X_LEDCNTRL_RANGE_TX_HALF	0x1
> -#define AFE440X_LEDCNTRL_RANGE_TX_FULL	0x2
> -#define AFE440X_LEDCNTRL_RANGE_TX_OFF	0x3
> -
> -/* AFE4403 CONTROL2 values */
> -#define AFE440X_CONTROL2_TX_REF_025	0x0
> -#define AFE440X_CONTROL2_TX_REF_050	0x1
> -#define AFE440X_CONTROL2_TX_REF_100	0x2
> -#define AFE440X_CONTROL2_TX_REF_075	0x3
> -
> -/* AFE4403 CONTROL3 values */
> -#define AFE440X_CONTROL3_CLK_DIV_2	0x0
> -#define AFE440X_CONTROL3_CLK_DIV_4	0x2
> -#define AFE440X_CONTROL3_CLK_DIV_6	0x3
> -#define AFE440X_CONTROL3_CLK_DIV_8	0x4
> -#define AFE440X_CONTROL3_CLK_DIV_12	0x5
> -#define AFE440X_CONTROL3_CLK_DIV_1	0x7
> -
> -/* AFE4403 TIAGAIN_CAP values */
> -#define AFE4403_TIAGAIN_CAP_5_P		0x0
> -#define AFE4403_TIAGAIN_CAP_10_P	0x1
> -#define AFE4403_TIAGAIN_CAP_20_P	0x2
> -#define AFE4403_TIAGAIN_CAP_30_P	0x3
> -#define AFE4403_TIAGAIN_CAP_55_P	0x8
> -#define AFE4403_TIAGAIN_CAP_155_P	0x10
> -
> -/* AFE4403 TIAGAIN_RES values */
> -#define AFE4403_TIAGAIN_RES_500_K	0x0
> -#define AFE4403_TIAGAIN_RES_250_K	0x1
> -#define AFE4403_TIAGAIN_RES_100_K	0x2
> -#define AFE4403_TIAGAIN_RES_50_K	0x3
> -#define AFE4403_TIAGAIN_RES_25_K	0x4
> -#define AFE4403_TIAGAIN_RES_10_K	0x5
> -#define AFE4403_TIAGAIN_RES_1_M		0x6
> -#define AFE4403_TIAGAIN_RES_NONE	0x7
> -
>  enum afe4403_fields {
>  	/* Gains */
>  	F_RF_LED1, F_CF_LED1,
> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
> index aa8770b..3a8131d 100644
> --- a/drivers/iio/health/afe4404.c
> +++ b/drivers/iio/health/afe4404.c
> @@ -51,26 +51,6 @@
>  /* AFE4404 CONTROL3 register fields */
>  #define AFE440X_CONTROL3_OSC_ENABLE	BIT(9)
>  
> -/* AFE4404 TIA_GAIN_CAP values */
> -#define AFE4404_TIA_GAIN_CAP_5_P	0x0
> -#define AFE4404_TIA_GAIN_CAP_2_5_P	0x1
> -#define AFE4404_TIA_GAIN_CAP_10_P	0x2
> -#define AFE4404_TIA_GAIN_CAP_7_5_P	0x3
> -#define AFE4404_TIA_GAIN_CAP_20_P	0x4
> -#define AFE4404_TIA_GAIN_CAP_17_5_P	0x5
> -#define AFE4404_TIA_GAIN_CAP_25_P	0x6
> -#define AFE4404_TIA_GAIN_CAP_22_5_P	0x7
> -
> -/* AFE4404 TIA_GAIN_RES values */
> -#define AFE4404_TIA_GAIN_RES_500_K	0x0
> -#define AFE4404_TIA_GAIN_RES_250_K	0x1
> -#define AFE4404_TIA_GAIN_RES_100_K	0x2
> -#define AFE4404_TIA_GAIN_RES_50_K	0x3
> -#define AFE4404_TIA_GAIN_RES_25_K	0x4
> -#define AFE4404_TIA_GAIN_RES_10_K	0x5
> -#define AFE4404_TIA_GAIN_RES_1_M	0x6
> -#define AFE4404_TIA_GAIN_RES_2_M	0x7
> -
>  enum afe4404_fields {
>  	/* Gains */
>  	F_TIA_GAIN_SEP, F_TIA_CF_SEP,
> 

Re: [PATCH 13/13] iio: health/afe4404: ENSEPGAIN is part of CONTROL2 register

[Jonathan Cameron]

On 01/05/16 21:37, Andrew F. Davis wrote:
> Rename this definition, no functional changes.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
Applied.
> ---
>  drivers/iio/health/afe4404.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
> index 3a8131d..4526640 100644
> --- a/drivers/iio/health/afe4404.c
> +++ b/drivers/iio/health/afe4404.c
> @@ -48,8 +48,8 @@
>  #define AFE4404_AVG_LED2_ALED2VAL	0x3f
>  #define AFE4404_AVG_LED1_ALED1VAL	0x40
>  
> -/* AFE4404 CONTROL3 register fields */
> -#define AFE440X_CONTROL3_OSC_ENABLE	BIT(9)
> +/* AFE4404 CONTROL2 register fields */
> +#define AFE440X_CONTROL2_OSC_ENABLE	BIT(9)
>  
>  enum afe4404_fields {
>  	/* Gains */
> @@ -398,7 +398,7 @@ static const struct reg_sequence afe4404_reg_sequences[] = {
>  	AFE4404_TIMING_PAIRS,
>  	{ AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN },
>  	{ AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN },
> -	{ AFE440X_CONTROL2, AFE440X_CONTROL3_OSC_ENABLE	},
> +	{ AFE440X_CONTROL2, AFE440X_CONTROL2_OSC_ENABLE	},
>  };
>  
>  static const struct regmap_range afe4404_yes_ranges[] = {
> 

Re: [PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405

[Jonathan Cameron]

On 01/05/16 21:36, Andrew F. Davis wrote:
> Hello all,
> 
> I will be posting a driver for the AFE4405 soon and in preparation
> for this I have made some changes to the existing drivers to better
> align them with the new part. Some of these changes are trivial, others
> change the sysfs entries, I understand these are considered ABIs and
> changes to even testing ABIs should not be made lightly, but my hope is
> that these changes will more accurately reflect the devices intended use
> and improve functionality as the first users begin to appear.
> 
> Thanks,
> Andrew

Hi Andrew,

Just a quick not to explain my reasoning for taking these which involve
some pretty major ABI surgery, whilst bouncing back much more trivial
changes.

Ultimately, whether we can change ABI comes down to 1 simple question.
'Will anyone notice and if so will it be an issue for them?'
We must not break userspace.

For the vast majority of IIO drivers, only simple tools along the lines
of generic_buffer or just sysfs file reads are all that is needed to
make 'use' of the data.  Obviously lots of users will go through one
of the more advanced libraries, but these are still mostly generic.

We had a long debate through numerous versions when Andrew was
originally submitting these two drivers to try and pin down a vaguely
standard ABI for them.  The result was indeed 'Vaguely' standard.
We moved in a good direction but sometimes a device is just too
specialized (unfortunately).

The very nature of these devices is that the data is only meaningful
(i.e. measuring what they are supposedly for) once a non trivial
amount of computation has been performed in userspace. Now, there
probably is enough information available to do pulse oximetry using
one of these devices, but it is certainly non trivial.

As such the interface is hopefully tightly coupled to the userspace
tools that TI provide allowing us to make changes without it
adversely effecting anyone.  As time moves on this may well not be
the case.  If anyone knows that there is existing non TI code out
there for this and we will be causing pain, let me know ASAP so that
we can work out a way forward.

So really it's a case of an educated guess that no one cares and
crossing fingers!  I think the improvements made here are worth
the risk.

Jonathan
> 
> Andrew F. Davis (13):
>   iio: health/afe440x: Fix kernel-doc format
>   iio: health/afe440x: Remove of_match_ptr and ifdefs
>   iio: health/afe440x: Remove unneeded initializers
>   iio: health/afe440x: Always use separate gain values
>   iio: health/afe440x: Fix scan_index assignment
>   iio: health/afe440x: Remove unneeded offset handling
>   iio: health/afe4404: Remove LED3 input channel
>   iio: health/afe440x: Remove channel names
>   iio: health/afe440x: Use regmap fields
>   iio: health/afe440x: Make gain settings a modifier for the stages
>   iio: health/afe440x: Match LED currents to stages
>   iio: health/afe440x: Remove unused definitions
>   iio: health/afe4404: ENSEPGAIN is part of CONTROL2 register
> 
>  .../ABI/testing/sysfs-bus-iio-health-afe440x       |  95 +++----
>  drivers/iio/health/afe4403.c                       | 299 ++++++++------------
>  drivers/iio/health/afe4404.c                       | 308 +++++++++------------
>  drivers/iio/health/afe440x.h                       |  48 +---
>  4 files changed, 295 insertions(+), 455 deletions(-)
>  rewrite Documentation/ABI/testing/sysfs-bus-iio-health-afe440x (72%)
> 

Re: [PATCH 04/13] iio: health/afe440x: Always use separate gain values

[Andrew F. Davis]

On 05/04/2016 05:02 AM, Jonathan Cameron wrote:
> On 01/05/16 21:36, Andrew F. Davis wrote:
>> Locking the two gain stages to the same setting adds no value for us,
>> so initialize them as unlocked and remove the sysfs for unlocking them.
>> This also allows us to greatly simplify showing and setting the gain
>> registers.
>>
>> Signed-off-by: Andrew F. Davis <afd@ti.com>
> Hmm. ABI change but as you said it's an improvement.
> 
> Honestly I doubt anyone is using this device without also using userspace that
> you are providing so lets apply this an cross our fingers that no one minds.
> 

Actually we don't even provide a Linux userspace for these yet, starting
to work on that is what prompted these changes. These parts have
traditionally been paired with a microcontroller to perform the work in
dedicated devices. This was put on my desk as they are hoping to start
enabling these to be integrated into smart devices running Linux
(Android). I'm not an expert in pulse metering, so my initial attempt at
an ABI was a bit off compared to what I now believe to be the "correct"
way to expose these parts.

Thanks again for tolerating all this madness :),
Andrew

> Applied.
>> ---
>>  .../ABI/testing/sysfs-bus-iio-health-afe440x       |  9 ----
>>  drivers/iio/health/afe4403.c                       | 60 ++++++----------------
>>  drivers/iio/health/afe4404.c                       | 60 ++++++----------------
>>  drivers/iio/health/afe440x.h                       | 15 ++----
>>  4 files changed, 37 insertions(+), 107 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
>> index 3740f25..b19053a 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
>> @@ -8,15 +8,6 @@ Description:
>>  		Transimpedance Amplifier. Y is 1 for Rf1 and Cf1, Y is 2 for
>>  		Rf2 and Cf2 values.
>>  
>> -What:		/sys/bus/iio/devices/iio:deviceX/tia_separate_en
>> -Date:		December 2015
>> -KernelVersion:
>> -Contact:	Andrew F. Davis <afd@ti.com>
>> -Description:
>> -		Enable or disable separate settings for the TransImpedance
>> -		Amplifier above, when disabled both values are set by the
>> -		first channel.
>> -
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_raw
>>  		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_ambient_raw
>>  Date:		December 2015
>> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
>> index 5484785..bcff528 100644
>> --- a/drivers/iio/health/afe4403.c
>> +++ b/drivers/iio/health/afe4403.c
>> @@ -180,9 +180,9 @@ static ssize_t afe440x_show_register(struct device *dev,
>>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>  	struct afe4403_data *afe = iio_priv(indio_dev);
>>  	struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr);
>> -	unsigned int reg_val, type;
>> +	unsigned int reg_val;
>>  	int vals[2];
>> -	int ret, val_len;
>> +	int ret;
>>  
>>  	ret = regmap_read(afe->regmap, afe440x_attr->reg, &reg_val);
>>  	if (ret)
>> @@ -191,27 +191,13 @@ static ssize_t afe440x_show_register(struct device *dev,
>>  	reg_val &= afe440x_attr->mask;
>>  	reg_val >>= afe440x_attr->shift;
>>  
>> -	switch (afe440x_attr->type) {
>> -	case SIMPLE:
>> -		type = IIO_VAL_INT;
>> -		val_len = 1;
>> -		vals[0] = reg_val;
>> -		break;
>> -	case RESISTANCE:
>> -	case CAPACITANCE:
>> -		type = IIO_VAL_INT_PLUS_MICRO;
>> -		val_len = 2;
>> -		if (reg_val < afe440x_attr->table_size) {
>> -			vals[0] = afe440x_attr->val_table[reg_val].integer;
>> -			vals[1] = afe440x_attr->val_table[reg_val].fract;
>> -			break;
>> -		}
>> +	if (reg_val >= afe440x_attr->table_size)
>>  		return -EINVAL;
>> -	default:
>> -		return -EINVAL;
>> -	}
>>  
>> -	return iio_format_value(buf, type, val_len, vals);
>> +	vals[0] = afe440x_attr->val_table[reg_val].integer;
>> +	vals[1] = afe440x_attr->val_table[reg_val].fract;
>> +
>> +	return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals);
>>  }
>>  
>>  static ssize_t afe440x_store_register(struct device *dev,
>> @@ -227,22 +213,12 @@ static ssize_t afe440x_store_register(struct device *dev,
>>  	if (ret)
>>  		return ret;
>>  
>> -	switch (afe440x_attr->type) {
>> -	case SIMPLE:
>> -		val = integer;
>> -		break;
>> -	case RESISTANCE:
>> -	case CAPACITANCE:
>> -		for (val = 0; val < afe440x_attr->table_size; val++)
>> -			if (afe440x_attr->val_table[val].integer == integer &&
>> -			    afe440x_attr->val_table[val].fract == fract)
>> -				break;
>> -		if (val == afe440x_attr->table_size)
>> -			return -EINVAL;
>> -		break;
>> -	default:
>> +	for (val = 0; val < afe440x_attr->table_size; val++)
>> +		if (afe440x_attr->val_table[val].integer == integer &&
>> +		    afe440x_attr->val_table[val].fract == fract)
>> +			break;
>> +	if (val == afe440x_attr->table_size)
>>  		return -EINVAL;
>> -	}
>>  
>>  	ret = regmap_update_bits(afe->regmap, afe440x_attr->reg,
>>  				 afe440x_attr->mask,
>> @@ -253,16 +229,13 @@ static ssize_t afe440x_store_register(struct device *dev,
>>  	return count;
>>  }
>>  
>> -static AFE440X_ATTR(tia_separate_en, AFE4403_TIAGAIN, AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0);
>> -
>> -static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table, ARRAY_SIZE(afe4403_res_table));
>> -static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, CAPACITANCE, afe4403_cap_table, ARRAY_SIZE(afe4403_cap_table));
>> +static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, afe4403_res_table);
>> +static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, afe4403_cap_table);
>>  
>> -static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table, ARRAY_SIZE(afe4403_res_table));
>> -static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, CAPACITANCE, afe4403_cap_table, ARRAY_SIZE(afe4403_cap_table));
>> +static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_res_table);
>> +static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_cap_table);
>>  
>>  static struct attribute *afe440x_attributes[] = {
>> -	&afe440x_attr_tia_separate_en.dev_attr.attr,
>>  	&afe440x_attr_tia_resistance1.dev_attr.attr,
>>  	&afe440x_attr_tia_capacitance1.dev_attr.attr,
>>  	&afe440x_attr_tia_resistance2.dev_attr.attr,
>> @@ -473,6 +446,7 @@ static const struct iio_trigger_ops afe4403_trigger_ops = {
>>  static const struct reg_sequence afe4403_reg_sequences[] = {
>>  	AFE4403_TIMING_PAIRS,
>>  	{ AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN },
>> +	{ AFE4403_TIAGAIN, AFE440X_TIAGAIN_ENSEPGAIN },
>>  };
>>  
>>  static const struct regmap_range afe4403_yes_ranges[] = {
>> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
>> index 2d4c522..b9c1666 100644
>> --- a/drivers/iio/health/afe4404.c
>> +++ b/drivers/iio/health/afe4404.c
>> @@ -193,9 +193,9 @@ static ssize_t afe440x_show_register(struct device *dev,
>>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>  	struct afe4404_data *afe = iio_priv(indio_dev);
>>  	struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr);
>> -	unsigned int reg_val, type;
>> +	unsigned int reg_val;
>>  	int vals[2];
>> -	int ret, val_len;
>> +	int ret;
>>  
>>  	ret = regmap_read(afe->regmap, afe440x_attr->reg, &reg_val);
>>  	if (ret)
>> @@ -204,27 +204,13 @@ static ssize_t afe440x_show_register(struct device *dev,
>>  	reg_val &= afe440x_attr->mask;
>>  	reg_val >>= afe440x_attr->shift;
>>  
>> -	switch (afe440x_attr->type) {
>> -	case SIMPLE:
>> -		type = IIO_VAL_INT;
>> -		val_len = 1;
>> -		vals[0] = reg_val;
>> -		break;
>> -	case RESISTANCE:
>> -	case CAPACITANCE:
>> -		type = IIO_VAL_INT_PLUS_MICRO;
>> -		val_len = 2;
>> -		if (reg_val < afe440x_attr->table_size) {
>> -			vals[0] = afe440x_attr->val_table[reg_val].integer;
>> -			vals[1] = afe440x_attr->val_table[reg_val].fract;
>> -			break;
>> -		}
>> +	if (reg_val >= afe440x_attr->table_size)
>>  		return -EINVAL;
>> -	default:
>> -		return -EINVAL;
>> -	}
>>  
>> -	return iio_format_value(buf, type, val_len, vals);
>> +	vals[0] = afe440x_attr->val_table[reg_val].integer;
>> +	vals[1] = afe440x_attr->val_table[reg_val].fract;
>> +
>> +	return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals);
>>  }
>>  
>>  static ssize_t afe440x_store_register(struct device *dev,
>> @@ -240,22 +226,12 @@ static ssize_t afe440x_store_register(struct device *dev,
>>  	if (ret)
>>  		return ret;
>>  
>> -	switch (afe440x_attr->type) {
>> -	case SIMPLE:
>> -		val = integer;
>> -		break;
>> -	case RESISTANCE:
>> -	case CAPACITANCE:
>> -		for (val = 0; val < afe440x_attr->table_size; val++)
>> -			if (afe440x_attr->val_table[val].integer == integer &&
>> -			    afe440x_attr->val_table[val].fract == fract)
>> -				break;
>> -		if (val == afe440x_attr->table_size)
>> -			return -EINVAL;
>> -		break;
>> -	default:
>> +	for (val = 0; val < afe440x_attr->table_size; val++)
>> +		if (afe440x_attr->val_table[val].integer == integer &&
>> +		    afe440x_attr->val_table[val].fract == fract)
>> +			break;
>> +	if (val == afe440x_attr->table_size)
>>  		return -EINVAL;
>> -	}
>>  
>>  	ret = regmap_update_bits(afe->regmap, afe440x_attr->reg,
>>  				 afe440x_attr->mask,
>> @@ -266,16 +242,13 @@ static ssize_t afe440x_store_register(struct device *dev,
>>  	return count;
>>  }
>>  
>> -static AFE440X_ATTR(tia_separate_en, AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0);
>> -
>> -static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, RESISTANCE, afe4404_res_table, ARRAY_SIZE(afe4404_res_table));
>> -static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, CAPACITANCE, afe4404_cap_table, ARRAY_SIZE(afe4404_cap_table));
>> +static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, afe4404_res_table);
>> +static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, afe4404_cap_table);
>>  
>> -static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, RESISTANCE, afe4404_res_table, ARRAY_SIZE(afe4404_res_table));
>> -static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, CAPACITANCE, afe4404_cap_table, ARRAY_SIZE(afe4404_cap_table));
>> +static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, afe4404_res_table);
>> +static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, afe4404_cap_table);
>>  
>>  static struct attribute *afe440x_attributes[] = {
>> -	&afe440x_attr_tia_separate_en.dev_attr.attr,
>>  	&afe440x_attr_tia_resistance1.dev_attr.attr,
>>  	&afe440x_attr_tia_capacitance1.dev_attr.attr,
>>  	&afe440x_attr_tia_resistance2.dev_attr.attr,
>> @@ -443,6 +416,7 @@ static const struct iio_trigger_ops afe4404_trigger_ops = {
>>  static const struct reg_sequence afe4404_reg_sequences[] = {
>>  	AFE4404_TIMING_PAIRS,
>>  	{ AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN },
>> +	{ AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN },
>>  	{ AFE440X_CONTROL2, AFE440X_CONTROL3_OSC_ENABLE	},
>>  };
>>  
>> diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h
>> index c671ab7..544bbab 100644
>> --- a/drivers/iio/health/afe440x.h
>> +++ b/drivers/iio/health/afe440x.h
>> @@ -71,8 +71,7 @@
>>  #define AFE440X_CONTROL1_TIMEREN	BIT(8)
>>  
>>  /* TIAGAIN register fields */
>> -#define AFE440X_TIAGAIN_ENSEPGAIN_MASK	BIT(15)
>> -#define AFE440X_TIAGAIN_ENSEPGAIN_SHIFT	15
>> +#define AFE440X_TIAGAIN_ENSEPGAIN	BIT(15)
>>  
>>  /* CONTROL2 register fields */
>>  #define AFE440X_CONTROL2_PDN_AFE	BIT(0)
>> @@ -133,12 +132,6 @@ struct afe440x_reg_info {
>>  		.output = true,					\
>>  	}
>>  
>> -enum afe440x_reg_type {
>> -	SIMPLE,
>> -	RESISTANCE,
>> -	CAPACITANCE,
>> -};
>> -
>>  struct afe440x_val_table {
>>  	int integer;
>>  	int fract;
>> @@ -167,7 +160,6 @@ struct afe440x_attr {
>>  	unsigned int reg;
>>  	unsigned int shift;
>>  	unsigned int mask;
>> -	enum afe440x_reg_type type;
>>  	const struct afe440x_val_table *val_table;
>>  	unsigned int table_size;
>>  };
>> @@ -175,7 +167,7 @@ struct afe440x_attr {
>>  #define to_afe440x_attr(_dev_attr)				\
>>  	container_of(_dev_attr, struct afe440x_attr, dev_attr)
>>  
>> -#define AFE440X_ATTR(_name, _reg, _field, _type, _table, _size)	\
>> +#define AFE440X_ATTR(_name, _reg, _field, _table)		\
>>  	struct afe440x_attr afe440x_attr_##_name = {		\
>>  		.dev_attr = __ATTR(_name, (S_IRUGO | S_IWUSR),	\
>>  				   afe440x_show_register,	\
>> @@ -183,9 +175,8 @@ struct afe440x_attr {
>>  		.reg = _reg,					\
>>  		.shift = _field ## _SHIFT,			\
>>  		.mask = _field ## _MASK,			\
>> -		.type = _type,					\
>>  		.val_table = _table,				\
>> -		.table_size = _size,				\
>> +		.table_size = ARRAY_SIZE(_table),		\
>>  	}
>>  
>>  #endif /* _AFE440X_H */
>>
> 

Re: [PATCH 08/13] iio: health/afe440x: Remove channel names

[Andrew F. Davis]

On 05/04/2016 05:08 AM, Jonathan Cameron wrote:
> On 01/05/16 21:36, Andrew F. Davis wrote:
>> These AFEs have 4 ADC mesuring stages (called LED2, ALED2, LED1, and
>> ALED1 in the datasheet), we map these as channels, these stages can serve
>> different purposes depending on the application. For instance the AFE4404
>> has an additional LED (LED3), this LED can be timed to be active during
>> stage 2 (or anystage, but the datasheet describes this case and the name
>> of the stage reflects this use). This ability is used further in upcoming
>> parts that tie the front-end gain and the LED timings together. For these
>> reasons we remove explicit naming the channels.
>>
>> Without channel names it is best that the index numbers are in order to
>> match the stage number, reorder the channel numbers.
>>
>> Signed-off-by: Andrew F. Davis <afd@ti.com>
> Youch - this is rather major surgery.
> 
> Hmm. Given how non standard an ABI we ended up with in the first place
> again, let us cross our fingers that all the userspace for this is
> effectively under your control currently.
> 

Yeah, as far as I know we are the first to start building something on
top of this, and that is going to just be an Android HAL, so after that
any usage should still be abstracted a bit from the ABI, at least until
native users appear.

> Getting a little more nervous about this - but worth the risk I think
> for the simpler result.
> 

Using the datasheet names in the sysfs entries was probably wrong from
the start, if I remember correctly you even suggested against it
originally (probably should have listened :)).

in_intensity_ledY_raw -> in_intensityY_raw

works much better, especially with all the different names/uses they
give each stage in the upcoming AFE4405.

> Applied
> 

Thanks,
Andrew

> Jonathan
>> ---
>>  .../ABI/testing/sysfs-bus-iio-health-afe440x       | 39 ++++++++++------------
>>  drivers/iio/health/afe4403.c                       | 28 ++++++++--------
>>  drivers/iio/health/afe4404.c                       | 30 ++++++++---------
>>  drivers/iio/health/afe440x.h                       |  8 ++---
>>  4 files changed, 51 insertions(+), 54 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
>> index b19053a..a067073 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
>> @@ -8,38 +8,35 @@ Description:
>>  		Transimpedance Amplifier. Y is 1 for Rf1 and Cf1, Y is 2 for
>>  		Rf2 and Cf2 values.
>>  
>> -What:		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_raw
>> -		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_ambient_raw
>> -Date:		December 2015
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_intensityY_raw
>> +Date:		May 2016
>>  KernelVersion:
>>  Contact:	Andrew F. Davis <afd@ti.com>
>>  Description:
>>  		Get measured values from the ADC for these stages. Y is the
>> -		specific LED number. The values are expressed in 24-bit twos
>> -		complement.
>> -
>> -What:		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY-ledY_ambient_raw
>> -Date:		December 2015
>> -KernelVersion:
>> -Contact:	Andrew F. Davis <afd@ti.com>
>> -Description:
>> -		Get differential values from the ADC for these stages. Y is the
>> -		specific LED number. The values are expressed in 24-bit twos
>> -		complement for the specified LEDs.
>> +		specific stage number corresponding to datasheet stage names
>> +		as follows:
>> +		1 -> LED2
>> +		2 -> ALED2/LED3
>> +		3 -> LED1
>> +		4 -> ALED1/LED4
>> +		Note that channels 5 and 6 represent LED2-ALED2 and LED1-ALED1
>> +		respectively which simply helper channels containing the
>> +		calculated difference in the value of stage 1 - 2 and 3 - 4.
>> +		The values are expressed in 24-bit twos complement.
>>  
>> -What:		/sys/bus/iio/devices/iio:deviceX/out_current_ledY_offset
>> -		/sys/bus/iio/devices/iio:deviceX/out_current_ledY_ambient_offset
>> -Date:		December 2015
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_intensityY_offset
>> +Date:		May 2016
>>  KernelVersion:
>>  Contact:	Andrew F. Davis <afd@ti.com>
>>  Description:
>>  		Get and set the offset cancellation DAC setting for these
>>  		stages. The values are expressed in 5-bit sign-magnitude.
>>  
>> -What:		/sys/bus/iio/devices/iio:deviceX/out_current_ledY_raw
>> -Date:		December 2015
>> +What:		/sys/bus/iio/devices/iio:deviceX/out_currentY_raw
>> +Date:		May 2016
>>  KernelVersion:
>>  Contact:	Andrew F. Davis <afd@ti.com>
>>  Description:
>> -		Get and set the LED current for the specified LED. Y is the
>> -		specific LED number.
>> +		Get and set the LED current for the specified LED active during
>> +		this stage. Y is the specific stage number.
>> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
>> index cac6090..4a58064 100644
>> --- a/drivers/iio/health/afe4403.c
>> +++ b/drivers/iio/health/afe4403.c
>> @@ -121,38 +121,38 @@ struct afe4403_data {
>>  };
>>  
>>  enum afe4403_chan_id {
>> +	LED2 = 1,
>> +	ALED2,
>>  	LED1,
>>  	ALED1,
>> -	LED2,
>> -	ALED2,
>> -	LED1_ALED1,
>>  	LED2_ALED2,
>> +	LED1_ALED1,
>>  	ILED1,
>>  	ILED2,
>>  };
>>  
>>  static const struct afe440x_reg_info afe4403_reg_info[] = {
>> -	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, 0, NULL),
>> -	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, 0, NULL),
>>  	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, 0, NULL),
>>  	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, 0, NULL),
>> -	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
>> +	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, 0, NULL),
>> +	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, 0, NULL),
>>  	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
>> +	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
>>  	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED1),
>>  	[ILED2] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED2),
>>  };
>>  
>>  static const struct iio_chan_spec afe4403_channels[] = {
>>  	/* ADC values */
>> -	AFE440X_INTENSITY_CHAN(LED1, "led1", 0),
>> -	AFE440X_INTENSITY_CHAN(ALED1, "led1_ambient", 0),
>> -	AFE440X_INTENSITY_CHAN(LED2, "led2", 0),
>> -	AFE440X_INTENSITY_CHAN(ALED2, "led2_ambient", 0),
>> -	AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0),
>> -	AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0),
>> +	AFE440X_INTENSITY_CHAN(LED2, 0),
>> +	AFE440X_INTENSITY_CHAN(ALED2, 0),
>> +	AFE440X_INTENSITY_CHAN(LED1, 0),
>> +	AFE440X_INTENSITY_CHAN(ALED1, 0),
>> +	AFE440X_INTENSITY_CHAN(LED2_ALED2, 0),
>> +	AFE440X_INTENSITY_CHAN(LED1_ALED1, 0),
>>  	/* LED current */
>> -	AFE440X_CURRENT_CHAN(ILED1, "led1"),
>> -	AFE440X_CURRENT_CHAN(ILED2, "led2"),
>> +	AFE440X_CURRENT_CHAN(ILED1),
>> +	AFE440X_CURRENT_CHAN(ILED2),
>>  };
>>  
>>  static const struct afe440x_val_table afe4403_res_table[] = {
>> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
>> index 2edb7d7..7806a45 100644
>> --- a/drivers/iio/health/afe4404.c
>> +++ b/drivers/iio/health/afe4404.c
>> @@ -122,24 +122,24 @@ struct afe4404_data {
>>  };
>>  
>>  enum afe4404_chan_id {
>> +	LED2 = 1,
>> +	ALED2,
>>  	LED1,
>>  	ALED1,
>> -	LED2,
>> -	ALED2,
>> -	LED1_ALED1,
>>  	LED2_ALED2,
>> +	LED1_ALED1,
>>  	ILED1,
>>  	ILED2,
>>  	ILED3,
>>  };
>>  
>>  static const struct afe440x_reg_info afe4404_reg_info[] = {
>> -	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED1),
>> -	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED1),
>>  	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED2),
>>  	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED2),
>> -	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
>> +	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED1),
>> +	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED1),
>>  	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
>> +	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
>>  	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED1),
>>  	[ILED2] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED2),
>>  	[ILED3] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED3),
>> @@ -147,16 +147,16 @@ static const struct afe440x_reg_info afe4404_reg_info[] = {
>>  
>>  static const struct iio_chan_spec afe4404_channels[] = {
>>  	/* ADC values */
>> -	AFE440X_INTENSITY_CHAN(LED1, "led1", BIT(IIO_CHAN_INFO_OFFSET)),
>> -	AFE440X_INTENSITY_CHAN(ALED1, "led1_ambient", BIT(IIO_CHAN_INFO_OFFSET)),
>> -	AFE440X_INTENSITY_CHAN(LED2, "led2", BIT(IIO_CHAN_INFO_OFFSET)),
>> -	AFE440X_INTENSITY_CHAN(ALED2, "led2_ambient", BIT(IIO_CHAN_INFO_OFFSET)),
>> -	AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0),
>> -	AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0),
>> +	AFE440X_INTENSITY_CHAN(LED2, BIT(IIO_CHAN_INFO_OFFSET)),
>> +	AFE440X_INTENSITY_CHAN(ALED2, BIT(IIO_CHAN_INFO_OFFSET)),
>> +	AFE440X_INTENSITY_CHAN(LED1, BIT(IIO_CHAN_INFO_OFFSET)),
>> +	AFE440X_INTENSITY_CHAN(ALED1, BIT(IIO_CHAN_INFO_OFFSET)),
>> +	AFE440X_INTENSITY_CHAN(LED2_ALED2, 0),
>> +	AFE440X_INTENSITY_CHAN(LED1_ALED1, 0),
>>  	/* LED current */
>> -	AFE440X_CURRENT_CHAN(ILED1, "led1"),
>> -	AFE440X_CURRENT_CHAN(ILED2, "led2"),
>> -	AFE440X_CURRENT_CHAN(ILED3, "led3"),
>> +	AFE440X_CURRENT_CHAN(ILED1),
>> +	AFE440X_CURRENT_CHAN(ILED2),
>> +	AFE440X_CURRENT_CHAN(ILED3),
>>  };
>>  
>>  static const struct afe440x_val_table afe4404_res_table[] = {
>> diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h
>> index 583d071..713972f 100644
>> --- a/drivers/iio/health/afe440x.h
>> +++ b/drivers/iio/health/afe440x.h
>> @@ -103,7 +103,7 @@ struct afe440x_reg_info {
>>  		.mask = _sm ## _MASK,				\
>>  	}
>>  
>> -#define AFE440X_INTENSITY_CHAN(_index, _name, _mask)		\
>> +#define AFE440X_INTENSITY_CHAN(_index, _mask)			\
>>  	{							\
>>  		.type = IIO_INTENSITY,				\
>>  		.channel = _index,				\
>> @@ -115,20 +115,20 @@ struct afe440x_reg_info {
>>  				.storagebits = 32,		\
>>  				.endianness = IIO_CPU,		\
>>  		},						\
>> -		.extend_name = _name,				\
>>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
>>  			_mask,					\
>> +		.indexed = true,				\
>>  	}
>>  
>> -#define AFE440X_CURRENT_CHAN(_index, _name)			\
>> +#define AFE440X_CURRENT_CHAN(_index)				\
>>  	{							\
>>  		.type = IIO_CURRENT,				\
>>  		.channel = _index,				\
>>  		.address = _index,				\
>>  		.scan_index = -1,				\
>> -		.extend_name = _name,				\
>>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
>>  			BIT(IIO_CHAN_INFO_SCALE),		\
>> +		.indexed = true,				\
>>  		.output = true,					\
>>  	}
>>  
>>
> 

Re: [PATCH 09/13] iio: health/afe440x: Use regmap fields

[Andrew F. Davis]

On 05/04/2016 05:10 AM, Jonathan Cameron wrote:
> On 01/05/16 21:36, Andrew F. Davis wrote:
>> These drivers can use regmap fields to access fields in registers, this
>> allows us to remove some macros/defines and simplify code, do this here.
>>
>> Signed-off-by: Andrew F. Davis <afd@ti.com>
> This could almost be a poster boy for why regmap field stuff is good ;)
> So often it ends up more marginal than this.
> 

I know, I am really pleased how well it worked out here.

Andrew

> Applied.
> 
> Jonathan
>> ---
>>  drivers/iio/health/afe4403.c | 118 ++++++++++++++++----------------
>>  drivers/iio/health/afe4404.c | 156 ++++++++++++++++++++++---------------------
>>  drivers/iio/health/afe440x.h |  25 +------
>>  3 files changed, 144 insertions(+), 155 deletions(-)
>>
>> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
>> index 4a58064..1950155 100644
>> --- a/drivers/iio/health/afe4403.c
>> +++ b/drivers/iio/health/afe4403.c
>> @@ -39,32 +39,6 @@
>>  #define AFE4403_TIAGAIN			0x20
>>  #define AFE4403_TIA_AMB_GAIN		0x21
>>  
>> -/* AFE4403 GAIN register fields */
>> -#define AFE4403_TIAGAIN_RES_MASK	GENMASK(2, 0)
>> -#define AFE4403_TIAGAIN_RES_SHIFT	0
>> -#define AFE4403_TIAGAIN_CAP_MASK	GENMASK(7, 3)
>> -#define AFE4403_TIAGAIN_CAP_SHIFT	3
>> -
>> -/* AFE4403 LEDCNTRL register fields */
>> -#define AFE440X_LEDCNTRL_LED1_MASK		GENMASK(15, 8)
>> -#define AFE440X_LEDCNTRL_LED1_SHIFT		8
>> -#define AFE440X_LEDCNTRL_LED2_MASK		GENMASK(7, 0)
>> -#define AFE440X_LEDCNTRL_LED2_SHIFT		0
>> -#define AFE440X_LEDCNTRL_LED_RANGE_MASK		GENMASK(17, 16)
>> -#define AFE440X_LEDCNTRL_LED_RANGE_SHIFT	16
>> -
>> -/* AFE4403 CONTROL2 register fields */
>> -#define AFE440X_CONTROL2_PWR_DWN_TX	BIT(2)
>> -#define AFE440X_CONTROL2_EN_SLOW_DIAG	BIT(8)
>> -#define AFE440X_CONTROL2_DIAG_OUT_TRI	BIT(10)
>> -#define AFE440X_CONTROL2_TX_BRDG_MOD	BIT(11)
>> -#define AFE440X_CONTROL2_TX_REF_MASK	GENMASK(18, 17)
>> -#define AFE440X_CONTROL2_TX_REF_SHIFT	17
>> -
>> -/* AFE4404 NULL fields */
>> -#define NULL_MASK	0
>> -#define NULL_SHIFT	0
>> -
>>  /* AFE4403 LEDCNTRL values */
>>  #define AFE440X_LEDCNTRL_RANGE_TX_HALF	0x1
>>  #define AFE440X_LEDCNTRL_RANGE_TX_FULL	0x2
>> @@ -102,11 +76,35 @@
>>  #define AFE4403_TIAGAIN_RES_1_M		0x6
>>  #define AFE4403_TIAGAIN_RES_NONE	0x7
>>  
>> +enum afe4403_fields {
>> +	/* Gains */
>> +	F_RF_LED1, F_CF_LED1,
>> +	F_RF_LED, F_CF_LED,
>> +
>> +	/* LED Current */
>> +	F_ILED1, F_ILED2,
>> +
>> +	/* sentinel */
>> +	F_MAX_FIELDS
>> +};
>> +
>> +static const struct reg_field afe4403_reg_fields[] = {
>> +	/* Gains */
>> +	[F_RF_LED1]	= REG_FIELD(AFE4403_TIAGAIN, 0, 2),
>> +	[F_CF_LED1]	= REG_FIELD(AFE4403_TIAGAIN, 3, 7),
>> +	[F_RF_LED]	= REG_FIELD(AFE4403_TIA_AMB_GAIN, 0, 2),
>> +	[F_CF_LED]	= REG_FIELD(AFE4403_TIA_AMB_GAIN, 3, 7),
>> +	/* LED Current */
>> +	[F_ILED1]	= REG_FIELD(AFE440X_LEDCNTRL, 0, 7),
>> +	[F_ILED2]	= REG_FIELD(AFE440X_LEDCNTRL, 8, 15),
>> +};
>> +
>>  /**
>>   * struct afe4403_data - AFE4403 device instance data
>>   * @dev: Device structure
>>   * @spi: SPI device handle
>>   * @regmap: Register map of the device
>> + * @fields: Register fields of the device
>>   * @regulator: Pointer to the regulator for the IC
>>   * @trig: IIO trigger for this device
>>   * @irq: ADC_RDY line interrupt number
>> @@ -115,6 +113,7 @@ struct afe4403_data {
>>  	struct device *dev;
>>  	struct spi_device *spi;
>>  	struct regmap *regmap;
>> +	struct regmap_field *fields[F_MAX_FIELDS];
>>  	struct regulator *regulator;
>>  	struct iio_trigger *trig;
>>  	int irq;
>> @@ -131,15 +130,18 @@ enum afe4403_chan_id {
>>  	ILED2,
>>  };
>>  
>> -static const struct afe440x_reg_info afe4403_reg_info[] = {
>> -	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, 0, NULL),
>> -	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, 0, NULL),
>> -	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, 0, NULL),
>> -	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, 0, NULL),
>> -	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
>> -	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
>> -	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED1),
>> -	[ILED2] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED2),
>> +static const unsigned int afe4403_channel_values[] = {
>> +	[LED2] = AFE440X_LED2VAL,
>> +	[ALED2] = AFE440X_ALED2VAL,
>> +	[LED1] = AFE440X_LED1VAL,
>> +	[ALED1] = AFE440X_ALED1VAL,
>> +	[LED2_ALED2] = AFE440X_LED2_ALED2VAL,
>> +	[LED1_ALED1] = AFE440X_LED1_ALED1VAL,
>> +};
>> +
>> +static const unsigned int afe4403_channel_leds[] = {
>> +	[ILED1] = F_ILED1,
>> +	[ILED2] = F_ILED2,
>>  };
>>  
>>  static const struct iio_chan_spec afe4403_channels[] = {
>> @@ -184,13 +186,10 @@ static ssize_t afe440x_show_register(struct device *dev,
>>  	int vals[2];
>>  	int ret;
>>  
>> -	ret = regmap_read(afe->regmap, afe440x_attr->reg, &reg_val);
>> +	ret = regmap_field_read(afe->fields[afe440x_attr->field], &reg_val);
>>  	if (ret)
>>  		return ret;
>>  
>> -	reg_val &= afe440x_attr->mask;
>> -	reg_val >>= afe440x_attr->shift;
>> -
>>  	if (reg_val >= afe440x_attr->table_size)
>>  		return -EINVAL;
>>  
>> @@ -220,20 +219,18 @@ static ssize_t afe440x_store_register(struct device *dev,
>>  	if (val == afe440x_attr->table_size)
>>  		return -EINVAL;
>>  
>> -	ret = regmap_update_bits(afe->regmap, afe440x_attr->reg,
>> -				 afe440x_attr->mask,
>> -				 (val << afe440x_attr->shift));
>> +	ret = regmap_field_write(afe->fields[afe440x_attr->field], val);
>>  	if (ret)
>>  		return ret;
>>  
>>  	return count;
>>  }
>>  
>> -static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, afe4403_res_table);
>> -static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, afe4403_cap_table);
>> +static AFE440X_ATTR(tia_resistance1, F_RF_LED1, afe4403_res_table);
>> +static AFE440X_ATTR(tia_capacitance1, F_CF_LED1, afe4403_cap_table);
>>  
>> -static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_res_table);
>> -static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_cap_table);
>> +static AFE440X_ATTR(tia_resistance2, F_RF_LED, afe4403_res_table);
>> +static AFE440X_ATTR(tia_capacitance2, F_CF_LED, afe4403_cap_table);
>>  
>>  static struct attribute *afe440x_attributes[] = {
>>  	&afe440x_attr_tia_resistance1.dev_attr.attr,
>> @@ -282,14 +279,15 @@ static int afe4403_read_raw(struct iio_dev *indio_dev,
>>  			    int *val, int *val2, long mask)
>>  {
>>  	struct afe4403_data *afe = iio_priv(indio_dev);
>> -	const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address];
>> +	unsigned int reg = afe4403_channel_values[chan->address];
>> +	unsigned int field = afe4403_channel_leds[chan->address];
>>  	int ret;
>>  
>>  	switch (chan->type) {
>>  	case IIO_INTENSITY:
>>  		switch (mask) {
>>  		case IIO_CHAN_INFO_RAW:
>> -			ret = afe4403_read(afe, reg_info.reg, val);
>> +			ret = afe4403_read(afe, reg, val);
>>  			if (ret)
>>  				return ret;
>>  			return IIO_VAL_INT;
>> @@ -298,11 +296,9 @@ static int afe4403_read_raw(struct iio_dev *indio_dev,
>>  	case IIO_CURRENT:
>>  		switch (mask) {
>>  		case IIO_CHAN_INFO_RAW:
>> -			ret = regmap_read(afe->regmap, reg_info.reg, val);
>> +			ret = regmap_field_read(afe->fields[field], val);
>>  			if (ret)
>>  				return ret;
>> -			*val &= reg_info.mask;
>> -			*val >>= reg_info.shift;
>>  			return IIO_VAL_INT;
>>  		case IIO_CHAN_INFO_SCALE:
>>  			*val = 0;
>> @@ -322,16 +318,13 @@ static int afe4403_write_raw(struct iio_dev *indio_dev,
>>  			     int val, int val2, long mask)
>>  {
>>  	struct afe4403_data *afe = iio_priv(indio_dev);
>> -	const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address];
>> +	unsigned int field = afe4403_channel_leds[chan->address];
>>  
>>  	switch (chan->type) {
>>  	case IIO_CURRENT:
>>  		switch (mask) {
>>  		case IIO_CHAN_INFO_RAW:
>> -			return regmap_update_bits(afe->regmap,
>> -				reg_info.reg,
>> -				reg_info.mask,
>> -				(val << reg_info.shift));
>> +			return regmap_field_write(afe->fields[field], val);
>>  		}
>>  		break;
>>  	default:
>> @@ -366,7 +359,7 @@ static irqreturn_t afe4403_trigger_handler(int irq, void *private)
>>  	for_each_set_bit(bit, indio_dev->active_scan_mask,
>>  			 indio_dev->masklength) {
>>  		ret = spi_write_then_read(afe->spi,
>> -					  &afe4403_reg_info[bit].reg, 1,
>> +					  &afe4403_channel_values[bit], 1,
>>  					  rx, 3);
>>  		if (ret)
>>  			goto err;
>> @@ -503,7 +496,7 @@ static int afe4403_probe(struct spi_device *spi)
>>  {
>>  	struct iio_dev *indio_dev;
>>  	struct afe4403_data *afe;
>> -	int ret;
>> +	int i, ret;
>>  
>>  	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*afe));
>>  	if (!indio_dev)
>> @@ -522,6 +515,15 @@ static int afe4403_probe(struct spi_device *spi)
>>  		return PTR_ERR(afe->regmap);
>>  	}
>>  
>> +	for (i = 0; i < F_MAX_FIELDS; i++) {
>> +		afe->fields[i] = devm_regmap_field_alloc(afe->dev, afe->regmap,
>> +							 afe4403_reg_fields[i]);
>> +		if (IS_ERR(afe->fields[i])) {
>> +			dev_err(afe->dev, "Unable to allocate regmap fields\n");
>> +			return PTR_ERR(afe->fields[i]);
>> +		}
>> +	}
>> +
>>  	afe->regulator = devm_regulator_get(afe->dev, "tx_sup");
>>  	if (IS_ERR(afe->regulator)) {
>>  		dev_err(afe->dev, "Unable to get regulator\n");
>> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
>> index 7806a45..0d1af4a 100644
>> --- a/drivers/iio/health/afe4404.c
>> +++ b/drivers/iio/health/afe4404.c
>> @@ -48,43 +48,9 @@
>>  #define AFE4404_AVG_LED2_ALED2VAL	0x3f
>>  #define AFE4404_AVG_LED1_ALED1VAL	0x40
>>  
>> -/* AFE4404 GAIN register fields */
>> -#define AFE4404_TIA_GAIN_RES_MASK	GENMASK(2, 0)
>> -#define AFE4404_TIA_GAIN_RES_SHIFT	0
>> -#define AFE4404_TIA_GAIN_CAP_MASK	GENMASK(5, 3)
>> -#define AFE4404_TIA_GAIN_CAP_SHIFT	3
>> -
>> -/* AFE4404 LEDCNTRL register fields */
>> -#define AFE4404_LEDCNTRL_ILED1_MASK	GENMASK(5, 0)
>> -#define AFE4404_LEDCNTRL_ILED1_SHIFT	0
>> -#define AFE4404_LEDCNTRL_ILED2_MASK	GENMASK(11, 6)
>> -#define AFE4404_LEDCNTRL_ILED2_SHIFT	6
>> -#define AFE4404_LEDCNTRL_ILED3_MASK	GENMASK(17, 12)
>> -#define AFE4404_LEDCNTRL_ILED3_SHIFT	12
>> -
>> -/* AFE4404 CONTROL2 register fields */
>> -#define AFE440X_CONTROL2_ILED_2X_MASK	BIT(17)
>> -#define AFE440X_CONTROL2_ILED_2X_SHIFT	17
>> -
>>  /* AFE4404 CONTROL3 register fields */
>>  #define AFE440X_CONTROL3_OSC_ENABLE	BIT(9)
>>  
>> -/* AFE4404 OFFDAC register current fields */
>> -#define AFE4404_OFFDAC_CURR_LED1_MASK	GENMASK(9, 5)
>> -#define AFE4404_OFFDAC_CURR_LED1_SHIFT	5
>> -#define AFE4404_OFFDAC_CURR_LED2_MASK	GENMASK(19, 15)
>> -#define AFE4404_OFFDAC_CURR_LED2_SHIFT	15
>> -#define AFE4404_OFFDAC_CURR_LED3_MASK	GENMASK(4, 0)
>> -#define AFE4404_OFFDAC_CURR_LED3_SHIFT	0
>> -#define AFE4404_OFFDAC_CURR_ALED1_MASK	GENMASK(14, 10)
>> -#define AFE4404_OFFDAC_CURR_ALED1_SHIFT	10
>> -#define AFE4404_OFFDAC_CURR_ALED2_MASK	GENMASK(4, 0)
>> -#define AFE4404_OFFDAC_CURR_ALED2_SHIFT	0
>> -
>> -/* AFE4404 NULL fields */
>> -#define NULL_MASK	0
>> -#define NULL_SHIFT	0
>> -
>>  /* AFE4404 TIA_GAIN_CAP values */
>>  #define AFE4404_TIA_GAIN_CAP_5_P	0x0
>>  #define AFE4404_TIA_GAIN_CAP_2_5_P	0x1
>> @@ -105,10 +71,43 @@
>>  #define AFE4404_TIA_GAIN_RES_1_M	0x6
>>  #define AFE4404_TIA_GAIN_RES_2_M	0x7
>>  
>> +enum afe4404_fields {
>> +	/* Gains */
>> +	F_TIA_GAIN_SEP, F_TIA_CF_SEP,
>> +	F_TIA_GAIN, TIA_CF,
>> +
>> +	/* LED Current */
>> +	F_ILED1, F_ILED2, F_ILED3,
>> +
>> +	/* Offset DAC */
>> +	F_OFFDAC_AMB2, F_OFFDAC_LED1, F_OFFDAC_AMB1, F_OFFDAC_LED2,
>> +
>> +	/* sentinel */
>> +	F_MAX_FIELDS
>> +};
>> +
>> +static const struct reg_field afe4404_reg_fields[] = {
>> +	/* Gains */
>> +	[F_TIA_GAIN_SEP]	= REG_FIELD(AFE4404_TIA_GAIN_SEP, 0, 2),
>> +	[F_TIA_CF_SEP]		= REG_FIELD(AFE4404_TIA_GAIN_SEP, 3, 5),
>> +	[F_TIA_GAIN]		= REG_FIELD(AFE4404_TIA_GAIN, 0, 2),
>> +	[TIA_CF]		= REG_FIELD(AFE4404_TIA_GAIN, 3, 5),
>> +	/* LED Current */
>> +	[F_ILED1]		= REG_FIELD(AFE440X_LEDCNTRL, 0, 5),
>> +	[F_ILED2]		= REG_FIELD(AFE440X_LEDCNTRL, 6, 11),
>> +	[F_ILED3]		= REG_FIELD(AFE440X_LEDCNTRL, 12, 17),
>> +	/* Offset DAC */
>> +	[F_OFFDAC_AMB2]		= REG_FIELD(AFE4404_OFFDAC, 0, 4),
>> +	[F_OFFDAC_LED1]		= REG_FIELD(AFE4404_OFFDAC, 5, 9),
>> +	[F_OFFDAC_AMB1]		= REG_FIELD(AFE4404_OFFDAC, 10, 14),
>> +	[F_OFFDAC_LED2]		= REG_FIELD(AFE4404_OFFDAC, 15, 19),
>> +};
>> +
>>  /**
>>   * struct afe4404_data - AFE4404 device instance data
>>   * @dev: Device structure
>>   * @regmap: Register map of the device
>> + * @fields: Register fields of the device
>>   * @regulator: Pointer to the regulator for the IC
>>   * @trig: IIO trigger for this device
>>   * @irq: ADC_RDY line interrupt number
>> @@ -116,6 +115,7 @@
>>  struct afe4404_data {
>>  	struct device *dev;
>>  	struct regmap *regmap;
>> +	struct regmap_field *fields[F_MAX_FIELDS];
>>  	struct regulator *regulator;
>>  	struct iio_trigger *trig;
>>  	int irq;
>> @@ -133,16 +133,26 @@ enum afe4404_chan_id {
>>  	ILED3,
>>  };
>>  
>> -static const struct afe440x_reg_info afe4404_reg_info[] = {
>> -	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED2),
>> -	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED2),
>> -	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED1),
>> -	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED1),
>> -	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
>> -	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
>> -	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED1),
>> -	[ILED2] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED2),
>> -	[ILED3] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED3),
>> +static const unsigned int afe4404_channel_values[] = {
>> +	[LED2] = AFE440X_LED2VAL,
>> +	[ALED2] = AFE440X_ALED2VAL,
>> +	[LED1] = AFE440X_LED1VAL,
>> +	[ALED1] = AFE440X_ALED1VAL,
>> +	[LED2_ALED2] = AFE440X_LED2_ALED2VAL,
>> +	[LED1_ALED1] = AFE440X_LED1_ALED1VAL,
>> +};
>> +
>> +static const unsigned int afe4404_channel_leds[] = {
>> +	[ILED1] = F_ILED1,
>> +	[ILED2] = F_ILED2,
>> +	[ILED3] = F_ILED3,
>> +};
>> +
>> +static const unsigned int afe4404_channel_offdacs[] = {
>> +	[LED2] = F_OFFDAC_LED2,
>> +	[ALED2] = F_OFFDAC_AMB2,
>> +	[LED1] = F_OFFDAC_LED1,
>> +	[ALED1] = F_OFFDAC_AMB1,
>>  };
>>  
>>  static const struct iio_chan_spec afe4404_channels[] = {
>> @@ -194,13 +204,10 @@ static ssize_t afe440x_show_register(struct device *dev,
>>  	int vals[2];
>>  	int ret;
>>  
>> -	ret = regmap_read(afe->regmap, afe440x_attr->reg, &reg_val);
>> +	ret = regmap_field_read(afe->fields[afe440x_attr->field], &reg_val);
>>  	if (ret)
>>  		return ret;
>>  
>> -	reg_val &= afe440x_attr->mask;
>> -	reg_val >>= afe440x_attr->shift;
>> -
>>  	if (reg_val >= afe440x_attr->table_size)
>>  		return -EINVAL;
>>  
>> @@ -230,20 +237,18 @@ static ssize_t afe440x_store_register(struct device *dev,
>>  	if (val == afe440x_attr->table_size)
>>  		return -EINVAL;
>>  
>> -	ret = regmap_update_bits(afe->regmap, afe440x_attr->reg,
>> -				 afe440x_attr->mask,
>> -				 (val << afe440x_attr->shift));
>> +	ret = regmap_field_write(afe->fields[afe440x_attr->field], val);
>>  	if (ret)
>>  		return ret;
>>  
>>  	return count;
>>  }
>>  
>> -static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, afe4404_res_table);
>> -static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, afe4404_cap_table);
>> +static AFE440X_ATTR(tia_resistance1, F_TIA_GAIN, afe4404_res_table);
>> +static AFE440X_ATTR(tia_capacitance1, TIA_CF, afe4404_cap_table);
>>  
>> -static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, afe4404_res_table);
>> -static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, afe4404_cap_table);
>> +static AFE440X_ATTR(tia_resistance2, F_TIA_GAIN_SEP, afe4404_res_table);
>> +static AFE440X_ATTR(tia_capacitance2, F_TIA_CF_SEP, afe4404_cap_table);
>>  
>>  static struct attribute *afe440x_attributes[] = {
>>  	&afe440x_attr_tia_resistance1.dev_attr.attr,
>> @@ -264,35 +269,32 @@ static int afe4404_read_raw(struct iio_dev *indio_dev,
>>  			    int *val, int *val2, long mask)
>>  {
>>  	struct afe4404_data *afe = iio_priv(indio_dev);
>> -	const struct afe440x_reg_info reg_info = afe4404_reg_info[chan->address];
>> +	unsigned int value_reg = afe4404_channel_values[chan->address];
>> +	unsigned int led_field = afe4404_channel_leds[chan->address];
>> +	unsigned int offdac_field = afe4404_channel_offdacs[chan->address];
>>  	int ret;
>>  
>>  	switch (chan->type) {
>>  	case IIO_INTENSITY:
>>  		switch (mask) {
>>  		case IIO_CHAN_INFO_RAW:
>> -			ret = regmap_read(afe->regmap, reg_info.reg, val);
>> +			ret = regmap_read(afe->regmap, value_reg, val);
>>  			if (ret)
>>  				return ret;
>>  			return IIO_VAL_INT;
>>  		case IIO_CHAN_INFO_OFFSET:
>> -			ret = regmap_read(afe->regmap, reg_info.offreg,
>> -					  val);
>> +			ret = regmap_field_read(afe->fields[offdac_field], val);
>>  			if (ret)
>>  				return ret;
>> -			*val &= reg_info.mask;
>> -			*val >>= reg_info.shift;
>>  			return IIO_VAL_INT;
>>  		}
>>  		break;
>>  	case IIO_CURRENT:
>>  		switch (mask) {
>>  		case IIO_CHAN_INFO_RAW:
>> -			ret = regmap_read(afe->regmap, reg_info.reg, val);
>> +			ret = regmap_field_read(afe->fields[led_field], val);
>>  			if (ret)
>>  				return ret;
>> -			*val &= reg_info.mask;
>> -			*val >>= reg_info.shift;
>>  			return IIO_VAL_INT;
>>  		case IIO_CHAN_INFO_SCALE:
>>  			*val = 0;
>> @@ -312,25 +314,20 @@ static int afe4404_write_raw(struct iio_dev *indio_dev,
>>  			     int val, int val2, long mask)
>>  {
>>  	struct afe4404_data *afe = iio_priv(indio_dev);
>> -	const struct afe440x_reg_info reg_info = afe4404_reg_info[chan->address];
>> +	unsigned int led_field = afe4404_channel_leds[chan->address];
>> +	unsigned int offdac_field = afe4404_channel_offdacs[chan->address];
>>  
>>  	switch (chan->type) {
>>  	case IIO_INTENSITY:
>>  		switch (mask) {
>>  		case IIO_CHAN_INFO_OFFSET:
>> -			return regmap_update_bits(afe->regmap,
>> -				reg_info.offreg,
>> -				reg_info.mask,
>> -				(val << reg_info.shift));
>> +			return regmap_field_write(afe->fields[offdac_field], val);
>>  		}
>>  		break;
>>  	case IIO_CURRENT:
>>  		switch (mask) {
>>  		case IIO_CHAN_INFO_RAW:
>> -			return regmap_update_bits(afe->regmap,
>> -				reg_info.reg,
>> -				reg_info.mask,
>> -				(val << reg_info.shift));
>> +			return regmap_field_write(afe->fields[led_field], val);
>>  		}
>>  		break;
>>  	default:
>> @@ -357,7 +354,7 @@ static irqreturn_t afe4404_trigger_handler(int irq, void *private)
>>  
>>  	for_each_set_bit(bit, indio_dev->active_scan_mask,
>>  			 indio_dev->masklength) {
>> -		ret = regmap_read(afe->regmap, afe4404_reg_info[bit].reg,
>> +		ret = regmap_read(afe->regmap, afe4404_channel_values[bit],
>>  				  &buffer[i++]);
>>  		if (ret)
>>  			goto err;
>> @@ -490,7 +487,7 @@ static int afe4404_probe(struct i2c_client *client,
>>  {
>>  	struct iio_dev *indio_dev;
>>  	struct afe4404_data *afe;
>> -	int ret;
>> +	int i, ret;
>>  
>>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*afe));
>>  	if (!indio_dev)
>> @@ -508,6 +505,15 @@ static int afe4404_probe(struct i2c_client *client,
>>  		return PTR_ERR(afe->regmap);
>>  	}
>>  
>> +	for (i = 0; i < F_MAX_FIELDS; i++) {
>> +		afe->fields[i] = devm_regmap_field_alloc(afe->dev, afe->regmap,
>> +							 afe4404_reg_fields[i]);
>> +		if (IS_ERR(afe->fields[i])) {
>> +			dev_err(afe->dev, "Unable to allocate regmap fields\n");
>> +			return PTR_ERR(afe->fields[i]);
>> +		}
>> +	}
>> +
>>  	afe->regulator = devm_regulator_get(afe->dev, "tx_sup");
>>  	if (IS_ERR(afe->regulator)) {
>>  		dev_err(afe->dev, "Unable to get regulator\n");
>> diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h
>> index 713972f..1a0f247 100644
>> --- a/drivers/iio/health/afe440x.h
>> +++ b/drivers/iio/health/afe440x.h
>> @@ -88,21 +88,6 @@
>>  #define AFE440X_CONTROL0_WRITE		0x0
>>  #define AFE440X_CONTROL0_READ		0x1
>>  
>> -struct afe440x_reg_info {
>> -	unsigned int reg;
>> -	unsigned int offreg;
>> -	unsigned int shift;
>> -	unsigned int mask;
>> -};
>> -
>> -#define AFE440X_REG_INFO(_reg, _offreg, _sm)			\
>> -	{							\
>> -		.reg = _reg,					\
>> -		.offreg = _offreg,				\
>> -		.shift = _sm ## _SHIFT,				\
>> -		.mask = _sm ## _MASK,				\
>> -	}
>> -
>>  #define AFE440X_INTENSITY_CHAN(_index, _mask)			\
>>  	{							\
>>  		.type = IIO_INTENSITY,				\
>> @@ -157,9 +142,7 @@ static DEVICE_ATTR_RO(_name)
>>  
>>  struct afe440x_attr {
>>  	struct device_attribute dev_attr;
>> -	unsigned int reg;
>> -	unsigned int shift;
>> -	unsigned int mask;
>> +	unsigned int field;
>>  	const struct afe440x_val_table *val_table;
>>  	unsigned int table_size;
>>  };
>> @@ -167,14 +150,12 @@ struct afe440x_attr {
>>  #define to_afe440x_attr(_dev_attr)				\
>>  	container_of(_dev_attr, struct afe440x_attr, dev_attr)
>>  
>> -#define AFE440X_ATTR(_name, _reg, _field, _table)		\
>> +#define AFE440X_ATTR(_name, _field, _table)			\
>>  	struct afe440x_attr afe440x_attr_##_name = {		\
>>  		.dev_attr = __ATTR(_name, (S_IRUGO | S_IWUSR),	\
>>  				   afe440x_show_register,	\
>>  				   afe440x_store_register),	\
>> -		.reg = _reg,					\
>> -		.shift = _field ## _SHIFT,			\
>> -		.mask = _field ## _MASK,			\
>> +		.field = _field,				\
>>  		.val_table = _table,				\
>>  		.table_size = ARRAY_SIZE(_table),		\
>>  	}
>>
> 

Re: [PATCH 04/13] iio: health/afe440x: Always use separate gain values

[Jonathan Cameron]

On 4 May 2016 16:13:29 BST, "Andrew F. Davis" <afd@ti.com> wrote:
>On 05/04/2016 05:02 AM, Jonathan Cameron wrote:
>> On 01/05/16 21:36, Andrew F. Davis wrote:
>>> Locking the two gain stages to the same setting adds no value for
>us,
>>> so initialize them as unlocked and remove the sysfs for unlocking
>them.
>>> This also allows us to greatly simplify showing and setting the gain
>>> registers.
>>>
>>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>> Hmm. ABI change but as you said it's an improvement.
>> 
>> Honestly I doubt anyone is using this device without also using
>userspace that
>> you are providing so lets apply this an cross our fingers that no one
>minds.
>> 
>
>Actually we don't even provide a Linux userspace for these yet,
>starting
>to work on that is what prompted these changes. These parts have
>traditionally been paired with a microcontroller to perform the work in
>dedicated devices. This was put on my desk as they are hoping to start
>enabling these to be integrated into smart devices running Linux
>(Android). I'm not an expert in pulse metering, so my initial attempt
>at
>an ABI was a bit off compared to what I now believe to be the "correct"
>way to expose these parts.
*laughs*  sometimes the only way to find out is developing real products.
>
>Thanks again for tolerating all this madness :),

It's cool. I worked with some medics who loved pulse oximeters a few years back. 
Not sure what kit they were using though.

We were doing breathing measurement with structure light. Turns out you can get
 pulse from that (sort of). Plus all sorts of weird plots that medics love which make
 no sense at all from an engineering POV.

I like weird devices even if they can cause headaches :) Weird comes in many forms!

Jonathan

>Andrew
>
>> Applied.
>>> ---
>>>  .../ABI/testing/sysfs-bus-iio-health-afe440x       |  9 ----
>>>  drivers/iio/health/afe4403.c                       | 60
>++++++----------------
>>>  drivers/iio/health/afe4404.c                       | 60
>++++++----------------
>>>  drivers/iio/health/afe440x.h                       | 15 ++----
>>>  4 files changed, 37 insertions(+), 107 deletions(-)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
>b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
>>> index 3740f25..b19053a 100644
>>> --- a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
>>> @@ -8,15 +8,6 @@ Description:
>>>  		Transimpedance Amplifier. Y is 1 for Rf1 and Cf1, Y is 2 for
>>>  		Rf2 and Cf2 values.
>>>  
>>> -What:		/sys/bus/iio/devices/iio:deviceX/tia_separate_en
>>> -Date:		December 2015
>>> -KernelVersion:
>>> -Contact:	Andrew F. Davis <afd@ti.com>
>>> -Description:
>>> -		Enable or disable separate settings for the TransImpedance
>>> -		Amplifier above, when disabled both values are set by the
>>> -		first channel.
>>> -
>>>  What:		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_raw
>>>  		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_ambient_raw
>>>  Date:		December 2015
>>> diff --git a/drivers/iio/health/afe4403.c
>b/drivers/iio/health/afe4403.c
>>> index 5484785..bcff528 100644
>>> --- a/drivers/iio/health/afe4403.c
>>> +++ b/drivers/iio/health/afe4403.c
>>> @@ -180,9 +180,9 @@ static ssize_t afe440x_show_register(struct
>device *dev,
>>>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>>  	struct afe4403_data *afe = iio_priv(indio_dev);
>>>  	struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr);
>>> -	unsigned int reg_val, type;
>>> +	unsigned int reg_val;
>>>  	int vals[2];
>>> -	int ret, val_len;
>>> +	int ret;
>>>  
>>>  	ret = regmap_read(afe->regmap, afe440x_attr->reg, &reg_val);
>>>  	if (ret)
>>> @@ -191,27 +191,13 @@ static ssize_t afe440x_show_register(struct
>device *dev,
>>>  	reg_val &= afe440x_attr->mask;
>>>  	reg_val >>= afe440x_attr->shift;
>>>  
>>> -	switch (afe440x_attr->type) {
>>> -	case SIMPLE:
>>> -		type = IIO_VAL_INT;
>>> -		val_len = 1;
>>> -		vals[0] = reg_val;
>>> -		break;
>>> -	case RESISTANCE:
>>> -	case CAPACITANCE:
>>> -		type = IIO_VAL_INT_PLUS_MICRO;
>>> -		val_len = 2;
>>> -		if (reg_val < afe440x_attr->table_size) {
>>> -			vals[0] = afe440x_attr->val_table[reg_val].integer;
>>> -			vals[1] = afe440x_attr->val_table[reg_val].fract;
>>> -			break;
>>> -		}
>>> +	if (reg_val >= afe440x_attr->table_size)
>>>  		return -EINVAL;
>>> -	default:
>>> -		return -EINVAL;
>>> -	}
>>>  
>>> -	return iio_format_value(buf, type, val_len, vals);
>>> +	vals[0] = afe440x_attr->val_table[reg_val].integer;
>>> +	vals[1] = afe440x_attr->val_table[reg_val].fract;
>>> +
>>> +	return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals);
>>>  }
>>>  
>>>  static ssize_t afe440x_store_register(struct device *dev,
>>> @@ -227,22 +213,12 @@ static ssize_t afe440x_store_register(struct
>device *dev,
>>>  	if (ret)
>>>  		return ret;
>>>  
>>> -	switch (afe440x_attr->type) {
>>> -	case SIMPLE:
>>> -		val = integer;
>>> -		break;
>>> -	case RESISTANCE:
>>> -	case CAPACITANCE:
>>> -		for (val = 0; val < afe440x_attr->table_size; val++)
>>> -			if (afe440x_attr->val_table[val].integer == integer &&
>>> -			    afe440x_attr->val_table[val].fract == fract)
>>> -				break;
>>> -		if (val == afe440x_attr->table_size)
>>> -			return -EINVAL;
>>> -		break;
>>> -	default:
>>> +	for (val = 0; val < afe440x_attr->table_size; val++)
>>> +		if (afe440x_attr->val_table[val].integer == integer &&
>>> +		    afe440x_attr->val_table[val].fract == fract)
>>> +			break;
>>> +	if (val == afe440x_attr->table_size)
>>>  		return -EINVAL;
>>> -	}
>>>  
>>>  	ret = regmap_update_bits(afe->regmap, afe440x_attr->reg,
>>>  				 afe440x_attr->mask,
>>> @@ -253,16 +229,13 @@ static ssize_t afe440x_store_register(struct
>device *dev,
>>>  	return count;
>>>  }
>>>  
>>> -static AFE440X_ATTR(tia_separate_en, AFE4403_TIAGAIN,
>AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0);
>>> -
>>> -static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN,
>AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table,
>ARRAY_SIZE(afe4403_res_table));
>>> -static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN,
>AFE4403_TIAGAIN_CAP, CAPACITANCE, afe4403_cap_table,
>ARRAY_SIZE(afe4403_cap_table));
>>> +static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN,
>AFE4403_TIAGAIN_RES, afe4403_res_table);
>>> +static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN,
>AFE4403_TIAGAIN_CAP, afe4403_cap_table);
>>>  
>>> -static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN,
>AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table,
>ARRAY_SIZE(afe4403_res_table));
>>> -static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN,
>AFE4403_TIAGAIN_RES, CAPACITANCE, afe4403_cap_table,
>ARRAY_SIZE(afe4403_cap_table));
>>> +static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN,
>AFE4403_TIAGAIN_RES, afe4403_res_table);
>>> +static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN,
>AFE4403_TIAGAIN_RES, afe4403_cap_table);
>>>  
>>>  static struct attribute *afe440x_attributes[] = {
>>> -	&afe440x_attr_tia_separate_en.dev_attr.attr,
>>>  	&afe440x_attr_tia_resistance1.dev_attr.attr,
>>>  	&afe440x_attr_tia_capacitance1.dev_attr.attr,
>>>  	&afe440x_attr_tia_resistance2.dev_attr.attr,
>>> @@ -473,6 +446,7 @@ static const struct iio_trigger_ops
>afe4403_trigger_ops = {
>>>  static const struct reg_sequence afe4403_reg_sequences[] = {
>>>  	AFE4403_TIMING_PAIRS,
>>>  	{ AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN },
>>> +	{ AFE4403_TIAGAIN, AFE440X_TIAGAIN_ENSEPGAIN },
>>>  };
>>>  
>>>  static const struct regmap_range afe4403_yes_ranges[] = {
>>> diff --git a/drivers/iio/health/afe4404.c
>b/drivers/iio/health/afe4404.c
>>> index 2d4c522..b9c1666 100644
>>> --- a/drivers/iio/health/afe4404.c
>>> +++ b/drivers/iio/health/afe4404.c
>>> @@ -193,9 +193,9 @@ static ssize_t afe440x_show_register(struct
>device *dev,
>>>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>>  	struct afe4404_data *afe = iio_priv(indio_dev);
>>>  	struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr);
>>> -	unsigned int reg_val, type;
>>> +	unsigned int reg_val;
>>>  	int vals[2];
>>> -	int ret, val_len;
>>> +	int ret;
>>>  
>>>  	ret = regmap_read(afe->regmap, afe440x_attr->reg, &reg_val);
>>>  	if (ret)
>>> @@ -204,27 +204,13 @@ static ssize_t afe440x_show_register(struct
>device *dev,
>>>  	reg_val &= afe440x_attr->mask;
>>>  	reg_val >>= afe440x_attr->shift;
>>>  
>>> -	switch (afe440x_attr->type) {
>>> -	case SIMPLE:
>>> -		type = IIO_VAL_INT;
>>> -		val_len = 1;
>>> -		vals[0] = reg_val;
>>> -		break;
>>> -	case RESISTANCE:
>>> -	case CAPACITANCE:
>>> -		type = IIO_VAL_INT_PLUS_MICRO;
>>> -		val_len = 2;
>>> -		if (reg_val < afe440x_attr->table_size) {
>>> -			vals[0] = afe440x_attr->val_table[reg_val].integer;
>>> -			vals[1] = afe440x_attr->val_table[reg_val].fract;
>>> -			break;
>>> -		}
>>> +	if (reg_val >= afe440x_attr->table_size)
>>>  		return -EINVAL;
>>> -	default:
>>> -		return -EINVAL;
>>> -	}
>>>  
>>> -	return iio_format_value(buf, type, val_len, vals);
>>> +	vals[0] = afe440x_attr->val_table[reg_val].integer;
>>> +	vals[1] = afe440x_attr->val_table[reg_val].fract;
>>> +
>>> +	return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals);
>>>  }
>>>  
>>>  static ssize_t afe440x_store_register(struct device *dev,
>>> @@ -240,22 +226,12 @@ static ssize_t afe440x_store_register(struct
>device *dev,
>>>  	if (ret)
>>>  		return ret;
>>>  
>>> -	switch (afe440x_attr->type) {
>>> -	case SIMPLE:
>>> -		val = integer;
>>> -		break;
>>> -	case RESISTANCE:
>>> -	case CAPACITANCE:
>>> -		for (val = 0; val < afe440x_attr->table_size; val++)
>>> -			if (afe440x_attr->val_table[val].integer == integer &&
>>> -			    afe440x_attr->val_table[val].fract == fract)
>>> -				break;
>>> -		if (val == afe440x_attr->table_size)
>>> -			return -EINVAL;
>>> -		break;
>>> -	default:
>>> +	for (val = 0; val < afe440x_attr->table_size; val++)
>>> +		if (afe440x_attr->val_table[val].integer == integer &&
>>> +		    afe440x_attr->val_table[val].fract == fract)
>>> +			break;
>>> +	if (val == afe440x_attr->table_size)
>>>  		return -EINVAL;
>>> -	}
>>>  
>>>  	ret = regmap_update_bits(afe->regmap, afe440x_attr->reg,
>>>  				 afe440x_attr->mask,
>>> @@ -266,16 +242,13 @@ static ssize_t afe440x_store_register(struct
>device *dev,
>>>  	return count;
>>>  }
>>>  
>>> -static AFE440X_ATTR(tia_separate_en, AFE4404_TIA_GAIN_SEP,
>AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0);
>>> -
>>> -static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN,
>AFE4404_TIA_GAIN_RES, RESISTANCE, afe4404_res_table,
>ARRAY_SIZE(afe4404_res_table));
>>> -static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN,
>AFE4404_TIA_GAIN_CAP, CAPACITANCE, afe4404_cap_table,
>ARRAY_SIZE(afe4404_cap_table));
>>> +static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN,
>AFE4404_TIA_GAIN_RES, afe4404_res_table);
>>> +static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN,
>AFE4404_TIA_GAIN_CAP, afe4404_cap_table);
>>>  
>>> -static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP,
>AFE4404_TIA_GAIN_RES, RESISTANCE, afe4404_res_table,
>ARRAY_SIZE(afe4404_res_table));
>>> -static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP,
>AFE4404_TIA_GAIN_CAP, CAPACITANCE, afe4404_cap_table,
>ARRAY_SIZE(afe4404_cap_table));
>>> +static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP,
>AFE4404_TIA_GAIN_RES, afe4404_res_table);
>>> +static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP,
>AFE4404_TIA_GAIN_CAP, afe4404_cap_table);
>>>  
>>>  static struct attribute *afe440x_attributes[] = {
>>> -	&afe440x_attr_tia_separate_en.dev_attr.attr,
>>>  	&afe440x_attr_tia_resistance1.dev_attr.attr,
>>>  	&afe440x_attr_tia_capacitance1.dev_attr.attr,
>>>  	&afe440x_attr_tia_resistance2.dev_attr.attr,
>>> @@ -443,6 +416,7 @@ static const struct iio_trigger_ops
>afe4404_trigger_ops = {
>>>  static const struct reg_sequence afe4404_reg_sequences[] = {
>>>  	AFE4404_TIMING_PAIRS,
>>>  	{ AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN },
>>> +	{ AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN },
>>>  	{ AFE440X_CONTROL2, AFE440X_CONTROL3_OSC_ENABLE	},
>>>  };
>>>  
>>> diff --git a/drivers/iio/health/afe440x.h
>b/drivers/iio/health/afe440x.h
>>> index c671ab7..544bbab 100644
>>> --- a/drivers/iio/health/afe440x.h
>>> +++ b/drivers/iio/health/afe440x.h
>>> @@ -71,8 +71,7 @@
>>>  #define AFE440X_CONTROL1_TIMEREN	BIT(8)
>>>  
>>>  /* TIAGAIN register fields */
>>> -#define AFE440X_TIAGAIN_ENSEPGAIN_MASK	BIT(15)
>>> -#define AFE440X_TIAGAIN_ENSEPGAIN_SHIFT	15
>>> +#define AFE440X_TIAGAIN_ENSEPGAIN	BIT(15)
>>>  
>>>  /* CONTROL2 register fields */
>>>  #define AFE440X_CONTROL2_PDN_AFE	BIT(0)
>>> @@ -133,12 +132,6 @@ struct afe440x_reg_info {
>>>  		.output = true,					\
>>>  	}
>>>  
>>> -enum afe440x_reg_type {
>>> -	SIMPLE,
>>> -	RESISTANCE,
>>> -	CAPACITANCE,
>>> -};
>>> -
>>>  struct afe440x_val_table {
>>>  	int integer;
>>>  	int fract;
>>> @@ -167,7 +160,6 @@ struct afe440x_attr {
>>>  	unsigned int reg;
>>>  	unsigned int shift;
>>>  	unsigned int mask;
>>> -	enum afe440x_reg_type type;
>>>  	const struct afe440x_val_table *val_table;
>>>  	unsigned int table_size;
>>>  };
>>> @@ -175,7 +167,7 @@ struct afe440x_attr {
>>>  #define to_afe440x_attr(_dev_attr)				\
>>>  	container_of(_dev_attr, struct afe440x_attr, dev_attr)
>>>  
>>> -#define AFE440X_ATTR(_name, _reg, _field, _type, _table, _size)	\
>>> +#define AFE440X_ATTR(_name, _reg, _field, _table)		\
>>>  	struct afe440x_attr afe440x_attr_##_name = {		\
>>>  		.dev_attr = __ATTR(_name, (S_IRUGO | S_IWUSR),	\
>>>  				   afe440x_show_register,	\
>>> @@ -183,9 +175,8 @@ struct afe440x_attr {
>>>  		.reg = _reg,					\
>>>  		.shift = _field ## _SHIFT,			\
>>>  		.mask = _field ## _MASK,			\
>>> -		.type = _type,					\
>>>  		.val_table = _table,				\
>>> -		.table_size = _size,				\
>>> +		.table_size = ARRAY_SIZE(_table),		\
>>>  	}
>>>  
>>>  #endif /* _AFE440X_H */
>>>
>> 
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.