staging: iio: ad2s1200: Driver clean up

This patches removes unneeded slab.h header.

Signed-off-by David Veenstra <[email protected]>

---
Changes in v2:
- Introduced in this version.

drivers/staging/iio/resolver/ad2s1200.c | 1 -

 1 file changed, 1 deletion(-)

drivers/staging/iio/resolver/ad2s1200.c +0 -1

index aa62c64e9bc4..5d7ed0034422 100644
@@ -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>

This patches sorts all the includes in alphabetic order.

Signed-off-by David Veenstra <[email protected]>

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

drivers/staging/iio/resolver/ad2s1200.c +6 -6

index 5d7ed0034422..ffcdf4e8eb92 100644
@@ -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>

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

Signed-off-by David Veenstra <[email protected]>

---
 drivers/staging/iio/resolver/ad2s1200.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/staging/iio/resolver/ad2s1200.c +2 -2

index ffcdf4e8eb92..b6c3a3c8f7fe 100644
@@ -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,

Add blank lines to improve readability.

Signed-off-by David Veenstra <[email protected]>

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

drivers/staging/iio/resolver/ad2s1200.c +7 -0

index b6c3a3c8f7fe..357fe3c382b3 100644
@@ -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);

Add missing kernel docs to the ad2s1200 driver state.

Signed-off-by David Veenstra <[email protected]>

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

drivers/staging/iio/resolver/ad2s1200.c +0 -0

index 357fe3c382b3..f07aab7e7a35 100644

drivers/staging/iio/resolver/ad2s1200.c +5 -0

+ * @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;

Quoted 12 lines · On Fri, 20 Apr 2018 21:30:03 +0200 David Veenstra…

Add missing kernel docs to the ad2s1200 driver state.

Signed-off-by David Veenstra <[email protected]>

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

drivers/staging/iio/resolver/ad2s1200.c +0 -0

index 357fe3c382b3..f07aab7e7a35 100644

drivers/staging/iio/resolver/ad2s1200.c line 38

Quoted 6 lines

drivers/staging/iio/resolver/ad2s1200.c +3 -0

@@ -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.

Quoted 9 lines

drivers/staging/iio/resolver/ad2s1200.c +5 -0

+ * @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;

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

Signed-off-by David Veenstra <[email protected]>

---
 drivers/staging/iio/resolver/ad2s1200.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

drivers/staging/iio/resolver/ad2s1200.c +8 -5

index f07aab7e7a35..0a5fc9917e32 100644
@@ -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;
 

Quoted 4 lines · On Fri, 20 Apr 2018 21:30:19 +0200 David Veenstra…

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

Signed-off-by David Veenstra <[email protected]>

No significant gain in readability. Perhaps even a slight lost I'm
going to say no to this one.

Jonathan

Quoted 51 lines

---
 drivers/staging/iio/resolver/ad2s1200.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

drivers/staging/iio/resolver/ad2s1200.c +8 -5

index f07aab7e7a35..0a5fc9917e32 100644
@@ -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;
 

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 <[email protected]>

---
 drivers/staging/iio/resolver/ad2s1200.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

drivers/staging/iio/resolver/ad2s1200.c +1 -1

index 0a5fc9917e32..11ed9c7332e6 100644
@@ -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);

drivers/staging/iio/resolver/ad2s1200.c +0 -0

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

Quoted 30 lines · On Fri, 20 Apr 2018 21:30:32 +0200 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 <[email protected]>

---
 drivers/staging/iio/resolver/ad2s1200.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

drivers/staging/iio/resolver/ad2s1200.c +1 -1

index 0a5fc9917e32..11ed9c7332e6 100644
@@ -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;
 	}
 

drivers/staging/iio/resolver/ad2s1200.c static int ad2s1200_read_raw(struct iio_dev *indio_dev, · line 76

Quoted 1 line

drivers/staging/iio/resolver/ad2s1200.c +1 -0

+	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.

drivers/staging/iio/resolver/ad2s1200.c static int ad2s1200_read_raw(struct iio_dev *indio_dev, · line 82

Quoted 11 lines

drivers/staging/iio/resolver/ad2s1200.c +2 -4

 	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.

Quoted 3 lines

drivers/staging/iio/resolver/ad2s1200.c +0 -0

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

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 <[email protected]>

---
 drivers/staging/iio/resolver/ad2s1200.c | 51 ++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 14 deletions(-)

drivers/staging/iio/resolver/ad2s1200.c +37 -14

index 11ed9c7332e6..29a9bb666e7b 100644
@@ -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;

Quoted 7 lines · On Fri, 20 Apr 2018 21:30:44 +0200 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.

Quoted 1 line

Signed-off-by David Veenstra <[email protected]>

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

Quoted 97 lines

---
 drivers/staging/iio/resolver/ad2s1200.c | 51 ++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 14 deletions(-)

drivers/staging/iio/resolver/ad2s1200.c +37 -14

index 11ed9c7332e6..29a9bb666e7b 100644
@@ -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;

Add documentation for the added device tree bindings.

Signed-off-by David Veenstra <[email protected]>

---
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

Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt new file +16 -0

new file mode 100644
index 000000000000..85c009987878
@@ -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>;
+	};

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 <[email protected]>

---
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(-)

drivers/staging/iio/resolver/ad2s1200.c +9 -2

index 29a9bb666e7b..6c56257be3b1 100644
@@ -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) {

drivers/staging/iio/resolver/ad2s1200.c +40 -23

+			 */
+			*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),
 	}
 };
 

Quoted 9 lines · On Fri, 20 Apr 2018 21:31:09 +0200 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 <[email protected]>

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

Quoted 28 lines

---
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(-)

drivers/staging/iio/resolver/ad2s1200.c +9 -2

index 29a9bb666e7b..6c56257be3b1 100644
@@ -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) {

drivers/staging/iio/resolver/ad2s1200.c static int ad2s1200_read_raw(struct iio_dev *indio_dev, · line 81

Quoted 10 lines

drivers/staging/iio/resolver/ad2s1200.c +10 -0

+	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.

Quoted 81 lines

drivers/staging/iio/resolver/ad2s1200.c +40 -23

+			 */
+			*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),
 	}
 };
 

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 <[email protected]>

---
Change in v2:
- Introduces in this version.

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

Documentation/ABI/testing/sysfs-bus-iio +0 -0

index 6a5f34b4d5b9..8ad0e55f99ee 100644

Documentation/ABI/testing/sysfs-bus-iio +8 -0

+KernelVersion:	4.17
+Contact:	[email protected]
+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:	[email protected]
 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:	[email protected]
 Description:

Quoted 24 lines · On Fri, 20 Apr 2018 21:31:37 +0200 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 <[email protected]>

---
Change in v2:
- Introduces in this version.

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

Documentation/ABI/testing/sysfs-bus-iio +0 -0

index 6a5f34b4d5b9..8ad0e55f99ee 100644
@@ -190,6 +190,15 @@ Description:
 		but should match other such assignments on device).
 		Units after application of scale and offset are m/s^2.
 

Documentation/ABI/testing/sysfs-bus-iio Description: · line 195

Quoted 3 lines

Documentation/ABI/testing/sysfs-bus-iio +3 -0

+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

Quoted 25 lines

Documentation/ABI/testing/sysfs-bus-iio +8 -0

+KernelVersion:	4.17
+Contact:	[email protected]
+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:	[email protected]
 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:	[email protected]
 Description:

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 <[email protected]>

---
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(+)

drivers/staging/iio/resolver/ad2s1200.c +1 -0

index 6c56257be3b1..4f5dd28b174a 100644
@@ -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:

drivers/staging/iio/resolver/ad2s1200.c +1 -0

 		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,

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

Signed-off-by David Veenstra <[email protected]>

---
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

Documentation/devicetree/bindings/iio/resolver/ad2s1200.txt new file +16 -0

new file mode 100644
index 000000000000..85c009987878
@@ -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>;
+	};

Documentation/devicetree/bindings/staging/iio/resolver/ad2s1200.txt deleted +0 -16

deleted file mode 100644
index 85c009987878..000000000000
@@ -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>;
-	};

drivers/iio/Kconfig +1 -0

index b3c8c6ef0dff..4bec3ccbf4a1 100644
@@ -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

drivers/iio/Makefile +1 -0

index b16b2e9ddc40..1865361b8714 100644
@@ -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/

drivers/iio/resolver/Kconfig new file +17 -0

new file mode 100644
index 000000000000..2ced9f22aa70
@@ -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

drivers/iio/resolver/Makefile new file +5 -0

new file mode 100644
index 000000000000..4e1dccae07e7
@@ -0,0 +1,5 @@
+#
+# Makefile for Resolver/Synchro drivers
+#
+
+obj-$(CONFIG_AD2S1200) += ad2s1200.o

drivers/iio/resolver/ad2s1200.c new file +250 -0

new file mode 100644
index 000000000000..4f5dd28b174a
@@ -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 <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices AD2S1200/1205 Resolver to Digital SPI driver");
+MODULE_LICENSE("GPL v2");

drivers/staging/iio/resolver/Kconfig +0 -12

index 1c7e2860d6b7..6a469ee6101f 100644
@@ -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

drivers/staging/iio/resolver/Makefile +0 -1

index 14375e444ebf..8d901dc7500b 100644
@@ -3,5 +3,4 @@
 #
 
 obj-$(CONFIG_AD2S90) += ad2s90.o
-obj-$(CONFIG_AD2S1200) += ad2s1200.o
 obj-$(CONFIG_AD2S1210) += ad2s1210.o

drivers/staging/iio/resolver/ad2s1200.c deleted +0 -250

deleted file mode 100644
index 4f5dd28b174a..000000000000
@@ -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 <[email protected]>");
-MODULE_DESCRIPTION("Analog Devices AD2S1200/1205 Resolver to Digital SPI driver");
-MODULE_LICENSE("GPL v2");