Skip to contentSkip to conversation

staging: iio: ad2s1210: Switch to the gpio descriptor interface.

PATCH Applied Nishad Kamdar 4 participants 10 messages Oct 31, 2018 – Nov 3, 2018

Subsystems (4)
Staging IIO Analog Devices IIO drivers Staging - industrial io
lore
Share & tools
lore

Nishad Kamdar

author
#0 lore raw

v7 cover staging: iio: ad2s1210: Switch to the gpio descriptor interface.

Use the gpiod interface instead of the deprecated old non-descriptor

Changes in v7:
- Adds a level of indirection to read and write the gpio_desc to make the code simpler.
- Drop gpioin flag which decides how the GPIOs
are controlled as the GPIOs must be outputs
for the host as per the datasheet.
Changes in v6:
- Split device tree table addition and device tree support addition in two patches.
- Replace platform data with device tree support.
- Rename boolean property.
Changes in v5:
- Add device tree support.
- Add device tree table for matching vendor ID.
- Add Support for retrieving platform data from device tree.
Changes in v4:
- Add spaces after { and before } in gpios[] initialization.
- Check the correct pointer for error.
- Align the dev_err msg to existing format in the code.
Changes in v3:
- Use a pointer to pointer for gpio_desc in
struct ad2s1210_gpio as it will be used to modify a pointer.
- Use dot notation to initialize the structure.
- Use a pointer variable to avoid writing gpios[i].
Changes in v2:
- Use the spi_device struct embedded in st instead of passing it as an argument to ad2s1210_setup_gpios().
- Use an array of structs to reduce redundant code in in ad2s1210_setup_gpios().
- Remove ad2s1210_free_gpios() as devm API is being used.

Nishad Kamdar (3):
staging: iio: ad2s1210: Switch to the gpio descriptor interface
staging: iio: ad2s1210: Drop the gpioin flag.
staging: iio: ad2s1210: Add device tree table.

 drivers/staging/iio/resolver/ad2s1210.c | 132 +++++++++++-------------
 drivers/staging/iio/resolver/ad2s1210.h |  20 ----
 2 files changed, 62 insertions(+), 90 deletions(-)
Signature · 1 line
2.17.1
author
#1 lore raw

PATCH v7 1/3 staging: iio: ad2s1210: Switch to the gpio descriptor interface

Use the gpiod interface instead of the deprecated old non-descriptor interface.

Signed-off-by Nishad Kamdar <[email protected]>
---
 drivers/staging/iio/resolver/ad2s1210.c | 106 ++++++++++++++----------
 drivers/staging/iio/resolver/ad2s1210.h |   3 -
 2 files changed, 62 insertions(+), 47 deletions(-)
drivers/staging/iio/resolver/ad2s1210.c +62 -44
index ac13b99bd9cb..82ac9847f6f4 100644
@@ -15,7 +15,7 @@
 #include <linux/slab.h>
 #include <linux/sysfs.h>
 #include <linux/delay.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/module.h>
 
 #include <linux/iio/iio.h>

@@ -67,12 +67,42 @@ enum ad2s1210_mode {
 	MOD_RESERVED,
 };
 
+enum ad2s1210_gpios {
+	AD2S1210_SAMPLE,
+	AD2S1210_A0,
+	AD2S1210_A1,
+	AD2S1210_RES0,
+	AD2S1210_RES1,
+};
+
+struct ad2s1210_gpio {
+	const char *name;
+	unsigned long flags;
+};
+
+static const struct ad2s1210_gpio gpios_in[] = {
+	[AD2S1210_SAMPLE] = { .name = "adi,sample", .flags = GPIOD_IN },
+	[AD2S1210_A0] = { .name = "adi,a0", .flags = GPIOD_IN },
+	[AD2S1210_A1] = { .name = "adi,a1", .flags = GPIOD_IN },
+	[AD2S1210_RES0] = { .name = "adi,res0", .flags = GPIOD_IN },
+	[AD2S1210_RES1] = { .name = "adi,res1", .flags = GPIOD_IN },
+};
+
+static const struct ad2s1210_gpio gpios_out[] = {
+	[AD2S1210_SAMPLE] = { .name = "adi,sample", .flags = GPIOD_OUT_LOW },
+	[AD2S1210_A0] = { .name = "adi,a0", .flags = GPIOD_OUT_LOW },
+	[AD2S1210_A1] = { .name = "adi,a1", .flags = GPIOD_OUT_LOW },
+	[AD2S1210_RES0] = { .name = "adi,res0", .flags = GPIOD_OUT_LOW },
+	[AD2S1210_RES1] = { .name = "adi,res1", .flags = GPIOD_OUT_LOW },
+};
+
 static const unsigned int ad2s1210_resolution_value[] = { 10, 12, 14, 16 };
 
 struct ad2s1210_state {
 	const struct ad2s1210_platform_data *pdata;
 	struct mutex lock;
 	struct spi_device *sdev;
+	struct gpio_desc *gpios[5];
 	unsigned int fclkin;
 	unsigned int fexcit;
 	bool hysteresis;

@@ -91,8 +121,8 @@ static const int ad2s1210_mode_vals[4][2] = {
 static inline void ad2s1210_set_mode(enum ad2s1210_mode mode,
 				     struct ad2s1210_state *st)
 {
-	gpio_set_value(st->pdata->a[0], ad2s1210_mode_vals[mode][0]);
-	gpio_set_value(st->pdata->a[1], ad2s1210_mode_vals[mode][1]);
+	gpiod_set_value(st->gpios[AD2S1210_A0], ad2s1210_mode_vals[mode][0]);
+	gpiod_set_value(st->gpios[AD2S1210_A1], ad2s1210_mode_vals[mode][1]);
 	st->mode = mode;
 }
 

@@ -152,8 +182,8 @@ int ad2s1210_update_frequency_control_word(struct ad2s1210_state *st)
 
 static unsigned char ad2s1210_read_resolution_pin(struct ad2s1210_state *st)
 {
-	int resolution = (gpio_get_value(st->pdata->res[0]) << 1) |
-			  gpio_get_value(st->pdata->res[1]);
+	int resolution = (gpiod_get_value(st->gpios[AD2S1210_RES0]) << 1) |
+			  gpiod_get_value(st->gpios[AD2S1210_RES1]);
 
 	return ad2s1210_resolution_value[resolution];
 }

@@ -164,10 +194,10 @@ static const int ad2s1210_res_pins[4][2] = {
 
 static inline void ad2s1210_set_resolution_pin(struct ad2s1210_state *st)
 {
-	gpio_set_value(st->pdata->res[0],
-		       ad2s1210_res_pins[(st->resolution - 10) / 2][0]);
-	gpio_set_value(st->pdata->res[1],
-		       ad2s1210_res_pins[(st->resolution - 10) / 2][1]);
+	gpiod_set_value(st->gpios[AD2S1210_RES0],
+			ad2s1210_res_pins[(st->resolution - 10) / 2][0]);
+	gpiod_set_value(st->gpios[AD2S1210_RES1],
+			ad2s1210_res_pins[(st->resolution - 10) / 2][1]);
 }
 
 static inline int ad2s1210_soft_reset(struct ad2s1210_state *st)

@@ -401,15 +431,15 @@ static ssize_t ad2s1210_clear_fault(struct device *dev,
 	int ret;
 
 	mutex_lock(&st->lock);
-	gpio_set_value(st->pdata->sample, 0);
+	gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 0);
 	/* delay (2 * tck + 20) nano seconds */
 	udelay(1);
-	gpio_set_value(st->pdata->sample, 1);
+	gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 1);
 	ret = ad2s1210_config_read(st, AD2S1210_REG_FAULT);
 	if (ret < 0)
 		goto error_ret;
-	gpio_set_value(st->pdata->sample, 0);
-	gpio_set_value(st->pdata->sample, 1);
+	gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 0);
+	gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 1);
 error_ret:
 	mutex_unlock(&st->lock);
 

@@ -466,7 +496,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
 	s16 vel;
 
 	mutex_lock(&st->lock);
-	gpio_set_value(st->pdata->sample, 0);
+	gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 0);
 	/* delay (6 * tck + 20) nano seconds */
 	udelay(1);
 

@@ -512,7 +542,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
 	}
 
 error_ret:
-	gpio_set_value(st->pdata->sample, 1);
+	gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 1);
 	/* delay (2 * tck + 20) nano seconds */
 	udelay(1);
 	mutex_unlock(&st->lock);

@@ -630,30 +660,23 @@ static const struct iio_info ad2s1210_info = {
 
 static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
 {
-	unsigned long flags = st->pdata->gpioin ? GPIOF_DIR_IN : GPIOF_DIR_OUT;
-	struct gpio ad2s1210_gpios[] = {
-		{ st->pdata->sample, GPIOF_DIR_IN, "sample" },
-		{ st->pdata->a[0], flags, "a0" },
-		{ st->pdata->a[1], flags, "a1" },
-		{ st->pdata->res[0], flags, "res0" },
-		{ st->pdata->res[0], flags, "res1" },
-	};
-
-	return gpio_request_array(ad2s1210_gpios, ARRAY_SIZE(ad2s1210_gpios));
-}
-
-static void ad2s1210_free_gpios(struct ad2s1210_state *st)
-{
-	unsigned long flags = st->pdata->gpioin ? GPIOF_DIR_IN : GPIOF_DIR_OUT;
-	struct gpio ad2s1210_gpios[] = {
-		{ st->pdata->sample, GPIOF_DIR_IN, "sample" },
-		{ st->pdata->a[0], flags, "a0" },
-		{ st->pdata->a[1], flags, "a1" },
-		{ st->pdata->res[0], flags, "res0" },
-		{ st->pdata->res[0], flags, "res1" },
-	};
+	struct ad2s1210_gpio *pin = st->pdata->gpioin ? &gpios_in[0] : &gpios_out[0];
+	struct spi_device *spi = st->sdev;
+	int i, ret;
+
+	for (i = 0; i < ARRAY_SIZE(gpios_in); i++) {
+		st->gpios[i] = devm_gpiod_get(&spi->dev, pin[i].name,
+					      pin[i].flags);
+		if (IS_ERR(st->gpios[i])) {
+			ret = PTR_ERR(st->gpios[i]);
+			dev_err(&spi->dev,
+				"ad2s1210: failed to request %s GPIO: %d\n",
+				pin[i].name, ret);
+			return ret;
+		}
+	}
 
-	gpio_free_array(ad2s1210_gpios, ARRAY_SIZE(ad2s1210_gpios));
+	return 0;
 }
 
 static int ad2s1210_probe(struct spi_device *spi)

@@ -692,7 +715,7 @@ static int ad2s1210_probe(struct spi_device *spi)
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
-		goto error_free_gpios;
+		return ret;
 
 	st->fclkin = spi->max_speed_hz;
 	spi->mode = SPI_MODE_3;

@@ -700,10 +723,6 @@ static int ad2s1210_probe(struct spi_device *spi)
 	ad2s1210_initial(st);
 
 	return 0;
-
-error_free_gpios:
-	ad2s1210_free_gpios(st);
-	return ret;
 }
 
 static int ad2s1210_remove(struct spi_device *spi)

@@ -711,7 +730,6 @@ static int ad2s1210_remove(struct spi_device *spi)
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 
 	iio_device_unregister(indio_dev);
-	ad2s1210_free_gpios(iio_priv(indio_dev));
 
 	return 0;
 }
drivers/staging/iio/resolver/ad2s1210.h +0 -3
index e9b2147701fc..63d479b20a6c 100644
@@ -12,9 +12,6 @@
 #define _AD2S1210_H
 
 struct ad2s1210_platform_data {
-	unsigned int		sample;
-	unsigned int		a[2];
-	unsigned int		res[2];
 	bool			gpioin;
 };
 #endif /* _AD2S1210_H */
Signature · 1 line
2.17.1
author
#4 lore raw

PATCH v7 2/3 staging: iio: ad2s1210: Drop the gpioin flag.

In reply to Nishad Kamdar

Drop gpioin flag which decides how the GPIOs
are controlled as the GPIOs must be outputs
for the host as per the datasheet.

Signed-off-by Nishad Kamdar <[email protected]>
---
 drivers/staging/iio/resolver/ad2s1210.c | 45 ++++---------------------
 drivers/staging/iio/resolver/ad2s1210.h | 17 ----------
 2 files changed, 6 insertions(+), 56 deletions(-)
drivers/staging/iio/resolver/ad2s1210.c +6 -39
index 82ac9847f6f4..d3e7d5aad2c8 100644
@@ -80,15 +80,7 @@ struct ad2s1210_gpio {
 	unsigned long flags;
 };
 
-static const struct ad2s1210_gpio gpios_in[] = {
-	[AD2S1210_SAMPLE] = { .name = "adi,sample", .flags = GPIOD_IN },
-	[AD2S1210_A0] = { .name = "adi,a0", .flags = GPIOD_IN },
-	[AD2S1210_A1] = { .name = "adi,a1", .flags = GPIOD_IN },
-	[AD2S1210_RES0] = { .name = "adi,res0", .flags = GPIOD_IN },
-	[AD2S1210_RES1] = { .name = "adi,res1", .flags = GPIOD_IN },
-};
-
-static const struct ad2s1210_gpio gpios_out[] = {
+static const struct ad2s1210_gpio gpios[] = {
 	[AD2S1210_SAMPLE] = { .name = "adi,sample", .flags = GPIOD_OUT_LOW },
 	[AD2S1210_A0] = { .name = "adi,a0", .flags = GPIOD_OUT_LOW },
 	[AD2S1210_A1] = { .name = "adi,a1", .flags = GPIOD_OUT_LOW },

@@ -99,7 +91,6 @@ static const struct ad2s1210_gpio gpios_out[] = {
 static const unsigned int ad2s1210_resolution_value[] = { 10, 12, 14, 16 };
 
 struct ad2s1210_state {
-	const struct ad2s1210_platform_data *pdata;
 	struct mutex lock;
 	struct spi_device *sdev;
 	struct gpio_desc *gpios[5];

@@ -180,14 +171,6 @@ int ad2s1210_update_frequency_control_word(struct ad2s1210_state *st)
 	return ad2s1210_config_write(st, fcw);
 }
 
-static unsigned char ad2s1210_read_resolution_pin(struct ad2s1210_state *st)
-{
-	int resolution = (gpiod_get_value(st->gpios[AD2S1210_RES0]) << 1) |
-			  gpiod_get_value(st->gpios[AD2S1210_RES1]);
-
-	return ad2s1210_resolution_value[resolution];
-}
-
 static const int ad2s1210_res_pins[4][2] = {
 	{ 0, 0 }, {0, 1}, {1, 0}, {1, 1}
 };

@@ -333,13 +316,7 @@ static ssize_t ad2s1210_store_control(struct device *dev,
 	}
 	st->resolution
 		= ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
-	if (st->pdata->gpioin) {
-		data = ad2s1210_read_resolution_pin(st);
-		if (data != st->resolution)
-			dev_warn(dev, "ad2s1210: resolution settings not match\n");
-	} else {
-		ad2s1210_set_resolution_pin(st);
-	}
+	ad2s1210_set_resolution_pin(st);
 	ret = len;
 	st->hysteresis = !!(data & AD2S1210_ENABLE_HYSTERESIS);
 

@@ -395,13 +372,7 @@ static ssize_t ad2s1210_store_resolution(struct device *dev,
 	}
 	st->resolution
 		= ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
-	if (st->pdata->gpioin) {
-		data = ad2s1210_read_resolution_pin(st);
-		if (data != st->resolution)
-			dev_warn(dev, "ad2s1210: resolution settings not match\n");
-	} else {
-		ad2s1210_set_resolution_pin(st);
-	}
+	ad2s1210_set_resolution_pin(st);
 	ret = len;
 error_ret:
 	mutex_unlock(&st->lock);

@@ -622,10 +593,7 @@ static int ad2s1210_initial(struct ad2s1210_state *st)
 	int ret;
 
 	mutex_lock(&st->lock);
-	if (st->pdata->gpioin)
-		st->resolution = ad2s1210_read_resolution_pin(st);
-	else
-		ad2s1210_set_resolution_pin(st);
+	ad2s1210_set_resolution_pin(st);
 
 	ret = ad2s1210_config_write(st, AD2S1210_REG_CONTROL);
 	if (ret < 0)

@@ -660,11 +628,11 @@ static const struct iio_info ad2s1210_info = {
 
 static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
 {
-	struct ad2s1210_gpio *pin = st->pdata->gpioin ? &gpios_in[0] : &gpios_out[0];
+	struct ad2s1210_gpio *pin = &gpios[0];
 	struct spi_device *spi = st->sdev;
 	int i, ret;
 
-	for (i = 0; i < ARRAY_SIZE(gpios_in); i++) {
+	for (i = 0; i < ARRAY_SIZE(gpios); i++) {
 		st->gpios[i] = devm_gpiod_get(&spi->dev, pin[i].name,
 					      pin[i].flags);
 		if (IS_ERR(st->gpios[i])) {

@@ -692,7 +660,6 @@ static int ad2s1210_probe(struct spi_device *spi)
 	if (!indio_dev)
 		return -ENOMEM;
 	st = iio_priv(indio_dev);
-	st->pdata = spi->dev.platform_data;
 	ret = ad2s1210_setup_gpios(st);
 	if (ret < 0)
 		return ret;
drivers/staging/iio/resolver/ad2s1210.h +0 -17
index 63d479b20a6c..e69de29bb2d1 100644
@@ -1,17 +0,0 @@
-/*
- * ad2s1210.h plaform data for the ADI Resolver to Digital Converters:
- * AD2S1210
- *
- * 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.
- */
-#ifndef _AD2S1210_H
-#define _AD2S1210_H
-
-struct ad2s1210_platform_data {
-	bool			gpioin;
-};
-#endif /* _AD2S1210_H */
Signature · 1 line
2.17.1
author
#6 lore raw

PATCH v7 3/3 staging: iio: ad2s1210: Add device tree table.

In reply to Nishad Kamdar

Add device tree table for matching vendor ID.

Signed-off-by Nishad Kamdar <[email protected]>
---
 drivers/staging/iio/resolver/ad2s1210.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/staging/iio/resolver/ad2s1210.c +7 -0
index d3e7d5aad2c8..7c50def91a2b 100644
@@ -701,6 +701,12 @@ static int ad2s1210_remove(struct spi_device *spi)
 	return 0;
 }
 
+static const struct of_device_id ad2s1210_of_match[] = {
+	{ .compatible = "adi,ad2s1210", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad2s1210_of_match);
+
 static const struct spi_device_id ad2s1210_id[] = {
 	{ "ad2s1210" },
 	{}

@@ -710,6 +716,7 @@ MODULE_DEVICE_TABLE(spi, ad2s1210_id);
 static struct spi_driver ad2s1210_driver = {
 	.driver = {
 		.name = DRV_NAME,
+		.of_match_table = of_match_ptr(ad2s1210_of_match),
 	},
 	.probe = ad2s1210_probe,
 	.remove = ad2s1210_remove,
Signature · 1 line
2.17.1
#7 lore raw

Re: [PATCH v7 3/3] staging: iio: ad2s1210: Add device tree table.

Quoted 12 lines · Nishad Kamdar

Add device tree table for matching vendor ID.

Signed-off-by Nishad Kamdar <[email protected]>
---
 drivers/staging/iio/resolver/ad2s1210.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/staging/iio/resolver/ad2s1210.c +0 -0
index d3e7d5aad2c8..7c50def91a2b 100644

drivers/staging/iio/resolver/ad2s1210.c static int ad2s1210_remove(struct spi_device *spi) · line 708

Quoted 8 lines
drivers/staging/iio/resolver/ad2s1210.c +5 -0
@@ -701,6 +701,12 @@ static int ad2s1210_remove(struct spi_device *spi)
 	return 0;
 }
 
+static const struct of_device_id ad2s1210_of_match[] = {
+	{ .compatible = "adi,ad2s1210", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad2s1210_of_match);

I believe this needs to be documented at:

Documentation/devicetree/bindings/iio/resolver/ad2s1210.txt

Cc'ed to devictree list + Rob(DT Maintainer).

Just wondering why didn't it came up till now from the IIO reviewers ? v7!!

Signature · 4 lines
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology
maintainer
#8 lore raw

Re: [PATCH v7 3/3] staging: iio: ad2s1210: Add device tree table.

Quoted 24 lines · On Thu, 1 Nov 2018 21:05:09 +0530 Himanshu Jha <[email protected]>…
Quoted 12 lines · Nishad Kamdar

Add device tree table for matching vendor ID.

Signed-off-by Nishad Kamdar <[email protected]>
---
 drivers/staging/iio/resolver/ad2s1210.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/staging/iio/resolver/ad2s1210.c +0 -0
index d3e7d5aad2c8..7c50def91a2b 100644

drivers/staging/iio/resolver/ad2s1210.c static int ad2s1210_remove(struct spi_device *spi) · line 708

Quoted 8 lines
drivers/staging/iio/resolver/ad2s1210.c +5 -0
@@ -701,6 +701,12 @@ static int ad2s1210_remove(struct spi_device *spi)
 	return 0;
 }
 
+static const struct of_device_id ad2s1210_of_match[] = {
+	{ .compatible = "adi,ad2s1210", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad2s1210_of_match);

I believe this needs to be documented at:

Quoted 5 lines

Documentation/devicetree/bindings/iio/resolver/ad2s1210.txt

Cc'ed to devictree list + Rob(DT Maintainer).

Just wondering why didn't it came up till now from the IIO reviewers ? v7!!

Because in staging drivers graduations we often hold off doing the dt-bindings document until we have full visibility of where we are going.

A lot of them have dodgy DT bindings (and that might even be the reason they are in staging). What we don't want is to have a doc for a silly binding in the 'official' list as we'll have to support it for ever.

It needs documenting before moving out staging, but not necessarily now. Particularly as this device is complex and has a 'lot' of other stuff that isn't currently supported and quite possibly never will be. Some of that would have non obvious dt bindings if we did support it. For example we 'might' route the encoder outputs round to the inputs of a counter driver and end up with a complex entity representing the facilities that both fo them provide.

Agreed, the DT binding doc needs to come soon and before the move out staging, but I am quite happy with it being in the next series.

A line in the description to that effect would have been useful of course!

Jonathan

Quoted 2 lines
maintainer
#2 lore raw

Re: [PATCH v7 1/3] staging: iio: ad2s1210: Switch to the gpio descriptor interface

In reply to Nishad Kamdar
Quoted 4 lines · On Wed, 31 Oct 2018 21:28:52 +0530 Nishad Kamdar <[email protected]> wrote:

Use the gpiod interface instead of the deprecated old non-descriptor interface.

Signed-off-by Nishad Kamdar <[email protected]>

It would have been less 'noisy' to do these in the reverse order (drop the flag and support first, then do the gpiod conversion), but I suppose it doesn't really matter.

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

Thanks,

Jonathan

Quoted 225 lines
---
 drivers/staging/iio/resolver/ad2s1210.c | 106 ++++++++++++++----------
 drivers/staging/iio/resolver/ad2s1210.h |   3 -
 2 files changed, 62 insertions(+), 47 deletions(-)
drivers/staging/iio/resolver/ad2s1210.c +62 -44
index ac13b99bd9cb..82ac9847f6f4 100644
@@ -15,7 +15,7 @@
 #include <linux/slab.h>
 #include <linux/sysfs.h>
 #include <linux/delay.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/module.h>
 
 #include <linux/iio/iio.h>

@@ -67,12 +67,42 @@ enum ad2s1210_mode {
 	MOD_RESERVED,
 };
 
+enum ad2s1210_gpios {
+	AD2S1210_SAMPLE,
+	AD2S1210_A0,
+	AD2S1210_A1,
+	AD2S1210_RES0,
+	AD2S1210_RES1,
+};
+
+struct ad2s1210_gpio {
+	const char *name;
+	unsigned long flags;
+};
+
+static const struct ad2s1210_gpio gpios_in[] = {
+	[AD2S1210_SAMPLE] = { .name = "adi,sample", .flags = GPIOD_IN },
+	[AD2S1210_A0] = { .name = "adi,a0", .flags = GPIOD_IN },
+	[AD2S1210_A1] = { .name = "adi,a1", .flags = GPIOD_IN },
+	[AD2S1210_RES0] = { .name = "adi,res0", .flags = GPIOD_IN },
+	[AD2S1210_RES1] = { .name = "adi,res1", .flags = GPIOD_IN },
+};
+
+static const struct ad2s1210_gpio gpios_out[] = {
+	[AD2S1210_SAMPLE] = { .name = "adi,sample", .flags = GPIOD_OUT_LOW },
+	[AD2S1210_A0] = { .name = "adi,a0", .flags = GPIOD_OUT_LOW },
+	[AD2S1210_A1] = { .name = "adi,a1", .flags = GPIOD_OUT_LOW },
+	[AD2S1210_RES0] = { .name = "adi,res0", .flags = GPIOD_OUT_LOW },
+	[AD2S1210_RES1] = { .name = "adi,res1", .flags = GPIOD_OUT_LOW },
+};
+
 static const unsigned int ad2s1210_resolution_value[] = { 10, 12, 14, 16 };
 
 struct ad2s1210_state {
 	const struct ad2s1210_platform_data *pdata;
 	struct mutex lock;
 	struct spi_device *sdev;
+	struct gpio_desc *gpios[5];
 	unsigned int fclkin;
 	unsigned int fexcit;
 	bool hysteresis;

@@ -91,8 +121,8 @@ static const int ad2s1210_mode_vals[4][2] = {
 static inline void ad2s1210_set_mode(enum ad2s1210_mode mode,
 				     struct ad2s1210_state *st)
 {
-	gpio_set_value(st->pdata->a[0], ad2s1210_mode_vals[mode][0]);
-	gpio_set_value(st->pdata->a[1], ad2s1210_mode_vals[mode][1]);
+	gpiod_set_value(st->gpios[AD2S1210_A0], ad2s1210_mode_vals[mode][0]);
+	gpiod_set_value(st->gpios[AD2S1210_A1], ad2s1210_mode_vals[mode][1]);
 	st->mode = mode;
 }
 

@@ -152,8 +182,8 @@ int ad2s1210_update_frequency_control_word(struct ad2s1210_state *st)
 
 static unsigned char ad2s1210_read_resolution_pin(struct ad2s1210_state *st)
 {
-	int resolution = (gpio_get_value(st->pdata->res[0]) << 1) |
-			  gpio_get_value(st->pdata->res[1]);
+	int resolution = (gpiod_get_value(st->gpios[AD2S1210_RES0]) << 1) |
+			  gpiod_get_value(st->gpios[AD2S1210_RES1]);
 
 	return ad2s1210_resolution_value[resolution];
 }

@@ -164,10 +194,10 @@ static const int ad2s1210_res_pins[4][2] = {
 
 static inline void ad2s1210_set_resolution_pin(struct ad2s1210_state *st)
 {
-	gpio_set_value(st->pdata->res[0],
-		       ad2s1210_res_pins[(st->resolution - 10) / 2][0]);
-	gpio_set_value(st->pdata->res[1],
-		       ad2s1210_res_pins[(st->resolution - 10) / 2][1]);
+	gpiod_set_value(st->gpios[AD2S1210_RES0],
+			ad2s1210_res_pins[(st->resolution - 10) / 2][0]);
+	gpiod_set_value(st->gpios[AD2S1210_RES1],
+			ad2s1210_res_pins[(st->resolution - 10) / 2][1]);
 }
 
 static inline int ad2s1210_soft_reset(struct ad2s1210_state *st)

@@ -401,15 +431,15 @@ static ssize_t ad2s1210_clear_fault(struct device *dev,
 	int ret;
 
 	mutex_lock(&st->lock);
-	gpio_set_value(st->pdata->sample, 0);
+	gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 0);
 	/* delay (2 * tck + 20) nano seconds */
 	udelay(1);
-	gpio_set_value(st->pdata->sample, 1);
+	gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 1);
 	ret = ad2s1210_config_read(st, AD2S1210_REG_FAULT);
 	if (ret < 0)
 		goto error_ret;
-	gpio_set_value(st->pdata->sample, 0);
-	gpio_set_value(st->pdata->sample, 1);
+	gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 0);
+	gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 1);
 error_ret:
 	mutex_unlock(&st->lock);
 

@@ -466,7 +496,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
 	s16 vel;
 
 	mutex_lock(&st->lock);
-	gpio_set_value(st->pdata->sample, 0);
+	gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 0);
 	/* delay (6 * tck + 20) nano seconds */
 	udelay(1);
 

@@ -512,7 +542,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
 	}
 
 error_ret:
-	gpio_set_value(st->pdata->sample, 1);
+	gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 1);
 	/* delay (2 * tck + 20) nano seconds */
 	udelay(1);
 	mutex_unlock(&st->lock);

@@ -630,30 +660,23 @@ static const struct iio_info ad2s1210_info = {
 
 static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
 {
-	unsigned long flags = st->pdata->gpioin ? GPIOF_DIR_IN : GPIOF_DIR_OUT;
-	struct gpio ad2s1210_gpios[] = {
-		{ st->pdata->sample, GPIOF_DIR_IN, "sample" },
-		{ st->pdata->a[0], flags, "a0" },
-		{ st->pdata->a[1], flags, "a1" },
-		{ st->pdata->res[0], flags, "res0" },
-		{ st->pdata->res[0], flags, "res1" },
-	};
-
-	return gpio_request_array(ad2s1210_gpios, ARRAY_SIZE(ad2s1210_gpios));
-}
-
-static void ad2s1210_free_gpios(struct ad2s1210_state *st)
-{
-	unsigned long flags = st->pdata->gpioin ? GPIOF_DIR_IN : GPIOF_DIR_OUT;
-	struct gpio ad2s1210_gpios[] = {
-		{ st->pdata->sample, GPIOF_DIR_IN, "sample" },
-		{ st->pdata->a[0], flags, "a0" },
-		{ st->pdata->a[1], flags, "a1" },
-		{ st->pdata->res[0], flags, "res0" },
-		{ st->pdata->res[0], flags, "res1" },
-	};
+	struct ad2s1210_gpio *pin = st->pdata->gpioin ? &gpios_in[0] : &gpios_out[0];
+	struct spi_device *spi = st->sdev;
+	int i, ret;
+
+	for (i = 0; i < ARRAY_SIZE(gpios_in); i++) {
+		st->gpios[i] = devm_gpiod_get(&spi->dev, pin[i].name,
+					      pin[i].flags);
+		if (IS_ERR(st->gpios[i])) {
+			ret = PTR_ERR(st->gpios[i]);
+			dev_err(&spi->dev,
+				"ad2s1210: failed to request %s GPIO: %d\n",
+				pin[i].name, ret);
+			return ret;
+		}
+	}
 
-	gpio_free_array(ad2s1210_gpios, ARRAY_SIZE(ad2s1210_gpios));
+	return 0;
 }
 
 static int ad2s1210_probe(struct spi_device *spi)

@@ -692,7 +715,7 @@ static int ad2s1210_probe(struct spi_device *spi)
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
-		goto error_free_gpios;
+		return ret;
 
 	st->fclkin = spi->max_speed_hz;
 	spi->mode = SPI_MODE_3;

@@ -700,10 +723,6 @@ static int ad2s1210_probe(struct spi_device *spi)
 	ad2s1210_initial(st);
 
 	return 0;
-
-error_free_gpios:
-	ad2s1210_free_gpios(st);
-	return ret;
 }
 
 static int ad2s1210_remove(struct spi_device *spi)

@@ -711,7 +730,6 @@ static int ad2s1210_remove(struct spi_device *spi)
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 
 	iio_device_unregister(indio_dev);
-	ad2s1210_free_gpios(iio_priv(indio_dev));
 
 	return 0;
 }
drivers/staging/iio/resolver/ad2s1210.h +0 -3
index e9b2147701fc..63d479b20a6c 100644
@@ -12,9 +12,6 @@
 #define _AD2S1210_H
 
 struct ad2s1210_platform_data {
-	unsigned int		sample;
-	unsigned int		a[2];
-	unsigned int		res[2];
 	bool			gpioin;
 };
 #endif /* _AD2S1210_H */
maintainer
#5 lore raw

Re: [PATCH v7 2/3] staging: iio: ad2s1210: Drop the gpioin flag.

In reply to Nishad Kamdar
Quoted 5 lines · On Wed, 31 Oct 2018 21:29:53 +0530 Nishad Kamdar <[email protected]> wrote:

Drop gpioin flag which decides how the GPIOs
are controlled as the GPIOs must be outputs
for the host as per the datasheet.

Signed-off-by Nishad Kamdar <[email protected]>

Whilst this does the right thing, it doesn't take advantage of opportunities to clean up. I've made a few changes in applying. See below. Also added a note that this gets rid of the platform data. Note you need to do git rm drivers/staging/iio/resolver/ad2s1210.h to actually get rid of the file. + remove it from the includes. I did that as well whilst here.

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

Thanks,

Jonathan

Quoted 92 lines
---
 drivers/staging/iio/resolver/ad2s1210.c | 45 ++++---------------------
 drivers/staging/iio/resolver/ad2s1210.h | 17 ----------
 2 files changed, 6 insertions(+), 56 deletions(-)
drivers/staging/iio/resolver/ad2s1210.c +4 -36
index 82ac9847f6f4..d3e7d5aad2c8 100644
@@ -80,15 +80,7 @@ struct ad2s1210_gpio {
 	unsigned long flags;
 };
 
-static const struct ad2s1210_gpio gpios_in[] = {
-	[AD2S1210_SAMPLE] = { .name = "adi,sample", .flags = GPIOD_IN },
-	[AD2S1210_A0] = { .name = "adi,a0", .flags = GPIOD_IN },
-	[AD2S1210_A1] = { .name = "adi,a1", .flags = GPIOD_IN },
-	[AD2S1210_RES0] = { .name = "adi,res0", .flags = GPIOD_IN },
-	[AD2S1210_RES1] = { .name = "adi,res1", .flags = GPIOD_IN },
-};
-
-static const struct ad2s1210_gpio gpios_out[] = {
+static const struct ad2s1210_gpio gpios[] = {
 	[AD2S1210_SAMPLE] = { .name = "adi,sample", .flags = GPIOD_OUT_LOW },
 	[AD2S1210_A0] = { .name = "adi,a0", .flags = GPIOD_OUT_LOW },
 	[AD2S1210_A1] = { .name = "adi,a1", .flags = GPIOD_OUT_LOW },

@@ -99,7 +91,6 @@ static const struct ad2s1210_gpio gpios_out[] = {
 static const unsigned int ad2s1210_resolution_value[] = { 10, 12, 14, 16 };
 
 struct ad2s1210_state {
-	const struct ad2s1210_platform_data *pdata;
 	struct mutex lock;
 	struct spi_device *sdev;
 	struct gpio_desc *gpios[5];

@@ -180,14 +171,6 @@ int ad2s1210_update_frequency_control_word(struct ad2s1210_state *st)
 	return ad2s1210_config_write(st, fcw);
 }
 
-static unsigned char ad2s1210_read_resolution_pin(struct ad2s1210_state *st)
-{
-	int resolution = (gpiod_get_value(st->gpios[AD2S1210_RES0]) << 1) |
-			  gpiod_get_value(st->gpios[AD2S1210_RES1]);
-
-	return ad2s1210_resolution_value[resolution];
-}
-
 static const int ad2s1210_res_pins[4][2] = {
 	{ 0, 0 }, {0, 1}, {1, 0}, {1, 1}
 };

@@ -333,13 +316,7 @@ static ssize_t ad2s1210_store_control(struct device *dev,
 	}
 	st->resolution
 		= ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
-	if (st->pdata->gpioin) {
-		data = ad2s1210_read_resolution_pin(st);
-		if (data != st->resolution)
-			dev_warn(dev, "ad2s1210: resolution settings not match\n");
-	} else {
-		ad2s1210_set_resolution_pin(st);
-	}
+	ad2s1210_set_resolution_pin(st);
 	ret = len;
 	st->hysteresis = !!(data & AD2S1210_ENABLE_HYSTERESIS);
 

@@ -395,13 +372,7 @@ static ssize_t ad2s1210_store_resolution(struct device *dev,
 	}
 	st->resolution
 		= ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
-	if (st->pdata->gpioin) {
-		data = ad2s1210_read_resolution_pin(st);
-		if (data != st->resolution)
-			dev_warn(dev, "ad2s1210: resolution settings not match\n");
-	} else {
-		ad2s1210_set_resolution_pin(st);
-	}
+	ad2s1210_set_resolution_pin(st);
 	ret = len;
 error_ret:
 	mutex_unlock(&st->lock);

@@ -622,10 +593,7 @@ static int ad2s1210_initial(struct ad2s1210_state *st)
 	int ret;
 
 	mutex_lock(&st->lock);
-	if (st->pdata->gpioin)
-		st->resolution = ad2s1210_read_resolution_pin(st);
-	else
-		ad2s1210_set_resolution_pin(st);
+	ad2s1210_set_resolution_pin(st);
 
 	ret = ad2s1210_config_write(st, AD2S1210_REG_CONTROL);
 	if (ret < 0)

drivers/staging/iio/resolver/ad2s1210.c static const struct iio_info ad2s1210_info = { · line 631

Quoted 5 lines
drivers/staging/iio/resolver/ad2s1210.c +1 -1
@@ -660,11 +628,11 @@ static const struct iio_info ad2s1210_info = {
 
 static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
 {
-	struct ad2s1210_gpio *pin = st->pdata->gpioin ? &gpios_in[0] : &gpios_out[0];
+	struct ad2s1210_gpio *pin = &gpios[0];

This local parameter no longer does anything useful so dropped it.

Quoted 38 lines
drivers/staging/iio/resolver/ad2s1210.c +1 -2
 	struct spi_device *spi = st->sdev;
 	int i, ret;
 
-	for (i = 0; i < ARRAY_SIZE(gpios_in); i++) {
+	for (i = 0; i < ARRAY_SIZE(gpios); i++) {
 		st->gpios[i] = devm_gpiod_get(&spi->dev, pin[i].name,
 					      pin[i].flags);
 		if (IS_ERR(st->gpios[i])) {

@@ -692,7 +660,6 @@ static int ad2s1210_probe(struct spi_device *spi)
 	if (!indio_dev)
 		return -ENOMEM;
 	st = iio_priv(indio_dev);
-	st->pdata = spi->dev.platform_data;
 	ret = ad2s1210_setup_gpios(st);
 	if (ret < 0)
 		return ret;
drivers/staging/iio/resolver/ad2s1210.h +0 -17
index 63d479b20a6c..e69de29bb2d1 100644
@@ -1,17 +0,0 @@
-/*
- * ad2s1210.h plaform data for the ADI Resolver to Digital Converters:
- * AD2S1210
- *
- * 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.
- */
-#ifndef _AD2S1210_H
-#define _AD2S1210_H
-
-struct ad2s1210_platform_data {
-	bool			gpioin;
-};
-#endif /* _AD2S1210_H */
Applied
#9 lore raw

Re: [PATCH v7 3/3] staging: iio: ad2s1210: Add device tree table.

In reply to Jonathan Cameron
Quoted 48 lines · On Sat, 3 Nov 2018 12:39:27 +0000 Jonathan Cameron <[email protected]> wrote:
Quoted 24 lines · On Thu, 1 Nov 2018 21:05:09 +0530 Himanshu Jha <[email protected]>…
Quoted 12 lines · Nishad Kamdar

Add device tree table for matching vendor ID.

Signed-off-by Nishad Kamdar <[email protected]>
---
 drivers/staging/iio/resolver/ad2s1210.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/staging/iio/resolver/ad2s1210.c +0 -0
index d3e7d5aad2c8..7c50def91a2b 100644

drivers/staging/iio/resolver/ad2s1210.c static int ad2s1210_remove(struct spi_device *spi) · line 708

Quoted 8 lines
drivers/staging/iio/resolver/ad2s1210.c +5 -0
@@ -701,6 +701,12 @@ static int ad2s1210_remove(struct spi_device *spi)
 	return 0;
 }
 
+static const struct of_device_id ad2s1210_of_match[] = {
+	{ .compatible = "adi,ad2s1210", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad2s1210_of_match);

I believe this needs to be documented at:

Quoted 5 lines

Documentation/devicetree/bindings/iio/resolver/ad2s1210.txt

Cc'ed to devictree list + Rob(DT Maintainer).

Just wondering why didn't it came up till now from the IIO reviewers ? v7!!

Because in staging drivers graduations we often hold off doing the dt-bindings document until we have full visibility of where we are going.

A lot of them have dodgy DT bindings (and that might even be the reason they are in staging). What we don't want is to have a doc for a silly binding in the 'official' list as we'll have to support it for ever.

It needs documenting before moving out staging, but not necessarily now. Particularly as this device is complex and has a 'lot' of other stuff that isn't currently supported and quite possibly never will be. Some of that would have non obvious dt bindings if we did support it. For example we 'might' route the encoder outputs round to the inputs of a counter driver and end up with a complex entity representing the facilities that both fo them provide.

Quoted 6 lines

Agreed, the DT binding doc needs to come soon and before the move out staging, but I am quite happy with it being in the next series.

A line in the description to that effect would have been useful of course!

Applied, with a line on the intent to document once driver is cleaned up added.

Thanks,

Jonathan

Quoted 5 lines

Jonathan

Quoted 2 lines
#3 lore raw

Re: [PATCH v7 1/3] staging: iio: ad2s1210: Switch to the gpio descriptor interface

In reply to Jonathan Cameron
Quoted 6 lines · On Sat, 3 Nov 2018 12:45:09 +0000 Jonathan Cameron <[email protected]> wrote:
Quoted 3 lines · On Wed, 31 Oct 2018 21:28:52 +0530 Nishad Kamdar <[email protected]> wrote:

Use the gpiod interface instead of the deprecated old non-descriptor interface.

Quoted 7 lines
Quoted 1 line
Signed-off-by Nishad Kamdar <[email protected]>

It would have been less 'noisy' to do these in the reverse order (drop the flag and support first, then do the gpiod conversion), but I suppose it doesn't really matter.

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

actually, couple of minor changes.

Quoted 160 lines

Thanks,

Jonathan

Quoted 155 lines
---
 drivers/staging/iio/resolver/ad2s1210.c | 106 ++++++++++++++----------
 drivers/staging/iio/resolver/ad2s1210.h |   3 -
 2 files changed, 62 insertions(+), 47 deletions(-)
drivers/staging/iio/resolver/ad2s1210.c +45 -30
index ac13b99bd9cb..82ac9847f6f4 100644
@@ -15,7 +15,7 @@
 #include <linux/slab.h>
 #include <linux/sysfs.h>
 #include <linux/delay.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/module.h>
 
 #include <linux/iio/iio.h>

@@ -67,12 +67,42 @@ enum ad2s1210_mode {
 	MOD_RESERVED,
 };
 
+enum ad2s1210_gpios {
+	AD2S1210_SAMPLE,
+	AD2S1210_A0,
+	AD2S1210_A1,
+	AD2S1210_RES0,
+	AD2S1210_RES1,
+};
+
+struct ad2s1210_gpio {
+	const char *name;
+	unsigned long flags;
+};
+
+static const struct ad2s1210_gpio gpios_in[] = {
+	[AD2S1210_SAMPLE] = { .name = "adi,sample", .flags = GPIOD_IN },
+	[AD2S1210_A0] = { .name = "adi,a0", .flags = GPIOD_IN },
+	[AD2S1210_A1] = { .name = "adi,a1", .flags = GPIOD_IN },
+	[AD2S1210_RES0] = { .name = "adi,res0", .flags = GPIOD_IN },
+	[AD2S1210_RES1] = { .name = "adi,res1", .flags = GPIOD_IN },
+};
+
+static const struct ad2s1210_gpio gpios_out[] = {
+	[AD2S1210_SAMPLE] = { .name = "adi,sample", .flags = GPIOD_OUT_LOW },
+	[AD2S1210_A0] = { .name = "adi,a0", .flags = GPIOD_OUT_LOW },
+	[AD2S1210_A1] = { .name = "adi,a1", .flags = GPIOD_OUT_LOW },
+	[AD2S1210_RES0] = { .name = "adi,res0", .flags = GPIOD_OUT_LOW },
+	[AD2S1210_RES1] = { .name = "adi,res1", .flags = GPIOD_OUT_LOW },
+};
+
 static const unsigned int ad2s1210_resolution_value[] = { 10, 12, 14, 16 };
 
 struct ad2s1210_state {
 	const struct ad2s1210_platform_data *pdata;
 	struct mutex lock;
 	struct spi_device *sdev;
+	struct gpio_desc *gpios[5];
 	unsigned int fclkin;
 	unsigned int fexcit;
 	bool hysteresis;

@@ -91,8 +121,8 @@ static const int ad2s1210_mode_vals[4][2] = {
 static inline void ad2s1210_set_mode(enum ad2s1210_mode mode,
 				     struct ad2s1210_state *st)
 {
-	gpio_set_value(st->pdata->a[0], ad2s1210_mode_vals[mode][0]);
-	gpio_set_value(st->pdata->a[1], ad2s1210_mode_vals[mode][1]);
+	gpiod_set_value(st->gpios[AD2S1210_A0], ad2s1210_mode_vals[mode][0]);
+	gpiod_set_value(st->gpios[AD2S1210_A1], ad2s1210_mode_vals[mode][1]);
 	st->mode = mode;
 }
 

@@ -152,8 +182,8 @@ int ad2s1210_update_frequency_control_word(struct ad2s1210_state *st)
 
 static unsigned char ad2s1210_read_resolution_pin(struct ad2s1210_state *st)
 {
-	int resolution = (gpio_get_value(st->pdata->res[0]) << 1) |
-			  gpio_get_value(st->pdata->res[1]);
+	int resolution = (gpiod_get_value(st->gpios[AD2S1210_RES0]) << 1) |
+			  gpiod_get_value(st->gpios[AD2S1210_RES1]);
 
 	return ad2s1210_resolution_value[resolution];
 }

@@ -164,10 +194,10 @@ static const int ad2s1210_res_pins[4][2] = {
 
 static inline void ad2s1210_set_resolution_pin(struct ad2s1210_state *st)
 {
-	gpio_set_value(st->pdata->res[0],
-		       ad2s1210_res_pins[(st->resolution - 10) / 2][0]);
-	gpio_set_value(st->pdata->res[1],
-		       ad2s1210_res_pins[(st->resolution - 10) / 2][1]);
+	gpiod_set_value(st->gpios[AD2S1210_RES0],
+			ad2s1210_res_pins[(st->resolution - 10) / 2][0]);
+	gpiod_set_value(st->gpios[AD2S1210_RES1],
+			ad2s1210_res_pins[(st->resolution - 10) / 2][1]);
 }
 
 static inline int ad2s1210_soft_reset(struct ad2s1210_state *st)

@@ -401,15 +431,15 @@ static ssize_t ad2s1210_clear_fault(struct device *dev,
 	int ret;
 
 	mutex_lock(&st->lock);
-	gpio_set_value(st->pdata->sample, 0);
+	gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 0);
 	/* delay (2 * tck + 20) nano seconds */
 	udelay(1);
-	gpio_set_value(st->pdata->sample, 1);
+	gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 1);
 	ret = ad2s1210_config_read(st, AD2S1210_REG_FAULT);
 	if (ret < 0)
 		goto error_ret;
-	gpio_set_value(st->pdata->sample, 0);
-	gpio_set_value(st->pdata->sample, 1);
+	gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 0);
+	gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 1);
 error_ret:
 	mutex_unlock(&st->lock);
 

@@ -466,7 +496,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
 	s16 vel;
 
 	mutex_lock(&st->lock);
-	gpio_set_value(st->pdata->sample, 0);
+	gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 0);
 	/* delay (6 * tck + 20) nano seconds */
 	udelay(1);
 

@@ -512,7 +542,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
 	}
 
 error_ret:
-	gpio_set_value(st->pdata->sample, 1);
+	gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 1);
 	/* delay (2 * tck + 20) nano seconds */
 	udelay(1);
 	mutex_unlock(&st->lock);

@@ -630,30 +660,23 @@ static const struct iio_info ad2s1210_info = {
 
 static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
 {
-	unsigned long flags = st->pdata->gpioin ? GPIOF_DIR_IN : GPIOF_DIR_OUT;
-	struct gpio ad2s1210_gpios[] = {
-		{ st->pdata->sample, GPIOF_DIR_IN, "sample" },
-		{ st->pdata->a[0], flags, "a0" },
-		{ st->pdata->a[1], flags, "a1" },
-		{ st->pdata->res[0], flags, "res0" },
-		{ st->pdata->res[0], flags, "res1" },
-	};
-
-	return gpio_request_array(ad2s1210_gpios, ARRAY_SIZE(ad2s1210_gpios));
-}
-
-static void ad2s1210_free_gpios(struct ad2s1210_state *st)
-{
-	unsigned long flags = st->pdata->gpioin ? GPIOF_DIR_IN : GPIOF_DIR_OUT;
Quoted 8 lines

drivers/staging/iio/resolver/ad2s1210.c static const struct iio_info ad2s1210_info = { · line 663

Quoted 8 lines
drivers/staging/iio/resolver/ad2s1210.c +1 -7
-	struct gpio ad2s1210_gpios[] = {
-		{ st->pdata->sample, GPIOF_DIR_IN, "sample" },
-		{ st->pdata->a[0], flags, "a0" },
-		{ st->pdata->a[1], flags, "a1" },
-		{ st->pdata->res[0], flags, "res0" },
-		{ st->pdata->res[0], flags, "res1" },
-	};
+	struct ad2s1210_gpio *pin = st->pdata->gpioin ? &gpios_in[0] : &gpios_out[0];

const struct ad2s1210_gpio
and the line is too long.

Fixed both.

Quoted 63 lines
Quoted 62 lines
drivers/staging/iio/resolver/ad2s1210.c +16 -7
+	struct spi_device *spi = st->sdev;
+	int i, ret;
+
+	for (i = 0; i < ARRAY_SIZE(gpios_in); i++) {
+		st->gpios[i] = devm_gpiod_get(&spi->dev, pin[i].name,
+					      pin[i].flags);
+		if (IS_ERR(st->gpios[i])) {
+			ret = PTR_ERR(st->gpios[i]);
+			dev_err(&spi->dev,
+				"ad2s1210: failed to request %s GPIO: %d\n",
+				pin[i].name, ret);
+			return ret;
+		}
+	}
 
-	gpio_free_array(ad2s1210_gpios, ARRAY_SIZE(ad2s1210_gpios));
+	return 0;
 }
 
 static int ad2s1210_probe(struct spi_device *spi)

@@ -692,7 +715,7 @@ static int ad2s1210_probe(struct spi_device *spi)
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
-		goto error_free_gpios;
+		return ret;
 
 	st->fclkin = spi->max_speed_hz;
 	spi->mode = SPI_MODE_3;

@@ -700,10 +723,6 @@ static int ad2s1210_probe(struct spi_device *spi)
 	ad2s1210_initial(st);
 
 	return 0;
-
-error_free_gpios:
-	ad2s1210_free_gpios(st);
-	return ret;
 }
 
 static int ad2s1210_remove(struct spi_device *spi)

@@ -711,7 +730,6 @@ static int ad2s1210_remove(struct spi_device *spi)
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 
 	iio_device_unregister(indio_dev);
-	ad2s1210_free_gpios(iio_priv(indio_dev));
 
 	return 0;
 }
drivers/staging/iio/resolver/ad2s1210.h +0 -3
index e9b2147701fc..63d479b20a6c 100644
@@ -12,9 +12,6 @@
 #define _AD2S1210_H
 
 struct ad2s1210_platform_data {
-	unsigned int		sample;
-	unsigned int		a[2];
-	unsigned int		res[2];
 	bool			gpioin;
 };
 #endif /* _AD2S1210_H */