[PATCH v2 00/13] staging: iio: ad2s1200: Driver clean up

[PATCH v2 00/13] staging: iio: ad2s1200: Driver clean up

[David Veenstra]

For v1 of the patch see [1].

A summary of this patch:
1. Clean up of minor code style issues
2. Replace legacy GPIO ABI with modern ABI
4. Add scaling factor for angular position and angular velocity to
   match the sysfs IIO ABI
5. Add documentation for device tree binding, and angle channel
6. Move driver to main line

Changes in v2:
- Removed an unneeded include
- Instead of using inclination channel for the angular position,
  use angle channel
- Added documentation for device tree binding, and sysfs IIO ABI
  documentation for angle channel

For the scaling factors, a fractional approximation of Pi is needed.
As in v1 of the patch, the fraction 103993 / 33102 was chosen, which is
based on the OEIS series of nominator/denominator of convergents to
Pi (A002485 and A002486). I have looked at the implementation of
iio_read_channel_info, and compared using the fraction above, with
the fraction and offset used in the IIO_DEGREE_TO_RAD in iio.h, and
found the first method to be more accurate for all possible 12-bit values,
as the error is neglible. Also it does not cause overflow.

In [2] Jonathan suggested to remove usage of platform data altogether,
and asked for Michael Hennerich opinion. Since Michael didn't weigh in,
I have left this how it was.

Best regards,
David Veenstra

[1] https://marc.info/?l=linux-iio&m=152137920426820&w=2
[2] https://marc.info/?l=linux-iio&m=152181140619243&w=2

David Veenstra (13):
  staging: iio: ad2s1200: Remove unneeded include
  staging: iio: ad2s1200: Sort includes alphabetically
  staging: iio: ad2s1200: Reverse Christmas tree ordering
  staging: iio: ad2s1200: Add blank lines
  staging: iio: ad2s1200: Add kernel docs to driver state
  staging: iio: ad2s1200: Introduce variable for repeated value
  staging: iio: ad2s1200: Improve readability with be16_to_cpup
  staging: iio: ad2s1200: Replace legacy gpio API with modern API
  staging: iio: ad2s1200: Add documentation for device tree binding
  staging: iio: ad2s1200: Add scaling factor for angular velocity
    channel
  staging: iio: Documentation: Add missing sysfs docs for angle channel
  staging: iio: ad2s1200: Add scaling factor for angle channel
  staging: iio: ad2s1200: Move driver out of staging

 Documentation/ABI/testing/sysfs-bus-iio            |  11 +
 .../devicetree/bindings/iio/resolver/ad2s1200.txt  |  16 ++
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/resolver/Kconfig                       |  17 ++
 drivers/iio/resolver/Makefile                      |   5 +
 drivers/iio/resolver/ad2s1200.c                    | 250 +++++++++++++++++++++
 drivers/staging/iio/resolver/Kconfig               |  12 -
 drivers/staging/iio/resolver/Makefile              |   1 -
 drivers/staging/iio/resolver/ad2s1200.c            | 165 --------------
 10 files changed, 301 insertions(+), 178 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt
 create mode 100644 drivers/iio/resolver/Kconfig
 create mode 100644 drivers/iio/resolver/Makefile
 create mode 100644 drivers/iio/resolver/ad2s1200.c
 delete mode 100644 drivers/staging/iio/resolver/ad2s1200.c

-- 
2.16.2

[PATCH v2 00/13] staging: iio: ad2s1200: Driver clean up

[David Veenstra]

For v1 of the patch see [1].

A summary of this patch:
1. Clean up of minor code style issues
2. Replace legacy GPIO ABI with modern ABI
4. Add scaling factor for angular position and angular velocity to
   match the sysfs IIO ABI
5. Add documentation for device tree binding, and angle channel
6. Move driver to main line

Changes in v2:
- Removed an unneeded include
- Instead of using inclination channel for the angular position,
  use angle channel
- Added documentation for device tree binding, and sysfs IIO ABI
  documentation for angle channel

For the scaling factors, a fractional approximation of Pi is needed.
As in v1 of the patch, the fraction 103993 / 33102 was chosen, which is
based on the OEIS series of nominator/denominator of convergents to
Pi (A002485 and A002486). I have looked at the implementation of
iio_read_channel_info, and compared using the fraction above, with
the fraction and offset used in the IIO_DEGREE_TO_RAD in iio.h, and
found the first method to be more accurate for all possible 12-bit values,
as the error is neglible. Also it does not cause overflow.

In [2] Jonathan suggested to remove usage of platform data altogether,
and asked for Michael Hennerich opinion. Since Michael didn't weigh in,
I have left this how it was.

Best regards,
David Veenstra

[1] https://marc.info/?l=linux-iio&m=152137920426820&w=2
[2] https://marc.info/?l=linux-iio&m=152181140619243&w=2

David Veenstra (13):
  staging: iio: ad2s1200: Remove unneeded include
  staging: iio: ad2s1200: Sort includes alphabetically
  staging: iio: ad2s1200: Reverse Christmas tree ordering
  staging: iio: ad2s1200: Add blank lines
  staging: iio: ad2s1200: Add kernel docs to driver state
  staging: iio: ad2s1200: Introduce variable for repeated value
  staging: iio: ad2s1200: Improve readability with be16_to_cpup
  staging: iio: ad2s1200: Replace legacy gpio API with modern API
  staging: iio: ad2s1200: Add documentation for device tree binding
  staging: iio: ad2s1200: Add scaling factor for angular velocity
    channel
  staging: iio: Documentation: Add missing sysfs docs for angle channel
  staging: iio: ad2s1200: Add scaling factor for angle channel
  staging: iio: ad2s1200: Move driver out of staging

 Documentation/ABI/testing/sysfs-bus-iio            |  11 +
 .../devicetree/bindings/iio/resolver/ad2s1200.txt  |  16 ++
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/resolver/Kconfig                       |  17 ++
 drivers/iio/resolver/Makefile                      |   5 +
 drivers/iio/resolver/ad2s1200.c                    | 250 +++++++++++++++++++++
 drivers/staging/iio/resolver/Kconfig               |  12 -
 drivers/staging/iio/resolver/Makefile              |   1 -
 drivers/staging/iio/resolver/ad2s1200.c            | 165 --------------
 10 files changed, 301 insertions(+), 178 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt
 create mode 100644 drivers/iio/resolver/Kconfig
 create mode 100644 drivers/iio/resolver/Makefile
 create mode 100644 drivers/iio/resolver/ad2s1200.c
 delete mode 100644 drivers/staging/iio/resolver/ad2s1200.c

-- 
2.16.2

[PATCH v2 01/13] staging: iio: ad2s1200: Remove unneeded include

[David Veenstra]

This patches removes unneeded slab.h header.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
Changes in v2:
  - Introduced in this version.

drivers/staging/iio/resolver/ad2s1200.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index aa62c64e9bc4..5d7ed0034422 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -13,7 +13,6 @@
 #include <linux/mutex.h>
 #include <linux/device.h>
 #include <linux/spi/spi.h>
-#include <linux/slab.h>
 #include <linux/sysfs.h>
 #include <linux/delay.h>
 #include <linux/gpio.h>
-- 
2.16.2

[PATCH v2 01/13] staging: iio: ad2s1200: Remove unneeded include

[David Veenstra]

This patches removes unneeded slab.h header.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
Changes in v2:
  - Introduced in this version.

drivers/staging/iio/resolver/ad2s1200.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index aa62c64e9bc4..5d7ed0034422 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -13,7 +13,6 @@
 #include <linux/mutex.h>
 #include <linux/device.h>
 #include <linux/spi/spi.h>
-#include <linux/slab.h>
 #include <linux/sysfs.h>
 #include <linux/delay.h>
 #include <linux/gpio.h>
-- 
2.16.2

[PATCH v2 02/13] staging: iio: ad2s1200: Sort includes alphabetically

[David Veenstra]

This patches sorts all the includes in alphabetic order.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
 drivers/staging/iio/resolver/ad2s1200.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index 5d7ed0034422..ffcdf4e8eb92 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -9,15 +9,15 @@
  * published by the Free Software Foundation.
  *
  */
-#include <linux/types.h>
-#include <linux/mutex.h>
-#include <linux/device.h>
-#include <linux/spi/spi.h>
-#include <linux/sysfs.h>
+#include <linux/bitops.h>
 #include <linux/delay.h>
+#include <linux/device.h>
 #include <linux/gpio.h>
 #include <linux/module.h>
-#include <linux/bitops.h>
+#include <linux/mutex.h>
+#include <linux/spi/spi.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
-- 
2.16.2

[PATCH v2 02/13] staging: iio: ad2s1200: Sort includes alphabetically

[David Veenstra]

This patches sorts all the includes in alphabetic order.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
 drivers/staging/iio/resolver/ad2s1200.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index 5d7ed0034422..ffcdf4e8eb92 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -9,15 +9,15 @@
  * published by the Free Software Foundation.
  *
  */
-#include <linux/types.h>
-#include <linux/mutex.h>
-#include <linux/device.h>
-#include <linux/spi/spi.h>
-#include <linux/sysfs.h>
+#include <linux/bitops.h>
 #include <linux/delay.h>
+#include <linux/device.h>
 #include <linux/gpio.h>
 #include <linux/module.h>
-#include <linux/bitops.h>
+#include <linux/mutex.h>
+#include <linux/spi/spi.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
-- 
2.16.2

[PATCH v2 03/13] staging: iio: ad2s1200: Reverse Christmas tree ordering

[David Veenstra]

Reorders the variable declarations to prefer a reverse Christmas tree
order to improve readability.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
 drivers/staging/iio/resolver/ad2s1200.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index ffcdf4e8eb92..b6c3a3c8f7fe 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -46,9 +46,9 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 			     int *val2,
 			     long m)
 {
+	struct ad2s1200_state *st = iio_priv(indio_dev);
 	int ret = 0;
 	s16 vel;
-	struct ad2s1200_state *st = iio_priv(indio_dev);
 
 	mutex_lock(&st->lock);
 	gpio_set_value(st->sample, 0);
@@ -101,10 +101,10 @@ static const struct iio_info ad2s1200_info = {
 
 static int ad2s1200_probe(struct spi_device *spi)
 {
+	unsigned short *pins = spi->dev.platform_data;
 	struct ad2s1200_state *st;
 	struct iio_dev *indio_dev;
 	int pn, ret = 0;
-	unsigned short *pins = spi->dev.platform_data;
 
 	for (pn = 0; pn < AD2S1200_PN; pn++) {
 		ret = devm_gpio_request_one(&spi->dev, pins[pn], GPIOF_DIR_OUT,
-- 
2.16.2

[PATCH v2 03/13] staging: iio: ad2s1200: Reverse Christmas tree ordering

[David Veenstra]

Reorders the variable declarations to prefer a reverse Christmas tree
order to improve readability.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
 drivers/staging/iio/resolver/ad2s1200.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index ffcdf4e8eb92..b6c3a3c8f7fe 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -46,9 +46,9 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 			     int *val2,
 			     long m)
 {
+	struct ad2s1200_state *st = iio_priv(indio_dev);
 	int ret = 0;
 	s16 vel;
-	struct ad2s1200_state *st = iio_priv(indio_dev);
 
 	mutex_lock(&st->lock);
 	gpio_set_value(st->sample, 0);
@@ -101,10 +101,10 @@ static const struct iio_info ad2s1200_info = {
 
 static int ad2s1200_probe(struct spi_device *spi)
 {
+	unsigned short *pins = spi->dev.platform_data;
 	struct ad2s1200_state *st;
 	struct iio_dev *indio_dev;
 	int pn, ret = 0;
-	unsigned short *pins = spi->dev.platform_data;
 
 	for (pn = 0; pn < AD2S1200_PN; pn++) {
 		ret = devm_gpio_request_one(&spi->dev, pins[pn], GPIOF_DIR_OUT,
-- 
2.16.2

[PATCH v2 04/13] staging: iio: ad2s1200: Add blank lines

[David Veenstra]

Add blank lines to improve readability.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
 drivers/staging/iio/resolver/ad2s1200.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index b6c3a3c8f7fe..357fe3c382b3 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -9,6 +9,7 @@
  * published by the Free Software Foundation.
  *
  */
+
 #include <linux/bitops.h>
 #include <linux/delay.h>
 #include <linux/device.h>
@@ -52,10 +53,12 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 
 	mutex_lock(&st->lock);
 	gpio_set_value(st->sample, 0);
+
 	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
 	udelay(1);
 	gpio_set_value(st->sample, 1);
 	gpio_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
+
 	ret = spi_read(st->sdev, st->rx, 2);
 	if (ret < 0) {
 		mutex_unlock(&st->lock);
@@ -75,9 +78,11 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 		mutex_unlock(&st->lock);
 		return -EINVAL;
 	}
+
 	/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
 	udelay(1);
 	mutex_unlock(&st->lock);
+
 	return IIO_VAL_INT;
 }
 
@@ -115,9 +120,11 @@ static int ad2s1200_probe(struct spi_device *spi)
 			return ret;
 		}
 	}
+
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
 	if (!indio_dev)
 		return -ENOMEM;
+
 	spi_set_drvdata(spi, indio_dev);
 	st = iio_priv(indio_dev);
 	mutex_init(&st->lock);
-- 
2.16.2

[PATCH v2 04/13] staging: iio: ad2s1200: Add blank lines

[David Veenstra]

Add blank lines to improve readability.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
 drivers/staging/iio/resolver/ad2s1200.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index b6c3a3c8f7fe..357fe3c382b3 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -9,6 +9,7 @@
  * published by the Free Software Foundation.
  *
  */
+
 #include <linux/bitops.h>
 #include <linux/delay.h>
 #include <linux/device.h>
@@ -52,10 +53,12 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 
 	mutex_lock(&st->lock);
 	gpio_set_value(st->sample, 0);
+
 	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
 	udelay(1);
 	gpio_set_value(st->sample, 1);
 	gpio_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
+
 	ret = spi_read(st->sdev, st->rx, 2);
 	if (ret < 0) {
 		mutex_unlock(&st->lock);
@@ -75,9 +78,11 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 		mutex_unlock(&st->lock);
 		return -EINVAL;
 	}
+
 	/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
 	udelay(1);
 	mutex_unlock(&st->lock);
+
 	return IIO_VAL_INT;
 }
 
@@ -115,9 +120,11 @@ static int ad2s1200_probe(struct spi_device *spi)
 			return ret;
 		}
 	}
+
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
 	if (!indio_dev)
 		return -ENOMEM;
+
 	spi_set_drvdata(spi, indio_dev);
 	st = iio_priv(indio_dev);
 	mutex_init(&st->lock);
-- 
2.16.2

[PATCH v2 05/13] staging: iio: ad2s1200: Add kernel docs to driver state

[David Veenstra]

Add missing kernel docs to the ad2s1200 driver state.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
 drivers/staging/iio/resolver/ad2s1200.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index 357fe3c382b3..f07aab7e7a35 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -33,6 +33,14 @@
 /* clock period in nano second */
 #define AD2S1200_TSCLK	(1000000000 / AD2S1200_HZ)
 
+/**
+ * struct ad2s1200_state - driver instance specific data
+ * @lock:	protect driver state
+ * @sdev:	spi device
+ * @sample:	GPIO pin SAMPLE
+ * @rdvel:	GPIO pin RDVEL
+ * @rx:		buffer for spi transfers
+ */
 struct ad2s1200_state {
 	struct mutex lock;
 	struct spi_device *sdev;
-- 
2.16.2

[PATCH v2 05/13] staging: iio: ad2s1200: Add kernel docs to driver state

[David Veenstra]

Add missing kernel docs to the ad2s1200 driver state.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
 drivers/staging/iio/resolver/ad2s1200.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index 357fe3c382b3..f07aab7e7a35 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -33,6 +33,14 @@
 /* clock period in nano second */
 #define AD2S1200_TSCLK	(1000000000 / AD2S1200_HZ)
 
+/**
+ * struct ad2s1200_state - driver instance specific data
+ * @lock:	protect driver state
+ * @sdev:	spi device
+ * @sample:	GPIO pin SAMPLE
+ * @rdvel:	GPIO pin RDVEL
+ * @rx:		buffer for spi transfers
+ */
 struct ad2s1200_state {
 	struct mutex lock;
 	struct spi_device *sdev;
-- 
2.16.2

[PATCH v2 06/13] staging: iio: ad2s1200: Introduce variable for repeated value

[David Veenstra]

Add variable to hold &spi->dev in ad2s1200_probe. This value is repeatedly
used in ad2s1200_probe.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
 drivers/staging/iio/resolver/ad2s1200.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index f07aab7e7a35..0a5fc9917e32 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -117,19 +117,22 @@ static int ad2s1200_probe(struct spi_device *spi)
 	unsigned short *pins = spi->dev.platform_data;
 	struct ad2s1200_state *st;
 	struct iio_dev *indio_dev;
+	struct device *dev;
 	int pn, ret = 0;
 
+	dev = &spi->dev;
+
 	for (pn = 0; pn < AD2S1200_PN; pn++) {
-		ret = devm_gpio_request_one(&spi->dev, pins[pn], GPIOF_DIR_OUT,
+		ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
 					    DRV_NAME);
 		if (ret) {
-			dev_err(&spi->dev, "request gpio pin %d failed\n",
+			dev_err(dev, "request gpio pin %d failed\n",
 				pins[pn]);
 			return ret;
 		}
 	}
 
-	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
 	if (!indio_dev)
 		return -ENOMEM;
 
@@ -140,14 +143,14 @@ static int ad2s1200_probe(struct spi_device *spi)
 	st->sample = pins[0];
 	st->rdvel = pins[1];
 
-	indio_dev->dev.parent = &spi->dev;
+	indio_dev->dev.parent = dev;
 	indio_dev->info = &ad2s1200_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = ad2s1200_channels;
 	indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
 	indio_dev->name = spi_get_device_id(spi)->name;
 
-	ret = devm_iio_device_register(&spi->dev, indio_dev);
+	ret = devm_iio_device_register(dev, indio_dev);
 	if (ret)
 		return ret;
 
-- 
2.16.2

[PATCH v2 06/13] staging: iio: ad2s1200: Introduce variable for repeated value

[David Veenstra]

Add variable to hold &spi->dev in ad2s1200_probe. This value is repeatedly
used in ad2s1200_probe.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
 drivers/staging/iio/resolver/ad2s1200.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index f07aab7e7a35..0a5fc9917e32 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -117,19 +117,22 @@ static int ad2s1200_probe(struct spi_device *spi)
 	unsigned short *pins = spi->dev.platform_data;
 	struct ad2s1200_state *st;
 	struct iio_dev *indio_dev;
+	struct device *dev;
 	int pn, ret = 0;
 
+	dev = &spi->dev;
+
 	for (pn = 0; pn < AD2S1200_PN; pn++) {
-		ret = devm_gpio_request_one(&spi->dev, pins[pn], GPIOF_DIR_OUT,
+		ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
 					    DRV_NAME);
 		if (ret) {
-			dev_err(&spi->dev, "request gpio pin %d failed\n",
+			dev_err(dev, "request gpio pin %d failed\n",
 				pins[pn]);
 			return ret;
 		}
 	}
 
-	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
 	if (!indio_dev)
 		return -ENOMEM;
 
@@ -140,14 +143,14 @@ static int ad2s1200_probe(struct spi_device *spi)
 	st->sample = pins[0];
 	st->rdvel = pins[1];
 
-	indio_dev->dev.parent = &spi->dev;
+	indio_dev->dev.parent = dev;
 	indio_dev->info = &ad2s1200_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = ad2s1200_channels;
 	indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
 	indio_dev->name = spi_get_device_id(spi)->name;
 
-	ret = devm_iio_device_register(&spi->dev, indio_dev);
+	ret = devm_iio_device_register(dev, indio_dev);
 	if (ret)
 		return ret;
 
-- 
2.16.2

[PATCH v2 07/13] staging: iio: ad2s1200: Improve readability with be16_to_cpup

[David Veenstra]

The manual states that the data is contained in the upper 12 bits
of the 16 bits read by spi. The code that extracts these 12 bits
is correct for both be and le machines, but this is not clear
from a first glance.

To improve readability the relevant expressions are replaced
with equivalent expressions that use be16_to_cpup.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
 drivers/staging/iio/resolver/ad2s1200.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index 0a5fc9917e32..11ed9c7332e6 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -57,7 +57,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 {
 	struct ad2s1200_state *st = iio_priv(indio_dev);
 	int ret = 0;
-	s16 vel;
+	u16 vel;
 
 	mutex_lock(&st->lock);
 	gpio_set_value(st->sample, 0);
@@ -73,14 +73,13 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 		return ret;
 	}
 
+	vel = be16_to_cpup((__be16 *)st->rx);
 	switch (chan->type) {
 	case IIO_ANGL:
-		*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
+		*val = vel >> 4;
 		break;
 	case IIO_ANGL_VEL:
-		vel = (((s16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-		vel = sign_extend32(vel, 11);
-		*val = vel;
+		*val = sign_extend32((s16)vel >> 4, 11);
 		break;
 	default:
 		mutex_unlock(&st->lock);
-- 
2.16.2

[PATCH v2 07/13] staging: iio: ad2s1200: Improve readability with be16_to_cpup

[David Veenstra]

The manual states that the data is contained in the upper 12 bits
of the 16 bits read by spi. The code that extracts these 12 bits
is correct for both be and le machines, but this is not clear
from a first glance.

To improve readability the relevant expressions are replaced
with equivalent expressions that use be16_to_cpup.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
 drivers/staging/iio/resolver/ad2s1200.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index 0a5fc9917e32..11ed9c7332e6 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -57,7 +57,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 {
 	struct ad2s1200_state *st = iio_priv(indio_dev);
 	int ret = 0;
-	s16 vel;
+	u16 vel;
 
 	mutex_lock(&st->lock);
 	gpio_set_value(st->sample, 0);
@@ -73,14 +73,13 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 		return ret;
 	}
 
+	vel = be16_to_cpup((__be16 *)st->rx);
 	switch (chan->type) {
 	case IIO_ANGL:
-		*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
+		*val = vel >> 4;
 		break;
 	case IIO_ANGL_VEL:
-		vel = (((s16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-		vel = sign_extend32(vel, 11);
-		*val = vel;
+		*val = sign_extend32((s16)vel >> 4, 11);
 		break;
 	default:
 		mutex_unlock(&st->lock);
-- 
2.16.2

[PATCH v2 08/13] staging: iio: ad2s1200: Replace legacy gpio API with modern API

[David Veenstra]

The legacy, integer based gpio API is replaced with the descriptor
based API.

For compatibility, it is first tried to use the platform data to
request the gpio's. Otherwise, it looks for the "sample" and "rdvel"
gpio function.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
 drivers/staging/iio/resolver/ad2s1200.c | 51 ++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index 11ed9c7332e6..29a9bb666e7b 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -14,6 +14,7 @@
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/spi/spi.h>
@@ -44,8 +45,8 @@
 struct ad2s1200_state {
 	struct mutex lock;
 	struct spi_device *sdev;
-	int sample;
-	int rdvel;
+	struct gpio_desc *sample;
+	struct gpio_desc *rdvel;
 	u8 rx[2] ____cacheline_aligned;
 };
 
@@ -60,12 +61,12 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 	u16 vel;
 
 	mutex_lock(&st->lock);
-	gpio_set_value(st->sample, 0);
+	gpiod_set_value(st->sample, 0);
 
 	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
 	udelay(1);
-	gpio_set_value(st->sample, 1);
-	gpio_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
+	gpiod_set_value(st->sample, 1);
+	gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
 
 	ret = spi_read(st->sdev, st->rx, 2);
 	if (ret < 0) {
@@ -121,13 +122,18 @@ static int ad2s1200_probe(struct spi_device *spi)
 
 	dev = &spi->dev;
 
-	for (pn = 0; pn < AD2S1200_PN; pn++) {
-		ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
-					    DRV_NAME);
-		if (ret) {
-			dev_err(dev, "request gpio pin %d failed\n",
-				pins[pn]);
-			return ret;
+	if (pins) {
+		for (pn = 0; pn < AD2S1200_PN; pn++) {
+			ret = devm_gpio_request_one(dev, pins[pn],
+						    GPIOF_DIR_OUT,
+						    DRV_NAME);
+			if (ret) {
+				dev_err(dev,
+					"Failed to claim gpio %d\n: err=%d",
+					pins[pn],
+					ret);
+				return ret;
+			}
 		}
 	}
 
@@ -139,8 +145,25 @@ static int ad2s1200_probe(struct spi_device *spi)
 	st = iio_priv(indio_dev);
 	mutex_init(&st->lock);
 	st->sdev = spi;
-	st->sample = pins[0];
-	st->rdvel = pins[1];
+
+	if (pins) {
+		st->sample = gpio_to_desc(pins[0]);
+		st->rdvel = gpio_to_desc(pins[1]);
+	} else {
+		st->sample = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
+		if (IS_ERR(st->sample)) {
+			dev_err(dev, "Failed to claim SAMPLE gpio: err=%ld\n",
+				PTR_ERR(st->sample));
+			return PTR_ERR(st->sample);
+		}
+
+		st->rdvel = devm_gpiod_get(dev, "rdvel", GPIOD_OUT_LOW);
+		if (IS_ERR(st->rdvel)) {
+			dev_err(dev, "Failed to claim RDVEL gpio: err=%ld\n",
+				PTR_ERR(st->rdvel));
+			return PTR_ERR(st->rdvel);
+		}
+	}
 
 	indio_dev->dev.parent = dev;
 	indio_dev->info = &ad2s1200_info;
-- 
2.16.2

[PATCH v2 08/13] staging: iio: ad2s1200: Replace legacy gpio API with modern API

[David Veenstra]

The legacy, integer based gpio API is replaced with the descriptor
based API.

For compatibility, it is first tried to use the platform data to
request the gpio's. Otherwise, it looks for the "sample" and "rdvel"
gpio function.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
 drivers/staging/iio/resolver/ad2s1200.c | 51 ++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index 11ed9c7332e6..29a9bb666e7b 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -14,6 +14,7 @@
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/spi/spi.h>
@@ -44,8 +45,8 @@
 struct ad2s1200_state {
 	struct mutex lock;
 	struct spi_device *sdev;
-	int sample;
-	int rdvel;
+	struct gpio_desc *sample;
+	struct gpio_desc *rdvel;
 	u8 rx[2] ____cacheline_aligned;
 };
 
@@ -60,12 +61,12 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 	u16 vel;
 
 	mutex_lock(&st->lock);
-	gpio_set_value(st->sample, 0);
+	gpiod_set_value(st->sample, 0);
 
 	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
 	udelay(1);
-	gpio_set_value(st->sample, 1);
-	gpio_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
+	gpiod_set_value(st->sample, 1);
+	gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
 
 	ret = spi_read(st->sdev, st->rx, 2);
 	if (ret < 0) {
@@ -121,13 +122,18 @@ static int ad2s1200_probe(struct spi_device *spi)
 
 	dev = &spi->dev;
 
-	for (pn = 0; pn < AD2S1200_PN; pn++) {
-		ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
-					    DRV_NAME);
-		if (ret) {
-			dev_err(dev, "request gpio pin %d failed\n",
-				pins[pn]);
-			return ret;
+	if (pins) {
+		for (pn = 0; pn < AD2S1200_PN; pn++) {
+			ret = devm_gpio_request_one(dev, pins[pn],
+						    GPIOF_DIR_OUT,
+						    DRV_NAME);
+			if (ret) {
+				dev_err(dev,
+					"Failed to claim gpio %d\n: err=%d",
+					pins[pn],
+					ret);
+				return ret;
+			}
 		}
 	}
 
@@ -139,8 +145,25 @@ static int ad2s1200_probe(struct spi_device *spi)
 	st = iio_priv(indio_dev);
 	mutex_init(&st->lock);
 	st->sdev = spi;
-	st->sample = pins[0];
-	st->rdvel = pins[1];
+
+	if (pins) {
+		st->sample = gpio_to_desc(pins[0]);
+		st->rdvel = gpio_to_desc(pins[1]);
+	} else {
+		st->sample = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
+		if (IS_ERR(st->sample)) {
+			dev_err(dev, "Failed to claim SAMPLE gpio: err=%ld\n",
+				PTR_ERR(st->sample));
+			return PTR_ERR(st->sample);
+		}
+
+		st->rdvel = devm_gpiod_get(dev, "rdvel", GPIOD_OUT_LOW);
+		if (IS_ERR(st->rdvel)) {
+			dev_err(dev, "Failed to claim RDVEL gpio: err=%ld\n",
+				PTR_ERR(st->rdvel));
+			return PTR_ERR(st->rdvel);
+		}
+	}
 
 	indio_dev->dev.parent = dev;
 	indio_dev->info = &ad2s1200_info;
-- 
2.16.2

[PATCH v2 09/13] staging: iio: ad2s1200: Add documentation for device tree binding

[David Veenstra]

Add documentation for the added device tree bindings.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
Changes in v2:
  - Introduced in this version.

 .../bindings/staging/iio/resolver/ad2s1200.txt           | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt

diff --git a/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt b/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
new file mode 100644
index 000000000000..85c009987878
--- /dev/null
+++ b/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
@@ -0,0 +1,16 @@
+Analog Devices AD2S1200 Resolver-to-Digital Converter
+
+Required properties:
+ - compatible : should be "adi,ad2s1200"
+ - reg : the SPI chip select number of the device
+ - sample-gpios : The GPIO pin connected to the SAMPLE line of the AD2S1200
+ - rdvel-gpios : The GPIO pin connected to the RDVEL line of the AD2S1200
+
+Example:
+
+	resolver {
+		compatible = "adi,ad2s1200";
+		reg = <4>;
+		sample-gpios = <&gpio 5 GPIO_ACTIVE_HIGH>;
+		rdvel-gpios = <&gpio 6 GPIO_ACTIVE_HIGH>;
+	};
-- 
2.16.2

[PATCH v2 09/13] staging: iio: ad2s1200: Add documentation for device tree binding

[David Veenstra]

Add documentation for the added device tree bindings.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
Changes in v2:
  - Introduced in this version.

 .../bindings/staging/iio/resolver/ad2s1200.txt           | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt

diff --git a/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt b/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
new file mode 100644
index 000000000000..85c009987878
--- /dev/null
+++ b/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
@@ -0,0 +1,16 @@
+Analog Devices AD2S1200 Resolver-to-Digital Converter
+
+Required properties:
+ - compatible : should be "adi,ad2s1200"
+ - reg : the SPI chip select number of the device
+ - sample-gpios : The GPIO pin connected to the SAMPLE line of the AD2S1200
+ - rdvel-gpios : The GPIO pin connected to the RDVEL line of the AD2S1200
+
+Example:
+
+	resolver {
+		compatible = "adi,ad2s1200";
+		reg = <4>;
+		sample-gpios = <&gpio 5 GPIO_ACTIVE_HIGH>;
+		rdvel-gpios = <&gpio 6 GPIO_ACTIVE_HIGH>;
+	};
-- 
2.16.2

[PATCH v2 10/13] staging: iio: ad2s1200: Add scaling factor for angular velocity channel

[David Veenstra]

The sysfs iio ABI states radians per second is expected as the unit for
angular velocity, but the 12-bit angular velocity register has rps
as its unit. So a fractional scaling factor of approximately 2 * Pi is
added to the angular velocity channel.

The added comments will also be relevant for the scaling factor of
the angle channel.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
Changes in v2:
  - Move explanation of Pi approximation to top of switch statement,
    as this will also be relevant to angle channel.
  - Replaced 33102 / 2 with 16551 on line 84.

 drivers/staging/iio/resolver/ad2s1200.c | 84 +++++++++++++++++++++++----------
 1 file changed, 59 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index 29a9bb666e7b..6c56257be3b1 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -60,38 +60,71 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 	int ret = 0;
 	u16 vel;
 
-	mutex_lock(&st->lock);
-	gpiod_set_value(st->sample, 0);
+	/*
+	 * Below a fractional approximation of Pi is needed.
+	 * The following approximation will be used: 103993 / 33102.
+	 * This is accurate in 9 decimals places.
+	 *
+	 * This fraction is based on OEIS series of nominator/denominator
+	 * of convergents to Pi (A002485 and A002486).
+	 */
+	switch (m) {
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_ANGL_VEL:
+			/*
+			 * 2 * Pi ~= 2 * 103993 / 33102
+			 *
+			 * iio_convert_raw_to_processed uses integer
+			 * division. This will cause at most 5% error
+			 * (for very small values). But for 99.5% of the values
+			 * it will cause less that 1% error.
+			 */
+			*val = 103993;
+			*val2 = 16551;
+			return IIO_VAL_FRACTIONAL;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&st->lock);
+		gpiod_set_value(st->sample, 0);
+
+		/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
+		udelay(1);
+		gpiod_set_value(st->sample, 1);
+		gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
+
+		ret = spi_read(st->sdev, st->rx, 2);
+		if (ret < 0) {
+			mutex_unlock(&st->lock);
+			return ret;
+		}
 
-	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
-	udelay(1);
-	gpiod_set_value(st->sample, 1);
-	gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
+		vel = be16_to_cpup((__be16 *)st->rx);
+		switch (chan->type) {
+		case IIO_ANGL:
+			*val = vel >> 4;
+			break;
+		case IIO_ANGL_VEL:
+			*val = sign_extend32((s16)vel >> 4, 11);
+			break;
+		default:
+			mutex_unlock(&st->lock);
+			return -EINVAL;
+		}
 
-	ret = spi_read(st->sdev, st->rx, 2);
-	if (ret < 0) {
+		/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
+		udelay(1);
 		mutex_unlock(&st->lock);
-		return ret;
-	}
 
-	vel = be16_to_cpup((__be16 *)st->rx);
-	switch (chan->type) {
-	case IIO_ANGL:
-		*val = vel >> 4;
-		break;
-	case IIO_ANGL_VEL:
-		*val = sign_extend32((s16)vel >> 4, 11);
-		break;
+		return IIO_VAL_INT;
 	default:
-		mutex_unlock(&st->lock);
-		return -EINVAL;
+		break;
 	}
 
-	/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
-	udelay(1);
-	mutex_unlock(&st->lock);
-
-	return IIO_VAL_INT;
+	return -EINVAL;
 }
 
 static const struct iio_chan_spec ad2s1200_channels[] = {
@@ -105,6 +138,7 @@ static const struct iio_chan_spec ad2s1200_channels[] = {
 		.indexed = 1,
 		.channel = 0,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
 	}
 };
 
-- 
2.16.2

[PATCH v2 10/13] staging: iio: ad2s1200: Add scaling factor for angular velocity channel

[David Veenstra]

The sysfs iio ABI states radians per second is expected as the unit for
angular velocity, but the 12-bit angular velocity register has rps
as its unit. So a fractional scaling factor of approximately 2 * Pi is
added to the angular velocity channel.

The added comments will also be relevant for the scaling factor of
the angle channel.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
Changes in v2:
  - Move explanation of Pi approximation to top of switch statement,
    as this will also be relevant to angle channel.
  - Replaced 33102 / 2 with 16551 on line 84.

 drivers/staging/iio/resolver/ad2s1200.c | 84 +++++++++++++++++++++++----------
 1 file changed, 59 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index 29a9bb666e7b..6c56257be3b1 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -60,38 +60,71 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 	int ret = 0;
 	u16 vel;
 
-	mutex_lock(&st->lock);
-	gpiod_set_value(st->sample, 0);
+	/*
+	 * Below a fractional approximation of Pi is needed.
+	 * The following approximation will be used: 103993 / 33102.
+	 * This is accurate in 9 decimals places.
+	 *
+	 * This fraction is based on OEIS series of nominator/denominator
+	 * of convergents to Pi (A002485 and A002486).
+	 */
+	switch (m) {
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_ANGL_VEL:
+			/*
+			 * 2 * Pi ~= 2 * 103993 / 33102
+			 *
+			 * iio_convert_raw_to_processed uses integer
+			 * division. This will cause at most 5% error
+			 * (for very small values). But for 99.5% of the values
+			 * it will cause less that 1% error.
+			 */
+			*val = 103993;
+			*val2 = 16551;
+			return IIO_VAL_FRACTIONAL;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&st->lock);
+		gpiod_set_value(st->sample, 0);
+
+		/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
+		udelay(1);
+		gpiod_set_value(st->sample, 1);
+		gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
+
+		ret = spi_read(st->sdev, st->rx, 2);
+		if (ret < 0) {
+			mutex_unlock(&st->lock);
+			return ret;
+		}
 
-	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
-	udelay(1);
-	gpiod_set_value(st->sample, 1);
-	gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
+		vel = be16_to_cpup((__be16 *)st->rx);
+		switch (chan->type) {
+		case IIO_ANGL:
+			*val = vel >> 4;
+			break;
+		case IIO_ANGL_VEL:
+			*val = sign_extend32((s16)vel >> 4, 11);
+			break;
+		default:
+			mutex_unlock(&st->lock);
+			return -EINVAL;
+		}
 
-	ret = spi_read(st->sdev, st->rx, 2);
-	if (ret < 0) {
+		/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
+		udelay(1);
 		mutex_unlock(&st->lock);
-		return ret;
-	}
 
-	vel = be16_to_cpup((__be16 *)st->rx);
-	switch (chan->type) {
-	case IIO_ANGL:
-		*val = vel >> 4;
-		break;
-	case IIO_ANGL_VEL:
-		*val = sign_extend32((s16)vel >> 4, 11);
-		break;
+		return IIO_VAL_INT;
 	default:
-		mutex_unlock(&st->lock);
-		return -EINVAL;
+		break;
 	}
 
-	/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
-	udelay(1);
-	mutex_unlock(&st->lock);
-
-	return IIO_VAL_INT;
+	return -EINVAL;
 }
 
 static const struct iio_chan_spec ad2s1200_channels[] = {
@@ -105,6 +138,7 @@ static const struct iio_chan_spec ad2s1200_channels[] = {
 		.indexed = 1,
 		.channel = 0,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
 	}
 };
 
-- 
2.16.2

[PATCH v2 11/13] staging: iio: Documentation: Add missing sysfs docs for angle channel

[David Veenstra]

The iio resolver drivers in staging use angle channels. This patch
add missing documentation for this type of channel.

As was discussed in [1], radians is chosen as the unit, to match the
unit of angular velocity.

[1] https://marc.info/?l=linux-driver-devel&m=152190078308330&w=2

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
Change in v2:
  - Introduces in this version.

 Documentation/ABI/testing/sysfs-bus-iio | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 6a5f34b4d5b9..8ad0e55f99ee 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -190,6 +190,15 @@ Description:
 		but should match other such assignments on device).
 		Units after application of scale and offset are m/s^2.
 
+What:		/sys/bus/iio/devices/iio:deviceX/in_angl_x_raw
+What:		/sys/bus/iio/devices/iio:deviceX/in_angl_y_raw
+What:		/sys/bus/iio/devices/iio:deviceX/in_angl_z_raw
+KernelVersion:	4.17
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Angle about axis x, y or z (may be arbitrarily assigned). Units
+		after application of scale and offset are radians.
+
 What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_x_raw
 What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_y_raw
 What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_z_raw
@@ -297,6 +306,7 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_pressure_offset
 What:		/sys/bus/iio/devices/iio:deviceX/in_humidityrelative_offset
 What:		/sys/bus/iio/devices/iio:deviceX/in_magn_offset
 What:		/sys/bus/iio/devices/iio:deviceX/in_rot_offset
+What:		/sys/bus/iio/devices/iio:deviceX/in_angl_offset
 KernelVersion:	2.6.35
 Contact:	linux-iio@vger.kernel.org
 Description:
@@ -350,6 +360,7 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_humidityrelative_scale
 What:		/sys/bus/iio/devices/iio:deviceX/in_velocity_sqrt(x^2+y^2+z^2)_scale
 What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_scale
 What:		/sys/bus/iio/devices/iio:deviceX/in_countY_scale
+What:		/sys/bus/iio/devices/iio:deviceX/in_angl_scale
 KernelVersion:	2.6.35
 Contact:	linux-iio@vger.kernel.org
 Description:
-- 
2.16.2

[PATCH v2 11/13] staging: iio: Documentation: Add missing sysfs docs for angle channel

[David Veenstra]

The iio resolver drivers in staging use angle channels. This patch
add missing documentation for this type of channel.

As was discussed in [1], radians is chosen as the unit, to match the
unit of angular velocity.

[1] https://marc.info/?l=linux-driver-devel&m=152190078308330&w=2

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
Change in v2:
  - Introduces in this version.

 Documentation/ABI/testing/sysfs-bus-iio | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 6a5f34b4d5b9..8ad0e55f99ee 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -190,6 +190,15 @@ Description:
 		but should match other such assignments on device).
 		Units after application of scale and offset are m/s^2.
 
+What:		/sys/bus/iio/devices/iio:deviceX/in_angl_x_raw
+What:		/sys/bus/iio/devices/iio:deviceX/in_angl_y_raw
+What:		/sys/bus/iio/devices/iio:deviceX/in_angl_z_raw
+KernelVersion:	4.17
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Angle about axis x, y or z (may be arbitrarily assigned). Units
+		after application of scale and offset are radians.
+
 What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_x_raw
 What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_y_raw
 What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_z_raw
@@ -297,6 +306,7 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_pressure_offset
 What:		/sys/bus/iio/devices/iio:deviceX/in_humidityrelative_offset
 What:		/sys/bus/iio/devices/iio:deviceX/in_magn_offset
 What:		/sys/bus/iio/devices/iio:deviceX/in_rot_offset
+What:		/sys/bus/iio/devices/iio:deviceX/in_angl_offset
 KernelVersion:	2.6.35
 Contact:	linux-iio@vger.kernel.org
 Description:
@@ -350,6 +360,7 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_humidityrelative_scale
 What:		/sys/bus/iio/devices/iio:deviceX/in_velocity_sqrt(x^2+y^2+z^2)_scale
 What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_scale
 What:		/sys/bus/iio/devices/iio:deviceX/in_countY_scale
+What:		/sys/bus/iio/devices/iio:deviceX/in_angl_scale
 KernelVersion:	2.6.35
 Contact:	linux-iio@vger.kernel.org
 Description:
-- 
2.16.2

[PATCH v2 12/13] staging: iio: ad2s1200: Add scaling factor for angle channel

[David Veenstra]

A fractional scaling factor of approximately 2 * Pi / (2^12 -1) is added,
to scale the 12-bits angular position to radians.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
Changes in v2:
  - This patch replaces the patch that changed the
    the channel for angular position to inclination
    channel.

 drivers/staging/iio/resolver/ad2s1200.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index 6c56257be3b1..4f5dd28b174a 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -71,6 +71,17 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 	switch (m) {
 	case IIO_CHAN_INFO_SCALE:
 		switch (chan->type) {
+		case IIO_ANGL:
+			/*
+			 * 2 * Pi / (2^12 - 1) ~= 2 * 103993 / (33102 * 0xFFF)
+			 *
+			 * Since there only fit 3 whole radians in 360 degrees,
+			 * usage of iio_convert_raw_to_processed for this
+			 * channel will be highly inaccurate.
+			 */
+			*val = 103993;
+			*val2 = 16551 * 0xFFF;
+			return IIO_VAL_FRACTIONAL;
 		case IIO_ANGL_VEL:
 			/*
 			 * 2 * Pi ~= 2 * 103993 / 33102
@@ -133,6 +144,7 @@ static const struct iio_chan_spec ad2s1200_channels[] = {
 		.indexed = 1,
 		.channel = 0,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
 	}, {
 		.type = IIO_ANGL_VEL,
 		.indexed = 1,
-- 
2.16.2

[PATCH v2 12/13] staging: iio: ad2s1200: Add scaling factor for angle channel

[David Veenstra]

A fractional scaling factor of approximately 2 * Pi / (2^12 -1) is added,
to scale the 12-bits angular position to radians.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
Changes in v2:
  - This patch replaces the patch that changed the
    the channel for angular position to inclination
    channel.

 drivers/staging/iio/resolver/ad2s1200.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
index 6c56257be3b1..4f5dd28b174a 100644
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ b/drivers/staging/iio/resolver/ad2s1200.c
@@ -71,6 +71,17 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
 	switch (m) {
 	case IIO_CHAN_INFO_SCALE:
 		switch (chan->type) {
+		case IIO_ANGL:
+			/*
+			 * 2 * Pi / (2^12 - 1) ~= 2 * 103993 / (33102 * 0xFFF)
+			 *
+			 * Since there only fit 3 whole radians in 360 degrees,
+			 * usage of iio_convert_raw_to_processed for this
+			 * channel will be highly inaccurate.
+			 */
+			*val = 103993;
+			*val2 = 16551 * 0xFFF;
+			return IIO_VAL_FRACTIONAL;
 		case IIO_ANGL_VEL:
 			/*
 			 * 2 * Pi ~= 2 * 103993 / 33102
@@ -133,6 +144,7 @@ static const struct iio_chan_spec ad2s1200_channels[] = {
 		.indexed = 1,
 		.channel = 0,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
 	}, {
 		.type = IIO_ANGL_VEL,
 		.indexed = 1,
-- 
2.16.2

[PATCH v2 13/13] staging: iio: ad2s1200: Move driver out of staging

[David Veenstra]

Move the iio driver for the ad2s1200 resolver-to-digital
converter out of staging, into mainline iio subsystems.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
Changes in v2:
  - Added commit message.
  - Also move device tree binding documentation out of staging.
  - Disabled move detection.

 .../devicetree/bindings/iio/resolver/ad2s1200.txt  |  16 ++
 .../bindings/staging/iio/resolver/ad2s1200.txt     |  16 --
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/resolver/Kconfig                       |  17 ++
 drivers/iio/resolver/Makefile                      |   5 +
 drivers/iio/resolver/ad2s1200.c                    | 250 +++++++++++++++++++++
 drivers/staging/iio/resolver/Kconfig               |  12 -
 drivers/staging/iio/resolver/Makefile              |   1 -
 drivers/staging/iio/resolver/ad2s1200.c            | 250 ---------------------
 10 files changed, 290 insertions(+), 279 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt
 delete mode 100644 Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
 create mode 100644 drivers/iio/resolver/Kconfig
 create mode 100644 drivers/iio/resolver/Makefile
 create mode 100644 drivers/iio/resolver/ad2s1200.c
 delete mode 100644 drivers/staging/iio/resolver/ad2s1200.c

diff --git a/Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt b/Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt
new file mode 100644
index 000000000000..85c009987878
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt
@@ -0,0 +1,16 @@
+Analog Devices AD2S1200 Resolver-to-Digital Converter
+
+Required properties:
+ - compatible : should be "adi,ad2s1200"
+ - reg : the SPI chip select number of the device
+ - sample-gpios : The GPIO pin connected to the SAMPLE line of the AD2S1200
+ - rdvel-gpios : The GPIO pin connected to the RDVEL line of the AD2S1200
+
+Example:
+
+	resolver {
+		compatible = "adi,ad2s1200";
+		reg = <4>;
+		sample-gpios = <&gpio 5 GPIO_ACTIVE_HIGH>;
+		rdvel-gpios = <&gpio 6 GPIO_ACTIVE_HIGH>;
+	};
diff --git a/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt b/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
deleted file mode 100644
index 85c009987878..000000000000
--- a/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
+++ /dev/null
@@ -1,16 +0,0 @@
-Analog Devices AD2S1200 Resolver-to-Digital Converter
-
-Required properties:
- - compatible : should be "adi,ad2s1200"
- - reg : the SPI chip select number of the device
- - sample-gpios : The GPIO pin connected to the SAMPLE line of the AD2S1200
- - rdvel-gpios : The GPIO pin connected to the RDVEL line of the AD2S1200
-
-Example:
-
-	resolver {
-		compatible = "adi,ad2s1200";
-		reg = <4>;
-		sample-gpios = <&gpio 5 GPIO_ACTIVE_HIGH>;
-		rdvel-gpios = <&gpio 6 GPIO_ACTIVE_HIGH>;
-	};
diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index b3c8c6ef0dff..4bec3ccbf4a1 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -92,6 +92,7 @@ source "drivers/iio/potentiometer/Kconfig"
 source "drivers/iio/potentiostat/Kconfig"
 source "drivers/iio/pressure/Kconfig"
 source "drivers/iio/proximity/Kconfig"
+source "drivers/iio/resolver/Kconfig"
 source "drivers/iio/temperature/Kconfig"
 
 endif # IIO
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index b16b2e9ddc40..1865361b8714 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -35,5 +35,6 @@ obj-y += potentiometer/
 obj-y += potentiostat/
 obj-y += pressure/
 obj-y += proximity/
+obj-y += resolver/
 obj-y += temperature/
 obj-y += trigger/
diff --git a/drivers/iio/resolver/Kconfig b/drivers/iio/resolver/Kconfig
new file mode 100644
index 000000000000..2ced9f22aa70
--- /dev/null
+++ b/drivers/iio/resolver/Kconfig
@@ -0,0 +1,17 @@
+#
+# Resolver/Synchro drivers
+#
+menu "Resolver to digital converters"
+
+config AD2S1200
+	tristate "Analog Devices ad2s1200/ad2s1205 driver"
+	depends on SPI
+	depends on GPIOLIB || COMPILE_TEST
+	help
+	  Say yes here to build support for Analog Devices spi resolver
+	  to digital converters, ad2s1200 and ad2s1205, provides direct access
+	  via sysfs.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad2s1200.
+endmenu
diff --git a/drivers/iio/resolver/Makefile b/drivers/iio/resolver/Makefile
new file mode 100644
index 000000000000..4e1dccae07e7
--- /dev/null
+++ b/drivers/iio/resolver/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for Resolver/Synchro drivers
+#
+
+obj-$(CONFIG_AD2S1200) += ad2s1200.o
diff --git a/drivers/iio/resolver/ad2s1200.c b/drivers/iio/resolver/ad2s1200.c
new file mode 100644
index 000000000000..4f5dd28b174a
--- /dev/null
+++ b/drivers/iio/resolver/ad2s1200.c
@@ -0,0 +1,250 @@
+/*
+ * ad2s1200.c simple support for the ADI Resolver to Digital Converters:
+ * AD2S1200/1205
+ *
+ * Copyright (c) 2010-2010 Analog Devices Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/spi/spi.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#define DRV_NAME "ad2s1200"
+
+/* input pin sample and rdvel is controlled by driver */
+#define AD2S1200_PN	2
+
+/* input clock on serial interface */
+#define AD2S1200_HZ	8192000
+/* clock period in nano second */
+#define AD2S1200_TSCLK	(1000000000 / AD2S1200_HZ)
+
+/**
+ * struct ad2s1200_state - driver instance specific data
+ * @lock:	protect driver state
+ * @sdev:	spi device
+ * @sample:	GPIO pin SAMPLE
+ * @rdvel:	GPIO pin RDVEL
+ * @rx:		buffer for spi transfers
+ */
+struct ad2s1200_state {
+	struct mutex lock;
+	struct spi_device *sdev;
+	struct gpio_desc *sample;
+	struct gpio_desc *rdvel;
+	u8 rx[2] ____cacheline_aligned;
+};
+
+static int ad2s1200_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val,
+			     int *val2,
+			     long m)
+{
+	struct ad2s1200_state *st = iio_priv(indio_dev);
+	int ret = 0;
+	u16 vel;
+
+	/*
+	 * Below a fractional approximation of Pi is needed.
+	 * The following approximation will be used: 103993 / 33102.
+	 * This is accurate in 9 decimals places.
+	 *
+	 * This fraction is based on OEIS series of nominator/denominator
+	 * of convergents to Pi (A002485 and A002486).
+	 */
+	switch (m) {
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_ANGL:
+			/*
+			 * 2 * Pi / (2^12 - 1) ~= 2 * 103993 / (33102 * 0xFFF)
+			 *
+			 * Since there only fit 3 whole radians in 360 degrees,
+			 * usage of iio_convert_raw_to_processed for this
+			 * channel will be highly inaccurate.
+			 */
+			*val = 103993;
+			*val2 = 16551 * 0xFFF;
+			return IIO_VAL_FRACTIONAL;
+		case IIO_ANGL_VEL:
+			/*
+			 * 2 * Pi ~= 2 * 103993 / 33102
+			 *
+			 * iio_convert_raw_to_processed uses integer
+			 * division. This will cause at most 5% error
+			 * (for very small values). But for 99.5% of the values
+			 * it will cause less that 1% error.
+			 */
+			*val = 103993;
+			*val2 = 16551;
+			return IIO_VAL_FRACTIONAL;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&st->lock);
+		gpiod_set_value(st->sample, 0);
+
+		/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
+		udelay(1);
+		gpiod_set_value(st->sample, 1);
+		gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
+
+		ret = spi_read(st->sdev, st->rx, 2);
+		if (ret < 0) {
+			mutex_unlock(&st->lock);
+			return ret;
+		}
+
+		vel = be16_to_cpup((__be16 *)st->rx);
+		switch (chan->type) {
+		case IIO_ANGL:
+			*val = vel >> 4;
+			break;
+		case IIO_ANGL_VEL:
+			*val = sign_extend32((s16)vel >> 4, 11);
+			break;
+		default:
+			mutex_unlock(&st->lock);
+			return -EINVAL;
+		}
+
+		/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
+		udelay(1);
+		mutex_unlock(&st->lock);
+
+		return IIO_VAL_INT;
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_chan_spec ad2s1200_channels[] = {
+	{
+		.type = IIO_ANGL,
+		.indexed = 1,
+		.channel = 0,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+	}, {
+		.type = IIO_ANGL_VEL,
+		.indexed = 1,
+		.channel = 0,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+	}
+};
+
+static const struct iio_info ad2s1200_info = {
+	.read_raw = ad2s1200_read_raw,
+};
+
+static int ad2s1200_probe(struct spi_device *spi)
+{
+	unsigned short *pins = spi->dev.platform_data;
+	struct ad2s1200_state *st;
+	struct iio_dev *indio_dev;
+	struct device *dev;
+	int pn, ret = 0;
+
+	dev = &spi->dev;
+
+	if (pins) {
+		for (pn = 0; pn < AD2S1200_PN; pn++) {
+			ret = devm_gpio_request_one(dev, pins[pn],
+						    GPIOF_DIR_OUT,
+						    DRV_NAME);
+			if (ret) {
+				dev_err(dev,
+					"Failed to claim gpio %d\n: err=%d",
+					pins[pn],
+					ret);
+				return ret;
+			}
+		}
+	}
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	spi_set_drvdata(spi, indio_dev);
+	st = iio_priv(indio_dev);
+	mutex_init(&st->lock);
+	st->sdev = spi;
+
+	if (pins) {
+		st->sample = gpio_to_desc(pins[0]);
+		st->rdvel = gpio_to_desc(pins[1]);
+	} else {
+		st->sample = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
+		if (IS_ERR(st->sample)) {
+			dev_err(dev, "Failed to claim SAMPLE gpio: err=%ld\n",
+				PTR_ERR(st->sample));
+			return PTR_ERR(st->sample);
+		}
+
+		st->rdvel = devm_gpiod_get(dev, "rdvel", GPIOD_OUT_LOW);
+		if (IS_ERR(st->rdvel)) {
+			dev_err(dev, "Failed to claim RDVEL gpio: err=%ld\n",
+				PTR_ERR(st->rdvel));
+			return PTR_ERR(st->rdvel);
+		}
+	}
+
+	indio_dev->dev.parent = dev;
+	indio_dev->info = &ad2s1200_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = ad2s1200_channels;
+	indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
+	indio_dev->name = spi_get_device_id(spi)->name;
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret)
+		return ret;
+
+	spi->max_speed_hz = AD2S1200_HZ;
+	spi->mode = SPI_MODE_3;
+	spi_setup(spi);
+
+	return 0;
+}
+
+static const struct spi_device_id ad2s1200_id[] = {
+	{ "ad2s1200" },
+	{ "ad2s1205" },
+	{}
+};
+MODULE_DEVICE_TABLE(spi, ad2s1200_id);
+
+static struct spi_driver ad2s1200_driver = {
+	.driver = {
+		.name = DRV_NAME,
+	},
+	.probe = ad2s1200_probe,
+	.id_table = ad2s1200_id,
+};
+module_spi_driver(ad2s1200_driver);
+
+MODULE_AUTHOR("Graff Yang <graff.yang@gmail.com>");
+MODULE_DESCRIPTION("Analog Devices AD2S1200/1205 Resolver to Digital SPI driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/staging/iio/resolver/Kconfig b/drivers/staging/iio/resolver/Kconfig
index 1c7e2860d6b7..6a469ee6101f 100644
--- a/drivers/staging/iio/resolver/Kconfig
+++ b/drivers/staging/iio/resolver/Kconfig
@@ -13,18 +13,6 @@ config AD2S90
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad2s90.
 
-config AD2S1200
-	tristate "Analog Devices ad2s1200/ad2s1205 driver"
-	depends on SPI
-	depends on GPIOLIB || COMPILE_TEST
-	help
-	  Say yes here to build support for Analog Devices spi resolver
-	  to digital converters, ad2s1200 and ad2s1205, provides direct access
-	  via sysfs.
-
-	  To compile this driver as a module, choose M here: the
-	  module will be called ad2s1200.
-
 config AD2S1210
 	tristate "Analog Devices ad2s1210 driver"
 	depends on SPI
diff --git a/drivers/staging/iio/resolver/Makefile b/drivers/staging/iio/resolver/Makefile
index 14375e444ebf..8d901dc7500b 100644
--- a/drivers/staging/iio/resolver/Makefile
+++ b/drivers/staging/iio/resolver/Makefile
@@ -3,5 +3,4 @@
 #
 
 obj-$(CONFIG_AD2S90) += ad2s90.o
-obj-$(CONFIG_AD2S1200) += ad2s1200.o
 obj-$(CONFIG_AD2S1210) += ad2s1210.o
diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
deleted file mode 100644
index 4f5dd28b174a..000000000000
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ /dev/null
@@ -1,250 +0,0 @@
-/*
- * ad2s1200.c simple support for the ADI Resolver to Digital Converters:
- * AD2S1200/1205
- *
- * Copyright (c) 2010-2010 Analog Devices Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- */
-
-#include <linux/bitops.h>
-#include <linux/delay.h>
-#include <linux/device.h>
-#include <linux/gpio.h>
-#include <linux/gpio/consumer.h>
-#include <linux/module.h>
-#include <linux/mutex.h>
-#include <linux/spi/spi.h>
-#include <linux/sysfs.h>
-#include <linux/types.h>
-
-#include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
-
-#define DRV_NAME "ad2s1200"
-
-/* input pin sample and rdvel is controlled by driver */
-#define AD2S1200_PN	2
-
-/* input clock on serial interface */
-#define AD2S1200_HZ	8192000
-/* clock period in nano second */
-#define AD2S1200_TSCLK	(1000000000 / AD2S1200_HZ)
-
-/**
- * struct ad2s1200_state - driver instance specific data
- * @lock:	protect driver state
- * @sdev:	spi device
- * @sample:	GPIO pin SAMPLE
- * @rdvel:	GPIO pin RDVEL
- * @rx:		buffer for spi transfers
- */
-struct ad2s1200_state {
-	struct mutex lock;
-	struct spi_device *sdev;
-	struct gpio_desc *sample;
-	struct gpio_desc *rdvel;
-	u8 rx[2] ____cacheline_aligned;
-};
-
-static int ad2s1200_read_raw(struct iio_dev *indio_dev,
-			     struct iio_chan_spec const *chan,
-			     int *val,
-			     int *val2,
-			     long m)
-{
-	struct ad2s1200_state *st = iio_priv(indio_dev);
-	int ret = 0;
-	u16 vel;
-
-	/*
-	 * Below a fractional approximation of Pi is needed.
-	 * The following approximation will be used: 103993 / 33102.
-	 * This is accurate in 9 decimals places.
-	 *
-	 * This fraction is based on OEIS series of nominator/denominator
-	 * of convergents to Pi (A002485 and A002486).
-	 */
-	switch (m) {
-	case IIO_CHAN_INFO_SCALE:
-		switch (chan->type) {
-		case IIO_ANGL:
-			/*
-			 * 2 * Pi / (2^12 - 1) ~= 2 * 103993 / (33102 * 0xFFF)
-			 *
-			 * Since there only fit 3 whole radians in 360 degrees,
-			 * usage of iio_convert_raw_to_processed for this
-			 * channel will be highly inaccurate.
-			 */
-			*val = 103993;
-			*val2 = 16551 * 0xFFF;
-			return IIO_VAL_FRACTIONAL;
-		case IIO_ANGL_VEL:
-			/*
-			 * 2 * Pi ~= 2 * 103993 / 33102
-			 *
-			 * iio_convert_raw_to_processed uses integer
-			 * division. This will cause at most 5% error
-			 * (for very small values). But for 99.5% of the values
-			 * it will cause less that 1% error.
-			 */
-			*val = 103993;
-			*val2 = 16551;
-			return IIO_VAL_FRACTIONAL;
-		default:
-			return -EINVAL;
-		}
-		break;
-	case IIO_CHAN_INFO_RAW:
-		mutex_lock(&st->lock);
-		gpiod_set_value(st->sample, 0);
-
-		/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
-		udelay(1);
-		gpiod_set_value(st->sample, 1);
-		gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
-
-		ret = spi_read(st->sdev, st->rx, 2);
-		if (ret < 0) {
-			mutex_unlock(&st->lock);
-			return ret;
-		}
-
-		vel = be16_to_cpup((__be16 *)st->rx);
-		switch (chan->type) {
-		case IIO_ANGL:
-			*val = vel >> 4;
-			break;
-		case IIO_ANGL_VEL:
-			*val = sign_extend32((s16)vel >> 4, 11);
-			break;
-		default:
-			mutex_unlock(&st->lock);
-			return -EINVAL;
-		}
-
-		/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
-		udelay(1);
-		mutex_unlock(&st->lock);
-
-		return IIO_VAL_INT;
-	default:
-		break;
-	}
-
-	return -EINVAL;
-}
-
-static const struct iio_chan_spec ad2s1200_channels[] = {
-	{
-		.type = IIO_ANGL,
-		.indexed = 1,
-		.channel = 0,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
-	}, {
-		.type = IIO_ANGL_VEL,
-		.indexed = 1,
-		.channel = 0,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
-	}
-};
-
-static const struct iio_info ad2s1200_info = {
-	.read_raw = ad2s1200_read_raw,
-};
-
-static int ad2s1200_probe(struct spi_device *spi)
-{
-	unsigned short *pins = spi->dev.platform_data;
-	struct ad2s1200_state *st;
-	struct iio_dev *indio_dev;
-	struct device *dev;
-	int pn, ret = 0;
-
-	dev = &spi->dev;
-
-	if (pins) {
-		for (pn = 0; pn < AD2S1200_PN; pn++) {
-			ret = devm_gpio_request_one(dev, pins[pn],
-						    GPIOF_DIR_OUT,
-						    DRV_NAME);
-			if (ret) {
-				dev_err(dev,
-					"Failed to claim gpio %d\n: err=%d",
-					pins[pn],
-					ret);
-				return ret;
-			}
-		}
-	}
-
-	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
-	if (!indio_dev)
-		return -ENOMEM;
-
-	spi_set_drvdata(spi, indio_dev);
-	st = iio_priv(indio_dev);
-	mutex_init(&st->lock);
-	st->sdev = spi;
-
-	if (pins) {
-		st->sample = gpio_to_desc(pins[0]);
-		st->rdvel = gpio_to_desc(pins[1]);
-	} else {
-		st->sample = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
-		if (IS_ERR(st->sample)) {
-			dev_err(dev, "Failed to claim SAMPLE gpio: err=%ld\n",
-				PTR_ERR(st->sample));
-			return PTR_ERR(st->sample);
-		}
-
-		st->rdvel = devm_gpiod_get(dev, "rdvel", GPIOD_OUT_LOW);
-		if (IS_ERR(st->rdvel)) {
-			dev_err(dev, "Failed to claim RDVEL gpio: err=%ld\n",
-				PTR_ERR(st->rdvel));
-			return PTR_ERR(st->rdvel);
-		}
-	}
-
-	indio_dev->dev.parent = dev;
-	indio_dev->info = &ad2s1200_info;
-	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->channels = ad2s1200_channels;
-	indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
-	indio_dev->name = spi_get_device_id(spi)->name;
-
-	ret = devm_iio_device_register(dev, indio_dev);
-	if (ret)
-		return ret;
-
-	spi->max_speed_hz = AD2S1200_HZ;
-	spi->mode = SPI_MODE_3;
-	spi_setup(spi);
-
-	return 0;
-}
-
-static const struct spi_device_id ad2s1200_id[] = {
-	{ "ad2s1200" },
-	{ "ad2s1205" },
-	{}
-};
-MODULE_DEVICE_TABLE(spi, ad2s1200_id);
-
-static struct spi_driver ad2s1200_driver = {
-	.driver = {
-		.name = DRV_NAME,
-	},
-	.probe = ad2s1200_probe,
-	.id_table = ad2s1200_id,
-};
-module_spi_driver(ad2s1200_driver);
-
-MODULE_AUTHOR("Graff Yang <graff.yang@gmail.com>");
-MODULE_DESCRIPTION("Analog Devices AD2S1200/1205 Resolver to Digital SPI driver");
-MODULE_LICENSE("GPL v2");
-- 
2.16.2

[PATCH v2 13/13] staging: iio: ad2s1200: Move driver out of staging

[David Veenstra]

Move the iio driver for the ad2s1200 resolver-to-digital
converter out of staging, into mainline iio subsystems.

Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
---
Changes in v2:
  - Added commit message.
  - Also move device tree binding documentation out of staging.
  - Disabled move detection.

 .../devicetree/bindings/iio/resolver/ad2s1200.txt  |  16 ++
 .../bindings/staging/iio/resolver/ad2s1200.txt     |  16 --
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/resolver/Kconfig                       |  17 ++
 drivers/iio/resolver/Makefile                      |   5 +
 drivers/iio/resolver/ad2s1200.c                    | 250 +++++++++++++++++++++
 drivers/staging/iio/resolver/Kconfig               |  12 -
 drivers/staging/iio/resolver/Makefile              |   1 -
 drivers/staging/iio/resolver/ad2s1200.c            | 250 ---------------------
 10 files changed, 290 insertions(+), 279 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt
 delete mode 100644 Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
 create mode 100644 drivers/iio/resolver/Kconfig
 create mode 100644 drivers/iio/resolver/Makefile
 create mode 100644 drivers/iio/resolver/ad2s1200.c
 delete mode 100644 drivers/staging/iio/resolver/ad2s1200.c

diff --git a/Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt b/Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt
new file mode 100644
index 000000000000..85c009987878
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt
@@ -0,0 +1,16 @@
+Analog Devices AD2S1200 Resolver-to-Digital Converter
+
+Required properties:
+ - compatible : should be "adi,ad2s1200"
+ - reg : the SPI chip select number of the device
+ - sample-gpios : The GPIO pin connected to the SAMPLE line of the AD2S1200
+ - rdvel-gpios : The GPIO pin connected to the RDVEL line of the AD2S1200
+
+Example:
+
+	resolver {
+		compatible = "adi,ad2s1200";
+		reg = <4>;
+		sample-gpios = <&gpio 5 GPIO_ACTIVE_HIGH>;
+		rdvel-gpios = <&gpio 6 GPIO_ACTIVE_HIGH>;
+	};
diff --git a/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt b/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
deleted file mode 100644
index 85c009987878..000000000000
--- a/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
+++ /dev/null
@@ -1,16 +0,0 @@
-Analog Devices AD2S1200 Resolver-to-Digital Converter
-
-Required properties:
- - compatible : should be "adi,ad2s1200"
- - reg : the SPI chip select number of the device
- - sample-gpios : The GPIO pin connected to the SAMPLE line of the AD2S1200
- - rdvel-gpios : The GPIO pin connected to the RDVEL line of the AD2S1200
-
-Example:
-
-	resolver {
-		compatible = "adi,ad2s1200";
-		reg = <4>;
-		sample-gpios = <&gpio 5 GPIO_ACTIVE_HIGH>;
-		rdvel-gpios = <&gpio 6 GPIO_ACTIVE_HIGH>;
-	};
diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index b3c8c6ef0dff..4bec3ccbf4a1 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -92,6 +92,7 @@ source "drivers/iio/potentiometer/Kconfig"
 source "drivers/iio/potentiostat/Kconfig"
 source "drivers/iio/pressure/Kconfig"
 source "drivers/iio/proximity/Kconfig"
+source "drivers/iio/resolver/Kconfig"
 source "drivers/iio/temperature/Kconfig"
 
 endif # IIO
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index b16b2e9ddc40..1865361b8714 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -35,5 +35,6 @@ obj-y += potentiometer/
 obj-y += potentiostat/
 obj-y += pressure/
 obj-y += proximity/
+obj-y += resolver/
 obj-y += temperature/
 obj-y += trigger/
diff --git a/drivers/iio/resolver/Kconfig b/drivers/iio/resolver/Kconfig
new file mode 100644
index 000000000000..2ced9f22aa70
--- /dev/null
+++ b/drivers/iio/resolver/Kconfig
@@ -0,0 +1,17 @@
+#
+# Resolver/Synchro drivers
+#
+menu "Resolver to digital converters"
+
+config AD2S1200
+	tristate "Analog Devices ad2s1200/ad2s1205 driver"
+	depends on SPI
+	depends on GPIOLIB || COMPILE_TEST
+	help
+	  Say yes here to build support for Analog Devices spi resolver
+	  to digital converters, ad2s1200 and ad2s1205, provides direct access
+	  via sysfs.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad2s1200.
+endmenu
diff --git a/drivers/iio/resolver/Makefile b/drivers/iio/resolver/Makefile
new file mode 100644
index 000000000000..4e1dccae07e7
--- /dev/null
+++ b/drivers/iio/resolver/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for Resolver/Synchro drivers
+#
+
+obj-$(CONFIG_AD2S1200) += ad2s1200.o
diff --git a/drivers/iio/resolver/ad2s1200.c b/drivers/iio/resolver/ad2s1200.c
new file mode 100644
index 000000000000..4f5dd28b174a
--- /dev/null
+++ b/drivers/iio/resolver/ad2s1200.c
@@ -0,0 +1,250 @@
+/*
+ * ad2s1200.c simple support for the ADI Resolver to Digital Converters:
+ * AD2S1200/1205
+ *
+ * Copyright (c) 2010-2010 Analog Devices Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/spi/spi.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#define DRV_NAME "ad2s1200"
+
+/* input pin sample and rdvel is controlled by driver */
+#define AD2S1200_PN	2
+
+/* input clock on serial interface */
+#define AD2S1200_HZ	8192000
+/* clock period in nano second */
+#define AD2S1200_TSCLK	(1000000000 / AD2S1200_HZ)
+
+/**
+ * struct ad2s1200_state - driver instance specific data
+ * @lock:	protect driver state
+ * @sdev:	spi device
+ * @sample:	GPIO pin SAMPLE
+ * @rdvel:	GPIO pin RDVEL
+ * @rx:		buffer for spi transfers
+ */
+struct ad2s1200_state {
+	struct mutex lock;
+	struct spi_device *sdev;
+	struct gpio_desc *sample;
+	struct gpio_desc *rdvel;
+	u8 rx[2] ____cacheline_aligned;
+};
+
+static int ad2s1200_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val,
+			     int *val2,
+			     long m)
+{
+	struct ad2s1200_state *st = iio_priv(indio_dev);
+	int ret = 0;
+	u16 vel;
+
+	/*
+	 * Below a fractional approximation of Pi is needed.
+	 * The following approximation will be used: 103993 / 33102.
+	 * This is accurate in 9 decimals places.
+	 *
+	 * This fraction is based on OEIS series of nominator/denominator
+	 * of convergents to Pi (A002485 and A002486).
+	 */
+	switch (m) {
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_ANGL:
+			/*
+			 * 2 * Pi / (2^12 - 1) ~= 2 * 103993 / (33102 * 0xFFF)
+			 *
+			 * Since there only fit 3 whole radians in 360 degrees,
+			 * usage of iio_convert_raw_to_processed for this
+			 * channel will be highly inaccurate.
+			 */
+			*val = 103993;
+			*val2 = 16551 * 0xFFF;
+			return IIO_VAL_FRACTIONAL;
+		case IIO_ANGL_VEL:
+			/*
+			 * 2 * Pi ~= 2 * 103993 / 33102
+			 *
+			 * iio_convert_raw_to_processed uses integer
+			 * division. This will cause at most 5% error
+			 * (for very small values). But for 99.5% of the values
+			 * it will cause less that 1% error.
+			 */
+			*val = 103993;
+			*val2 = 16551;
+			return IIO_VAL_FRACTIONAL;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&st->lock);
+		gpiod_set_value(st->sample, 0);
+
+		/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
+		udelay(1);
+		gpiod_set_value(st->sample, 1);
+		gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
+
+		ret = spi_read(st->sdev, st->rx, 2);
+		if (ret < 0) {
+			mutex_unlock(&st->lock);
+			return ret;
+		}
+
+		vel = be16_to_cpup((__be16 *)st->rx);
+		switch (chan->type) {
+		case IIO_ANGL:
+			*val = vel >> 4;
+			break;
+		case IIO_ANGL_VEL:
+			*val = sign_extend32((s16)vel >> 4, 11);
+			break;
+		default:
+			mutex_unlock(&st->lock);
+			return -EINVAL;
+		}
+
+		/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
+		udelay(1);
+		mutex_unlock(&st->lock);
+
+		return IIO_VAL_INT;
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_chan_spec ad2s1200_channels[] = {
+	{
+		.type = IIO_ANGL,
+		.indexed = 1,
+		.channel = 0,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+	}, {
+		.type = IIO_ANGL_VEL,
+		.indexed = 1,
+		.channel = 0,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+	}
+};
+
+static const struct iio_info ad2s1200_info = {
+	.read_raw = ad2s1200_read_raw,
+};
+
+static int ad2s1200_probe(struct spi_device *spi)
+{
+	unsigned short *pins = spi->dev.platform_data;
+	struct ad2s1200_state *st;
+	struct iio_dev *indio_dev;
+	struct device *dev;
+	int pn, ret = 0;
+
+	dev = &spi->dev;
+
+	if (pins) {
+		for (pn = 0; pn < AD2S1200_PN; pn++) {
+			ret = devm_gpio_request_one(dev, pins[pn],
+						    GPIOF_DIR_OUT,
+						    DRV_NAME);
+			if (ret) {
+				dev_err(dev,
+					"Failed to claim gpio %d\n: err=%d",
+					pins[pn],
+					ret);
+				return ret;
+			}
+		}
+	}
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	spi_set_drvdata(spi, indio_dev);
+	st = iio_priv(indio_dev);
+	mutex_init(&st->lock);
+	st->sdev = spi;
+
+	if (pins) {
+		st->sample = gpio_to_desc(pins[0]);
+		st->rdvel = gpio_to_desc(pins[1]);
+	} else {
+		st->sample = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
+		if (IS_ERR(st->sample)) {
+			dev_err(dev, "Failed to claim SAMPLE gpio: err=%ld\n",
+				PTR_ERR(st->sample));
+			return PTR_ERR(st->sample);
+		}
+
+		st->rdvel = devm_gpiod_get(dev, "rdvel", GPIOD_OUT_LOW);
+		if (IS_ERR(st->rdvel)) {
+			dev_err(dev, "Failed to claim RDVEL gpio: err=%ld\n",
+				PTR_ERR(st->rdvel));
+			return PTR_ERR(st->rdvel);
+		}
+	}
+
+	indio_dev->dev.parent = dev;
+	indio_dev->info = &ad2s1200_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = ad2s1200_channels;
+	indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
+	indio_dev->name = spi_get_device_id(spi)->name;
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret)
+		return ret;
+
+	spi->max_speed_hz = AD2S1200_HZ;
+	spi->mode = SPI_MODE_3;
+	spi_setup(spi);
+
+	return 0;
+}
+
+static const struct spi_device_id ad2s1200_id[] = {
+	{ "ad2s1200" },
+	{ "ad2s1205" },
+	{}
+};
+MODULE_DEVICE_TABLE(spi, ad2s1200_id);
+
+static struct spi_driver ad2s1200_driver = {
+	.driver = {
+		.name = DRV_NAME,
+	},
+	.probe = ad2s1200_probe,
+	.id_table = ad2s1200_id,
+};
+module_spi_driver(ad2s1200_driver);
+
+MODULE_AUTHOR("Graff Yang <graff.yang@gmail.com>");
+MODULE_DESCRIPTION("Analog Devices AD2S1200/1205 Resolver to Digital SPI driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/staging/iio/resolver/Kconfig b/drivers/staging/iio/resolver/Kconfig
index 1c7e2860d6b7..6a469ee6101f 100644
--- a/drivers/staging/iio/resolver/Kconfig
+++ b/drivers/staging/iio/resolver/Kconfig
@@ -13,18 +13,6 @@ config AD2S90
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad2s90.
 
-config AD2S1200
-	tristate "Analog Devices ad2s1200/ad2s1205 driver"
-	depends on SPI
-	depends on GPIOLIB || COMPILE_TEST
-	help
-	  Say yes here to build support for Analog Devices spi resolver
-	  to digital converters, ad2s1200 and ad2s1205, provides direct access
-	  via sysfs.
-
-	  To compile this driver as a module, choose M here: the
-	  module will be called ad2s1200.
-
 config AD2S1210
 	tristate "Analog Devices ad2s1210 driver"
 	depends on SPI
diff --git a/drivers/staging/iio/resolver/Makefile b/drivers/staging/iio/resolver/Makefile
index 14375e444ebf..8d901dc7500b 100644
--- a/drivers/staging/iio/resolver/Makefile
+++ b/drivers/staging/iio/resolver/Makefile
@@ -3,5 +3,4 @@
 #
 
 obj-$(CONFIG_AD2S90) += ad2s90.o
-obj-$(CONFIG_AD2S1200) += ad2s1200.o
 obj-$(CONFIG_AD2S1210) += ad2s1210.o
diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
deleted file mode 100644
index 4f5dd28b174a..000000000000
--- a/drivers/staging/iio/resolver/ad2s1200.c
+++ /dev/null
@@ -1,250 +0,0 @@
-/*
- * ad2s1200.c simple support for the ADI Resolver to Digital Converters:
- * AD2S1200/1205
- *
- * Copyright (c) 2010-2010 Analog Devices Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- */
-
-#include <linux/bitops.h>
-#include <linux/delay.h>
-#include <linux/device.h>
-#include <linux/gpio.h>
-#include <linux/gpio/consumer.h>
-#include <linux/module.h>
-#include <linux/mutex.h>
-#include <linux/spi/spi.h>
-#include <linux/sysfs.h>
-#include <linux/types.h>
-
-#include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
-
-#define DRV_NAME "ad2s1200"
-
-/* input pin sample and rdvel is controlled by driver */
-#define AD2S1200_PN	2
-
-/* input clock on serial interface */
-#define AD2S1200_HZ	8192000
-/* clock period in nano second */
-#define AD2S1200_TSCLK	(1000000000 / AD2S1200_HZ)
-
-/**
- * struct ad2s1200_state - driver instance specific data
- * @lock:	protect driver state
- * @sdev:	spi device
- * @sample:	GPIO pin SAMPLE
- * @rdvel:	GPIO pin RDVEL
- * @rx:		buffer for spi transfers
- */
-struct ad2s1200_state {
-	struct mutex lock;
-	struct spi_device *sdev;
-	struct gpio_desc *sample;
-	struct gpio_desc *rdvel;
-	u8 rx[2] ____cacheline_aligned;
-};
-
-static int ad2s1200_read_raw(struct iio_dev *indio_dev,
-			     struct iio_chan_spec const *chan,
-			     int *val,
-			     int *val2,
-			     long m)
-{
-	struct ad2s1200_state *st = iio_priv(indio_dev);
-	int ret = 0;
-	u16 vel;
-
-	/*
-	 * Below a fractional approximation of Pi is needed.
-	 * The following approximation will be used: 103993 / 33102.
-	 * This is accurate in 9 decimals places.
-	 *
-	 * This fraction is based on OEIS series of nominator/denominator
-	 * of convergents to Pi (A002485 and A002486).
-	 */
-	switch (m) {
-	case IIO_CHAN_INFO_SCALE:
-		switch (chan->type) {
-		case IIO_ANGL:
-			/*
-			 * 2 * Pi / (2^12 - 1) ~= 2 * 103993 / (33102 * 0xFFF)
-			 *
-			 * Since there only fit 3 whole radians in 360 degrees,
-			 * usage of iio_convert_raw_to_processed for this
-			 * channel will be highly inaccurate.
-			 */
-			*val = 103993;
-			*val2 = 16551 * 0xFFF;
-			return IIO_VAL_FRACTIONAL;
-		case IIO_ANGL_VEL:
-			/*
-			 * 2 * Pi ~= 2 * 103993 / 33102
-			 *
-			 * iio_convert_raw_to_processed uses integer
-			 * division. This will cause at most 5% error
-			 * (for very small values). But for 99.5% of the values
-			 * it will cause less that 1% error.
-			 */
-			*val = 103993;
-			*val2 = 16551;
-			return IIO_VAL_FRACTIONAL;
-		default:
-			return -EINVAL;
-		}
-		break;
-	case IIO_CHAN_INFO_RAW:
-		mutex_lock(&st->lock);
-		gpiod_set_value(st->sample, 0);
-
-		/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
-		udelay(1);
-		gpiod_set_value(st->sample, 1);
-		gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
-
-		ret = spi_read(st->sdev, st->rx, 2);
-		if (ret < 0) {
-			mutex_unlock(&st->lock);
-			return ret;
-		}
-
-		vel = be16_to_cpup((__be16 *)st->rx);
-		switch (chan->type) {
-		case IIO_ANGL:
-			*val = vel >> 4;
-			break;
-		case IIO_ANGL_VEL:
-			*val = sign_extend32((s16)vel >> 4, 11);
-			break;
-		default:
-			mutex_unlock(&st->lock);
-			return -EINVAL;
-		}
-
-		/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
-		udelay(1);
-		mutex_unlock(&st->lock);
-
-		return IIO_VAL_INT;
-	default:
-		break;
-	}
-
-	return -EINVAL;
-}
-
-static const struct iio_chan_spec ad2s1200_channels[] = {
-	{
-		.type = IIO_ANGL,
-		.indexed = 1,
-		.channel = 0,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
-	}, {
-		.type = IIO_ANGL_VEL,
-		.indexed = 1,
-		.channel = 0,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
-	}
-};
-
-static const struct iio_info ad2s1200_info = {
-	.read_raw = ad2s1200_read_raw,
-};
-
-static int ad2s1200_probe(struct spi_device *spi)
-{
-	unsigned short *pins = spi->dev.platform_data;
-	struct ad2s1200_state *st;
-	struct iio_dev *indio_dev;
-	struct device *dev;
-	int pn, ret = 0;
-
-	dev = &spi->dev;
-
-	if (pins) {
-		for (pn = 0; pn < AD2S1200_PN; pn++) {
-			ret = devm_gpio_request_one(dev, pins[pn],
-						    GPIOF_DIR_OUT,
-						    DRV_NAME);
-			if (ret) {
-				dev_err(dev,
-					"Failed to claim gpio %d\n: err=%d",
-					pins[pn],
-					ret);
-				return ret;
-			}
-		}
-	}
-
-	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
-	if (!indio_dev)
-		return -ENOMEM;
-
-	spi_set_drvdata(spi, indio_dev);
-	st = iio_priv(indio_dev);
-	mutex_init(&st->lock);
-	st->sdev = spi;
-
-	if (pins) {
-		st->sample = gpio_to_desc(pins[0]);
-		st->rdvel = gpio_to_desc(pins[1]);
-	} else {
-		st->sample = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
-		if (IS_ERR(st->sample)) {
-			dev_err(dev, "Failed to claim SAMPLE gpio: err=%ld\n",
-				PTR_ERR(st->sample));
-			return PTR_ERR(st->sample);
-		}
-
-		st->rdvel = devm_gpiod_get(dev, "rdvel", GPIOD_OUT_LOW);
-		if (IS_ERR(st->rdvel)) {
-			dev_err(dev, "Failed to claim RDVEL gpio: err=%ld\n",
-				PTR_ERR(st->rdvel));
-			return PTR_ERR(st->rdvel);
-		}
-	}
-
-	indio_dev->dev.parent = dev;
-	indio_dev->info = &ad2s1200_info;
-	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->channels = ad2s1200_channels;
-	indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
-	indio_dev->name = spi_get_device_id(spi)->name;
-
-	ret = devm_iio_device_register(dev, indio_dev);
-	if (ret)
-		return ret;
-
-	spi->max_speed_hz = AD2S1200_HZ;
-	spi->mode = SPI_MODE_3;
-	spi_setup(spi);
-
-	return 0;
-}
-
-static const struct spi_device_id ad2s1200_id[] = {
-	{ "ad2s1200" },
-	{ "ad2s1205" },
-	{}
-};
-MODULE_DEVICE_TABLE(spi, ad2s1200_id);
-
-static struct spi_driver ad2s1200_driver = {
-	.driver = {
-		.name = DRV_NAME,
-	},
-	.probe = ad2s1200_probe,
-	.id_table = ad2s1200_id,
-};
-module_spi_driver(ad2s1200_driver);
-
-MODULE_AUTHOR("Graff Yang <graff.yang@gmail.com>");
-MODULE_DESCRIPTION("Analog Devices AD2S1200/1205 Resolver to Digital SPI driver");
-MODULE_LICENSE("GPL v2");
-- 
2.16.2

Re: [PATCH v2 01/13] staging: iio: ad2s1200: Remove unneeded include

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:28:32 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> This patches removes unneeded slab.h header.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
This one surprised me, but indeed there are no direct
users of any memory allocation in this file.

Applied to the togreg branch of iio.git and pushed out as
testing for the autobuilders to play with it.

Thanks,

Jonathan

> ---
> Changes in v2:
>   - Introduced in this version.
> 
> drivers/staging/iio/resolver/ad2s1200.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index aa62c64e9bc4..5d7ed0034422 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -13,7 +13,6 @@
>  #include <linux/mutex.h>
>  #include <linux/device.h>
>  #include <linux/spi/spi.h>
> -#include <linux/slab.h>
>  #include <linux/sysfs.h>
>  #include <linux/delay.h>
>  #include <linux/gpio.h>

Re: [PATCH v2 01/13] staging: iio: ad2s1200: Remove unneeded include

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:28:32 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> This patches removes unneeded slab.h header.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
This one surprised me, but indeed there are no direct
users of any memory allocation in this file.

Applied to the togreg branch of iio.git and pushed out as
testing for the autobuilders to play with it.

Thanks,

Jonathan

> ---
> Changes in v2:
>   - Introduced in this version.
> 
> drivers/staging/iio/resolver/ad2s1200.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index aa62c64e9bc4..5d7ed0034422 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -13,7 +13,6 @@
>  #include <linux/mutex.h>
>  #include <linux/device.h>
>  #include <linux/spi/spi.h>
> -#include <linux/slab.h>
>  #include <linux/sysfs.h>
>  #include <linux/delay.h>
>  #include <linux/gpio.h>

Re: [PATCH v2 02/13] staging: iio: ad2s1200: Sort includes alphabetically

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:28:52 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> This patches sorts all the includes in alphabetic order.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
Applied,

Thanks

Jonathan

> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index 5d7ed0034422..ffcdf4e8eb92 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -9,15 +9,15 @@
>   * published by the Free Software Foundation.
>   *
>   */
> -#include <linux/types.h>
> -#include <linux/mutex.h>
> -#include <linux/device.h>
> -#include <linux/spi/spi.h>
> -#include <linux/sysfs.h>
> +#include <linux/bitops.h>
>  #include <linux/delay.h>
> +#include <linux/device.h>
>  #include <linux/gpio.h>
>  #include <linux/module.h>
> -#include <linux/bitops.h>
> +#include <linux/mutex.h>
> +#include <linux/spi/spi.h>
> +#include <linux/sysfs.h>
> +#include <linux/types.h>
>  
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>

Re: [PATCH v2 02/13] staging: iio: ad2s1200: Sort includes alphabetically

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:28:52 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> This patches sorts all the includes in alphabetic order.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
Applied,

Thanks

Jonathan

> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index 5d7ed0034422..ffcdf4e8eb92 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -9,15 +9,15 @@
>   * published by the Free Software Foundation.
>   *
>   */
> -#include <linux/types.h>
> -#include <linux/mutex.h>
> -#include <linux/device.h>
> -#include <linux/spi/spi.h>
> -#include <linux/sysfs.h>
> +#include <linux/bitops.h>
>  #include <linux/delay.h>
> +#include <linux/device.h>
>  #include <linux/gpio.h>
>  #include <linux/module.h>
> -#include <linux/bitops.h>
> +#include <linux/mutex.h>
> +#include <linux/spi/spi.h>
> +#include <linux/sysfs.h>
> +#include <linux/types.h>
>  
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>

Re: [PATCH v2 03/13] staging: iio: ad2s1200: Reverse Christmas tree ordering

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:29:08 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> Reorders the variable declarations to prefer a reverse Christmas tree
> order to improve readability.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
Applied,

Thanks,

Jonathan

> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index ffcdf4e8eb92..b6c3a3c8f7fe 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -46,9 +46,9 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>  			     int *val2,
>  			     long m)
>  {
> +	struct ad2s1200_state *st = iio_priv(indio_dev);
>  	int ret = 0;
>  	s16 vel;
> -	struct ad2s1200_state *st = iio_priv(indio_dev);
>  
>  	mutex_lock(&st->lock);
>  	gpio_set_value(st->sample, 0);
> @@ -101,10 +101,10 @@ static const struct iio_info ad2s1200_info = {
>  
>  static int ad2s1200_probe(struct spi_device *spi)
>  {
> +	unsigned short *pins = spi->dev.platform_data;
>  	struct ad2s1200_state *st;
>  	struct iio_dev *indio_dev;
>  	int pn, ret = 0;
> -	unsigned short *pins = spi->dev.platform_data;
>  
>  	for (pn = 0; pn < AD2S1200_PN; pn++) {
>  		ret = devm_gpio_request_one(&spi->dev, pins[pn], GPIOF_DIR_OUT,

Re: [PATCH v2 03/13] staging: iio: ad2s1200: Reverse Christmas tree ordering

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:29:08 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> Reorders the variable declarations to prefer a reverse Christmas tree
> order to improve readability.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
Applied,

Thanks,

Jonathan

> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index ffcdf4e8eb92..b6c3a3c8f7fe 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -46,9 +46,9 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>  			     int *val2,
>  			     long m)
>  {
> +	struct ad2s1200_state *st = iio_priv(indio_dev);
>  	int ret = 0;
>  	s16 vel;
> -	struct ad2s1200_state *st = iio_priv(indio_dev);
>  
>  	mutex_lock(&st->lock);
>  	gpio_set_value(st->sample, 0);
> @@ -101,10 +101,10 @@ static const struct iio_info ad2s1200_info = {
>  
>  static int ad2s1200_probe(struct spi_device *spi)
>  {
> +	unsigned short *pins = spi->dev.platform_data;
>  	struct ad2s1200_state *st;
>  	struct iio_dev *indio_dev;
>  	int pn, ret = 0;
> -	unsigned short *pins = spi->dev.platform_data;
>  
>  	for (pn = 0; pn < AD2S1200_PN; pn++) {
>  		ret = devm_gpio_request_one(&spi->dev, pins[pn], GPIOF_DIR_OUT,

Re: [PATCH v2 04/13] staging: iio: ad2s1200: Add blank lines

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:29:52 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> Add blank lines to improve readability.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
Applied,

Thanks,

Jonathan

> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index b6c3a3c8f7fe..357fe3c382b3 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -9,6 +9,7 @@
>   * published by the Free Software Foundation.
>   *
>   */
> +
>  #include <linux/bitops.h>
>  #include <linux/delay.h>
>  #include <linux/device.h>
> @@ -52,10 +53,12 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>  
>  	mutex_lock(&st->lock);
>  	gpio_set_value(st->sample, 0);
> +
>  	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>  	udelay(1);
>  	gpio_set_value(st->sample, 1);
>  	gpio_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
> +
>  	ret = spi_read(st->sdev, st->rx, 2);
>  	if (ret < 0) {
>  		mutex_unlock(&st->lock);
> @@ -75,9 +78,11 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>  		mutex_unlock(&st->lock);
>  		return -EINVAL;
>  	}
> +
>  	/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
>  	udelay(1);
>  	mutex_unlock(&st->lock);
> +
>  	return IIO_VAL_INT;
>  }
>  
> @@ -115,9 +120,11 @@ static int ad2s1200_probe(struct spi_device *spi)
>  			return ret;
>  		}
>  	}
> +
>  	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
>  	if (!indio_dev)
>  		return -ENOMEM;
> +
>  	spi_set_drvdata(spi, indio_dev);
>  	st = iio_priv(indio_dev);
>  	mutex_init(&st->lock);

Re: [PATCH v2 04/13] staging: iio: ad2s1200: Add blank lines

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:29:52 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> Add blank lines to improve readability.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
Applied,

Thanks,

Jonathan

> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index b6c3a3c8f7fe..357fe3c382b3 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -9,6 +9,7 @@
>   * published by the Free Software Foundation.
>   *
>   */
> +
>  #include <linux/bitops.h>
>  #include <linux/delay.h>
>  #include <linux/device.h>
> @@ -52,10 +53,12 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>  
>  	mutex_lock(&st->lock);
>  	gpio_set_value(st->sample, 0);
> +
>  	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>  	udelay(1);
>  	gpio_set_value(st->sample, 1);
>  	gpio_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
> +
>  	ret = spi_read(st->sdev, st->rx, 2);
>  	if (ret < 0) {
>  		mutex_unlock(&st->lock);
> @@ -75,9 +78,11 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>  		mutex_unlock(&st->lock);
>  		return -EINVAL;
>  	}
> +
>  	/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
>  	udelay(1);
>  	mutex_unlock(&st->lock);
> +
>  	return IIO_VAL_INT;
>  }
>  
> @@ -115,9 +120,11 @@ static int ad2s1200_probe(struct spi_device *spi)
>  			return ret;
>  		}
>  	}
> +
>  	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
>  	if (!indio_dev)
>  		return -ENOMEM;
> +
>  	spi_set_drvdata(spi, indio_dev);
>  	st = iio_priv(indio_dev);
>  	mutex_init(&st->lock);

Re: [PATCH v2 05/13] staging: iio: ad2s1200: Add kernel docs to driver state

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:30:03 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> Add missing kernel docs to the ad2s1200 driver state.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index 357fe3c382b3..f07aab7e7a35 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -33,6 +33,14 @@
>  /* clock period in nano second */
>  #define AD2S1200_TSCLK	(1000000000 / AD2S1200_HZ)
>  
> +/**
> + * struct ad2s1200_state - driver instance specific data
> + * @lock:	protect driver state

This doc for locks needs to be more specific.  From a quick
glance I think it does two things.
1) Ensures that we don't have concurrent accesses changing the
gpio control lines.
2) Protects the rx buffer against concurrent accesses.

It doesn't have anything much to do with the rest of this state
structure.

> + * @sdev:	spi device
> + * @sample:	GPIO pin SAMPLE
> + * @rdvel:	GPIO pin RDVEL
> + * @rx:		buffer for spi transfers
> + */
>  struct ad2s1200_state {
>  	struct mutex lock;
>  	struct spi_device *sdev;

Re: [PATCH v2 05/13] staging: iio: ad2s1200: Add kernel docs to driver state

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:30:03 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> Add missing kernel docs to the ad2s1200 driver state.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index 357fe3c382b3..f07aab7e7a35 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -33,6 +33,14 @@
>  /* clock period in nano second */
>  #define AD2S1200_TSCLK	(1000000000 / AD2S1200_HZ)
>  
> +/**
> + * struct ad2s1200_state - driver instance specific data
> + * @lock:	protect driver state

This doc for locks needs to be more specific.  From a quick
glance I think it does two things.
1) Ensures that we don't have concurrent accesses changing the
gpio control lines.
2) Protects the rx buffer against concurrent accesses.

It doesn't have anything much to do with the rest of this state
structure.

> + * @sdev:	spi device
> + * @sample:	GPIO pin SAMPLE
> + * @rdvel:	GPIO pin RDVEL
> + * @rx:		buffer for spi transfers
> + */
>  struct ad2s1200_state {
>  	struct mutex lock;
>  	struct spi_device *sdev;

Re: [PATCH v2 06/13] staging: iio: ad2s1200: Introduce variable for repeated value

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:30:19 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> Add variable to hold &spi->dev in ad2s1200_probe. This value is repeatedly
> used in ad2s1200_probe.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
No significant gain in readability.   Perhaps even a slight lost I'm
going to say no to this one.

Jonathan

> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index f07aab7e7a35..0a5fc9917e32 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -117,19 +117,22 @@ static int ad2s1200_probe(struct spi_device *spi)
>  	unsigned short *pins = spi->dev.platform_data;
>  	struct ad2s1200_state *st;
>  	struct iio_dev *indio_dev;
> +	struct device *dev;
>  	int pn, ret = 0;
>  
> +	dev = &spi->dev;
> +
>  	for (pn = 0; pn < AD2S1200_PN; pn++) {
> -		ret = devm_gpio_request_one(&spi->dev, pins[pn], GPIOF_DIR_OUT,
> +		ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
>  					    DRV_NAME);
>  		if (ret) {
> -			dev_err(&spi->dev, "request gpio pin %d failed\n",
> +			dev_err(dev, "request gpio pin %d failed\n",
>  				pins[pn]);
>  			return ret;
>  		}
>  	}
>  
> -	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
>  	if (!indio_dev)
>  		return -ENOMEM;
>  
> @@ -140,14 +143,14 @@ static int ad2s1200_probe(struct spi_device *spi)
>  	st->sample = pins[0];
>  	st->rdvel = pins[1];
>  
> -	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->dev.parent = dev;
>  	indio_dev->info = &ad2s1200_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->channels = ad2s1200_channels;
>  	indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
>  	indio_dev->name = spi_get_device_id(spi)->name;
>  
> -	ret = devm_iio_device_register(&spi->dev, indio_dev);
> +	ret = devm_iio_device_register(dev, indio_dev);
>  	if (ret)
>  		return ret;
>  

Re: [PATCH v2 06/13] staging: iio: ad2s1200: Introduce variable for repeated value

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:30:19 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> Add variable to hold &spi->dev in ad2s1200_probe. This value is repeatedly
> used in ad2s1200_probe.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
No significant gain in readability.   Perhaps even a slight lost I'm
going to say no to this one.

Jonathan

> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index f07aab7e7a35..0a5fc9917e32 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -117,19 +117,22 @@ static int ad2s1200_probe(struct spi_device *spi)
>  	unsigned short *pins = spi->dev.platform_data;
>  	struct ad2s1200_state *st;
>  	struct iio_dev *indio_dev;
> +	struct device *dev;
>  	int pn, ret = 0;
>  
> +	dev = &spi->dev;
> +
>  	for (pn = 0; pn < AD2S1200_PN; pn++) {
> -		ret = devm_gpio_request_one(&spi->dev, pins[pn], GPIOF_DIR_OUT,
> +		ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
>  					    DRV_NAME);
>  		if (ret) {
> -			dev_err(&spi->dev, "request gpio pin %d failed\n",
> +			dev_err(dev, "request gpio pin %d failed\n",
>  				pins[pn]);
>  			return ret;
>  		}
>  	}
>  
> -	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
>  	if (!indio_dev)
>  		return -ENOMEM;
>  
> @@ -140,14 +143,14 @@ static int ad2s1200_probe(struct spi_device *spi)
>  	st->sample = pins[0];
>  	st->rdvel = pins[1];
>  
> -	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->dev.parent = dev;
>  	indio_dev->info = &ad2s1200_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->channels = ad2s1200_channels;
>  	indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
>  	indio_dev->name = spi_get_device_id(spi)->name;
>  
> -	ret = devm_iio_device_register(&spi->dev, indio_dev);
> +	ret = devm_iio_device_register(dev, indio_dev);
>  	if (ret)
>  		return ret;
>  

Re: [PATCH v2 07/13] staging: iio: ad2s1200: Improve readability with be16_to_cpup

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:30:32 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> The manual states that the data is contained in the upper 12 bits
> of the 16 bits read by spi. The code that extracts these 12 bits
> is correct for both be and le machines, but this is not clear
> from a first glance.
> 
> To improve readability the relevant expressions are replaced
> with equivalent expressions that use be16_to_cpup.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index 0a5fc9917e32..11ed9c7332e6 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -57,7 +57,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>  {
>  	struct ad2s1200_state *st = iio_priv(indio_dev);
>  	int ret = 0;
> -	s16 vel;
> +	u16 vel;
>  
>  	mutex_lock(&st->lock);
>  	gpio_set_value(st->sample, 0);
> @@ -73,14 +73,13 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>  		return ret;
>  	}
>  
> +	vel = be16_to_cpup((__be16 *)st->rx);
Well it isn't vel (velocity) in one case so now the name is misleading.

Also that type cast suggests a fairly obvious cleanup associated with this one.
Why not make st->rx a __be16 in the first pace avoiding the need to cast at all?

Then you could just have *val = be16_to_cpu(st->rx) >> 4 and similar.

>  	switch (chan->type) {
>  	case IIO_ANGL:
> -		*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
> +		*val = vel >> 4;
>  		break;
>  	case IIO_ANGL_VEL:
> -		vel = (((s16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
> -		vel = sign_extend32(vel, 11);
> -		*val = vel;
> +		*val = sign_extend32((s16)vel >> 4, 11);
If you were to use an intermediate that was s16 then the sign extend would
be automatic.  Perhaps it is clear to do it like this though.. 
Up to you.

>  		break;
>  	default:
>  		mutex_unlock(&st->lock);

Re: [PATCH v2 07/13] staging: iio: ad2s1200: Improve readability with be16_to_cpup

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:30:32 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> The manual states that the data is contained in the upper 12 bits
> of the 16 bits read by spi. The code that extracts these 12 bits
> is correct for both be and le machines, but this is not clear
> from a first glance.
> 
> To improve readability the relevant expressions are replaced
> with equivalent expressions that use be16_to_cpup.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index 0a5fc9917e32..11ed9c7332e6 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -57,7 +57,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>  {
>  	struct ad2s1200_state *st = iio_priv(indio_dev);
>  	int ret = 0;
> -	s16 vel;
> +	u16 vel;
>  
>  	mutex_lock(&st->lock);
>  	gpio_set_value(st->sample, 0);
> @@ -73,14 +73,13 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>  		return ret;
>  	}
>  
> +	vel = be16_to_cpup((__be16 *)st->rx);
Well it isn't vel (velocity) in one case so now the name is misleading.

Also that type cast suggests a fairly obvious cleanup associated with this one.
Why not make st->rx a __be16 in the first pace avoiding the need to cast at all?

Then you could just have *val = be16_to_cpu(st->rx) >> 4 and similar.

>  	switch (chan->type) {
>  	case IIO_ANGL:
> -		*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
> +		*val = vel >> 4;
>  		break;
>  	case IIO_ANGL_VEL:
> -		vel = (((s16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
> -		vel = sign_extend32(vel, 11);
> -		*val = vel;
> +		*val = sign_extend32((s16)vel >> 4, 11);
If you were to use an intermediate that was s16 then the sign extend would
be automatic.  Perhaps it is clear to do it like this though.. 
Up to you.

>  		break;
>  	default:
>  		mutex_unlock(&st->lock);

Re: [PATCH v2 08/13] staging: iio: ad2s1200: Replace legacy gpio API with modern API

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:30:44 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> The legacy, integer based gpio API is replaced with the descriptor
> based API.
> 
> For compatibility, it is first tried to use the platform data to
> request the gpio's. Otherwise, it looks for the "sample" and "rdvel"
> gpio function.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
I would suggest that we simply force any out of tree users of the
platform data to update the platform data.  Drop the the old
versions entirely..

We can be mean to out of tree board files so let us make our own
lives easy.

Jonathan

> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 51 ++++++++++++++++++++++++---------
>  1 file changed, 37 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index 11ed9c7332e6..29a9bb666e7b 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -14,6 +14,7 @@
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/spi/spi.h>
> @@ -44,8 +45,8 @@
>  struct ad2s1200_state {
>  	struct mutex lock;
>  	struct spi_device *sdev;
> -	int sample;
> -	int rdvel;
> +	struct gpio_desc *sample;
> +	struct gpio_desc *rdvel;
>  	u8 rx[2] ____cacheline_aligned;
>  };
>  
> @@ -60,12 +61,12 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>  	u16 vel;
>  
>  	mutex_lock(&st->lock);
> -	gpio_set_value(st->sample, 0);
> +	gpiod_set_value(st->sample, 0);
>  
>  	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>  	udelay(1);
> -	gpio_set_value(st->sample, 1);
> -	gpio_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
> +	gpiod_set_value(st->sample, 1);
> +	gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>  
>  	ret = spi_read(st->sdev, st->rx, 2);
>  	if (ret < 0) {
> @@ -121,13 +122,18 @@ static int ad2s1200_probe(struct spi_device *spi)
>  
>  	dev = &spi->dev;
>  
> -	for (pn = 0; pn < AD2S1200_PN; pn++) {
> -		ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
> -					    DRV_NAME);
> -		if (ret) {
> -			dev_err(dev, "request gpio pin %d failed\n",
> -				pins[pn]);
> -			return ret;
> +	if (pins) {
> +		for (pn = 0; pn < AD2S1200_PN; pn++) {
> +			ret = devm_gpio_request_one(dev, pins[pn],
> +						    GPIOF_DIR_OUT,
> +						    DRV_NAME);
> +			if (ret) {
> +				dev_err(dev,
> +					"Failed to claim gpio %d\n: err=%d",
> +					pins[pn],
> +					ret);
> +				return ret;
> +			}
>  		}
>  	}
>  
> @@ -139,8 +145,25 @@ static int ad2s1200_probe(struct spi_device *spi)
>  	st = iio_priv(indio_dev);
>  	mutex_init(&st->lock);
>  	st->sdev = spi;
> -	st->sample = pins[0];
> -	st->rdvel = pins[1];
> +
> +	if (pins) {
> +		st->sample = gpio_to_desc(pins[0]);
> +		st->rdvel = gpio_to_desc(pins[1]);
> +	} else {
> +		st->sample = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
> +		if (IS_ERR(st->sample)) {
> +			dev_err(dev, "Failed to claim SAMPLE gpio: err=%ld\n",
> +				PTR_ERR(st->sample));
> +			return PTR_ERR(st->sample);
> +		}
> +
> +		st->rdvel = devm_gpiod_get(dev, "rdvel", GPIOD_OUT_LOW);
> +		if (IS_ERR(st->rdvel)) {
> +			dev_err(dev, "Failed to claim RDVEL gpio: err=%ld\n",
> +				PTR_ERR(st->rdvel));
> +			return PTR_ERR(st->rdvel);
> +		}
> +	}
>  
>  	indio_dev->dev.parent = dev;
>  	indio_dev->info = &ad2s1200_info;

Re: [PATCH v2 08/13] staging: iio: ad2s1200: Replace legacy gpio API with modern API

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:30:44 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> The legacy, integer based gpio API is replaced with the descriptor
> based API.
> 
> For compatibility, it is first tried to use the platform data to
> request the gpio's. Otherwise, it looks for the "sample" and "rdvel"
> gpio function.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
I would suggest that we simply force any out of tree users of the
platform data to update the platform data.  Drop the the old
versions entirely..

We can be mean to out of tree board files so let us make our own
lives easy.

Jonathan

> ---
>  drivers/staging/iio/resolver/ad2s1200.c | 51 ++++++++++++++++++++++++---------
>  1 file changed, 37 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index 11ed9c7332e6..29a9bb666e7b 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -14,6 +14,7 @@
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/spi/spi.h>
> @@ -44,8 +45,8 @@
>  struct ad2s1200_state {
>  	struct mutex lock;
>  	struct spi_device *sdev;
> -	int sample;
> -	int rdvel;
> +	struct gpio_desc *sample;
> +	struct gpio_desc *rdvel;
>  	u8 rx[2] ____cacheline_aligned;
>  };
>  
> @@ -60,12 +61,12 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>  	u16 vel;
>  
>  	mutex_lock(&st->lock);
> -	gpio_set_value(st->sample, 0);
> +	gpiod_set_value(st->sample, 0);
>  
>  	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>  	udelay(1);
> -	gpio_set_value(st->sample, 1);
> -	gpio_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
> +	gpiod_set_value(st->sample, 1);
> +	gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>  
>  	ret = spi_read(st->sdev, st->rx, 2);
>  	if (ret < 0) {
> @@ -121,13 +122,18 @@ static int ad2s1200_probe(struct spi_device *spi)
>  
>  	dev = &spi->dev;
>  
> -	for (pn = 0; pn < AD2S1200_PN; pn++) {
> -		ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
> -					    DRV_NAME);
> -		if (ret) {
> -			dev_err(dev, "request gpio pin %d failed\n",
> -				pins[pn]);
> -			return ret;
> +	if (pins) {
> +		for (pn = 0; pn < AD2S1200_PN; pn++) {
> +			ret = devm_gpio_request_one(dev, pins[pn],
> +						    GPIOF_DIR_OUT,
> +						    DRV_NAME);
> +			if (ret) {
> +				dev_err(dev,
> +					"Failed to claim gpio %d\n: err=%d",
> +					pins[pn],
> +					ret);
> +				return ret;
> +			}
>  		}
>  	}
>  
> @@ -139,8 +145,25 @@ static int ad2s1200_probe(struct spi_device *spi)
>  	st = iio_priv(indio_dev);
>  	mutex_init(&st->lock);
>  	st->sdev = spi;
> -	st->sample = pins[0];
> -	st->rdvel = pins[1];
> +
> +	if (pins) {
> +		st->sample = gpio_to_desc(pins[0]);
> +		st->rdvel = gpio_to_desc(pins[1]);
> +	} else {
> +		st->sample = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
> +		if (IS_ERR(st->sample)) {
> +			dev_err(dev, "Failed to claim SAMPLE gpio: err=%ld\n",
> +				PTR_ERR(st->sample));
> +			return PTR_ERR(st->sample);
> +		}
> +
> +		st->rdvel = devm_gpiod_get(dev, "rdvel", GPIOD_OUT_LOW);
> +		if (IS_ERR(st->rdvel)) {
> +			dev_err(dev, "Failed to claim RDVEL gpio: err=%ld\n",
> +				PTR_ERR(st->rdvel));
> +			return PTR_ERR(st->rdvel);
> +		}
> +	}
>  
>  	indio_dev->dev.parent = dev;
>  	indio_dev->info = &ad2s1200_info;

Re: [PATCH v2 09/13] staging: iio: ad2s1200: Add documentation for device tree binding

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:30:54 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> Add documentation for the added device tree bindings.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
Straight forward, but please introduce it directly in
bindings/iio/resolver rather than moving it.

The binding doesn't require a driver to be outside of staging
so there would be no harm in doing this directly.

Jonathan

> ---
> Changes in v2:
>   - Introduced in this version.
> 
>  .../bindings/staging/iio/resolver/ad2s1200.txt           | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
> 
> diff --git a/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt b/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
> new file mode 100644
> index 000000000000..85c009987878
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
> @@ -0,0 +1,16 @@
> +Analog Devices AD2S1200 Resolver-to-Digital Converter
> +
> +Required properties:
> + - compatible : should be "adi,ad2s1200"
> + - reg : the SPI chip select number of the device
> + - sample-gpios : The GPIO pin connected to the SAMPLE line of the AD2S1200
> + - rdvel-gpios : The GPIO pin connected to the RDVEL line of the AD2S1200
> +
> +Example:
> +
> +	resolver {
> +		compatible = "adi,ad2s1200";
> +		reg = <4>;
> +		sample-gpios = <&gpio 5 GPIO_ACTIVE_HIGH>;
> +		rdvel-gpios = <&gpio 6 GPIO_ACTIVE_HIGH>;
> +	};

Re: [PATCH v2 09/13] staging: iio: ad2s1200: Add documentation for device tree binding

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:30:54 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> Add documentation for the added device tree bindings.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
Straight forward, but please introduce it directly in
bindings/iio/resolver rather than moving it.

The binding doesn't require a driver to be outside of staging
so there would be no harm in doing this directly.

Jonathan

> ---
> Changes in v2:
>   - Introduced in this version.
> 
>  .../bindings/staging/iio/resolver/ad2s1200.txt           | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
> 
> diff --git a/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt b/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
> new file mode 100644
> index 000000000000..85c009987878
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
> @@ -0,0 +1,16 @@
> +Analog Devices AD2S1200 Resolver-to-Digital Converter
> +
> +Required properties:
> + - compatible : should be "adi,ad2s1200"
> + - reg : the SPI chip select number of the device
> + - sample-gpios : The GPIO pin connected to the SAMPLE line of the AD2S1200
> + - rdvel-gpios : The GPIO pin connected to the RDVEL line of the AD2S1200
> +
> +Example:
> +
> +	resolver {
> +		compatible = "adi,ad2s1200";
> +		reg = <4>;
> +		sample-gpios = <&gpio 5 GPIO_ACTIVE_HIGH>;
> +		rdvel-gpios = <&gpio 6 GPIO_ACTIVE_HIGH>;
> +	};

Re: [PATCH v2 10/13] staging: iio: ad2s1200: Add scaling factor for angular velocity channel

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:31:09 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> The sysfs iio ABI states radians per second is expected as the unit for
> angular velocity, but the 12-bit angular velocity register has rps
> as its unit. So a fractional scaling factor of approximately 2 * Pi is
> added to the angular velocity channel.
> 
> The added comments will also be relevant for the scaling factor of
> the angle channel.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
Comment inline.  The function you are talking about isn't used
in the majority of likely use cases for this part.  The maths will actually
be done in userspace (which can use floating point).

Thanks,

Jonathan

> ---
> Changes in v2:
>   - Move explanation of Pi approximation to top of switch statement,
>     as this will also be relevant to angle channel.
>   - Replaced 33102 / 2 with 16551 on line 84.
> 
>  drivers/staging/iio/resolver/ad2s1200.c | 84 +++++++++++++++++++++++----------
>  1 file changed, 59 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index 29a9bb666e7b..6c56257be3b1 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -60,38 +60,71 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>  	int ret = 0;
>  	u16 vel;
>  
> -	mutex_lock(&st->lock);
> -	gpiod_set_value(st->sample, 0);
> +	/*
> +	 * Below a fractional approximation of Pi is needed.
> +	 * The following approximation will be used: 103993 / 33102.
> +	 * This is accurate in 9 decimals places.
> +	 *
> +	 * This fraction is based on OEIS series of nominator/denominator
> +	 * of convergents to Pi (A002485 and A002486).
> +	 */
> +	switch (m) {
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_ANGL_VEL:
> +			/*
> +			 * 2 * Pi ~= 2 * 103993 / 33102
> +			 *
> +			 * iio_convert_raw_to_processed uses integer
> +			 * division. This will cause at most 5% error
> +			 * (for very small values). But for 99.5% of the values
> +			 * it will cause less that 1% error.
This is an interesting comment, but relies on anyone actually
using iio_convert_raw_to_processed with this device.

I would imagine that 'in kernel' users of a resolver (who would use
that function) will be few and far between.  Mostly this will just
get passed to userspace.  That involves this being converted to
a decimal.  I would just specify it as one in the first place.

> +			 */
> +			*val = 103993;
> +			*val2 = 16551;
> +			return IIO_VAL_FRACTIONAL;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&st->lock);
> +		gpiod_set_value(st->sample, 0);
> +
> +		/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
> +		udelay(1);
> +		gpiod_set_value(st->sample, 1);
> +		gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
> +
> +		ret = spi_read(st->sdev, st->rx, 2);
> +		if (ret < 0) {
> +			mutex_unlock(&st->lock);
> +			return ret;
> +		}
>  
> -	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
> -	udelay(1);
> -	gpiod_set_value(st->sample, 1);
> -	gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
> +		vel = be16_to_cpup((__be16 *)st->rx);
> +		switch (chan->type) {
> +		case IIO_ANGL:
> +			*val = vel >> 4;
> +			break;
> +		case IIO_ANGL_VEL:
> +			*val = sign_extend32((s16)vel >> 4, 11);
> +			break;
> +		default:
> +			mutex_unlock(&st->lock);
> +			return -EINVAL;
> +		}
>  
> -	ret = spi_read(st->sdev, st->rx, 2);
> -	if (ret < 0) {
> +		/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
> +		udelay(1);
>  		mutex_unlock(&st->lock);
> -		return ret;
> -	}
>  
> -	vel = be16_to_cpup((__be16 *)st->rx);
> -	switch (chan->type) {
> -	case IIO_ANGL:
> -		*val = vel >> 4;
> -		break;
> -	case IIO_ANGL_VEL:
> -		*val = sign_extend32((s16)vel >> 4, 11);
> -		break;
> +		return IIO_VAL_INT;
>  	default:
> -		mutex_unlock(&st->lock);
> -		return -EINVAL;
> +		break;
>  	}
>  
> -	/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
> -	udelay(1);
> -	mutex_unlock(&st->lock);
> -
> -	return IIO_VAL_INT;
> +	return -EINVAL;
>  }
>  
>  static const struct iio_chan_spec ad2s1200_channels[] = {
> @@ -105,6 +138,7 @@ static const struct iio_chan_spec ad2s1200_channels[] = {
>  		.indexed = 1,
>  		.channel = 0,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
>  	}
>  };
>  

Re: [PATCH v2 10/13] staging: iio: ad2s1200: Add scaling factor for angular velocity channel

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:31:09 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> The sysfs iio ABI states radians per second is expected as the unit for
> angular velocity, but the 12-bit angular velocity register has rps
> as its unit. So a fractional scaling factor of approximately 2 * Pi is
> added to the angular velocity channel.
> 
> The added comments will also be relevant for the scaling factor of
> the angle channel.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
Comment inline.  The function you are talking about isn't used
in the majority of likely use cases for this part.  The maths will actually
be done in userspace (which can use floating point).

Thanks,

Jonathan

> ---
> Changes in v2:
>   - Move explanation of Pi approximation to top of switch statement,
>     as this will also be relevant to angle channel.
>   - Replaced 33102 / 2 with 16551 on line 84.
> 
>  drivers/staging/iio/resolver/ad2s1200.c | 84 +++++++++++++++++++++++----------
>  1 file changed, 59 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index 29a9bb666e7b..6c56257be3b1 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -60,38 +60,71 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>  	int ret = 0;
>  	u16 vel;
>  
> -	mutex_lock(&st->lock);
> -	gpiod_set_value(st->sample, 0);
> +	/*
> +	 * Below a fractional approximation of Pi is needed.
> +	 * The following approximation will be used: 103993 / 33102.
> +	 * This is accurate in 9 decimals places.
> +	 *
> +	 * This fraction is based on OEIS series of nominator/denominator
> +	 * of convergents to Pi (A002485 and A002486).
> +	 */
> +	switch (m) {
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_ANGL_VEL:
> +			/*
> +			 * 2 * Pi ~= 2 * 103993 / 33102
> +			 *
> +			 * iio_convert_raw_to_processed uses integer
> +			 * division. This will cause at most 5% error
> +			 * (for very small values). But for 99.5% of the values
> +			 * it will cause less that 1% error.
This is an interesting comment, but relies on anyone actually
using iio_convert_raw_to_processed with this device.

I would imagine that 'in kernel' users of a resolver (who would use
that function) will be few and far between.  Mostly this will just
get passed to userspace.  That involves this being converted to
a decimal.  I would just specify it as one in the first place.

> +			 */
> +			*val = 103993;
> +			*val2 = 16551;
> +			return IIO_VAL_FRACTIONAL;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&st->lock);
> +		gpiod_set_value(st->sample, 0);
> +
> +		/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
> +		udelay(1);
> +		gpiod_set_value(st->sample, 1);
> +		gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
> +
> +		ret = spi_read(st->sdev, st->rx, 2);
> +		if (ret < 0) {
> +			mutex_unlock(&st->lock);
> +			return ret;
> +		}
>  
> -	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
> -	udelay(1);
> -	gpiod_set_value(st->sample, 1);
> -	gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
> +		vel = be16_to_cpup((__be16 *)st->rx);
> +		switch (chan->type) {
> +		case IIO_ANGL:
> +			*val = vel >> 4;
> +			break;
> +		case IIO_ANGL_VEL:
> +			*val = sign_extend32((s16)vel >> 4, 11);
> +			break;
> +		default:
> +			mutex_unlock(&st->lock);
> +			return -EINVAL;
> +		}
>  
> -	ret = spi_read(st->sdev, st->rx, 2);
> -	if (ret < 0) {
> +		/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
> +		udelay(1);
>  		mutex_unlock(&st->lock);
> -		return ret;
> -	}
>  
> -	vel = be16_to_cpup((__be16 *)st->rx);
> -	switch (chan->type) {
> -	case IIO_ANGL:
> -		*val = vel >> 4;
> -		break;
> -	case IIO_ANGL_VEL:
> -		*val = sign_extend32((s16)vel >> 4, 11);
> -		break;
> +		return IIO_VAL_INT;
>  	default:
> -		mutex_unlock(&st->lock);
> -		return -EINVAL;
> +		break;
>  	}
>  
> -	/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
> -	udelay(1);
> -	mutex_unlock(&st->lock);
> -
> -	return IIO_VAL_INT;
> +	return -EINVAL;
>  }
>  
>  static const struct iio_chan_spec ad2s1200_channels[] = {
> @@ -105,6 +138,7 @@ static const struct iio_chan_spec ad2s1200_channels[] = {
>  		.indexed = 1,
>  		.channel = 0,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
>  	}
>  };
>  

Re: [PATCH v2 11/13] staging: iio: Documentation: Add missing sysfs docs for angle channel

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:31:37 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> The iio resolver drivers in staging use angle channels. This patch
> add missing documentation for this type of channel.
> 
> As was discussed in [1], radians is chosen as the unit, to match the
> unit of angular velocity.
> 
> [1] https://marc.info/?l=linux-driver-devel&m=152190078308330&w=2
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> ---
> Change in v2:
>   - Introduces in this version.
> 
>  Documentation/ABI/testing/sysfs-bus-iio | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 6a5f34b4d5b9..8ad0e55f99ee 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -190,6 +190,15 @@ Description:
>  		but should match other such assignments on device).
>  		Units after application of scale and offset are m/s^2.
>  
> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_x_raw
> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_y_raw
> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_z_raw
This surprised me.  A resolver is not going to inherently have any
notion of a particular axis.
Would expect.
in_angl_raw

Jonathan

> +KernelVersion:	4.17
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Angle about axis x, y or z (may be arbitrarily assigned). Units
> +		after application of scale and offset are radians.
> +
>  What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_x_raw
>  What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_y_raw
>  What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_z_raw
> @@ -297,6 +306,7 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_pressure_offset
>  What:		/sys/bus/iio/devices/iio:deviceX/in_humidityrelative_offset
>  What:		/sys/bus/iio/devices/iio:deviceX/in_magn_offset
>  What:		/sys/bus/iio/devices/iio:deviceX/in_rot_offset
> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_offset
>  KernelVersion:	2.6.35
>  Contact:	linux-iio@vger.kernel.org
>  Description:
> @@ -350,6 +360,7 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_humidityrelative_scale
>  What:		/sys/bus/iio/devices/iio:deviceX/in_velocity_sqrt(x^2+y^2+z^2)_scale
>  What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_scale
>  What:		/sys/bus/iio/devices/iio:deviceX/in_countY_scale
> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_scale
>  KernelVersion:	2.6.35
>  Contact:	linux-iio@vger.kernel.org
>  Description:

Re: [PATCH v2 11/13] staging: iio: Documentation: Add missing sysfs docs for angle channel

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:31:37 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> The iio resolver drivers in staging use angle channels. This patch
> add missing documentation for this type of channel.
> 
> As was discussed in [1], radians is chosen as the unit, to match the
> unit of angular velocity.
> 
> [1] https://marc.info/?l=linux-driver-devel&m=152190078308330&w=2
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> ---
> Change in v2:
>   - Introduces in this version.
> 
>  Documentation/ABI/testing/sysfs-bus-iio | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 6a5f34b4d5b9..8ad0e55f99ee 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -190,6 +190,15 @@ Description:
>  		but should match other such assignments on device).
>  		Units after application of scale and offset are m/s^2.
>  
> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_x_raw
> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_y_raw
> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_z_raw
This surprised me.  A resolver is not going to inherently have any
notion of a particular axis.
Would expect.
in_angl_raw

Jonathan

> +KernelVersion:	4.17
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Angle about axis x, y or z (may be arbitrarily assigned). Units
> +		after application of scale and offset are radians.
> +
>  What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_x_raw
>  What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_y_raw
>  What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_z_raw
> @@ -297,6 +306,7 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_pressure_offset
>  What:		/sys/bus/iio/devices/iio:deviceX/in_humidityrelative_offset
>  What:		/sys/bus/iio/devices/iio:deviceX/in_magn_offset
>  What:		/sys/bus/iio/devices/iio:deviceX/in_rot_offset
> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_offset
>  KernelVersion:	2.6.35
>  Contact:	linux-iio@vger.kernel.org
>  Description:
> @@ -350,6 +360,7 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_humidityrelative_scale
>  What:		/sys/bus/iio/devices/iio:deviceX/in_velocity_sqrt(x^2+y^2+z^2)_scale
>  What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_scale
>  What:		/sys/bus/iio/devices/iio:deviceX/in_countY_scale
> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_scale
>  KernelVersion:	2.6.35
>  Contact:	linux-iio@vger.kernel.org
>  Description:

Re: [PATCH v2 12/13] staging: iio: ad2s1200: Add scaling factor for angle channel

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:31:48 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> A fractional scaling factor of approximately 2 * Pi / (2^12 -1) is added,
> to scale the 12-bits angular position to radians.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> ---
> Changes in v2:
>   - This patch replaces the patch that changed the
>     the channel for angular position to inclination
>     channel.
> 
>  drivers/staging/iio/resolver/ad2s1200.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index 6c56257be3b1..4f5dd28b174a 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -71,6 +71,17 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>  	switch (m) {
>  	case IIO_CHAN_INFO_SCALE:
>  		switch (chan->type) {
> +		case IIO_ANGL:
> +			/*
> +			 * 2 * Pi / (2^12 - 1) ~= 2 * 103993 / (33102 * 0xFFF)
> +			 *
> +			 * Since there only fit 3 whole radians in 360 degrees,
> +			 * usage of iio_convert_raw_to_processed for this
> +			 * channel will be highly inaccurate.
> +			 */
> +			*val = 103993;
> +			*val2 = 16551 * 0xFFF;
> +			return IIO_VAL_FRACTIONAL;
Again, use IIO_VAL_INT_PLUS_MICRO and specify it as a decimal.  There is
no need to jump through these hoops when we are unlikely to have any users
that need to keep to integer only maths.

Thanks,

Jonathan

>  		case IIO_ANGL_VEL:
>  			/*
>  			 * 2 * Pi ~= 2 * 103993 / 33102
> @@ -133,6 +144,7 @@ static const struct iio_chan_spec ad2s1200_channels[] = {
>  		.indexed = 1,
>  		.channel = 0,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
>  	}, {
>  		.type = IIO_ANGL_VEL,
>  		.indexed = 1,

Re: [PATCH v2 12/13] staging: iio: ad2s1200: Add scaling factor for angle channel

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:31:48 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> A fractional scaling factor of approximately 2 * Pi / (2^12 -1) is added,
> to scale the 12-bits angular position to radians.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> ---
> Changes in v2:
>   - This patch replaces the patch that changed the
>     the channel for angular position to inclination
>     channel.
> 
>  drivers/staging/iio/resolver/ad2s1200.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> index 6c56257be3b1..4f5dd28b174a 100644
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ b/drivers/staging/iio/resolver/ad2s1200.c
> @@ -71,6 +71,17 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>  	switch (m) {
>  	case IIO_CHAN_INFO_SCALE:
>  		switch (chan->type) {
> +		case IIO_ANGL:
> +			/*
> +			 * 2 * Pi / (2^12 - 1) ~= 2 * 103993 / (33102 * 0xFFF)
> +			 *
> +			 * Since there only fit 3 whole radians in 360 degrees,
> +			 * usage of iio_convert_raw_to_processed for this
> +			 * channel will be highly inaccurate.
> +			 */
> +			*val = 103993;
> +			*val2 = 16551 * 0xFFF;
> +			return IIO_VAL_FRACTIONAL;
Again, use IIO_VAL_INT_PLUS_MICRO and specify it as a decimal.  There is
no need to jump through these hoops when we are unlikely to have any users
that need to keep to integer only maths.

Thanks,

Jonathan

>  		case IIO_ANGL_VEL:
>  			/*
>  			 * 2 * Pi ~= 2 * 103993 / 33102
> @@ -133,6 +144,7 @@ static const struct iio_chan_spec ad2s1200_channels[] = {
>  		.indexed = 1,
>  		.channel = 0,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
>  	}, {
>  		.type = IIO_ANGL_VEL,
>  		.indexed = 1,

Re: [PATCH v2 13/13] staging: iio: ad2s1200: Move driver out of staging

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:32:01 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> Move the iio driver for the ad2s1200 resolver-to-digital
> converter out of staging, into mainline iio subsystems.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
I'll look at this once the minor issues with the other patches
are cleared up (so in v3!)

Thanks for your hard work on this. It seems to be heading rapidly
in the right direction.

Jonathan

> ---
> Changes in v2:
>   - Added commit message.
>   - Also move device tree binding documentation out of staging.
>   - Disabled move detection.
> 
>  .../devicetree/bindings/iio/resolver/ad2s1200.txt  |  16 ++
>  .../bindings/staging/iio/resolver/ad2s1200.txt     |  16 --
>  drivers/iio/Kconfig                                |   1 +
>  drivers/iio/Makefile                               |   1 +
>  drivers/iio/resolver/Kconfig                       |  17 ++
>  drivers/iio/resolver/Makefile                      |   5 +
>  drivers/iio/resolver/ad2s1200.c                    | 250 +++++++++++++++++++++
>  drivers/staging/iio/resolver/Kconfig               |  12 -
>  drivers/staging/iio/resolver/Makefile              |   1 -
>  drivers/staging/iio/resolver/ad2s1200.c            | 250 ---------------------
>  10 files changed, 290 insertions(+), 279 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt
>  delete mode 100644 Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
>  create mode 100644 drivers/iio/resolver/Kconfig
>  create mode 100644 drivers/iio/resolver/Makefile
>  create mode 100644 drivers/iio/resolver/ad2s1200.c
>  delete mode 100644 drivers/staging/iio/resolver/ad2s1200.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt b/Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt
> new file mode 100644
> index 000000000000..85c009987878
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt
> @@ -0,0 +1,16 @@
> +Analog Devices AD2S1200 Resolver-to-Digital Converter
> +
> +Required properties:
> + - compatible : should be "adi,ad2s1200"
> + - reg : the SPI chip select number of the device
> + - sample-gpios : The GPIO pin connected to the SAMPLE line of the AD2S1200
> + - rdvel-gpios : The GPIO pin connected to the RDVEL line of the AD2S1200
> +
> +Example:
> +
> +	resolver {
> +		compatible = "adi,ad2s1200";
> +		reg = <4>;
> +		sample-gpios = <&gpio 5 GPIO_ACTIVE_HIGH>;
> +		rdvel-gpios = <&gpio 6 GPIO_ACTIVE_HIGH>;
> +	};
> diff --git a/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt b/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
> deleted file mode 100644
> index 85c009987878..000000000000
> --- a/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
> +++ /dev/null
> @@ -1,16 +0,0 @@
> -Analog Devices AD2S1200 Resolver-to-Digital Converter
> -
> -Required properties:
> - - compatible : should be "adi,ad2s1200"
> - - reg : the SPI chip select number of the device
> - - sample-gpios : The GPIO pin connected to the SAMPLE line of the AD2S1200
> - - rdvel-gpios : The GPIO pin connected to the RDVEL line of the AD2S1200
> -
> -Example:
> -
> -	resolver {
> -		compatible = "adi,ad2s1200";
> -		reg = <4>;
> -		sample-gpios = <&gpio 5 GPIO_ACTIVE_HIGH>;
> -		rdvel-gpios = <&gpio 6 GPIO_ACTIVE_HIGH>;
> -	};
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index b3c8c6ef0dff..4bec3ccbf4a1 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -92,6 +92,7 @@ source "drivers/iio/potentiometer/Kconfig"
>  source "drivers/iio/potentiostat/Kconfig"
>  source "drivers/iio/pressure/Kconfig"
>  source "drivers/iio/proximity/Kconfig"
> +source "drivers/iio/resolver/Kconfig"
>  source "drivers/iio/temperature/Kconfig"
>  
>  endif # IIO
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index b16b2e9ddc40..1865361b8714 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -35,5 +35,6 @@ obj-y += potentiometer/
>  obj-y += potentiostat/
>  obj-y += pressure/
>  obj-y += proximity/
> +obj-y += resolver/
>  obj-y += temperature/
>  obj-y += trigger/
> diff --git a/drivers/iio/resolver/Kconfig b/drivers/iio/resolver/Kconfig
> new file mode 100644
> index 000000000000..2ced9f22aa70
> --- /dev/null
> +++ b/drivers/iio/resolver/Kconfig
> @@ -0,0 +1,17 @@
> +#
> +# Resolver/Synchro drivers
> +#
> +menu "Resolver to digital converters"
> +
> +config AD2S1200
> +	tristate "Analog Devices ad2s1200/ad2s1205 driver"
> +	depends on SPI
> +	depends on GPIOLIB || COMPILE_TEST
> +	help
> +	  Say yes here to build support for Analog Devices spi resolver
> +	  to digital converters, ad2s1200 and ad2s1205, provides direct access
> +	  via sysfs.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ad2s1200.
> +endmenu
> diff --git a/drivers/iio/resolver/Makefile b/drivers/iio/resolver/Makefile
> new file mode 100644
> index 000000000000..4e1dccae07e7
> --- /dev/null
> +++ b/drivers/iio/resolver/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# Makefile for Resolver/Synchro drivers
> +#
> +
> +obj-$(CONFIG_AD2S1200) += ad2s1200.o
> diff --git a/drivers/iio/resolver/ad2s1200.c b/drivers/iio/resolver/ad2s1200.c
> new file mode 100644
> index 000000000000..4f5dd28b174a
> --- /dev/null
> +++ b/drivers/iio/resolver/ad2s1200.c
> @@ -0,0 +1,250 @@
> +/*
> + * ad2s1200.c simple support for the ADI Resolver to Digital Converters:
> + * AD2S1200/1205
> + *
> + * Copyright (c) 2010-2010 Analog Devices Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/spi/spi.h>
> +#include <linux/sysfs.h>
> +#include <linux/types.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define DRV_NAME "ad2s1200"
> +
> +/* input pin sample and rdvel is controlled by driver */
> +#define AD2S1200_PN	2
> +
> +/* input clock on serial interface */
> +#define AD2S1200_HZ	8192000
> +/* clock period in nano second */
> +#define AD2S1200_TSCLK	(1000000000 / AD2S1200_HZ)
> +
> +/**
> + * struct ad2s1200_state - driver instance specific data
> + * @lock:	protect driver state
> + * @sdev:	spi device
> + * @sample:	GPIO pin SAMPLE
> + * @rdvel:	GPIO pin RDVEL
> + * @rx:		buffer for spi transfers
> + */
> +struct ad2s1200_state {
> +	struct mutex lock;
> +	struct spi_device *sdev;
> +	struct gpio_desc *sample;
> +	struct gpio_desc *rdvel;
> +	u8 rx[2] ____cacheline_aligned;
> +};
> +
> +static int ad2s1200_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int *val,
> +			     int *val2,
> +			     long m)
> +{
> +	struct ad2s1200_state *st = iio_priv(indio_dev);
> +	int ret = 0;
> +	u16 vel;
> +
> +	/*
> +	 * Below a fractional approximation of Pi is needed.
> +	 * The following approximation will be used: 103993 / 33102.
> +	 * This is accurate in 9 decimals places.
> +	 *
> +	 * This fraction is based on OEIS series of nominator/denominator
> +	 * of convergents to Pi (A002485 and A002486).
> +	 */
> +	switch (m) {
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_ANGL:
> +			/*
> +			 * 2 * Pi / (2^12 - 1) ~= 2 * 103993 / (33102 * 0xFFF)
> +			 *
> +			 * Since there only fit 3 whole radians in 360 degrees,
> +			 * usage of iio_convert_raw_to_processed for this
> +			 * channel will be highly inaccurate.
> +			 */
> +			*val = 103993;
> +			*val2 = 16551 * 0xFFF;
> +			return IIO_VAL_FRACTIONAL;
> +		case IIO_ANGL_VEL:
> +			/*
> +			 * 2 * Pi ~= 2 * 103993 / 33102
> +			 *
> +			 * iio_convert_raw_to_processed uses integer
> +			 * division. This will cause at most 5% error
> +			 * (for very small values). But for 99.5% of the values
> +			 * it will cause less that 1% error.
> +			 */
> +			*val = 103993;
> +			*val2 = 16551;
> +			return IIO_VAL_FRACTIONAL;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&st->lock);
> +		gpiod_set_value(st->sample, 0);
> +
> +		/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
> +		udelay(1);
> +		gpiod_set_value(st->sample, 1);
> +		gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
> +
> +		ret = spi_read(st->sdev, st->rx, 2);
> +		if (ret < 0) {
> +			mutex_unlock(&st->lock);
> +			return ret;
> +		}
> +
> +		vel = be16_to_cpup((__be16 *)st->rx);
> +		switch (chan->type) {
> +		case IIO_ANGL:
> +			*val = vel >> 4;
> +			break;
> +		case IIO_ANGL_VEL:
> +			*val = sign_extend32((s16)vel >> 4, 11);
> +			break;
> +		default:
> +			mutex_unlock(&st->lock);
> +			return -EINVAL;
> +		}
> +
> +		/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
> +		udelay(1);
> +		mutex_unlock(&st->lock);
> +
> +		return IIO_VAL_INT;
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct iio_chan_spec ad2s1200_channels[] = {
> +	{
> +		.type = IIO_ANGL,
> +		.indexed = 1,
> +		.channel = 0,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> +	}, {
> +		.type = IIO_ANGL_VEL,
> +		.indexed = 1,
> +		.channel = 0,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> +	}
> +};
> +
> +static const struct iio_info ad2s1200_info = {
> +	.read_raw = ad2s1200_read_raw,
> +};
> +
> +static int ad2s1200_probe(struct spi_device *spi)
> +{
> +	unsigned short *pins = spi->dev.platform_data;
> +	struct ad2s1200_state *st;
> +	struct iio_dev *indio_dev;
> +	struct device *dev;
> +	int pn, ret = 0;
> +
> +	dev = &spi->dev;
> +
> +	if (pins) {
> +		for (pn = 0; pn < AD2S1200_PN; pn++) {
> +			ret = devm_gpio_request_one(dev, pins[pn],
> +						    GPIOF_DIR_OUT,
> +						    DRV_NAME);
> +			if (ret) {
> +				dev_err(dev,
> +					"Failed to claim gpio %d\n: err=%d",
> +					pins[pn],
> +					ret);
> +				return ret;
> +			}
> +		}
> +	}
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	spi_set_drvdata(spi, indio_dev);
> +	st = iio_priv(indio_dev);
> +	mutex_init(&st->lock);
> +	st->sdev = spi;
> +
> +	if (pins) {
> +		st->sample = gpio_to_desc(pins[0]);
> +		st->rdvel = gpio_to_desc(pins[1]);
> +	} else {
> +		st->sample = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
> +		if (IS_ERR(st->sample)) {
> +			dev_err(dev, "Failed to claim SAMPLE gpio: err=%ld\n",
> +				PTR_ERR(st->sample));
> +			return PTR_ERR(st->sample);
> +		}
> +
> +		st->rdvel = devm_gpiod_get(dev, "rdvel", GPIOD_OUT_LOW);
> +		if (IS_ERR(st->rdvel)) {
> +			dev_err(dev, "Failed to claim RDVEL gpio: err=%ld\n",
> +				PTR_ERR(st->rdvel));
> +			return PTR_ERR(st->rdvel);
> +		}
> +	}
> +
> +	indio_dev->dev.parent = dev;
> +	indio_dev->info = &ad2s1200_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = ad2s1200_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +
> +	ret = devm_iio_device_register(dev, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	spi->max_speed_hz = AD2S1200_HZ;
> +	spi->mode = SPI_MODE_3;
> +	spi_setup(spi);
> +
> +	return 0;
> +}
> +
> +static const struct spi_device_id ad2s1200_id[] = {
> +	{ "ad2s1200" },
> +	{ "ad2s1205" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(spi, ad2s1200_id);
> +
> +static struct spi_driver ad2s1200_driver = {
> +	.driver = {
> +		.name = DRV_NAME,
> +	},
> +	.probe = ad2s1200_probe,
> +	.id_table = ad2s1200_id,
> +};
> +module_spi_driver(ad2s1200_driver);
> +
> +MODULE_AUTHOR("Graff Yang <graff.yang@gmail.com>");
> +MODULE_DESCRIPTION("Analog Devices AD2S1200/1205 Resolver to Digital SPI driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/staging/iio/resolver/Kconfig b/drivers/staging/iio/resolver/Kconfig
> index 1c7e2860d6b7..6a469ee6101f 100644
> --- a/drivers/staging/iio/resolver/Kconfig
> +++ b/drivers/staging/iio/resolver/Kconfig
> @@ -13,18 +13,6 @@ config AD2S90
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ad2s90.
>  
> -config AD2S1200
> -	tristate "Analog Devices ad2s1200/ad2s1205 driver"
> -	depends on SPI
> -	depends on GPIOLIB || COMPILE_TEST
> -	help
> -	  Say yes here to build support for Analog Devices spi resolver
> -	  to digital converters, ad2s1200 and ad2s1205, provides direct access
> -	  via sysfs.
> -
> -	  To compile this driver as a module, choose M here: the
> -	  module will be called ad2s1200.
> -
>  config AD2S1210
>  	tristate "Analog Devices ad2s1210 driver"
>  	depends on SPI
> diff --git a/drivers/staging/iio/resolver/Makefile b/drivers/staging/iio/resolver/Makefile
> index 14375e444ebf..8d901dc7500b 100644
> --- a/drivers/staging/iio/resolver/Makefile
> +++ b/drivers/staging/iio/resolver/Makefile
> @@ -3,5 +3,4 @@
>  #
>  
>  obj-$(CONFIG_AD2S90) += ad2s90.o
> -obj-$(CONFIG_AD2S1200) += ad2s1200.o
>  obj-$(CONFIG_AD2S1210) += ad2s1210.o
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> deleted file mode 100644
> index 4f5dd28b174a..000000000000
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ /dev/null
> @@ -1,250 +0,0 @@
> -/*
> - * ad2s1200.c simple support for the ADI Resolver to Digital Converters:
> - * AD2S1200/1205
> - *
> - * Copyright (c) 2010-2010 Analog Devices Inc.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> - *
> - */
> -
> -#include <linux/bitops.h>
> -#include <linux/delay.h>
> -#include <linux/device.h>
> -#include <linux/gpio.h>
> -#include <linux/gpio/consumer.h>
> -#include <linux/module.h>
> -#include <linux/mutex.h>
> -#include <linux/spi/spi.h>
> -#include <linux/sysfs.h>
> -#include <linux/types.h>
> -
> -#include <linux/iio/iio.h>
> -#include <linux/iio/sysfs.h>
> -
> -#define DRV_NAME "ad2s1200"
> -
> -/* input pin sample and rdvel is controlled by driver */
> -#define AD2S1200_PN	2
> -
> -/* input clock on serial interface */
> -#define AD2S1200_HZ	8192000
> -/* clock period in nano second */
> -#define AD2S1200_TSCLK	(1000000000 / AD2S1200_HZ)
> -
> -/**
> - * struct ad2s1200_state - driver instance specific data
> - * @lock:	protect driver state
> - * @sdev:	spi device
> - * @sample:	GPIO pin SAMPLE
> - * @rdvel:	GPIO pin RDVEL
> - * @rx:		buffer for spi transfers
> - */
> -struct ad2s1200_state {
> -	struct mutex lock;
> -	struct spi_device *sdev;
> -	struct gpio_desc *sample;
> -	struct gpio_desc *rdvel;
> -	u8 rx[2] ____cacheline_aligned;
> -};
> -
> -static int ad2s1200_read_raw(struct iio_dev *indio_dev,
> -			     struct iio_chan_spec const *chan,
> -			     int *val,
> -			     int *val2,
> -			     long m)
> -{
> -	struct ad2s1200_state *st = iio_priv(indio_dev);
> -	int ret = 0;
> -	u16 vel;
> -
> -	/*
> -	 * Below a fractional approximation of Pi is needed.
> -	 * The following approximation will be used: 103993 / 33102.
> -	 * This is accurate in 9 decimals places.
> -	 *
> -	 * This fraction is based on OEIS series of nominator/denominator
> -	 * of convergents to Pi (A002485 and A002486).
> -	 */
> -	switch (m) {
> -	case IIO_CHAN_INFO_SCALE:
> -		switch (chan->type) {
> -		case IIO_ANGL:
> -			/*
> -			 * 2 * Pi / (2^12 - 1) ~= 2 * 103993 / (33102 * 0xFFF)
> -			 *
> -			 * Since there only fit 3 whole radians in 360 degrees,
> -			 * usage of iio_convert_raw_to_processed for this
> -			 * channel will be highly inaccurate.
> -			 */
> -			*val = 103993;
> -			*val2 = 16551 * 0xFFF;
> -			return IIO_VAL_FRACTIONAL;
> -		case IIO_ANGL_VEL:
> -			/*
> -			 * 2 * Pi ~= 2 * 103993 / 33102
> -			 *
> -			 * iio_convert_raw_to_processed uses integer
> -			 * division. This will cause at most 5% error
> -			 * (for very small values). But for 99.5% of the values
> -			 * it will cause less that 1% error.
> -			 */
> -			*val = 103993;
> -			*val2 = 16551;
> -			return IIO_VAL_FRACTIONAL;
> -		default:
> -			return -EINVAL;
> -		}
> -		break;
> -	case IIO_CHAN_INFO_RAW:
> -		mutex_lock(&st->lock);
> -		gpiod_set_value(st->sample, 0);
> -
> -		/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
> -		udelay(1);
> -		gpiod_set_value(st->sample, 1);
> -		gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
> -
> -		ret = spi_read(st->sdev, st->rx, 2);
> -		if (ret < 0) {
> -			mutex_unlock(&st->lock);
> -			return ret;
> -		}
> -
> -		vel = be16_to_cpup((__be16 *)st->rx);
> -		switch (chan->type) {
> -		case IIO_ANGL:
> -			*val = vel >> 4;
> -			break;
> -		case IIO_ANGL_VEL:
> -			*val = sign_extend32((s16)vel >> 4, 11);
> -			break;
> -		default:
> -			mutex_unlock(&st->lock);
> -			return -EINVAL;
> -		}
> -
> -		/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
> -		udelay(1);
> -		mutex_unlock(&st->lock);
> -
> -		return IIO_VAL_INT;
> -	default:
> -		break;
> -	}
> -
> -	return -EINVAL;
> -}
> -
> -static const struct iio_chan_spec ad2s1200_channels[] = {
> -	{
> -		.type = IIO_ANGL,
> -		.indexed = 1,
> -		.channel = 0,
> -		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> -		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> -	}, {
> -		.type = IIO_ANGL_VEL,
> -		.indexed = 1,
> -		.channel = 0,
> -		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> -		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> -	}
> -};
> -
> -static const struct iio_info ad2s1200_info = {
> -	.read_raw = ad2s1200_read_raw,
> -};
> -
> -static int ad2s1200_probe(struct spi_device *spi)
> -{
> -	unsigned short *pins = spi->dev.platform_data;
> -	struct ad2s1200_state *st;
> -	struct iio_dev *indio_dev;
> -	struct device *dev;
> -	int pn, ret = 0;
> -
> -	dev = &spi->dev;
> -
> -	if (pins) {
> -		for (pn = 0; pn < AD2S1200_PN; pn++) {
> -			ret = devm_gpio_request_one(dev, pins[pn],
> -						    GPIOF_DIR_OUT,
> -						    DRV_NAME);
> -			if (ret) {
> -				dev_err(dev,
> -					"Failed to claim gpio %d\n: err=%d",
> -					pins[pn],
> -					ret);
> -				return ret;
> -			}
> -		}
> -	}
> -
> -	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> -	if (!indio_dev)
> -		return -ENOMEM;
> -
> -	spi_set_drvdata(spi, indio_dev);
> -	st = iio_priv(indio_dev);
> -	mutex_init(&st->lock);
> -	st->sdev = spi;
> -
> -	if (pins) {
> -		st->sample = gpio_to_desc(pins[0]);
> -		st->rdvel = gpio_to_desc(pins[1]);
> -	} else {
> -		st->sample = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
> -		if (IS_ERR(st->sample)) {
> -			dev_err(dev, "Failed to claim SAMPLE gpio: err=%ld\n",
> -				PTR_ERR(st->sample));
> -			return PTR_ERR(st->sample);
> -		}
> -
> -		st->rdvel = devm_gpiod_get(dev, "rdvel", GPIOD_OUT_LOW);
> -		if (IS_ERR(st->rdvel)) {
> -			dev_err(dev, "Failed to claim RDVEL gpio: err=%ld\n",
> -				PTR_ERR(st->rdvel));
> -			return PTR_ERR(st->rdvel);
> -		}
> -	}
> -
> -	indio_dev->dev.parent = dev;
> -	indio_dev->info = &ad2s1200_info;
> -	indio_dev->modes = INDIO_DIRECT_MODE;
> -	indio_dev->channels = ad2s1200_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
> -	indio_dev->name = spi_get_device_id(spi)->name;
> -
> -	ret = devm_iio_device_register(dev, indio_dev);
> -	if (ret)
> -		return ret;
> -
> -	spi->max_speed_hz = AD2S1200_HZ;
> -	spi->mode = SPI_MODE_3;
> -	spi_setup(spi);
> -
> -	return 0;
> -}
> -
> -static const struct spi_device_id ad2s1200_id[] = {
> -	{ "ad2s1200" },
> -	{ "ad2s1205" },
> -	{}
> -};
> -MODULE_DEVICE_TABLE(spi, ad2s1200_id);
> -
> -static struct spi_driver ad2s1200_driver = {
> -	.driver = {
> -		.name = DRV_NAME,
> -	},
> -	.probe = ad2s1200_probe,
> -	.id_table = ad2s1200_id,
> -};
> -module_spi_driver(ad2s1200_driver);
> -
> -MODULE_AUTHOR("Graff Yang <graff.yang@gmail.com>");
> -MODULE_DESCRIPTION("Analog Devices AD2S1200/1205 Resolver to Digital SPI driver");
> -MODULE_LICENSE("GPL v2");

Re: [PATCH v2 13/13] staging: iio: ad2s1200: Move driver out of staging

[Jonathan Cameron]

On Fri, 20 Apr 2018 21:32:01 +0200
David Veenstra <davidjulianveenstra@gmail.com> wrote:

> Move the iio driver for the ad2s1200 resolver-to-digital
> converter out of staging, into mainline iio subsystems.
> 
> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
I'll look at this once the minor issues with the other patches
are cleared up (so in v3!)

Thanks for your hard work on this. It seems to be heading rapidly
in the right direction.

Jonathan

> ---
> Changes in v2:
>   - Added commit message.
>   - Also move device tree binding documentation out of staging.
>   - Disabled move detection.
> 
>  .../devicetree/bindings/iio/resolver/ad2s1200.txt  |  16 ++
>  .../bindings/staging/iio/resolver/ad2s1200.txt     |  16 --
>  drivers/iio/Kconfig                                |   1 +
>  drivers/iio/Makefile                               |   1 +
>  drivers/iio/resolver/Kconfig                       |  17 ++
>  drivers/iio/resolver/Makefile                      |   5 +
>  drivers/iio/resolver/ad2s1200.c                    | 250 +++++++++++++++++++++
>  drivers/staging/iio/resolver/Kconfig               |  12 -
>  drivers/staging/iio/resolver/Makefile              |   1 -
>  drivers/staging/iio/resolver/ad2s1200.c            | 250 ---------------------
>  10 files changed, 290 insertions(+), 279 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt
>  delete mode 100644 Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
>  create mode 100644 drivers/iio/resolver/Kconfig
>  create mode 100644 drivers/iio/resolver/Makefile
>  create mode 100644 drivers/iio/resolver/ad2s1200.c
>  delete mode 100644 drivers/staging/iio/resolver/ad2s1200.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt b/Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt
> new file mode 100644
> index 000000000000..85c009987878
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt
> @@ -0,0 +1,16 @@
> +Analog Devices AD2S1200 Resolver-to-Digital Converter
> +
> +Required properties:
> + - compatible : should be "adi,ad2s1200"
> + - reg : the SPI chip select number of the device
> + - sample-gpios : The GPIO pin connected to the SAMPLE line of the AD2S1200
> + - rdvel-gpios : The GPIO pin connected to the RDVEL line of the AD2S1200
> +
> +Example:
> +
> +	resolver {
> +		compatible = "adi,ad2s1200";
> +		reg = <4>;
> +		sample-gpios = <&gpio 5 GPIO_ACTIVE_HIGH>;
> +		rdvel-gpios = <&gpio 6 GPIO_ACTIVE_HIGH>;
> +	};
> diff --git a/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt b/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
> deleted file mode 100644
> index 85c009987878..000000000000
> --- a/Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt
> +++ /dev/null
> @@ -1,16 +0,0 @@
> -Analog Devices AD2S1200 Resolver-to-Digital Converter
> -
> -Required properties:
> - - compatible : should be "adi,ad2s1200"
> - - reg : the SPI chip select number of the device
> - - sample-gpios : The GPIO pin connected to the SAMPLE line of the AD2S1200
> - - rdvel-gpios : The GPIO pin connected to the RDVEL line of the AD2S1200
> -
> -Example:
> -
> -	resolver {
> -		compatible = "adi,ad2s1200";
> -		reg = <4>;
> -		sample-gpios = <&gpio 5 GPIO_ACTIVE_HIGH>;
> -		rdvel-gpios = <&gpio 6 GPIO_ACTIVE_HIGH>;
> -	};
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index b3c8c6ef0dff..4bec3ccbf4a1 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -92,6 +92,7 @@ source "drivers/iio/potentiometer/Kconfig"
>  source "drivers/iio/potentiostat/Kconfig"
>  source "drivers/iio/pressure/Kconfig"
>  source "drivers/iio/proximity/Kconfig"
> +source "drivers/iio/resolver/Kconfig"
>  source "drivers/iio/temperature/Kconfig"
>  
>  endif # IIO
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index b16b2e9ddc40..1865361b8714 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -35,5 +35,6 @@ obj-y += potentiometer/
>  obj-y += potentiostat/
>  obj-y += pressure/
>  obj-y += proximity/
> +obj-y += resolver/
>  obj-y += temperature/
>  obj-y += trigger/
> diff --git a/drivers/iio/resolver/Kconfig b/drivers/iio/resolver/Kconfig
> new file mode 100644
> index 000000000000..2ced9f22aa70
> --- /dev/null
> +++ b/drivers/iio/resolver/Kconfig
> @@ -0,0 +1,17 @@
> +#
> +# Resolver/Synchro drivers
> +#
> +menu "Resolver to digital converters"
> +
> +config AD2S1200
> +	tristate "Analog Devices ad2s1200/ad2s1205 driver"
> +	depends on SPI
> +	depends on GPIOLIB || COMPILE_TEST
> +	help
> +	  Say yes here to build support for Analog Devices spi resolver
> +	  to digital converters, ad2s1200 and ad2s1205, provides direct access
> +	  via sysfs.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ad2s1200.
> +endmenu
> diff --git a/drivers/iio/resolver/Makefile b/drivers/iio/resolver/Makefile
> new file mode 100644
> index 000000000000..4e1dccae07e7
> --- /dev/null
> +++ b/drivers/iio/resolver/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# Makefile for Resolver/Synchro drivers
> +#
> +
> +obj-$(CONFIG_AD2S1200) += ad2s1200.o
> diff --git a/drivers/iio/resolver/ad2s1200.c b/drivers/iio/resolver/ad2s1200.c
> new file mode 100644
> index 000000000000..4f5dd28b174a
> --- /dev/null
> +++ b/drivers/iio/resolver/ad2s1200.c
> @@ -0,0 +1,250 @@
> +/*
> + * ad2s1200.c simple support for the ADI Resolver to Digital Converters:
> + * AD2S1200/1205
> + *
> + * Copyright (c) 2010-2010 Analog Devices Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/spi/spi.h>
> +#include <linux/sysfs.h>
> +#include <linux/types.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define DRV_NAME "ad2s1200"
> +
> +/* input pin sample and rdvel is controlled by driver */
> +#define AD2S1200_PN	2
> +
> +/* input clock on serial interface */
> +#define AD2S1200_HZ	8192000
> +/* clock period in nano second */
> +#define AD2S1200_TSCLK	(1000000000 / AD2S1200_HZ)
> +
> +/**
> + * struct ad2s1200_state - driver instance specific data
> + * @lock:	protect driver state
> + * @sdev:	spi device
> + * @sample:	GPIO pin SAMPLE
> + * @rdvel:	GPIO pin RDVEL
> + * @rx:		buffer for spi transfers
> + */
> +struct ad2s1200_state {
> +	struct mutex lock;
> +	struct spi_device *sdev;
> +	struct gpio_desc *sample;
> +	struct gpio_desc *rdvel;
> +	u8 rx[2] ____cacheline_aligned;
> +};
> +
> +static int ad2s1200_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int *val,
> +			     int *val2,
> +			     long m)
> +{
> +	struct ad2s1200_state *st = iio_priv(indio_dev);
> +	int ret = 0;
> +	u16 vel;
> +
> +	/*
> +	 * Below a fractional approximation of Pi is needed.
> +	 * The following approximation will be used: 103993 / 33102.
> +	 * This is accurate in 9 decimals places.
> +	 *
> +	 * This fraction is based on OEIS series of nominator/denominator
> +	 * of convergents to Pi (A002485 and A002486).
> +	 */
> +	switch (m) {
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_ANGL:
> +			/*
> +			 * 2 * Pi / (2^12 - 1) ~= 2 * 103993 / (33102 * 0xFFF)
> +			 *
> +			 * Since there only fit 3 whole radians in 360 degrees,
> +			 * usage of iio_convert_raw_to_processed for this
> +			 * channel will be highly inaccurate.
> +			 */
> +			*val = 103993;
> +			*val2 = 16551 * 0xFFF;
> +			return IIO_VAL_FRACTIONAL;
> +		case IIO_ANGL_VEL:
> +			/*
> +			 * 2 * Pi ~= 2 * 103993 / 33102
> +			 *
> +			 * iio_convert_raw_to_processed uses integer
> +			 * division. This will cause at most 5% error
> +			 * (for very small values). But for 99.5% of the values
> +			 * it will cause less that 1% error.
> +			 */
> +			*val = 103993;
> +			*val2 = 16551;
> +			return IIO_VAL_FRACTIONAL;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&st->lock);
> +		gpiod_set_value(st->sample, 0);
> +
> +		/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
> +		udelay(1);
> +		gpiod_set_value(st->sample, 1);
> +		gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
> +
> +		ret = spi_read(st->sdev, st->rx, 2);
> +		if (ret < 0) {
> +			mutex_unlock(&st->lock);
> +			return ret;
> +		}
> +
> +		vel = be16_to_cpup((__be16 *)st->rx);
> +		switch (chan->type) {
> +		case IIO_ANGL:
> +			*val = vel >> 4;
> +			break;
> +		case IIO_ANGL_VEL:
> +			*val = sign_extend32((s16)vel >> 4, 11);
> +			break;
> +		default:
> +			mutex_unlock(&st->lock);
> +			return -EINVAL;
> +		}
> +
> +		/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
> +		udelay(1);
> +		mutex_unlock(&st->lock);
> +
> +		return IIO_VAL_INT;
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct iio_chan_spec ad2s1200_channels[] = {
> +	{
> +		.type = IIO_ANGL,
> +		.indexed = 1,
> +		.channel = 0,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> +	}, {
> +		.type = IIO_ANGL_VEL,
> +		.indexed = 1,
> +		.channel = 0,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> +	}
> +};
> +
> +static const struct iio_info ad2s1200_info = {
> +	.read_raw = ad2s1200_read_raw,
> +};
> +
> +static int ad2s1200_probe(struct spi_device *spi)
> +{
> +	unsigned short *pins = spi->dev.platform_data;
> +	struct ad2s1200_state *st;
> +	struct iio_dev *indio_dev;
> +	struct device *dev;
> +	int pn, ret = 0;
> +
> +	dev = &spi->dev;
> +
> +	if (pins) {
> +		for (pn = 0; pn < AD2S1200_PN; pn++) {
> +			ret = devm_gpio_request_one(dev, pins[pn],
> +						    GPIOF_DIR_OUT,
> +						    DRV_NAME);
> +			if (ret) {
> +				dev_err(dev,
> +					"Failed to claim gpio %d\n: err=%d",
> +					pins[pn],
> +					ret);
> +				return ret;
> +			}
> +		}
> +	}
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	spi_set_drvdata(spi, indio_dev);
> +	st = iio_priv(indio_dev);
> +	mutex_init(&st->lock);
> +	st->sdev = spi;
> +
> +	if (pins) {
> +		st->sample = gpio_to_desc(pins[0]);
> +		st->rdvel = gpio_to_desc(pins[1]);
> +	} else {
> +		st->sample = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
> +		if (IS_ERR(st->sample)) {
> +			dev_err(dev, "Failed to claim SAMPLE gpio: err=%ld\n",
> +				PTR_ERR(st->sample));
> +			return PTR_ERR(st->sample);
> +		}
> +
> +		st->rdvel = devm_gpiod_get(dev, "rdvel", GPIOD_OUT_LOW);
> +		if (IS_ERR(st->rdvel)) {
> +			dev_err(dev, "Failed to claim RDVEL gpio: err=%ld\n",
> +				PTR_ERR(st->rdvel));
> +			return PTR_ERR(st->rdvel);
> +		}
> +	}
> +
> +	indio_dev->dev.parent = dev;
> +	indio_dev->info = &ad2s1200_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = ad2s1200_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +
> +	ret = devm_iio_device_register(dev, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	spi->max_speed_hz = AD2S1200_HZ;
> +	spi->mode = SPI_MODE_3;
> +	spi_setup(spi);
> +
> +	return 0;
> +}
> +
> +static const struct spi_device_id ad2s1200_id[] = {
> +	{ "ad2s1200" },
> +	{ "ad2s1205" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(spi, ad2s1200_id);
> +
> +static struct spi_driver ad2s1200_driver = {
> +	.driver = {
> +		.name = DRV_NAME,
> +	},
> +	.probe = ad2s1200_probe,
> +	.id_table = ad2s1200_id,
> +};
> +module_spi_driver(ad2s1200_driver);
> +
> +MODULE_AUTHOR("Graff Yang <graff.yang@gmail.com>");
> +MODULE_DESCRIPTION("Analog Devices AD2S1200/1205 Resolver to Digital SPI driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/staging/iio/resolver/Kconfig b/drivers/staging/iio/resolver/Kconfig
> index 1c7e2860d6b7..6a469ee6101f 100644
> --- a/drivers/staging/iio/resolver/Kconfig
> +++ b/drivers/staging/iio/resolver/Kconfig
> @@ -13,18 +13,6 @@ config AD2S90
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ad2s90.
>  
> -config AD2S1200
> -	tristate "Analog Devices ad2s1200/ad2s1205 driver"
> -	depends on SPI
> -	depends on GPIOLIB || COMPILE_TEST
> -	help
> -	  Say yes here to build support for Analog Devices spi resolver
> -	  to digital converters, ad2s1200 and ad2s1205, provides direct access
> -	  via sysfs.
> -
> -	  To compile this driver as a module, choose M here: the
> -	  module will be called ad2s1200.
> -
>  config AD2S1210
>  	tristate "Analog Devices ad2s1210 driver"
>  	depends on SPI
> diff --git a/drivers/staging/iio/resolver/Makefile b/drivers/staging/iio/resolver/Makefile
> index 14375e444ebf..8d901dc7500b 100644
> --- a/drivers/staging/iio/resolver/Makefile
> +++ b/drivers/staging/iio/resolver/Makefile
> @@ -3,5 +3,4 @@
>  #
>  
>  obj-$(CONFIG_AD2S90) += ad2s90.o
> -obj-$(CONFIG_AD2S1200) += ad2s1200.o
>  obj-$(CONFIG_AD2S1210) += ad2s1210.o
> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
> deleted file mode 100644
> index 4f5dd28b174a..000000000000
> --- a/drivers/staging/iio/resolver/ad2s1200.c
> +++ /dev/null
> @@ -1,250 +0,0 @@
> -/*
> - * ad2s1200.c simple support for the ADI Resolver to Digital Converters:
> - * AD2S1200/1205
> - *
> - * Copyright (c) 2010-2010 Analog Devices Inc.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> - *
> - */
> -
> -#include <linux/bitops.h>
> -#include <linux/delay.h>
> -#include <linux/device.h>
> -#include <linux/gpio.h>
> -#include <linux/gpio/consumer.h>
> -#include <linux/module.h>
> -#include <linux/mutex.h>
> -#include <linux/spi/spi.h>
> -#include <linux/sysfs.h>
> -#include <linux/types.h>
> -
> -#include <linux/iio/iio.h>
> -#include <linux/iio/sysfs.h>
> -
> -#define DRV_NAME "ad2s1200"
> -
> -/* input pin sample and rdvel is controlled by driver */
> -#define AD2S1200_PN	2
> -
> -/* input clock on serial interface */
> -#define AD2S1200_HZ	8192000
> -/* clock period in nano second */
> -#define AD2S1200_TSCLK	(1000000000 / AD2S1200_HZ)
> -
> -/**
> - * struct ad2s1200_state - driver instance specific data
> - * @lock:	protect driver state
> - * @sdev:	spi device
> - * @sample:	GPIO pin SAMPLE
> - * @rdvel:	GPIO pin RDVEL
> - * @rx:		buffer for spi transfers
> - */
> -struct ad2s1200_state {
> -	struct mutex lock;
> -	struct spi_device *sdev;
> -	struct gpio_desc *sample;
> -	struct gpio_desc *rdvel;
> -	u8 rx[2] ____cacheline_aligned;
> -};
> -
> -static int ad2s1200_read_raw(struct iio_dev *indio_dev,
> -			     struct iio_chan_spec const *chan,
> -			     int *val,
> -			     int *val2,
> -			     long m)
> -{
> -	struct ad2s1200_state *st = iio_priv(indio_dev);
> -	int ret = 0;
> -	u16 vel;
> -
> -	/*
> -	 * Below a fractional approximation of Pi is needed.
> -	 * The following approximation will be used: 103993 / 33102.
> -	 * This is accurate in 9 decimals places.
> -	 *
> -	 * This fraction is based on OEIS series of nominator/denominator
> -	 * of convergents to Pi (A002485 and A002486).
> -	 */
> -	switch (m) {
> -	case IIO_CHAN_INFO_SCALE:
> -		switch (chan->type) {
> -		case IIO_ANGL:
> -			/*
> -			 * 2 * Pi / (2^12 - 1) ~= 2 * 103993 / (33102 * 0xFFF)
> -			 *
> -			 * Since there only fit 3 whole radians in 360 degrees,
> -			 * usage of iio_convert_raw_to_processed for this
> -			 * channel will be highly inaccurate.
> -			 */
> -			*val = 103993;
> -			*val2 = 16551 * 0xFFF;
> -			return IIO_VAL_FRACTIONAL;
> -		case IIO_ANGL_VEL:
> -			/*
> -			 * 2 * Pi ~= 2 * 103993 / 33102
> -			 *
> -			 * iio_convert_raw_to_processed uses integer
> -			 * division. This will cause at most 5% error
> -			 * (for very small values). But for 99.5% of the values
> -			 * it will cause less that 1% error.
> -			 */
> -			*val = 103993;
> -			*val2 = 16551;
> -			return IIO_VAL_FRACTIONAL;
> -		default:
> -			return -EINVAL;
> -		}
> -		break;
> -	case IIO_CHAN_INFO_RAW:
> -		mutex_lock(&st->lock);
> -		gpiod_set_value(st->sample, 0);
> -
> -		/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
> -		udelay(1);
> -		gpiod_set_value(st->sample, 1);
> -		gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
> -
> -		ret = spi_read(st->sdev, st->rx, 2);
> -		if (ret < 0) {
> -			mutex_unlock(&st->lock);
> -			return ret;
> -		}
> -
> -		vel = be16_to_cpup((__be16 *)st->rx);
> -		switch (chan->type) {
> -		case IIO_ANGL:
> -			*val = vel >> 4;
> -			break;
> -		case IIO_ANGL_VEL:
> -			*val = sign_extend32((s16)vel >> 4, 11);
> -			break;
> -		default:
> -			mutex_unlock(&st->lock);
> -			return -EINVAL;
> -		}
> -
> -		/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
> -		udelay(1);
> -		mutex_unlock(&st->lock);
> -
> -		return IIO_VAL_INT;
> -	default:
> -		break;
> -	}
> -
> -	return -EINVAL;
> -}
> -
> -static const struct iio_chan_spec ad2s1200_channels[] = {
> -	{
> -		.type = IIO_ANGL,
> -		.indexed = 1,
> -		.channel = 0,
> -		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> -		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> -	}, {
> -		.type = IIO_ANGL_VEL,
> -		.indexed = 1,
> -		.channel = 0,
> -		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> -		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> -	}
> -};
> -
> -static const struct iio_info ad2s1200_info = {
> -	.read_raw = ad2s1200_read_raw,
> -};
> -
> -static int ad2s1200_probe(struct spi_device *spi)
> -{
> -	unsigned short *pins = spi->dev.platform_data;
> -	struct ad2s1200_state *st;
> -	struct iio_dev *indio_dev;
> -	struct device *dev;
> -	int pn, ret = 0;
> -
> -	dev = &spi->dev;
> -
> -	if (pins) {
> -		for (pn = 0; pn < AD2S1200_PN; pn++) {
> -			ret = devm_gpio_request_one(dev, pins[pn],
> -						    GPIOF_DIR_OUT,
> -						    DRV_NAME);
> -			if (ret) {
> -				dev_err(dev,
> -					"Failed to claim gpio %d\n: err=%d",
> -					pins[pn],
> -					ret);
> -				return ret;
> -			}
> -		}
> -	}
> -
> -	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> -	if (!indio_dev)
> -		return -ENOMEM;
> -
> -	spi_set_drvdata(spi, indio_dev);
> -	st = iio_priv(indio_dev);
> -	mutex_init(&st->lock);
> -	st->sdev = spi;
> -
> -	if (pins) {
> -		st->sample = gpio_to_desc(pins[0]);
> -		st->rdvel = gpio_to_desc(pins[1]);
> -	} else {
> -		st->sample = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
> -		if (IS_ERR(st->sample)) {
> -			dev_err(dev, "Failed to claim SAMPLE gpio: err=%ld\n",
> -				PTR_ERR(st->sample));
> -			return PTR_ERR(st->sample);
> -		}
> -
> -		st->rdvel = devm_gpiod_get(dev, "rdvel", GPIOD_OUT_LOW);
> -		if (IS_ERR(st->rdvel)) {
> -			dev_err(dev, "Failed to claim RDVEL gpio: err=%ld\n",
> -				PTR_ERR(st->rdvel));
> -			return PTR_ERR(st->rdvel);
> -		}
> -	}
> -
> -	indio_dev->dev.parent = dev;
> -	indio_dev->info = &ad2s1200_info;
> -	indio_dev->modes = INDIO_DIRECT_MODE;
> -	indio_dev->channels = ad2s1200_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
> -	indio_dev->name = spi_get_device_id(spi)->name;
> -
> -	ret = devm_iio_device_register(dev, indio_dev);
> -	if (ret)
> -		return ret;
> -
> -	spi->max_speed_hz = AD2S1200_HZ;
> -	spi->mode = SPI_MODE_3;
> -	spi_setup(spi);
> -
> -	return 0;
> -}
> -
> -static const struct spi_device_id ad2s1200_id[] = {
> -	{ "ad2s1200" },
> -	{ "ad2s1205" },
> -	{}
> -};
> -MODULE_DEVICE_TABLE(spi, ad2s1200_id);
> -
> -static struct spi_driver ad2s1200_driver = {
> -	.driver = {
> -		.name = DRV_NAME,
> -	},
> -	.probe = ad2s1200_probe,
> -	.id_table = ad2s1200_id,
> -};
> -module_spi_driver(ad2s1200_driver);
> -
> -MODULE_AUTHOR("Graff Yang <graff.yang@gmail.com>");
> -MODULE_DESCRIPTION("Analog Devices AD2S1200/1205 Resolver to Digital SPI driver");
> -MODULE_LICENSE("GPL v2");

Re: [PATCH v2 05/13] staging: iio: ad2s1200: Add kernel docs to driver state

[David Julian Veenstra]

On 21, April 2018 18:49, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:30:03 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> Add missing kernel docs to the ad2s1200 driver state.
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
>> ---
>>  drivers/staging/iio/resolver/ad2s1200.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index 357fe3c382b3..f07aab7e7a35 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -33,6 +33,14 @@
>>  /* clock period in nano second */
>>  #define AD2S1200_TSCLK	(1000000000 / AD2S1200_HZ)
>>  
>> +/**
>> + * struct ad2s1200_state - driver instance specific data
>> + * @lock:	protect driver state
>
> This doc for locks needs to be more specific.  From a quick
> glance I think it does two things.
> 1) Ensures that we don't have concurrent accesses changing the
> gpio control lines.
> 2) Protects the rx buffer against concurrent accesses.
>
> It doesn't have anything much to do with the rest of this state
> structure.

You're right. The lock is to prevent concurrent spi reads. I'll
add additional explanation in v3.

Best regards,
David Veenstra

>
>> + * @sdev:	spi device
>> + * @sample:	GPIO pin SAMPLE
>> + * @rdvel:	GPIO pin RDVEL
>> + * @rx:		buffer for spi transfers
>> + */
>>  struct ad2s1200_state {
>>  	struct mutex lock;
>>  	struct spi_device *sdev;

Re: [PATCH v2 05/13] staging: iio: ad2s1200: Add kernel docs to driver state

[David Julian Veenstra]

On 21, April 2018 18:49, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:30:03 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> Add missing kernel docs to the ad2s1200 driver state.
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
>> ---
>>  drivers/staging/iio/resolver/ad2s1200.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index 357fe3c382b3..f07aab7e7a35 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -33,6 +33,14 @@
>>  /* clock period in nano second */
>>  #define AD2S1200_TSCLK	(1000000000 / AD2S1200_HZ)
>>  
>> +/**
>> + * struct ad2s1200_state - driver instance specific data
>> + * @lock:	protect driver state
>
> This doc for locks needs to be more specific.  From a quick
> glance I think it does two things.
> 1) Ensures that we don't have concurrent accesses changing the
> gpio control lines.
> 2) Protects the rx buffer against concurrent accesses.
>
> It doesn't have anything much to do with the rest of this state
> structure.

You're right. The lock is to prevent concurrent spi reads. I'll
add additional explanation in v3.

Best regards,
David Veenstra

>
>> + * @sdev:	spi device
>> + * @sample:	GPIO pin SAMPLE
>> + * @rdvel:	GPIO pin RDVEL
>> + * @rx:		buffer for spi transfers
>> + */
>>  struct ad2s1200_state {
>>  	struct mutex lock;
>>  	struct spi_device *sdev;

Re: [PATCH v2 06/13] staging: iio: ad2s1200: Introduce variable for repeated value

[David Julian Veenstra]

On 21, April 2018 18:50, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:30:19 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> Add variable to hold &spi->dev in ad2s1200_probe. This value is repeatedly
>> used in ad2s1200_probe.
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> No significant gain in readability.   Perhaps even a slight lost I'm
> going to say no to this one.

We discussed this in [1]. But after removal of platform data it will
be even less worth it. I'll remove this patch in v3.

Best regards,
David Veenstra

[1] https://marc.info/?l=linux-iio&m=152190029808229&w=2

>
> Jonathan
>
>> ---
>>  drivers/staging/iio/resolver/ad2s1200.c | 13 ++++++++-----
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index f07aab7e7a35..0a5fc9917e32 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -117,19 +117,22 @@ static int ad2s1200_probe(struct spi_device *spi)
>>  	unsigned short *pins = spi->dev.platform_data;
>>  	struct ad2s1200_state *st;
>>  	struct iio_dev *indio_dev;
>> +	struct device *dev;
>>  	int pn, ret = 0;
>>  
>> +	dev = &spi->dev;
>> +
>>  	for (pn = 0; pn < AD2S1200_PN; pn++) {
>> -		ret = devm_gpio_request_one(&spi->dev, pins[pn], GPIOF_DIR_OUT,
>> +		ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
>>  					    DRV_NAME);
>>  		if (ret) {
>> -			dev_err(&spi->dev, "request gpio pin %d failed\n",
>> +			dev_err(dev, "request gpio pin %d failed\n",
>>  				pins[pn]);
>>  			return ret;
>>  		}
>>  	}
>>  
>> -	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
>> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
>>  	if (!indio_dev)
>>  		return -ENOMEM;
>>  
>> @@ -140,14 +143,14 @@ static int ad2s1200_probe(struct spi_device *spi)
>>  	st->sample = pins[0];
>>  	st->rdvel = pins[1];
>>  
>> -	indio_dev->dev.parent = &spi->dev;
>> +	indio_dev->dev.parent = dev;
>>  	indio_dev->info = &ad2s1200_info;
>>  	indio_dev->modes = INDIO_DIRECT_MODE;
>>  	indio_dev->channels = ad2s1200_channels;
>>  	indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
>>  	indio_dev->name = spi_get_device_id(spi)->name;
>>  
>> -	ret = devm_iio_device_register(&spi->dev, indio_dev);
>> +	ret = devm_iio_device_register(dev, indio_dev);
>>  	if (ret)
>>  		return ret;
>>  

Re: [PATCH v2 06/13] staging: iio: ad2s1200: Introduce variable for repeated value

[David Julian Veenstra]

On 21, April 2018 18:50, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:30:19 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> Add variable to hold &spi->dev in ad2s1200_probe. This value is repeatedly
>> used in ad2s1200_probe.
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> No significant gain in readability.   Perhaps even a slight lost I'm
> going to say no to this one.

We discussed this in [1]. But after removal of platform data it will
be even less worth it. I'll remove this patch in v3.

Best regards,
David Veenstra

[1] https://marc.info/?l=linux-iio&m=152190029808229&w=2

>
> Jonathan
>
>> ---
>>  drivers/staging/iio/resolver/ad2s1200.c | 13 ++++++++-----
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index f07aab7e7a35..0a5fc9917e32 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -117,19 +117,22 @@ static int ad2s1200_probe(struct spi_device *spi)
>>  	unsigned short *pins = spi->dev.platform_data;
>>  	struct ad2s1200_state *st;
>>  	struct iio_dev *indio_dev;
>> +	struct device *dev;
>>  	int pn, ret = 0;
>>  
>> +	dev = &spi->dev;
>> +
>>  	for (pn = 0; pn < AD2S1200_PN; pn++) {
>> -		ret = devm_gpio_request_one(&spi->dev, pins[pn], GPIOF_DIR_OUT,
>> +		ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
>>  					    DRV_NAME);
>>  		if (ret) {
>> -			dev_err(&spi->dev, "request gpio pin %d failed\n",
>> +			dev_err(dev, "request gpio pin %d failed\n",
>>  				pins[pn]);
>>  			return ret;
>>  		}
>>  	}
>>  
>> -	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
>> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
>>  	if (!indio_dev)
>>  		return -ENOMEM;
>>  
>> @@ -140,14 +143,14 @@ static int ad2s1200_probe(struct spi_device *spi)
>>  	st->sample = pins[0];
>>  	st->rdvel = pins[1];
>>  
>> -	indio_dev->dev.parent = &spi->dev;
>> +	indio_dev->dev.parent = dev;
>>  	indio_dev->info = &ad2s1200_info;
>>  	indio_dev->modes = INDIO_DIRECT_MODE;
>>  	indio_dev->channels = ad2s1200_channels;
>>  	indio_dev->num_channels = ARRAY_SIZE(ad2s1200_channels);
>>  	indio_dev->name = spi_get_device_id(spi)->name;
>>  
>> -	ret = devm_iio_device_register(&spi->dev, indio_dev);
>> +	ret = devm_iio_device_register(dev, indio_dev);
>>  	if (ret)
>>  		return ret;
>>  

Re: [PATCH v2 07/13] staging: iio: ad2s1200: Improve readability with be16_to_cpup

[David Julian Veenstra]

On 21, April 2018 18:55, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:30:32 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> The manual states that the data is contained in the upper 12 bits
>> of the 16 bits read by spi. The code that extracts these 12 bits
>> is correct for both be and le machines, but this is not clear
>> from a first glance.
>> 
>> To improve readability the relevant expressions are replaced
>> with equivalent expressions that use be16_to_cpup.
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
>> ---
>>  drivers/staging/iio/resolver/ad2s1200.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index 0a5fc9917e32..11ed9c7332e6 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -57,7 +57,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  {
>>  	struct ad2s1200_state *st = iio_priv(indio_dev);
>>  	int ret = 0;
>> -	s16 vel;
>> +	u16 vel;
>>  
>>  	mutex_lock(&st->lock);
>>  	gpio_set_value(st->sample, 0);
>> @@ -73,14 +73,13 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  		return ret;
>>  	}
>>  
>> +	vel = be16_to_cpup((__be16 *)st->rx);
> Well it isn't vel (velocity) in one case so now the name is misleading.
>
> Also that type cast suggests a fairly obvious cleanup associated with this one.
> Why not make st->rx a __be16 in the first pace avoiding the need to cast at all?
>
> Then you could just have *val = be16_to_cpu(st->rx) >> 4 and similar.

Agreed on naming of vel, and type of st-rx. I'll change this for v3.

Best regards,
David Veenstra

>
>>  	switch (chan->type) {
>>  	case IIO_ANGL:
>> -		*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
>> +		*val = vel >> 4;
>>  		break;
>>  	case IIO_ANGL_VEL:
>> -		vel = (((s16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
>> -		vel = sign_extend32(vel, 11);
>> -		*val = vel;
>> +		*val = sign_extend32((s16)vel >> 4, 11);
> If you were to use an intermediate that was s16 then the sign extend would
> be automatic.  Perhaps it is clear to do it like this though.. 
> Up to you.
>
>>  		break;
>>  	default:
>>  		mutex_unlock(&st->lock);

Re: [PATCH v2 07/13] staging: iio: ad2s1200: Improve readability with be16_to_cpup

[David Julian Veenstra]

On 21, April 2018 18:55, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:30:32 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> The manual states that the data is contained in the upper 12 bits
>> of the 16 bits read by spi. The code that extracts these 12 bits
>> is correct for both be and le machines, but this is not clear
>> from a first glance.
>> 
>> To improve readability the relevant expressions are replaced
>> with equivalent expressions that use be16_to_cpup.
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
>> ---
>>  drivers/staging/iio/resolver/ad2s1200.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index 0a5fc9917e32..11ed9c7332e6 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -57,7 +57,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  {
>>  	struct ad2s1200_state *st = iio_priv(indio_dev);
>>  	int ret = 0;
>> -	s16 vel;
>> +	u16 vel;
>>  
>>  	mutex_lock(&st->lock);
>>  	gpio_set_value(st->sample, 0);
>> @@ -73,14 +73,13 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  		return ret;
>>  	}
>>  
>> +	vel = be16_to_cpup((__be16 *)st->rx);
> Well it isn't vel (velocity) in one case so now the name is misleading.
>
> Also that type cast suggests a fairly obvious cleanup associated with this one.
> Why not make st->rx a __be16 in the first pace avoiding the need to cast at all?
>
> Then you could just have *val = be16_to_cpu(st->rx) >> 4 and similar.

Agreed on naming of vel, and type of st-rx. I'll change this for v3.

Best regards,
David Veenstra

>
>>  	switch (chan->type) {
>>  	case IIO_ANGL:
>> -		*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
>> +		*val = vel >> 4;
>>  		break;
>>  	case IIO_ANGL_VEL:
>> -		vel = (((s16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
>> -		vel = sign_extend32(vel, 11);
>> -		*val = vel;
>> +		*val = sign_extend32((s16)vel >> 4, 11);
> If you were to use an intermediate that was s16 then the sign extend would
> be automatic.  Perhaps it is clear to do it like this though.. 
> Up to you.
>
>>  		break;
>>  	default:
>>  		mutex_unlock(&st->lock);

Re: [PATCH v2 08/13] staging: iio: ad2s1200: Replace legacy gpio API with modern API

[David Julian Veenstra]

On 21, April 2018 18:58, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:30:44 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> The legacy, integer based gpio API is replaced with the descriptor
>> based API.
>> 
>> For compatibility, it is first tried to use the platform data to
>> request the gpio's. Otherwise, it looks for the "sample" and "rdvel"
>> gpio function.
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> I would suggest that we simply force any out of tree users of the
> platform data to update the platform data.  Drop the the old
> versions entirely..
>
> We can be mean to out of tree board files so let us make our own
> lives easy.

Alright, I'll remove usage of platform data for v3.

Best regards,
David Veenstra

>
> Jonathan
>
>> ---
>>  drivers/staging/iio/resolver/ad2s1200.c | 51 ++++++++++++++++++++++++---------
>>  1 file changed, 37 insertions(+), 14 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index 11ed9c7332e6..29a9bb666e7b 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -14,6 +14,7 @@
>>  #include <linux/delay.h>
>>  #include <linux/device.h>
>>  #include <linux/gpio.h>
>> +#include <linux/gpio/consumer.h>
>>  #include <linux/module.h>
>>  #include <linux/mutex.h>
>>  #include <linux/spi/spi.h>
>> @@ -44,8 +45,8 @@
>>  struct ad2s1200_state {
>>  	struct mutex lock;
>>  	struct spi_device *sdev;
>> -	int sample;
>> -	int rdvel;
>> +	struct gpio_desc *sample;
>> +	struct gpio_desc *rdvel;
>>  	u8 rx[2] ____cacheline_aligned;
>>  };
>>  
>> @@ -60,12 +61,12 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  	u16 vel;
>>  
>>  	mutex_lock(&st->lock);
>> -	gpio_set_value(st->sample, 0);
>> +	gpiod_set_value(st->sample, 0);
>>  
>>  	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>>  	udelay(1);
>> -	gpio_set_value(st->sample, 1);
>> -	gpio_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>> +	gpiod_set_value(st->sample, 1);
>> +	gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>>  
>>  	ret = spi_read(st->sdev, st->rx, 2);
>>  	if (ret < 0) {
>> @@ -121,13 +122,18 @@ static int ad2s1200_probe(struct spi_device *spi)
>>  
>>  	dev = &spi->dev;
>>  
>> -	for (pn = 0; pn < AD2S1200_PN; pn++) {
>> -		ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
>> -					    DRV_NAME);
>> -		if (ret) {
>> -			dev_err(dev, "request gpio pin %d failed\n",
>> -				pins[pn]);
>> -			return ret;
>> +	if (pins) {
>> +		for (pn = 0; pn < AD2S1200_PN; pn++) {
>> +			ret = devm_gpio_request_one(dev, pins[pn],
>> +						    GPIOF_DIR_OUT,
>> +						    DRV_NAME);
>> +			if (ret) {
>> +				dev_err(dev,
>> +					"Failed to claim gpio %d\n: err=%d",
>> +					pins[pn],
>> +					ret);
>> +				return ret;
>> +			}
>>  		}
>>  	}
>>  
>> @@ -139,8 +145,25 @@ static int ad2s1200_probe(struct spi_device *spi)
>>  	st = iio_priv(indio_dev);
>>  	mutex_init(&st->lock);
>>  	st->sdev = spi;
>> -	st->sample = pins[0];
>> -	st->rdvel = pins[1];
>> +
>> +	if (pins) {
>> +		st->sample = gpio_to_desc(pins[0]);
>> +		st->rdvel = gpio_to_desc(pins[1]);
>> +	} else {
>> +		st->sample = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
>> +		if (IS_ERR(st->sample)) {
>> +			dev_err(dev, "Failed to claim SAMPLE gpio: err=%ld\n",
>> +				PTR_ERR(st->sample));
>> +			return PTR_ERR(st->sample);
>> +		}
>> +
>> +		st->rdvel = devm_gpiod_get(dev, "rdvel", GPIOD_OUT_LOW);
>> +		if (IS_ERR(st->rdvel)) {
>> +			dev_err(dev, "Failed to claim RDVEL gpio: err=%ld\n",
>> +				PTR_ERR(st->rdvel));
>> +			return PTR_ERR(st->rdvel);
>> +		}
>> +	}
>>  
>>  	indio_dev->dev.parent = dev;
>>  	indio_dev->info = &ad2s1200_info;

Re: [PATCH v2 08/13] staging: iio: ad2s1200: Replace legacy gpio API with modern API

[David Julian Veenstra]

On 21, April 2018 18:58, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:30:44 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> The legacy, integer based gpio API is replaced with the descriptor
>> based API.
>> 
>> For compatibility, it is first tried to use the platform data to
>> request the gpio's. Otherwise, it looks for the "sample" and "rdvel"
>> gpio function.
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> I would suggest that we simply force any out of tree users of the
> platform data to update the platform data.  Drop the the old
> versions entirely..
>
> We can be mean to out of tree board files so let us make our own
> lives easy.

Alright, I'll remove usage of platform data for v3.

Best regards,
David Veenstra

>
> Jonathan
>
>> ---
>>  drivers/staging/iio/resolver/ad2s1200.c | 51 ++++++++++++++++++++++++---------
>>  1 file changed, 37 insertions(+), 14 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index 11ed9c7332e6..29a9bb666e7b 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -14,6 +14,7 @@
>>  #include <linux/delay.h>
>>  #include <linux/device.h>
>>  #include <linux/gpio.h>
>> +#include <linux/gpio/consumer.h>
>>  #include <linux/module.h>
>>  #include <linux/mutex.h>
>>  #include <linux/spi/spi.h>
>> @@ -44,8 +45,8 @@
>>  struct ad2s1200_state {
>>  	struct mutex lock;
>>  	struct spi_device *sdev;
>> -	int sample;
>> -	int rdvel;
>> +	struct gpio_desc *sample;
>> +	struct gpio_desc *rdvel;
>>  	u8 rx[2] ____cacheline_aligned;
>>  };
>>  
>> @@ -60,12 +61,12 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  	u16 vel;
>>  
>>  	mutex_lock(&st->lock);
>> -	gpio_set_value(st->sample, 0);
>> +	gpiod_set_value(st->sample, 0);
>>  
>>  	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>>  	udelay(1);
>> -	gpio_set_value(st->sample, 1);
>> -	gpio_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>> +	gpiod_set_value(st->sample, 1);
>> +	gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>>  
>>  	ret = spi_read(st->sdev, st->rx, 2);
>>  	if (ret < 0) {
>> @@ -121,13 +122,18 @@ static int ad2s1200_probe(struct spi_device *spi)
>>  
>>  	dev = &spi->dev;
>>  
>> -	for (pn = 0; pn < AD2S1200_PN; pn++) {
>> -		ret = devm_gpio_request_one(dev, pins[pn], GPIOF_DIR_OUT,
>> -					    DRV_NAME);
>> -		if (ret) {
>> -			dev_err(dev, "request gpio pin %d failed\n",
>> -				pins[pn]);
>> -			return ret;
>> +	if (pins) {
>> +		for (pn = 0; pn < AD2S1200_PN; pn++) {
>> +			ret = devm_gpio_request_one(dev, pins[pn],
>> +						    GPIOF_DIR_OUT,
>> +						    DRV_NAME);
>> +			if (ret) {
>> +				dev_err(dev,
>> +					"Failed to claim gpio %d\n: err=%d",
>> +					pins[pn],
>> +					ret);
>> +				return ret;
>> +			}
>>  		}
>>  	}
>>  
>> @@ -139,8 +145,25 @@ static int ad2s1200_probe(struct spi_device *spi)
>>  	st = iio_priv(indio_dev);
>>  	mutex_init(&st->lock);
>>  	st->sdev = spi;
>> -	st->sample = pins[0];
>> -	st->rdvel = pins[1];
>> +
>> +	if (pins) {
>> +		st->sample = gpio_to_desc(pins[0]);
>> +		st->rdvel = gpio_to_desc(pins[1]);
>> +	} else {
>> +		st->sample = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
>> +		if (IS_ERR(st->sample)) {
>> +			dev_err(dev, "Failed to claim SAMPLE gpio: err=%ld\n",
>> +				PTR_ERR(st->sample));
>> +			return PTR_ERR(st->sample);
>> +		}
>> +
>> +		st->rdvel = devm_gpiod_get(dev, "rdvel", GPIOD_OUT_LOW);
>> +		if (IS_ERR(st->rdvel)) {
>> +			dev_err(dev, "Failed to claim RDVEL gpio: err=%ld\n",
>> +				PTR_ERR(st->rdvel));
>> +			return PTR_ERR(st->rdvel);
>> +		}
>> +	}
>>  
>>  	indio_dev->dev.parent = dev;
>>  	indio_dev->info = &ad2s1200_info;

Re: [PATCH v2 10/13] staging: iio: ad2s1200: Add scaling factor for angular velocity channel

[David Julian Veenstra]

On 21, April 2018 19:07, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:31:09 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> The sysfs iio ABI states radians per second is expected as the unit for
>> angular velocity, but the 12-bit angular velocity register has rps
>> as its unit. So a fractional scaling factor of approximately 2 * Pi is
>> added to the angular velocity channel.
>> 
>> The added comments will also be relevant for the scaling factor of
>> the angle channel.
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> Comment inline.  The function you are talking about isn't used
> in the majority of likely use cases for this part.  The maths will actually
> be done in userspace (which can use floating point).
>
> Thanks,
>
> Jonathan
>
>> ---
>> Changes in v2:
>>   - Move explanation of Pi approximation to top of switch statement,
>>     as this will also be relevant to angle channel.
>>   - Replaced 33102 / 2 with 16551 on line 84.
>> 
>>  drivers/staging/iio/resolver/ad2s1200.c | 84 +++++++++++++++++++++++----------
>>  1 file changed, 59 insertions(+), 25 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index 29a9bb666e7b..6c56257be3b1 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -60,38 +60,71 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  	int ret = 0;
>>  	u16 vel;
>>  
>> -	mutex_lock(&st->lock);
>> -	gpiod_set_value(st->sample, 0);
>> +	/*
>> +	 * Below a fractional approximation of Pi is needed.
>> +	 * The following approximation will be used: 103993 / 33102.
>> +	 * This is accurate in 9 decimals places.
>> +	 *
>> +	 * This fraction is based on OEIS series of nominator/denominator
>> +	 * of convergents to Pi (A002485 and A002486).
>> +	 */
>> +	switch (m) {
>> +	case IIO_CHAN_INFO_SCALE:
>> +		switch (chan->type) {
>> +		case IIO_ANGL_VEL:
>> +			/*
>> +			 * 2 * Pi ~= 2 * 103993 / 33102
>> +			 *
>> +			 * iio_convert_raw_to_processed uses integer
>> +			 * division. This will cause at most 5% error
>> +			 * (for very small values). But for 99.5% of the values
>> +			 * it will cause less that 1% error.
> This is an interesting comment, but relies on anyone actually
> using iio_convert_raw_to_processed with this device.
>
> I would imagine that 'in kernel' users of a resolver (who would use
> that function) will be few and far between.  Mostly this will just
> get passed to userspace.  That involves this being converted to
> a decimal.  I would just specify it as one in the first place.

Hmm, I didn't realize that it might never be used in kernel. A
decimal representation is indeed a lot more clear. I'll change
scale value type to IIO_VAL_INT_PLUS_MICRO for both the angular
velocity and angel channel.

Best regards,
David Veenstra

>
>> +			 */
>> +			*val = 103993;
>> +			*val2 = 16551;
>> +			return IIO_VAL_FRACTIONAL;
>> +		default:
>> +			return -EINVAL;
>> +		}
>> +		break;
>> +	case IIO_CHAN_INFO_RAW:
>> +		mutex_lock(&st->lock);
>> +		gpiod_set_value(st->sample, 0);
>> +
>> +		/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>> +		udelay(1);
>> +		gpiod_set_value(st->sample, 1);
>> +		gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>> +
>> +		ret = spi_read(st->sdev, st->rx, 2);
>> +		if (ret < 0) {
>> +			mutex_unlock(&st->lock);
>> +			return ret;
>> +		}
>>  
>> -	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>> -	udelay(1);
>> -	gpiod_set_value(st->sample, 1);
>> -	gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>> +		vel = be16_to_cpup((__be16 *)st->rx);
>> +		switch (chan->type) {
>> +		case IIO_ANGL:
>> +			*val = vel >> 4;
>> +			break;
>> +		case IIO_ANGL_VEL:
>> +			*val = sign_extend32((s16)vel >> 4, 11);
>> +			break;
>> +		default:
>> +			mutex_unlock(&st->lock);
>> +			return -EINVAL;
>> +		}
>>  
>> -	ret = spi_read(st->sdev, st->rx, 2);
>> -	if (ret < 0) {
>> +		/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
>> +		udelay(1);
>>  		mutex_unlock(&st->lock);
>> -		return ret;
>> -	}
>>  
>> -	vel = be16_to_cpup((__be16 *)st->rx);
>> -	switch (chan->type) {
>> -	case IIO_ANGL:
>> -		*val = vel >> 4;
>> -		break;
>> -	case IIO_ANGL_VEL:
>> -		*val = sign_extend32((s16)vel >> 4, 11);
>> -		break;
>> +		return IIO_VAL_INT;
>>  	default:
>> -		mutex_unlock(&st->lock);
>> -		return -EINVAL;
>> +		break;
>>  	}
>>  
>> -	/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
>> -	udelay(1);
>> -	mutex_unlock(&st->lock);
>> -
>> -	return IIO_VAL_INT;
>> +	return -EINVAL;
>>  }
>>  
>>  static const struct iio_chan_spec ad2s1200_channels[] = {
>> @@ -105,6 +138,7 @@ static const struct iio_chan_spec ad2s1200_channels[] = {
>>  		.indexed = 1,
>>  		.channel = 0,
>>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
>>  	}
>>  };
>>  

Re: [PATCH v2 10/13] staging: iio: ad2s1200: Add scaling factor for angular velocity channel

[David Julian Veenstra]

On 21, April 2018 19:07, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:31:09 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> The sysfs iio ABI states radians per second is expected as the unit for
>> angular velocity, but the 12-bit angular velocity register has rps
>> as its unit. So a fractional scaling factor of approximately 2 * Pi is
>> added to the angular velocity channel.
>> 
>> The added comments will also be relevant for the scaling factor of
>> the angle channel.
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> Comment inline.  The function you are talking about isn't used
> in the majority of likely use cases for this part.  The maths will actually
> be done in userspace (which can use floating point).
>
> Thanks,
>
> Jonathan
>
>> ---
>> Changes in v2:
>>   - Move explanation of Pi approximation to top of switch statement,
>>     as this will also be relevant to angle channel.
>>   - Replaced 33102 / 2 with 16551 on line 84.
>> 
>>  drivers/staging/iio/resolver/ad2s1200.c | 84 +++++++++++++++++++++++----------
>>  1 file changed, 59 insertions(+), 25 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index 29a9bb666e7b..6c56257be3b1 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -60,38 +60,71 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  	int ret = 0;
>>  	u16 vel;
>>  
>> -	mutex_lock(&st->lock);
>> -	gpiod_set_value(st->sample, 0);
>> +	/*
>> +	 * Below a fractional approximation of Pi is needed.
>> +	 * The following approximation will be used: 103993 / 33102.
>> +	 * This is accurate in 9 decimals places.
>> +	 *
>> +	 * This fraction is based on OEIS series of nominator/denominator
>> +	 * of convergents to Pi (A002485 and A002486).
>> +	 */
>> +	switch (m) {
>> +	case IIO_CHAN_INFO_SCALE:
>> +		switch (chan->type) {
>> +		case IIO_ANGL_VEL:
>> +			/*
>> +			 * 2 * Pi ~= 2 * 103993 / 33102
>> +			 *
>> +			 * iio_convert_raw_to_processed uses integer
>> +			 * division. This will cause at most 5% error
>> +			 * (for very small values). But for 99.5% of the values
>> +			 * it will cause less that 1% error.
> This is an interesting comment, but relies on anyone actually
> using iio_convert_raw_to_processed with this device.
>
> I would imagine that 'in kernel' users of a resolver (who would use
> that function) will be few and far between.  Mostly this will just
> get passed to userspace.  That involves this being converted to
> a decimal.  I would just specify it as one in the first place.

Hmm, I didn't realize that it might never be used in kernel. A
decimal representation is indeed a lot more clear. I'll change
scale value type to IIO_VAL_INT_PLUS_MICRO for both the angular
velocity and angel channel.

Best regards,
David Veenstra

>
>> +			 */
>> +			*val = 103993;
>> +			*val2 = 16551;
>> +			return IIO_VAL_FRACTIONAL;
>> +		default:
>> +			return -EINVAL;
>> +		}
>> +		break;
>> +	case IIO_CHAN_INFO_RAW:
>> +		mutex_lock(&st->lock);
>> +		gpiod_set_value(st->sample, 0);
>> +
>> +		/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>> +		udelay(1);
>> +		gpiod_set_value(st->sample, 1);
>> +		gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>> +
>> +		ret = spi_read(st->sdev, st->rx, 2);
>> +		if (ret < 0) {
>> +			mutex_unlock(&st->lock);
>> +			return ret;
>> +		}
>>  
>> -	/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>> -	udelay(1);
>> -	gpiod_set_value(st->sample, 1);
>> -	gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>> +		vel = be16_to_cpup((__be16 *)st->rx);
>> +		switch (chan->type) {
>> +		case IIO_ANGL:
>> +			*val = vel >> 4;
>> +			break;
>> +		case IIO_ANGL_VEL:
>> +			*val = sign_extend32((s16)vel >> 4, 11);
>> +			break;
>> +		default:
>> +			mutex_unlock(&st->lock);
>> +			return -EINVAL;
>> +		}
>>  
>> -	ret = spi_read(st->sdev, st->rx, 2);
>> -	if (ret < 0) {
>> +		/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
>> +		udelay(1);
>>  		mutex_unlock(&st->lock);
>> -		return ret;
>> -	}
>>  
>> -	vel = be16_to_cpup((__be16 *)st->rx);
>> -	switch (chan->type) {
>> -	case IIO_ANGL:
>> -		*val = vel >> 4;
>> -		break;
>> -	case IIO_ANGL_VEL:
>> -		*val = sign_extend32((s16)vel >> 4, 11);
>> -		break;
>> +		return IIO_VAL_INT;
>>  	default:
>> -		mutex_unlock(&st->lock);
>> -		return -EINVAL;
>> +		break;
>>  	}
>>  
>> -	/* delay (2 * AD2S1200_TSCLK + 20) ns for sample pulse */
>> -	udelay(1);
>> -	mutex_unlock(&st->lock);
>> -
>> -	return IIO_VAL_INT;
>> +	return -EINVAL;
>>  }
>>  
>>  static const struct iio_chan_spec ad2s1200_channels[] = {
>> @@ -105,6 +138,7 @@ static const struct iio_chan_spec ad2s1200_channels[] = {
>>  		.indexed = 1,
>>  		.channel = 0,
>>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
>>  	}
>>  };
>>  

Re: [PATCH v2 11/13] staging: iio: Documentation: Add missing sysfs docs for angle channel

[David Julian Veenstra]

On 21, April 2018 19:08, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:31:37 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> The iio resolver drivers in staging use angle channels. This patch
>> add missing documentation for this type of channel.
>> 
>> As was discussed in [1], radians is chosen as the unit, to match the
>> unit of angular velocity.
>> 
>> [1] https://marc.info/?l=linux-driver-devel&m=152190078308330&w=2
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
>> ---
>> Change in v2:
>>   - Introduces in this version.
>> 
>>  Documentation/ABI/testing/sysfs-bus-iio | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>> 
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
>> index 6a5f34b4d5b9..8ad0e55f99ee 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio
>> @@ -190,6 +190,15 @@ Description:
>>  		but should match other such assignments on device).
>>  		Units after application of scale and offset are m/s^2.
>>  
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_x_raw
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_y_raw
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_z_raw
> This surprised me.  A resolver is not going to inherently have any
> notion of a particular axis.
> Would expect.
> in_angl_raw

For the attributes that were added, I tried to match it with
the angular velocity counterpart. But it indeed doesn't make
much sense to have an axis for a resolver. I'll remove axis
modifier for v3.

Best regards,
David Veenstra

>
> Jonathan
>
>> +KernelVersion:	4.17
>> +Contact:	linux-iio@vger.kernel.org
>> +Description:
>> +		Angle about axis x, y or z (may be arbitrarily assigned). Units
>> +		after application of scale and offset are radians.
>> +
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_x_raw
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_y_raw
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_z_raw
>> @@ -297,6 +306,7 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_pressure_offset
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_humidityrelative_offset
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_magn_offset
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_rot_offset
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_offset
>>  KernelVersion:	2.6.35
>>  Contact:	linux-iio@vger.kernel.org
>>  Description:
>> @@ -350,6 +360,7 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_humidityrelative_scale
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_velocity_sqrt(x^2+y^2+z^2)_scale
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_scale
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_countY_scale
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_scale
>>  KernelVersion:	2.6.35
>>  Contact:	linux-iio@vger.kernel.org
>>  Description:

Re: [PATCH v2 11/13] staging: iio: Documentation: Add missing sysfs docs for angle channel

[David Julian Veenstra]

On 21, April 2018 19:08, Jonathan Cameron wrote:

> On Fri, 20 Apr 2018 21:31:37 +0200
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> The iio resolver drivers in staging use angle channels. This patch
>> add missing documentation for this type of channel.
>> 
>> As was discussed in [1], radians is chosen as the unit, to match the
>> unit of angular velocity.
>> 
>> [1] https://marc.info/?l=linux-driver-devel&m=152190078308330&w=2
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
>> ---
>> Change in v2:
>>   - Introduces in this version.
>> 
>>  Documentation/ABI/testing/sysfs-bus-iio | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>> 
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
>> index 6a5f34b4d5b9..8ad0e55f99ee 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio
>> @@ -190,6 +190,15 @@ Description:
>>  		but should match other such assignments on device).
>>  		Units after application of scale and offset are m/s^2.
>>  
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_x_raw
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_y_raw
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_z_raw
> This surprised me.  A resolver is not going to inherently have any
> notion of a particular axis.
> Would expect.
> in_angl_raw

For the attributes that were added, I tried to match it with
the angular velocity counterpart. But it indeed doesn't make
much sense to have an axis for a resolver. I'll remove axis
modifier for v3.

Best regards,
David Veenstra

>
> Jonathan
>
>> +KernelVersion:	4.17
>> +Contact:	linux-iio@vger.kernel.org
>> +Description:
>> +		Angle about axis x, y or z (may be arbitrarily assigned). Units
>> +		after application of scale and offset are radians.
>> +
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_x_raw
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_y_raw
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_z_raw
>> @@ -297,6 +306,7 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_pressure_offset
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_humidityrelative_offset
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_magn_offset
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_rot_offset
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_offset
>>  KernelVersion:	2.6.35
>>  Contact:	linux-iio@vger.kernel.org
>>  Description:
>> @@ -350,6 +360,7 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_humidityrelative_scale
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_velocity_sqrt(x^2+y^2+z^2)_scale
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_scale
>>  What:		/sys/bus/iio/devices/iio:deviceX/in_countY_scale
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_angl_scale
>>  KernelVersion:	2.6.35
>>  Contact:	linux-iio@vger.kernel.org
>>  Description: