2018-11-09 07:34:56

by Nishad Kamdar

[permalink] [raw]
Subject: [PATCH v3 0/4] staging: iio: ad7816: Switch to the gpio descriptor interface

Changes in v4:
- Drop busy pin in case of AD7818.
- Set RD/WR pin and CONVST pin as outputs.
- Add device tree table.

Nishad Kamdar (4):
staging: iio: ad7816: Switch to the gpio descriptor interface
staging: iio: ad7816: Do not use busy_pin in case of AD7818
staging: iio: ad7816: Set RD/WR pin and CONVST pin as outputs.
staging: iio: ad7816: Add device tree table.

drivers/staging/iio/adc/ad7816.c | 111 +++++++++++++++++--------------
1 file changed, 60 insertions(+), 51 deletions(-)

--
2.17.1



2018-11-09 07:36:15

by Nishad Kamdar

[permalink] [raw]
Subject: [PATCH v3 1/4] staging: iio: ad7816: Switch to the gpio descriptor interface

Use the gpiod interface for rdwr_pin, convert_pin and busy_pin
instead of the deprecated old non-descriptor interface.

Signed-off-by: Nishad Kamdar <[email protected]>
---
drivers/staging/iio/adc/ad7816.c | 80 ++++++++++++++------------------
1 file changed, 34 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index bf76a8620bdb..12c4e0ab4713 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -7,7 +7,7 @@
*/

#include <linux/interrupt.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/device.h>
#include <linux/kernel.h>
#include <linux/slab.h>
@@ -44,9 +44,9 @@

struct ad7816_chip_info {
struct spi_device *spi_dev;
- u16 rdwr_pin;
- u16 convert_pin;
- u16 busy_pin;
+ struct gpio_desc *rdwr_pin;
+ struct gpio_desc *convert_pin;
+ struct gpio_desc *busy_pin;
u8 oti_data[AD7816_CS_MAX + 1];
u8 channel_id; /* 0 always be temperature */
u8 mode;
@@ -61,28 +61,28 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
int ret = 0;
__be16 buf;

- gpio_set_value(chip->rdwr_pin, 1);
- gpio_set_value(chip->rdwr_pin, 0);
+ gpiod_set_value(chip->rdwr_pin, 1);
+ gpiod_set_value(chip->rdwr_pin, 0);
ret = spi_write(spi_dev, &chip->channel_id, sizeof(chip->channel_id));
if (ret < 0) {
dev_err(&spi_dev->dev, "SPI channel setting error\n");
return ret;
}
- gpio_set_value(chip->rdwr_pin, 1);
+ gpiod_set_value(chip->rdwr_pin, 1);

if (chip->mode == AD7816_PD) { /* operating mode 2 */
- gpio_set_value(chip->convert_pin, 1);
- gpio_set_value(chip->convert_pin, 0);
+ gpiod_set_value(chip->convert_pin, 1);
+ gpiod_set_value(chip->convert_pin, 0);
} else { /* operating mode 1 */
- gpio_set_value(chip->convert_pin, 0);
- gpio_set_value(chip->convert_pin, 1);
+ gpiod_set_value(chip->convert_pin, 0);
+ gpiod_set_value(chip->convert_pin, 1);
}

- while (gpio_get_value(chip->busy_pin))
+ while (gpiod_get_value(chip->busy_pin))
cpu_relax();

- gpio_set_value(chip->rdwr_pin, 0);
- gpio_set_value(chip->rdwr_pin, 1);
+ gpiod_set_value(chip->rdwr_pin, 0);
+ gpiod_set_value(chip->rdwr_pin, 1);
ret = spi_read(spi_dev, &buf, sizeof(*data));
if (ret < 0) {
dev_err(&spi_dev->dev, "SPI data read error\n");
@@ -99,8 +99,8 @@ static int ad7816_spi_write(struct ad7816_chip_info *chip, u8 data)
struct spi_device *spi_dev = chip->spi_dev;
int ret = 0;

- gpio_set_value(chip->rdwr_pin, 1);
- gpio_set_value(chip->rdwr_pin, 0);
+ gpiod_set_value(chip->rdwr_pin, 1);
+ gpiod_set_value(chip->rdwr_pin, 0);
ret = spi_write(spi_dev, &data, sizeof(data));
if (ret < 0)
dev_err(&spi_dev->dev, "SPI oti data write error\n");
@@ -129,10 +129,10 @@ static ssize_t ad7816_store_mode(struct device *dev,
struct ad7816_chip_info *chip = iio_priv(indio_dev);

if (strcmp(buf, "full")) {
- gpio_set_value(chip->rdwr_pin, 1);
+ gpiod_set_value(chip->rdwr_pin, 1);
chip->mode = AD7816_FULL;
} else {
- gpio_set_value(chip->rdwr_pin, 0);
+ gpiod_set_value(chip->rdwr_pin, 0);
chip->mode = AD7816_PD;
}

@@ -345,15 +345,9 @@ static int ad7816_probe(struct spi_device *spi_dev)
{
struct ad7816_chip_info *chip;
struct iio_dev *indio_dev;
- unsigned short *pins = dev_get_platdata(&spi_dev->dev);
int ret = 0;
int i;

- if (!pins) {
- dev_err(&spi_dev->dev, "No necessary GPIO platform data.\n");
- return -EINVAL;
- }
-
indio_dev = devm_iio_device_alloc(&spi_dev->dev, sizeof(*chip));
if (!indio_dev)
return -ENOMEM;
@@ -364,34 +358,28 @@ static int ad7816_probe(struct spi_device *spi_dev)
chip->spi_dev = spi_dev;
for (i = 0; i <= AD7816_CS_MAX; i++)
chip->oti_data[i] = 203;
- chip->rdwr_pin = pins[0];
- chip->convert_pin = pins[1];
- chip->busy_pin = pins[2];
-
- ret = devm_gpio_request(&spi_dev->dev, chip->rdwr_pin,
- spi_get_device_id(spi_dev)->name);
- if (ret) {
- dev_err(&spi_dev->dev, "Fail to request rdwr gpio PIN %d.\n",
- chip->rdwr_pin);
+
+ chip->rdwr_pin = devm_gpiod_get(&spi_dev->dev, "rdwr", GPIOD_IN);
+ if (IS_ERR(chip->rdwr_pin)) {
+ ret = PTR_ERR(chip->rdwr_pin);
+ dev_err(&spi_dev->dev, "Failed to request rdwr GPIO: %d\n",
+ ret);
return ret;
}
- gpio_direction_input(chip->rdwr_pin);
- ret = devm_gpio_request(&spi_dev->dev, chip->convert_pin,
- spi_get_device_id(spi_dev)->name);
- if (ret) {
- dev_err(&spi_dev->dev, "Fail to request convert gpio PIN %d.\n",
- chip->convert_pin);
+ chip->convert_pin = devm_gpiod_get(&spi_dev->dev, "convert", GPIOD_IN);
+ if (IS_ERR(chip->convert_pin)) {
+ ret = PTR_ERR(chip->convert_pin);
+ dev_err(&spi_dev->dev, "Failed to request convert GPIO: %d\n",
+ ret);
return ret;
}
- gpio_direction_input(chip->convert_pin);
- ret = devm_gpio_request(&spi_dev->dev, chip->busy_pin,
- spi_get_device_id(spi_dev)->name);
- if (ret) {
- dev_err(&spi_dev->dev, "Fail to request busy gpio PIN %d.\n",
- chip->busy_pin);
+ chip->busy_pin = devm_gpiod_get(&spi_dev->dev, "busy", GPIOD_IN);
+ if (IS_ERR(chip->busy_pin)) {
+ ret = PTR_ERR(chip->busy_pin);
+ dev_err(&spi_dev->dev, "Failed to request busy GPIO: %d\n",
+ ret);
return ret;
}
- gpio_direction_input(chip->busy_pin);

indio_dev->name = spi_get_device_id(spi_dev)->name;
indio_dev->dev.parent = &spi_dev->dev;
--
2.17.1


2018-11-09 07:38:30

by Nishad Kamdar

[permalink] [raw]
Subject: [PATCH v3 2/4] staging: iio: ad7816: Do not use busy_pin in case of AD7818

AD7818 does not support busy_pin functionality as per datasheet.
Hence drop busy_pin when AD7818 is used.

Signed-off-by: Nishad Kamdar <[email protected]>
---
drivers/staging/iio/adc/ad7816.c | 35 ++++++++++++++++++++++----------
1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index 12c4e0ab4713..3cda5cd09365 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -43,6 +43,7 @@
*/

struct ad7816_chip_info {
+ kernel_ulong_t id;
struct spi_device *spi_dev;
struct gpio_desc *rdwr_pin;
struct gpio_desc *convert_pin;
@@ -52,6 +53,12 @@ struct ad7816_chip_info {
u8 mode;
};

+enum ad7816_type {
+ ID_AD7816,
+ ID_AD7817,
+ ID_AD7818,
+};
+
/*
* ad7816 data access by SPI
*/
@@ -78,8 +85,10 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
gpiod_set_value(chip->convert_pin, 1);
}

- while (gpiod_get_value(chip->busy_pin))
- cpu_relax();
+ if (chip->id == ID_AD7816 || chip->id == ID_AD7817) {
+ while (gpiod_get_value(chip->busy_pin))
+ cpu_relax();
+ }

gpiod_set_value(chip->rdwr_pin, 0);
gpiod_set_value(chip->rdwr_pin, 1);
@@ -359,6 +368,7 @@ static int ad7816_probe(struct spi_device *spi_dev)
for (i = 0; i <= AD7816_CS_MAX; i++)
chip->oti_data[i] = 203;

+ chip->id = spi_get_device_id(spi_dev)->driver_data;
chip->rdwr_pin = devm_gpiod_get(&spi_dev->dev, "rdwr", GPIOD_IN);
if (IS_ERR(chip->rdwr_pin)) {
ret = PTR_ERR(chip->rdwr_pin);
@@ -373,12 +383,15 @@ static int ad7816_probe(struct spi_device *spi_dev)
ret);
return ret;
}
- chip->busy_pin = devm_gpiod_get(&spi_dev->dev, "busy", GPIOD_IN);
- if (IS_ERR(chip->busy_pin)) {
- ret = PTR_ERR(chip->busy_pin);
- dev_err(&spi_dev->dev, "Failed to request busy GPIO: %d\n",
- ret);
- return ret;
+ if (chip->id == ID_AD7816 || chip->id == ID_AD7817) {
+ chip->busy_pin = devm_gpiod_get(&spi_dev->dev, "busy",
+ GPIOD_IN);
+ if (IS_ERR(chip->busy_pin)) {
+ ret = PTR_ERR(chip->busy_pin);
+ dev_err(&spi_dev->dev, "Failed to request busy GPIO: %d\n",
+ ret);
+ return ret;
+ }
}

indio_dev->name = spi_get_device_id(spi_dev)->name;
@@ -409,9 +422,9 @@ static int ad7816_probe(struct spi_device *spi_dev)
}

static const struct spi_device_id ad7816_id[] = {
- { "ad7816", 0 },
- { "ad7817", 0 },
- { "ad7818", 0 },
+ { "ad7816", ID_AD7816 },
+ { "ad7817", ID_AD7817 },
+ { "ad7818", ID_AD7818 },
{}
};

--
2.17.1


2018-11-09 07:39:25

by Nishad Kamdar

[permalink] [raw]
Subject: [PATCH v3 3/4] staging: iio: ad7816: Set RD/WR pin and CONVST pin as outputs.

The RD/WR pin and CONVST pin are logical inputs to the AD78xx
chip as per the datasheet. Hence convert them to outputs.

Signed-off-by: Nishad Kamdar <[email protected]>
---
drivers/staging/iio/adc/ad7816.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index 3cda5cd09365..a2fead85cd46 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -369,14 +369,15 @@ static int ad7816_probe(struct spi_device *spi_dev)
chip->oti_data[i] = 203;

chip->id = spi_get_device_id(spi_dev)->driver_data;
- chip->rdwr_pin = devm_gpiod_get(&spi_dev->dev, "rdwr", GPIOD_IN);
+ chip->rdwr_pin = devm_gpiod_get(&spi_dev->dev, "rdwr", GPIOD_OUT_HIGH);
if (IS_ERR(chip->rdwr_pin)) {
ret = PTR_ERR(chip->rdwr_pin);
dev_err(&spi_dev->dev, "Failed to request rdwr GPIO: %d\n",
ret);
return ret;
}
- chip->convert_pin = devm_gpiod_get(&spi_dev->dev, "convert", GPIOD_IN);
+ chip->convert_pin = devm_gpiod_get(&spi_dev->dev, "convert",
+ GPIOD_OUT_HIGH);
if (IS_ERR(chip->convert_pin)) {
ret = PTR_ERR(chip->convert_pin);
dev_err(&spi_dev->dev, "Failed to request convert GPIO: %d\n",
--
2.17.1


2018-11-09 07:41:07

by Nishad Kamdar

[permalink] [raw]
Subject: [PATCH v3 4/4] staging: iio: ad7816: Add device tree table.

Add device tree table for matching vendor ID.

Signed-off-by: Nishad Kamdar <[email protected]>
---
drivers/staging/iio/adc/ad7816.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index a2fead85cd46..b8a9149fbac1 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -422,6 +422,12 @@ static int ad7816_probe(struct spi_device *spi_dev)
return 0;
}

+static const struct of_device_id ad7816_of_match[] = {
+ { .compatible = "adi,ad7816", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ad7816_of_match);
+
static const struct spi_device_id ad7816_id[] = {
{ "ad7816", ID_AD7816 },
{ "ad7817", ID_AD7817 },
@@ -434,6 +440,7 @@ MODULE_DEVICE_TABLE(spi, ad7816_id);
static struct spi_driver ad7816_driver = {
.driver = {
.name = "ad7816",
+ .of_match_table = of_match_ptr(ad7816_of_match),
},
.probe = ad7816_probe,
.id_table = ad7816_id,
--
2.17.1


2018-11-09 08:07:29

by Alexandru Ardelean

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] staging: iio: ad7816: Switch to the gpio descriptor interface

On Fri, 2018-11-09 at 13:05 +0530, Nishad Kamdar wrote:
> Use the gpiod interface for rdwr_pin, convert_pin and busy_pin
> instead of the deprecated old non-descriptor interface.
>

Patch looks good.
I do have some thoughts about it. See inline.


> Signed-off-by: Nishad Kamdar <[email protected]>
> ---
> drivers/staging/iio/adc/ad7816.c | 80 ++++++++++++++------------------
> 1 file changed, 34 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7816.c
> b/drivers/staging/iio/adc/ad7816.c
> index bf76a8620bdb..12c4e0ab4713 100644
> --- a/drivers/staging/iio/adc/ad7816.c
> +++ b/drivers/staging/iio/adc/ad7816.c
> @@ -7,7 +7,7 @@
> */
>
> #include <linux/interrupt.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/device.h>
> #include <linux/kernel.h>
> #include <linux/slab.h>
> @@ -44,9 +44,9 @@
>
> struct ad7816_chip_info {
> struct spi_device *spi_dev;
> - u16 rdwr_pin;
> - u16 convert_pin;
> - u16 busy_pin;
> + struct gpio_desc *rdwr_pin;
> + struct gpio_desc *convert_pin;
> + struct gpio_desc *busy_pin;

Only if you want to do a re-spin, here's an idea.
If you don't want to, feel free to disregard, since your patch is good.

You could compact things a bit; I know this wasn't what the initial code
was doing, but it's an option.

Something like:

enum {
GPIO_RWDR,
GPIO_CONVERT,
GPIO_BUSY,
__GPIO_MAX,
}

Then, what you end up having is:

struct ad7816_chip_info {
struct spi_device *spi_dev;
- u16 rdwr_pin;
- u16 convert_pin;
- u16 busy_pin;
+ struct gpio_desc *gpios[__GPIO_MAX];


// Continued below


> u8 oti_data[AD7816_CS_MAX + 1];
> u8 channel_id; /* 0 always be temperature */
> u8 mode;
> @@ -61,28 +61,28 @@ static int ad7816_spi_read(struct ad7816_chip_info
> *chip, u16 *data)
> int ret = 0;
> __be16 buf;
>
> - gpio_set_value(chip->rdwr_pin, 1);
> - gpio_set_value(chip->rdwr_pin, 0);
> + gpiod_set_value(chip->rdwr_pin, 1);
> + gpiod_set_value(chip->rdwr_pin, 0);

Obviously, in the above context, this becomes:
+ gpiod_set_value(chip->gpios[GPIO_RWDR], 1);
+ gpiod_set_value(chip->gpios[GPIO_RWDR], 0);

> ret = spi_write(spi_dev, &chip->channel_id, sizeof(chip-
> >channel_id));
> if (ret < 0) {
> dev_err(&spi_dev->dev, "SPI channel setting error\n");
> return ret;
> }
> - gpio_set_value(chip->rdwr_pin, 1);
> + gpiod_set_value(chip->rdwr_pin, 1);
>
> if (chip->mode == AD7816_PD) { /* operating mode 2 */
> - gpio_set_value(chip->convert_pin, 1);
> - gpio_set_value(chip->convert_pin, 0);
> + gpiod_set_value(chip->convert_pin, 1);
> + gpiod_set_value(chip->convert_pin, 0);
> } else { /* operating mode 1 */
> - gpio_set_value(chip->convert_pin, 0);
> - gpio_set_value(chip->convert_pin, 1);
> + gpiod_set_value(chip->convert_pin, 0);
> + gpiod_set_value(chip->convert_pin, 1);
> }
>
> - while (gpio_get_value(chip->busy_pin))
> + while (gpiod_get_value(chip->busy_pin))
> cpu_relax();
>
> - gpio_set_value(chip->rdwr_pin, 0);
> - gpio_set_value(chip->rdwr_pin, 1);
> + gpiod_set_value(chip->rdwr_pin, 0);
> + gpiod_set_value(chip->rdwr_pin, 1);
> ret = spi_read(spi_dev, &buf, sizeof(*data));
> if (ret < 0) {
> dev_err(&spi_dev->dev, "SPI data read error\n");
> @@ -99,8 +99,8 @@ static int ad7816_spi_write(struct ad7816_chip_info
> *chip, u8 data)
> struct spi_device *spi_dev = chip->spi_dev;
> int ret = 0;
>
> - gpio_set_value(chip->rdwr_pin, 1);
> - gpio_set_value(chip->rdwr_pin, 0);
> + gpiod_set_value(chip->rdwr_pin, 1);
> + gpiod_set_value(chip->rdwr_pin, 0);
> ret = spi_write(spi_dev, &data, sizeof(data));
> if (ret < 0)
> dev_err(&spi_dev->dev, "SPI oti data write error\n");
> @@ -129,10 +129,10 @@ static ssize_t ad7816_store_mode(struct device
> *dev,
> struct ad7816_chip_info *chip = iio_priv(indio_dev);
>
> if (strcmp(buf, "full")) {
> - gpio_set_value(chip->rdwr_pin, 1);
> + gpiod_set_value(chip->rdwr_pin, 1);
> chip->mode = AD7816_FULL;
> } else {
> - gpio_set_value(chip->rdwr_pin, 0);
> + gpiod_set_value(chip->rdwr_pin, 0);
> chip->mode = AD7816_PD;
> }
>
> @@ -345,15 +345,9 @@ static int ad7816_probe(struct spi_device *spi_dev)
> {
> struct ad7816_chip_info *chip;
> struct iio_dev *indio_dev;
> - unsigned short *pins = dev_get_platdata(&spi_dev->dev);
> int ret = 0;
> int i;
>
> - if (!pins) {
> - dev_err(&spi_dev->dev, "No necessary GPIO platform
> data.\n");
> - return -EINVAL;
> - }
> -
> indio_dev = devm_iio_device_alloc(&spi_dev->dev, sizeof(*chip));
> if (!indio_dev)
> return -ENOMEM;
> @@ -364,34 +358,28 @@ static int ad7816_probe(struct spi_device *spi_dev)
> chip->spi_dev = spi_dev;
> for (i = 0; i <= AD7816_CS_MAX; i++)
> chip->oti_data[i] = 203;
> - chip->rdwr_pin = pins[0];
> - chip->convert_pin = pins[1];
> - chip->busy_pin = pins[2];
> -
> - ret = devm_gpio_request(&spi_dev->dev, chip->rdwr_pin,
> - spi_get_device_id(spi_dev)->name);
> - if (ret) {
> - dev_err(&spi_dev->dev, "Fail to request rdwr gpio PIN
> %d.\n",
> - chip->rdwr_pin);
> +
> + chip->rdwr_pin = devm_gpiod_get(&spi_dev->dev, "rdwr", GPIOD_IN);
> + if (IS_ERR(chip->rdwr_pin)) {
> + ret = PTR_ERR(chip->rdwr_pin);
> + dev_err(&spi_dev->dev, "Failed to request rdwr GPIO: %d\n",
> + ret);
> return ret;
> }

But the real benefit if this approach comes here.
You could do:

static const char *gpio_names[__GPIO_MAX] = {
[GPIO_RWDR] = "rwdr",
[GPIO_CONVERT] = "convert",
[GPIO_BUSY] = "busy",
};

// code for patch2
bool have_busy_pin = (chip->id == ID_AD7816 || chip->id == ID_AD7817);
// end code for patch2


for (i = 0; i < __GPIO_MAX; i++) {
// code for patch2
if (i == GPIO_BUSY && !have_busy_pin)
continue;
// end code for patch2

chip->gpios[i] = devm_gpiod_get(&spi_dev->dev, gpio_names[i],
GPIOD_IN);
if (IS_ERR(chip->gpios[i])) {
ret = PTR_ERR(chip->gpios[i]);
dev_err(&spi_dev->dev, "Failed to request rdwr GPIO: %d\n",
ret;
return ret;
}
}

Then, in your patch2 (i.e " staging: iio: ad7816: Do not use busy_pin in
case of AD7818"), what you could do, is to add that code (along with the
other ID code).

And finaly what you could do in patch2, is:

while (chip->gpios[GPIO_BUSY] &&
gpiod_get_value(chip->busy_pin))
cpu_relax();



> - gpio_direction_input(chip->rdwr_pin);
> - ret = devm_gpio_request(&spi_dev->dev, chip->convert_pin,
> - spi_get_device_id(spi_dev)->name);
> - if (ret) {
> - dev_err(&spi_dev->dev, "Fail to request convert gpio PIN
> %d.\n",
> - chip->convert_pin);
> + chip->convert_pin = devm_gpiod_get(&spi_dev->dev, "convert",
> GPIOD_IN);
> + if (IS_ERR(chip->convert_pin)) {
> + ret = PTR_ERR(chip->convert_pin);
> + dev_err(&spi_dev->dev, "Failed to request convert GPIO:
> %d\n",
> + ret);
> return ret;
> }
> - gpio_direction_input(chip->convert_pin);
> - ret = devm_gpio_request(&spi_dev->dev, chip->busy_pin,
> - spi_get_device_id(spi_dev)->name);
> - if (ret) {
> - dev_err(&spi_dev->dev, "Fail to request busy gpio PIN
> %d.\n",
> - chip->busy_pin);
> + chip->busy_pin = devm_gpiod_get(&spi_dev->dev, "busy", GPIOD_IN);
> + if (IS_ERR(chip->busy_pin)) {
> + ret = PTR_ERR(chip->busy_pin);
> + dev_err(&spi_dev->dev, "Failed to request busy GPIO: %d\n",
> + ret);
> return ret;
> }
> - gpio_direction_input(chip->busy_pin);
>
> indio_dev->name = spi_get_device_id(spi_dev)->name;
> indio_dev->dev.parent = &spi_dev->dev;

2018-11-09 08:09:27

by Alexandru Ardelean

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] staging: iio: ad7816: Switch to the gpio descriptor interface

On Fri, 2018-11-09 at 08:05 +0000, Ardelean, Alexandru wrote:
> On Fri, 2018-11-09 at 13:05 +0530, Nishad Kamdar wrote:
> > Use the gpiod interface for rdwr_pin, convert_pin and busy_pin
> > instead of the deprecated old non-descriptor interface.
> >
>
> Patch looks good.
> I do have some thoughts about it. See inline.
>

Nevermind what I just said here; I just saw patch3: `iio: ad7816: Set RD/WR
pin and CONVST pin as outputs.`

This looks good as is in the context of the patch series.

>
> > Signed-off-by: Nishad Kamdar <[email protected]>
> > ---
> > drivers/staging/iio/adc/ad7816.c | 80 ++++++++++++++------------------
> > 1 file changed, 34 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7816.c
> > b/drivers/staging/iio/adc/ad7816.c
> > index bf76a8620bdb..12c4e0ab4713 100644
> > --- a/drivers/staging/iio/adc/ad7816.c
> > +++ b/drivers/staging/iio/adc/ad7816.c
> > @@ -7,7 +7,7 @@
> > */
> >
> > #include <linux/interrupt.h>
> > -#include <linux/gpio.h>
> > +#include <linux/gpio/consumer.h>
> > #include <linux/device.h>
> > #include <linux/kernel.h>
> > #include <linux/slab.h>
> > @@ -44,9 +44,9 @@
> >
> > struct ad7816_chip_info {
> > struct spi_device *spi_dev;
> > - u16 rdwr_pin;
> > - u16 convert_pin;
> > - u16 busy_pin;
> > + struct gpio_desc *rdwr_pin;
> > + struct gpio_desc *convert_pin;
> > + struct gpio_desc *busy_pin;
>
> Only if you want to do a re-spin, here's an idea.
> If you don't want to, feel free to disregard, since your patch is good.
>
> You could compact things a bit; I know this wasn't what the initial code
> was doing, but it's an option.
>
> Something like:
>
> enum {
> GPIO_RWDR,
> GPIO_CONVERT,
> GPIO_BUSY,
> __GPIO_MAX,
> }
>
> Then, what you end up having is:
>
> struct ad7816_chip_info {
> struct spi_device *spi_dev;
> - u16 rdwr_pin;
> - u16 convert_pin;
> - u16 busy_pin;
> + struct gpio_desc *gpios[__GPIO_MAX];
>
>
> // Continued below
>
>
> > u8 oti_data[AD7816_CS_MAX + 1];
> > u8 channel_id; /* 0 always be temperature */
> > u8 mode;
> > @@ -61,28 +61,28 @@ static int ad7816_spi_read(struct ad7816_chip_info
> > *chip, u16 *data)
> > int ret = 0;
> > __be16 buf;
> >
> > - gpio_set_value(chip->rdwr_pin, 1);
> > - gpio_set_value(chip->rdwr_pin, 0);
> > + gpiod_set_value(chip->rdwr_pin, 1);
> > + gpiod_set_value(chip->rdwr_pin, 0);
>
> Obviously, in the above context, this becomes:
> + gpiod_set_value(chip->gpios[GPIO_RWDR], 1);
> + gpiod_set_value(chip->gpios[GPIO_RWDR], 0);
>
> > ret = spi_write(spi_dev, &chip->channel_id, sizeof(chip-
> > > channel_id));
> >
> > if (ret < 0) {
> > dev_err(&spi_dev->dev, "SPI channel setting error\n");
> > return ret;
> > }
> > - gpio_set_value(chip->rdwr_pin, 1);
> > + gpiod_set_value(chip->rdwr_pin, 1);
> >
> > if (chip->mode == AD7816_PD) { /* operating mode 2 */
> > - gpio_set_value(chip->convert_pin, 1);
> > - gpio_set_value(chip->convert_pin, 0);
> > + gpiod_set_value(chip->convert_pin, 1);
> > + gpiod_set_value(chip->convert_pin, 0);
> > } else { /* operating mode 1 */
> > - gpio_set_value(chip->convert_pin, 0);
> > - gpio_set_value(chip->convert_pin, 1);
> > + gpiod_set_value(chip->convert_pin, 0);
> > + gpiod_set_value(chip->convert_pin, 1);
> > }
> >
> > - while (gpio_get_value(chip->busy_pin))
> > + while (gpiod_get_value(chip->busy_pin))
> > cpu_relax();
> >
> > - gpio_set_value(chip->rdwr_pin, 0);
> > - gpio_set_value(chip->rdwr_pin, 1);
> > + gpiod_set_value(chip->rdwr_pin, 0);
> > + gpiod_set_value(chip->rdwr_pin, 1);
> > ret = spi_read(spi_dev, &buf, sizeof(*data));
> > if (ret < 0) {
> > dev_err(&spi_dev->dev, "SPI data read error\n");
> > @@ -99,8 +99,8 @@ static int ad7816_spi_write(struct ad7816_chip_info
> > *chip, u8 data)
> > struct spi_device *spi_dev = chip->spi_dev;
> > int ret = 0;
> >
> > - gpio_set_value(chip->rdwr_pin, 1);
> > - gpio_set_value(chip->rdwr_pin, 0);
> > + gpiod_set_value(chip->rdwr_pin, 1);
> > + gpiod_set_value(chip->rdwr_pin, 0);
> > ret = spi_write(spi_dev, &data, sizeof(data));
> > if (ret < 0)
> > dev_err(&spi_dev->dev, "SPI oti data write error\n");
> > @@ -129,10 +129,10 @@ static ssize_t ad7816_store_mode(struct device
> > *dev,
> > struct ad7816_chip_info *chip = iio_priv(indio_dev);
> >
> > if (strcmp(buf, "full")) {
> > - gpio_set_value(chip->rdwr_pin, 1);
> > + gpiod_set_value(chip->rdwr_pin, 1);
> > chip->mode = AD7816_FULL;
> > } else {
> > - gpio_set_value(chip->rdwr_pin, 0);
> > + gpiod_set_value(chip->rdwr_pin, 0);
> > chip->mode = AD7816_PD;
> > }
> >
> > @@ -345,15 +345,9 @@ static int ad7816_probe(struct spi_device
> > *spi_dev)
> > {
> > struct ad7816_chip_info *chip;
> > struct iio_dev *indio_dev;
> > - unsigned short *pins = dev_get_platdata(&spi_dev->dev);
> > int ret = 0;
> > int i;
> >
> > - if (!pins) {
> > - dev_err(&spi_dev->dev, "No necessary GPIO platform
> > data.\n");
> > - return -EINVAL;
> > - }
> > -
> > indio_dev = devm_iio_device_alloc(&spi_dev->dev, sizeof(*chip));
> > if (!indio_dev)
> > return -ENOMEM;
> > @@ -364,34 +358,28 @@ static int ad7816_probe(struct spi_device
> > *spi_dev)
> > chip->spi_dev = spi_dev;
> > for (i = 0; i <= AD7816_CS_MAX; i++)
> > chip->oti_data[i] = 203;
> > - chip->rdwr_pin = pins[0];
> > - chip->convert_pin = pins[1];
> > - chip->busy_pin = pins[2];
> > -
> > - ret = devm_gpio_request(&spi_dev->dev, chip->rdwr_pin,
> > - spi_get_device_id(spi_dev)->name);
> > - if (ret) {
> > - dev_err(&spi_dev->dev, "Fail to request rdwr gpio PIN
> > %d.\n",
> > - chip->rdwr_pin);
> > +
> > + chip->rdwr_pin = devm_gpiod_get(&spi_dev->dev, "rdwr", GPIOD_IN);
> > + if (IS_ERR(chip->rdwr_pin)) {
> > + ret = PTR_ERR(chip->rdwr_pin);
> > + dev_err(&spi_dev->dev, "Failed to request rdwr GPIO: %d\n",
> > + ret);
> > return ret;
> > }
>
> But the real benefit if this approach comes here.
> You could do:
>
> static const char *gpio_names[__GPIO_MAX] = {
> [GPIO_RWDR] = "rwdr",
> [GPIO_CONVERT] = "convert",
> [GPIO_BUSY] = "busy",
> };
>
> // code for patch2
> bool have_busy_pin = (chip->id == ID_AD7816 || chip->id == ID_AD7817);
> // end code for patch2
>
>
> for (i = 0; i < __GPIO_MAX; i++) {
> // code for patch2
> if (i == GPIO_BUSY && !have_busy_pin)
> continue;
> // end code for patch2
>
> chip->gpios[i] = devm_gpiod_get(&spi_dev->dev, gpio_names[i],
> GPIOD_IN);
> if (IS_ERR(chip->gpios[i])) {
> ret = PTR_ERR(chip->gpios[i]);
> dev_err(&spi_dev->dev, "Failed to request rdwr GPIO: %d\n",
> ret;
> return ret;
> }
> }
>
> Then, in your patch2 (i.e " staging: iio: ad7816: Do not use busy_pin in
> case of AD7818"), what you could do, is to add that code (along with the
> other ID code).
>
> And finaly what you could do in patch2, is:
>
> while (chip->gpios[GPIO_BUSY] &&
> gpiod_get_value(chip->busy_pin))
> cpu_relax();
>
>
>
> > - gpio_direction_input(chip->rdwr_pin);
> > - ret = devm_gpio_request(&spi_dev->dev, chip->convert_pin,
> > - spi_get_device_id(spi_dev)->name);
> > - if (ret) {
> > - dev_err(&spi_dev->dev, "Fail to request convert gpio PIN
> > %d.\n",
> > - chip->convert_pin);
> > + chip->convert_pin = devm_gpiod_get(&spi_dev->dev, "convert",
> > GPIOD_IN);
> > + if (IS_ERR(chip->convert_pin)) {
> > + ret = PTR_ERR(chip->convert_pin);
> > + dev_err(&spi_dev->dev, "Failed to request convert GPIO:
> > %d\n",
> > + ret);
> > return ret;
> > }
> > - gpio_direction_input(chip->convert_pin);
> > - ret = devm_gpio_request(&spi_dev->dev, chip->busy_pin,
> > - spi_get_device_id(spi_dev)->name);
> > - if (ret) {
> > - dev_err(&spi_dev->dev, "Fail to request busy gpio PIN
> > %d.\n",
> > - chip->busy_pin);
> > + chip->busy_pin = devm_gpiod_get(&spi_dev->dev, "busy", GPIOD_IN);
> > + if (IS_ERR(chip->busy_pin)) {
> > + ret = PTR_ERR(chip->busy_pin);
> > + dev_err(&spi_dev->dev, "Failed to request busy GPIO: %d\n",
> > + ret);
> > return ret;
> > }
> > - gpio_direction_input(chip->busy_pin);
> >
> > indio_dev->name = spi_get_device_id(spi_dev)->name;
> > indio_dev->dev.parent = &spi_dev->dev;

2018-11-09 08:16:02

by Alexandru Ardelean

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] staging: iio: ad7816: Add device tree table.

On Fri, 2018-11-09 at 13:08 +0530, Nishad Kamdar wrote:
> Add device tree table for matching vendor ID.

One comment inline for this.

Thanks
Alex

>
> Signed-off-by: Nishad Kamdar <[email protected]>
> ---
> drivers/staging/iio/adc/ad7816.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/staging/iio/adc/ad7816.c
> b/drivers/staging/iio/adc/ad7816.c
> index a2fead85cd46..b8a9149fbac1 100644
> --- a/drivers/staging/iio/adc/ad7816.c
> +++ b/drivers/staging/iio/adc/ad7816.c
> @@ -422,6 +422,12 @@ static int ad7816_probe(struct spi_device *spi_dev)
> return 0;
> }
>
> +static const struct of_device_id ad7816_of_match[] = {
> + { .compatible = "adi,ad7816", },

I think you also need to add:
+ { .compatible = "adi,ad7817", },
+ { .compatible = "adi,ad7818", },

> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ad7816_of_match);
> +
> static const struct spi_device_id ad7816_id[] = {
> { "ad7816", ID_AD7816 },
> { "ad7817", ID_AD7817 },
> @@ -434,6 +440,7 @@ MODULE_DEVICE_TABLE(spi, ad7816_id);
> static struct spi_driver ad7816_driver = {
> .driver = {
> .name = "ad7816",
> + .of_match_table = of_match_ptr(ad7816_of_match),
> },
> .probe = ad7816_probe,
> .id_table = ad7816_id,

2018-11-11 12:24:48

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] staging: iio: ad7816: Switch to the gpio descriptor interface

On Fri, 9 Nov 2018 13:05:17 +0530
Nishad Kamdar <[email protected]> wrote:

> Use the gpiod interface for rdwr_pin, convert_pin and busy_pin
> instead of the deprecated old non-descriptor interface.
>
> Signed-off-by: Nishad Kamdar <[email protected]>
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> ---
> drivers/staging/iio/adc/ad7816.c | 80 ++++++++++++++------------------
> 1 file changed, 34 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
> index bf76a8620bdb..12c4e0ab4713 100644
> --- a/drivers/staging/iio/adc/ad7816.c
> +++ b/drivers/staging/iio/adc/ad7816.c
> @@ -7,7 +7,7 @@
> */
>
> #include <linux/interrupt.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/device.h>
> #include <linux/kernel.h>
> #include <linux/slab.h>
> @@ -44,9 +44,9 @@
>
> struct ad7816_chip_info {
> struct spi_device *spi_dev;
> - u16 rdwr_pin;
> - u16 convert_pin;
> - u16 busy_pin;
> + struct gpio_desc *rdwr_pin;
> + struct gpio_desc *convert_pin;
> + struct gpio_desc *busy_pin;
> u8 oti_data[AD7816_CS_MAX + 1];
> u8 channel_id; /* 0 always be temperature */
> u8 mode;
> @@ -61,28 +61,28 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
> int ret = 0;
> __be16 buf;
>
> - gpio_set_value(chip->rdwr_pin, 1);
> - gpio_set_value(chip->rdwr_pin, 0);
> + gpiod_set_value(chip->rdwr_pin, 1);
> + gpiod_set_value(chip->rdwr_pin, 0);
> ret = spi_write(spi_dev, &chip->channel_id, sizeof(chip->channel_id));
> if (ret < 0) {
> dev_err(&spi_dev->dev, "SPI channel setting error\n");
> return ret;
> }
> - gpio_set_value(chip->rdwr_pin, 1);
> + gpiod_set_value(chip->rdwr_pin, 1);
>
> if (chip->mode == AD7816_PD) { /* operating mode 2 */
> - gpio_set_value(chip->convert_pin, 1);
> - gpio_set_value(chip->convert_pin, 0);
> + gpiod_set_value(chip->convert_pin, 1);
> + gpiod_set_value(chip->convert_pin, 0);
> } else { /* operating mode 1 */
> - gpio_set_value(chip->convert_pin, 0);
> - gpio_set_value(chip->convert_pin, 1);
> + gpiod_set_value(chip->convert_pin, 0);
> + gpiod_set_value(chip->convert_pin, 1);
> }
>
> - while (gpio_get_value(chip->busy_pin))
> + while (gpiod_get_value(chip->busy_pin))
> cpu_relax();
>
> - gpio_set_value(chip->rdwr_pin, 0);
> - gpio_set_value(chip->rdwr_pin, 1);
> + gpiod_set_value(chip->rdwr_pin, 0);
> + gpiod_set_value(chip->rdwr_pin, 1);
> ret = spi_read(spi_dev, &buf, sizeof(*data));
> if (ret < 0) {
> dev_err(&spi_dev->dev, "SPI data read error\n");
> @@ -99,8 +99,8 @@ static int ad7816_spi_write(struct ad7816_chip_info *chip, u8 data)
> struct spi_device *spi_dev = chip->spi_dev;
> int ret = 0;
>
> - gpio_set_value(chip->rdwr_pin, 1);
> - gpio_set_value(chip->rdwr_pin, 0);
> + gpiod_set_value(chip->rdwr_pin, 1);
> + gpiod_set_value(chip->rdwr_pin, 0);
> ret = spi_write(spi_dev, &data, sizeof(data));
> if (ret < 0)
> dev_err(&spi_dev->dev, "SPI oti data write error\n");
> @@ -129,10 +129,10 @@ static ssize_t ad7816_store_mode(struct device *dev,
> struct ad7816_chip_info *chip = iio_priv(indio_dev);
>
> if (strcmp(buf, "full")) {
> - gpio_set_value(chip->rdwr_pin, 1);
> + gpiod_set_value(chip->rdwr_pin, 1);
> chip->mode = AD7816_FULL;
> } else {
> - gpio_set_value(chip->rdwr_pin, 0);
> + gpiod_set_value(chip->rdwr_pin, 0);
> chip->mode = AD7816_PD;
> }
>
> @@ -345,15 +345,9 @@ static int ad7816_probe(struct spi_device *spi_dev)
> {
> struct ad7816_chip_info *chip;
> struct iio_dev *indio_dev;
> - unsigned short *pins = dev_get_platdata(&spi_dev->dev);
> int ret = 0;
> int i;
>
> - if (!pins) {
> - dev_err(&spi_dev->dev, "No necessary GPIO platform data.\n");
> - return -EINVAL;
> - }
> -
> indio_dev = devm_iio_device_alloc(&spi_dev->dev, sizeof(*chip));
> if (!indio_dev)
> return -ENOMEM;
> @@ -364,34 +358,28 @@ static int ad7816_probe(struct spi_device *spi_dev)
> chip->spi_dev = spi_dev;
> for (i = 0; i <= AD7816_CS_MAX; i++)
> chip->oti_data[i] = 203;
> - chip->rdwr_pin = pins[0];
> - chip->convert_pin = pins[1];
> - chip->busy_pin = pins[2];
> -
> - ret = devm_gpio_request(&spi_dev->dev, chip->rdwr_pin,
> - spi_get_device_id(spi_dev)->name);
> - if (ret) {
> - dev_err(&spi_dev->dev, "Fail to request rdwr gpio PIN %d.\n",
> - chip->rdwr_pin);
> +
> + chip->rdwr_pin = devm_gpiod_get(&spi_dev->dev, "rdwr", GPIOD_IN);
> + if (IS_ERR(chip->rdwr_pin)) {
> + ret = PTR_ERR(chip->rdwr_pin);
> + dev_err(&spi_dev->dev, "Failed to request rdwr GPIO: %d\n",
> + ret);
> return ret;
> }
> - gpio_direction_input(chip->rdwr_pin);
> - ret = devm_gpio_request(&spi_dev->dev, chip->convert_pin,
> - spi_get_device_id(spi_dev)->name);
> - if (ret) {
> - dev_err(&spi_dev->dev, "Fail to request convert gpio PIN %d.\n",
> - chip->convert_pin);
> + chip->convert_pin = devm_gpiod_get(&spi_dev->dev, "convert", GPIOD_IN);
> + if (IS_ERR(chip->convert_pin)) {
> + ret = PTR_ERR(chip->convert_pin);
> + dev_err(&spi_dev->dev, "Failed to request convert GPIO: %d\n",
> + ret);
> return ret;
> }
> - gpio_direction_input(chip->convert_pin);
> - ret = devm_gpio_request(&spi_dev->dev, chip->busy_pin,
> - spi_get_device_id(spi_dev)->name);
> - if (ret) {
> - dev_err(&spi_dev->dev, "Fail to request busy gpio PIN %d.\n",
> - chip->busy_pin);
> + chip->busy_pin = devm_gpiod_get(&spi_dev->dev, "busy", GPIOD_IN);
> + if (IS_ERR(chip->busy_pin)) {
> + ret = PTR_ERR(chip->busy_pin);
> + dev_err(&spi_dev->dev, "Failed to request busy GPIO: %d\n",
> + ret);
> return ret;
> }
> - gpio_direction_input(chip->busy_pin);
>
> indio_dev->name = spi_get_device_id(spi_dev)->name;
> indio_dev->dev.parent = &spi_dev->dev;


2018-11-11 12:32:36

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] staging: iio: ad7816: Switch to the gpio descriptor interface

On Sun, 11 Nov 2018 12:24:05 +0000
Jonathan Cameron <[email protected]> wrote:

> On Fri, 9 Nov 2018 13:05:17 +0530
> Nishad Kamdar <[email protected]> wrote:
>
> > Use the gpiod interface for rdwr_pin, convert_pin and busy_pin
> > instead of the deprecated old non-descriptor interface.
> >
> > Signed-off-by: Nishad Kamdar <[email protected]>
> Applied to the togreg branch of iio.git and pushed out as testing
> for the autobuilders to play with it.
Actually no it isn't, because I applied the previous version back in
October. Please avoid resending patches that have already been applied.
It gets very confusing!

Jonathan


>
> Thanks,
>
> Jonathan
>
> > ---
> > drivers/staging/iio/adc/ad7816.c | 80 ++++++++++++++------------------
> > 1 file changed, 34 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
> > index bf76a8620bdb..12c4e0ab4713 100644
> > --- a/drivers/staging/iio/adc/ad7816.c
> > +++ b/drivers/staging/iio/adc/ad7816.c
> > @@ -7,7 +7,7 @@
> > */
> >
> > #include <linux/interrupt.h>
> > -#include <linux/gpio.h>
> > +#include <linux/gpio/consumer.h>
> > #include <linux/device.h>
> > #include <linux/kernel.h>
> > #include <linux/slab.h>
> > @@ -44,9 +44,9 @@
> >
> > struct ad7816_chip_info {
> > struct spi_device *spi_dev;
> > - u16 rdwr_pin;
> > - u16 convert_pin;
> > - u16 busy_pin;
> > + struct gpio_desc *rdwr_pin;
> > + struct gpio_desc *convert_pin;
> > + struct gpio_desc *busy_pin;
> > u8 oti_data[AD7816_CS_MAX + 1];
> > u8 channel_id; /* 0 always be temperature */
> > u8 mode;
> > @@ -61,28 +61,28 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
> > int ret = 0;
> > __be16 buf;
> >
> > - gpio_set_value(chip->rdwr_pin, 1);
> > - gpio_set_value(chip->rdwr_pin, 0);
> > + gpiod_set_value(chip->rdwr_pin, 1);
> > + gpiod_set_value(chip->rdwr_pin, 0);
> > ret = spi_write(spi_dev, &chip->channel_id, sizeof(chip->channel_id));
> > if (ret < 0) {
> > dev_err(&spi_dev->dev, "SPI channel setting error\n");
> > return ret;
> > }
> > - gpio_set_value(chip->rdwr_pin, 1);
> > + gpiod_set_value(chip->rdwr_pin, 1);
> >
> > if (chip->mode == AD7816_PD) { /* operating mode 2 */
> > - gpio_set_value(chip->convert_pin, 1);
> > - gpio_set_value(chip->convert_pin, 0);
> > + gpiod_set_value(chip->convert_pin, 1);
> > + gpiod_set_value(chip->convert_pin, 0);
> > } else { /* operating mode 1 */
> > - gpio_set_value(chip->convert_pin, 0);
> > - gpio_set_value(chip->convert_pin, 1);
> > + gpiod_set_value(chip->convert_pin, 0);
> > + gpiod_set_value(chip->convert_pin, 1);
> > }
> >
> > - while (gpio_get_value(chip->busy_pin))
> > + while (gpiod_get_value(chip->busy_pin))
> > cpu_relax();
> >
> > - gpio_set_value(chip->rdwr_pin, 0);
> > - gpio_set_value(chip->rdwr_pin, 1);
> > + gpiod_set_value(chip->rdwr_pin, 0);
> > + gpiod_set_value(chip->rdwr_pin, 1);
> > ret = spi_read(spi_dev, &buf, sizeof(*data));
> > if (ret < 0) {
> > dev_err(&spi_dev->dev, "SPI data read error\n");
> > @@ -99,8 +99,8 @@ static int ad7816_spi_write(struct ad7816_chip_info *chip, u8 data)
> > struct spi_device *spi_dev = chip->spi_dev;
> > int ret = 0;
> >
> > - gpio_set_value(chip->rdwr_pin, 1);
> > - gpio_set_value(chip->rdwr_pin, 0);
> > + gpiod_set_value(chip->rdwr_pin, 1);
> > + gpiod_set_value(chip->rdwr_pin, 0);
> > ret = spi_write(spi_dev, &data, sizeof(data));
> > if (ret < 0)
> > dev_err(&spi_dev->dev, "SPI oti data write error\n");
> > @@ -129,10 +129,10 @@ static ssize_t ad7816_store_mode(struct device *dev,
> > struct ad7816_chip_info *chip = iio_priv(indio_dev);
> >
> > if (strcmp(buf, "full")) {
> > - gpio_set_value(chip->rdwr_pin, 1);
> > + gpiod_set_value(chip->rdwr_pin, 1);
> > chip->mode = AD7816_FULL;
> > } else {
> > - gpio_set_value(chip->rdwr_pin, 0);
> > + gpiod_set_value(chip->rdwr_pin, 0);
> > chip->mode = AD7816_PD;
> > }
> >
> > @@ -345,15 +345,9 @@ static int ad7816_probe(struct spi_device *spi_dev)
> > {
> > struct ad7816_chip_info *chip;
> > struct iio_dev *indio_dev;
> > - unsigned short *pins = dev_get_platdata(&spi_dev->dev);
> > int ret = 0;
> > int i;
> >
> > - if (!pins) {
> > - dev_err(&spi_dev->dev, "No necessary GPIO platform data.\n");
> > - return -EINVAL;
> > - }
> > -
> > indio_dev = devm_iio_device_alloc(&spi_dev->dev, sizeof(*chip));
> > if (!indio_dev)
> > return -ENOMEM;
> > @@ -364,34 +358,28 @@ static int ad7816_probe(struct spi_device *spi_dev)
> > chip->spi_dev = spi_dev;
> > for (i = 0; i <= AD7816_CS_MAX; i++)
> > chip->oti_data[i] = 203;
> > - chip->rdwr_pin = pins[0];
> > - chip->convert_pin = pins[1];
> > - chip->busy_pin = pins[2];
> > -
> > - ret = devm_gpio_request(&spi_dev->dev, chip->rdwr_pin,
> > - spi_get_device_id(spi_dev)->name);
> > - if (ret) {
> > - dev_err(&spi_dev->dev, "Fail to request rdwr gpio PIN %d.\n",
> > - chip->rdwr_pin);
> > +
> > + chip->rdwr_pin = devm_gpiod_get(&spi_dev->dev, "rdwr", GPIOD_IN);
> > + if (IS_ERR(chip->rdwr_pin)) {
> > + ret = PTR_ERR(chip->rdwr_pin);
> > + dev_err(&spi_dev->dev, "Failed to request rdwr GPIO: %d\n",
> > + ret);
> > return ret;
> > }
> > - gpio_direction_input(chip->rdwr_pin);
> > - ret = devm_gpio_request(&spi_dev->dev, chip->convert_pin,
> > - spi_get_device_id(spi_dev)->name);
> > - if (ret) {
> > - dev_err(&spi_dev->dev, "Fail to request convert gpio PIN %d.\n",
> > - chip->convert_pin);
> > + chip->convert_pin = devm_gpiod_get(&spi_dev->dev, "convert", GPIOD_IN);
> > + if (IS_ERR(chip->convert_pin)) {
> > + ret = PTR_ERR(chip->convert_pin);
> > + dev_err(&spi_dev->dev, "Failed to request convert GPIO: %d\n",
> > + ret);
> > return ret;
> > }
> > - gpio_direction_input(chip->convert_pin);
> > - ret = devm_gpio_request(&spi_dev->dev, chip->busy_pin,
> > - spi_get_device_id(spi_dev)->name);
> > - if (ret) {
> > - dev_err(&spi_dev->dev, "Fail to request busy gpio PIN %d.\n",
> > - chip->busy_pin);
> > + chip->busy_pin = devm_gpiod_get(&spi_dev->dev, "busy", GPIOD_IN);
> > + if (IS_ERR(chip->busy_pin)) {
> > + ret = PTR_ERR(chip->busy_pin);
> > + dev_err(&spi_dev->dev, "Failed to request busy GPIO: %d\n",
> > + ret);
> > return ret;
> > }
> > - gpio_direction_input(chip->busy_pin);
> >
> > indio_dev->name = spi_get_device_id(spi_dev)->name;
> > indio_dev->dev.parent = &spi_dev->dev;
>


2018-11-11 12:35:49

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 2/4] staging: iio: ad7816: Do not use busy_pin in case of AD7818

On Fri, 9 Nov 2018 13:06:24 +0530
Nishad Kamdar <[email protected]> wrote:

> AD7818 does not support busy_pin functionality as per datasheet.
> Hence drop busy_pin when AD7818 is used.
>
> Signed-off-by: Nishad Kamdar <[email protected]>
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

A slight concern that I can't immediately find an answer to,
is whether we should be waiting a short while for the ad7818 given
we can't wait on the gpio? We can add this in a follow up patch
if it is necessary.

Thanks,

Jonathan
> ---
> drivers/staging/iio/adc/ad7816.c | 35 ++++++++++++++++++++++----------
> 1 file changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
> index 12c4e0ab4713..3cda5cd09365 100644
> --- a/drivers/staging/iio/adc/ad7816.c
> +++ b/drivers/staging/iio/adc/ad7816.c
> @@ -43,6 +43,7 @@
> */
>
> struct ad7816_chip_info {
> + kernel_ulong_t id;
> struct spi_device *spi_dev;
> struct gpio_desc *rdwr_pin;
> struct gpio_desc *convert_pin;
> @@ -52,6 +53,12 @@ struct ad7816_chip_info {
> u8 mode;
> };
>
> +enum ad7816_type {
> + ID_AD7816,
> + ID_AD7817,
> + ID_AD7818,
> +};
> +
> /*
> * ad7816 data access by SPI
> */
> @@ -78,8 +85,10 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
> gpiod_set_value(chip->convert_pin, 1);
> }
>
> - while (gpiod_get_value(chip->busy_pin))
> - cpu_relax();
> + if (chip->id == ID_AD7816 || chip->id == ID_AD7817) {
> + while (gpiod_get_value(chip->busy_pin))
> + cpu_relax();
> + }
>
> gpiod_set_value(chip->rdwr_pin, 0);
> gpiod_set_value(chip->rdwr_pin, 1);
> @@ -359,6 +368,7 @@ static int ad7816_probe(struct spi_device *spi_dev)
> for (i = 0; i <= AD7816_CS_MAX; i++)
> chip->oti_data[i] = 203;
>
> + chip->id = spi_get_device_id(spi_dev)->driver_data;
> chip->rdwr_pin = devm_gpiod_get(&spi_dev->dev, "rdwr", GPIOD_IN);
> if (IS_ERR(chip->rdwr_pin)) {
> ret = PTR_ERR(chip->rdwr_pin);
> @@ -373,12 +383,15 @@ static int ad7816_probe(struct spi_device *spi_dev)
> ret);
> return ret;
> }
> - chip->busy_pin = devm_gpiod_get(&spi_dev->dev, "busy", GPIOD_IN);
> - if (IS_ERR(chip->busy_pin)) {
> - ret = PTR_ERR(chip->busy_pin);
> - dev_err(&spi_dev->dev, "Failed to request busy GPIO: %d\n",
> - ret);
> - return ret;
> + if (chip->id == ID_AD7816 || chip->id == ID_AD7817) {
> + chip->busy_pin = devm_gpiod_get(&spi_dev->dev, "busy",
> + GPIOD_IN);
> + if (IS_ERR(chip->busy_pin)) {
> + ret = PTR_ERR(chip->busy_pin);
> + dev_err(&spi_dev->dev, "Failed to request busy GPIO: %d\n",
> + ret);
> + return ret;
> + }
> }
>
> indio_dev->name = spi_get_device_id(spi_dev)->name;
> @@ -409,9 +422,9 @@ static int ad7816_probe(struct spi_device *spi_dev)
> }
>
> static const struct spi_device_id ad7816_id[] = {
> - { "ad7816", 0 },
> - { "ad7817", 0 },
> - { "ad7818", 0 },
> + { "ad7816", ID_AD7816 },
> + { "ad7817", ID_AD7817 },
> + { "ad7818", ID_AD7818 },
> {}
> };
>


2018-11-11 12:37:21

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 3/4] staging: iio: ad7816: Set RD/WR pin and CONVST pin as outputs.

On Fri, 9 Nov 2018 13:07:18 +0530
Nishad Kamdar <[email protected]> wrote:

> The RD/WR pin and CONVST pin are logical inputs to the AD78xx
> chip as per the datasheet. Hence convert them to outputs.
>
> Signed-off-by: Nishad Kamdar <[email protected]>
Hi Nishad,

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

This is the sort of patch we might want to think about backporting
for stable, but it is sufficiently complex to do that I' think we'll
wait for someone to actually request it.

Thanks,

Jonathan

> ---
> drivers/staging/iio/adc/ad7816.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
> index 3cda5cd09365..a2fead85cd46 100644
> --- a/drivers/staging/iio/adc/ad7816.c
> +++ b/drivers/staging/iio/adc/ad7816.c
> @@ -369,14 +369,15 @@ static int ad7816_probe(struct spi_device *spi_dev)
> chip->oti_data[i] = 203;
>
> chip->id = spi_get_device_id(spi_dev)->driver_data;
> - chip->rdwr_pin = devm_gpiod_get(&spi_dev->dev, "rdwr", GPIOD_IN);
> + chip->rdwr_pin = devm_gpiod_get(&spi_dev->dev, "rdwr", GPIOD_OUT_HIGH);
> if (IS_ERR(chip->rdwr_pin)) {
> ret = PTR_ERR(chip->rdwr_pin);
> dev_err(&spi_dev->dev, "Failed to request rdwr GPIO: %d\n",
> ret);
> return ret;
> }
> - chip->convert_pin = devm_gpiod_get(&spi_dev->dev, "convert", GPIOD_IN);
> + chip->convert_pin = devm_gpiod_get(&spi_dev->dev, "convert",
> + GPIOD_OUT_HIGH);
> if (IS_ERR(chip->convert_pin)) {
> ret = PTR_ERR(chip->convert_pin);
> dev_err(&spi_dev->dev, "Failed to request convert GPIO: %d\n",


2018-11-11 12:38:44

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] staging: iio: ad7816: Add device tree table.

On Fri, 9 Nov 2018 08:11:57 +0000
"Ardelean, Alexandru" <[email protected]> wrote:

> On Fri, 2018-11-09 at 13:08 +0530, Nishad Kamdar wrote:
> > Add device tree table for matching vendor ID.
>
> One comment inline for this.
>
> Thanks
> Alex
>
> >
> > Signed-off-by: Nishad Kamdar <[email protected]>
> > ---
> > drivers/staging/iio/adc/ad7816.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/staging/iio/adc/ad7816.c
> > b/drivers/staging/iio/adc/ad7816.c
> > index a2fead85cd46..b8a9149fbac1 100644
> > --- a/drivers/staging/iio/adc/ad7816.c
> > +++ b/drivers/staging/iio/adc/ad7816.c
> > @@ -422,6 +422,12 @@ static int ad7816_probe(struct spi_device *spi_dev)
> > return 0;
> > }
> >
> > +static const struct of_device_id ad7816_of_match[] = {
> > + { .compatible = "adi,ad7816", },
>
> I think you also need to add:
> + { .compatible = "adi,ad7817", },
> + { .compatible = "adi,ad7818", },
>

Yes, please add all parts to the dt binding.
Particularly as they are functionally different in this case!

Please send a v4 for just this patch as the rest have now been
applied.

Thanks,


Jonathan

> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, ad7816_of_match);
> > +
> > static const struct spi_device_id ad7816_id[] = {
> > { "ad7816", ID_AD7816 },
> > { "ad7817", ID_AD7817 },
> > @@ -434,6 +440,7 @@ MODULE_DEVICE_TABLE(spi, ad7816_id);
> > static struct spi_driver ad7816_driver = {
> > .driver = {
> > .name = "ad7816",
> > + .of_match_table = of_match_ptr(ad7816_of_match),
> > },
> > .probe = ad7816_probe,
> > .id_table = ad7816_id,


2018-11-14 16:32:48

by Nishad Kamdar

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] staging: iio: ad7816: Add device tree table.

On Fri, Nov 09, 2018 at 08:11:57AM +0000, Ardelean, Alexandru wrote:
> On Fri, 2018-11-09 at 13:08 +0530, Nishad Kamdar wrote:
> > Add device tree table for matching vendor ID.
>
> One comment inline for this.
>
> Thanks
> Alex
>
> >
> > Signed-off-by: Nishad Kamdar <[email protected]>
> > ---
> > drivers/staging/iio/adc/ad7816.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/staging/iio/adc/ad7816.c
> > b/drivers/staging/iio/adc/ad7816.c
> > index a2fead85cd46..b8a9149fbac1 100644
> > --- a/drivers/staging/iio/adc/ad7816.c
> > +++ b/drivers/staging/iio/adc/ad7816.c
> > @@ -422,6 +422,12 @@ static int ad7816_probe(struct spi_device *spi_dev)
> > return 0;
> > }
> >
> > +static const struct of_device_id ad7816_of_match[] = {
> > + { .compatible = "adi,ad7816", },
>
> I think you also need to add:
> + { .compatible = "adi,ad7817", },
> + { .compatible = "adi,ad7818", },
>
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, ad7816_of_match);
> > +
> > static const struct spi_device_id ad7816_id[] = {
> > { "ad7816", ID_AD7816 },
> > { "ad7817", ID_AD7817 },
> > @@ -434,6 +440,7 @@ MODULE_DEVICE_TABLE(spi, ad7816_id);
> > static struct spi_driver ad7816_driver = {
> > .driver = {
> > .name = "ad7816",
> > + .of_match_table = of_match_ptr(ad7816_of_match),
> > },
> > .probe = ad7816_probe,
> > .id_table = ad7816_id,

Ok. I'll do that.

Thanks for the review.

regards,
Nishad

2018-11-14 16:56:41

by Nishad Kamdar

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] staging: iio: ad7816: Add device tree table.

On Sun, Nov 11, 2018 at 12:38:02PM +0000, Jonathan Cameron wrote:
> On Fri, 9 Nov 2018 08:11:57 +0000
> "Ardelean, Alexandru" <[email protected]> wrote:
>
> > On Fri, 2018-11-09 at 13:08 +0530, Nishad Kamdar wrote:
> > > Add device tree table for matching vendor ID.
> >
> > One comment inline for this.
> >
> > Thanks
> > Alex
> >
> > >
> > > Signed-off-by: Nishad Kamdar <[email protected]>
> > > ---
> > > drivers/staging/iio/adc/ad7816.c | 7 +++++++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/drivers/staging/iio/adc/ad7816.c
> > > b/drivers/staging/iio/adc/ad7816.c
> > > index a2fead85cd46..b8a9149fbac1 100644
> > > --- a/drivers/staging/iio/adc/ad7816.c
> > > +++ b/drivers/staging/iio/adc/ad7816.c
> > > @@ -422,6 +422,12 @@ static int ad7816_probe(struct spi_device *spi_dev)
> > > return 0;
> > > }
> > >
> > > +static const struct of_device_id ad7816_of_match[] = {
> > > + { .compatible = "adi,ad7816", },
> >
> > I think you also need to add:
> > + { .compatible = "adi,ad7817", },
> > + { .compatible = "adi,ad7818", },
> >
>
> Yes, please add all parts to the dt binding.
> Particularly as they are functionally different in this case!
>
> Please send a v4 for just this patch as the rest have now been
> applied.
>
> Thanks,
>
>
> Jonathan
>
> > > + { }
> > > +};
> > > +MODULE_DEVICE_TABLE(of, ad7816_of_match);
> > > +
> > > static const struct spi_device_id ad7816_id[] = {
> > > { "ad7816", ID_AD7816 },
> > > { "ad7817", ID_AD7817 },
> > > @@ -434,6 +440,7 @@ MODULE_DEVICE_TABLE(spi, ad7816_id);
> > > static struct spi_driver ad7816_driver = {
> > > .driver = {
> > > .name = "ad7816",
> > > + .of_match_table = of_match_ptr(ad7816_of_match),
> > > },
> > > .probe = ad7816_probe,
> > > .id_table = ad7816_id,
>

Ok. I'll do that.

Thanks for the reivew.

regards,
Nishad

2018-11-14 17:09:12

by Nishad Kamdar

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] staging: iio: ad7816: Switch to the gpio descriptor interface

On Sun, Nov 11, 2018 at 12:31:54PM +0000, Jonathan Cameron wrote:
> On Sun, 11 Nov 2018 12:24:05 +0000
> Jonathan Cameron <[email protected]> wrote:
>
> > On Fri, 9 Nov 2018 13:05:17 +0530
> > Nishad Kamdar <[email protected]> wrote:
> >
> > > Use the gpiod interface for rdwr_pin, convert_pin and busy_pin
> > > instead of the deprecated old non-descriptor interface.
> > >
> > > Signed-off-by: Nishad Kamdar <[email protected]>
> > Applied to the togreg branch of iio.git and pushed out as testing
> > for the autobuilders to play with it.
> Actually no it isn't, because I applied the previous version back in
> October. Please avoid resending patches that have already been applied.
> It gets very confusing!
>
> Jonathan
>

Sorry about that. I'll keep that in mind.

Thanks for the review.

regards,
Nishad

>
> >
> > Thanks,
> >
> > Jonathan
> >
> > > ---
> > > drivers/staging/iio/adc/ad7816.c | 80 ++++++++++++++------------------
> > > 1 file changed, 34 insertions(+), 46 deletions(-)
> > >
> > > diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
> > > index bf76a8620bdb..12c4e0ab4713 100644
> > > --- a/drivers/staging/iio/adc/ad7816.c
> > > +++ b/drivers/staging/iio/adc/ad7816.c
> > > @@ -7,7 +7,7 @@
> > > */
> > >
> > > #include <linux/interrupt.h>
> > > -#include <linux/gpio.h>
> > > +#include <linux/gpio/consumer.h>
> > > #include <linux/device.h>
> > > #include <linux/kernel.h>
> > > #include <linux/slab.h>
> > > @@ -44,9 +44,9 @@
> > >
> > > struct ad7816_chip_info {
> > > struct spi_device *spi_dev;
> > > - u16 rdwr_pin;
> > > - u16 convert_pin;
> > > - u16 busy_pin;
> > > + struct gpio_desc *rdwr_pin;
> > > + struct gpio_desc *convert_pin;
> > > + struct gpio_desc *busy_pin;
> > > u8 oti_data[AD7816_CS_MAX + 1];
> > > u8 channel_id; /* 0 always be temperature */
> > > u8 mode;
> > > @@ -61,28 +61,28 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
> > > int ret = 0;
> > > __be16 buf;
> > >
> > > - gpio_set_value(chip->rdwr_pin, 1);
> > > - gpio_set_value(chip->rdwr_pin, 0);
> > > + gpiod_set_value(chip->rdwr_pin, 1);
> > > + gpiod_set_value(chip->rdwr_pin, 0);
> > > ret = spi_write(spi_dev, &chip->channel_id, sizeof(chip->channel_id));
> > > if (ret < 0) {
> > > dev_err(&spi_dev->dev, "SPI channel setting error\n");
> > > return ret;
> > > }
> > > - gpio_set_value(chip->rdwr_pin, 1);
> > > + gpiod_set_value(chip->rdwr_pin, 1);
> > >
> > > if (chip->mode == AD7816_PD) { /* operating mode 2 */
> > > - gpio_set_value(chip->convert_pin, 1);
> > > - gpio_set_value(chip->convert_pin, 0);
> > > + gpiod_set_value(chip->convert_pin, 1);
> > > + gpiod_set_value(chip->convert_pin, 0);
> > > } else { /* operating mode 1 */
> > > - gpio_set_value(chip->convert_pin, 0);
> > > - gpio_set_value(chip->convert_pin, 1);
> > > + gpiod_set_value(chip->convert_pin, 0);
> > > + gpiod_set_value(chip->convert_pin, 1);
> > > }
> > >
> > > - while (gpio_get_value(chip->busy_pin))
> > > + while (gpiod_get_value(chip->busy_pin))
> > > cpu_relax();
> > >
> > > - gpio_set_value(chip->rdwr_pin, 0);
> > > - gpio_set_value(chip->rdwr_pin, 1);
> > > + gpiod_set_value(chip->rdwr_pin, 0);
> > > + gpiod_set_value(chip->rdwr_pin, 1);
> > > ret = spi_read(spi_dev, &buf, sizeof(*data));
> > > if (ret < 0) {
> > > dev_err(&spi_dev->dev, "SPI data read error\n");
> > > @@ -99,8 +99,8 @@ static int ad7816_spi_write(struct ad7816_chip_info *chip, u8 data)
> > > struct spi_device *spi_dev = chip->spi_dev;
> > > int ret = 0;
> > >
> > > - gpio_set_value(chip->rdwr_pin, 1);
> > > - gpio_set_value(chip->rdwr_pin, 0);
> > > + gpiod_set_value(chip->rdwr_pin, 1);
> > > + gpiod_set_value(chip->rdwr_pin, 0);
> > > ret = spi_write(spi_dev, &data, sizeof(data));
> > > if (ret < 0)
> > > dev_err(&spi_dev->dev, "SPI oti data write error\n");
> > > @@ -129,10 +129,10 @@ static ssize_t ad7816_store_mode(struct device *dev,
> > > struct ad7816_chip_info *chip = iio_priv(indio_dev);
> > >
> > > if (strcmp(buf, "full")) {
> > > - gpio_set_value(chip->rdwr_pin, 1);
> > > + gpiod_set_value(chip->rdwr_pin, 1);
> > > chip->mode = AD7816_FULL;
> > > } else {
> > > - gpio_set_value(chip->rdwr_pin, 0);
> > > + gpiod_set_value(chip->rdwr_pin, 0);
> > > chip->mode = AD7816_PD;
> > > }
> > >
> > > @@ -345,15 +345,9 @@ static int ad7816_probe(struct spi_device *spi_dev)
> > > {
> > > struct ad7816_chip_info *chip;
> > > struct iio_dev *indio_dev;
> > > - unsigned short *pins = dev_get_platdata(&spi_dev->dev);
> > > int ret = 0;
> > > int i;
> > >
> > > - if (!pins) {
> > > - dev_err(&spi_dev->dev, "No necessary GPIO platform data.\n");
> > > - return -EINVAL;
> > > - }
> > > -
> > > indio_dev = devm_iio_device_alloc(&spi_dev->dev, sizeof(*chip));
> > > if (!indio_dev)
> > > return -ENOMEM;
> > > @@ -364,34 +358,28 @@ static int ad7816_probe(struct spi_device *spi_dev)
> > > chip->spi_dev = spi_dev;
> > > for (i = 0; i <= AD7816_CS_MAX; i++)
> > > chip->oti_data[i] = 203;
> > > - chip->rdwr_pin = pins[0];
> > > - chip->convert_pin = pins[1];
> > > - chip->busy_pin = pins[2];
> > > -
> > > - ret = devm_gpio_request(&spi_dev->dev, chip->rdwr_pin,
> > > - spi_get_device_id(spi_dev)->name);
> > > - if (ret) {
> > > - dev_err(&spi_dev->dev, "Fail to request rdwr gpio PIN %d.\n",
> > > - chip->rdwr_pin);
> > > +
> > > + chip->rdwr_pin = devm_gpiod_get(&spi_dev->dev, "rdwr", GPIOD_IN);
> > > + if (IS_ERR(chip->rdwr_pin)) {
> > > + ret = PTR_ERR(chip->rdwr_pin);
> > > + dev_err(&spi_dev->dev, "Failed to request rdwr GPIO: %d\n",
> > > + ret);
> > > return ret;
> > > }
> > > - gpio_direction_input(chip->rdwr_pin);
> > > - ret = devm_gpio_request(&spi_dev->dev, chip->convert_pin,
> > > - spi_get_device_id(spi_dev)->name);
> > > - if (ret) {
> > > - dev_err(&spi_dev->dev, "Fail to request convert gpio PIN %d.\n",
> > > - chip->convert_pin);
> > > + chip->convert_pin = devm_gpiod_get(&spi_dev->dev, "convert", GPIOD_IN);
> > > + if (IS_ERR(chip->convert_pin)) {
> > > + ret = PTR_ERR(chip->convert_pin);
> > > + dev_err(&spi_dev->dev, "Failed to request convert GPIO: %d\n",
> > > + ret);
> > > return ret;
> > > }
> > > - gpio_direction_input(chip->convert_pin);
> > > - ret = devm_gpio_request(&spi_dev->dev, chip->busy_pin,
> > > - spi_get_device_id(spi_dev)->name);
> > > - if (ret) {
> > > - dev_err(&spi_dev->dev, "Fail to request busy gpio PIN %d.\n",
> > > - chip->busy_pin);
> > > + chip->busy_pin = devm_gpiod_get(&spi_dev->dev, "busy", GPIOD_IN);
> > > + if (IS_ERR(chip->busy_pin)) {
> > > + ret = PTR_ERR(chip->busy_pin);
> > > + dev_err(&spi_dev->dev, "Failed to request busy GPIO: %d\n",
> > > + ret);
> > > return ret;
> > > }
> > > - gpio_direction_input(chip->busy_pin);
> > >
> > > indio_dev->name = spi_get_device_id(spi_dev)->name;
> > > indio_dev->dev.parent = &spi_dev->dev;
> >
>