* [PATCH v3 0/4] staging: iio: ad7816: Switch to the gpio descriptor interface
@ 2018-11-09 7:34 Nishad Kamdar
2018-11-09 7:35 ` [PATCH v3 1/4] " Nishad Kamdar
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: Nishad Kamdar @ 2018-11-09 7:34 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio, devel,
linux-kernel
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 1/4] staging: iio: ad7816: Switch to the gpio descriptor interface
2018-11-09 7:34 [PATCH v3 0/4] staging: iio: ad7816: Switch to the gpio descriptor interface Nishad Kamdar
@ 2018-11-09 7:35 ` Nishad Kamdar
2018-11-09 8:05 ` Ardelean, Alexandru
2018-11-11 12:24 ` Jonathan Cameron
2018-11-09 7:36 ` [PATCH v3 2/4] staging: iio: ad7816: Do not use busy_pin in case of AD7818 Nishad Kamdar
` (2 subsequent siblings)
3 siblings, 2 replies; 19+ messages in thread
From: Nishad Kamdar @ 2018-11-09 7:35 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio, devel,
linux-kernel
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 <nishadkamdar@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 2/4] staging: iio: ad7816: Do not use busy_pin in case of AD7818
2018-11-09 7:34 [PATCH v3 0/4] staging: iio: ad7816: Switch to the gpio descriptor interface Nishad Kamdar
2018-11-09 7:35 ` [PATCH v3 1/4] " Nishad Kamdar
@ 2018-11-09 7:36 ` Nishad Kamdar
2018-11-11 12:35 ` Jonathan Cameron
2018-11-09 7:37 ` [PATCH v3 3/4] staging: iio: ad7816: Set RD/WR pin and CONVST pin as outputs Nishad Kamdar
2018-11-09 7:38 ` [PATCH v3 4/4] staging: iio: ad7816: Add device tree table Nishad Kamdar
3 siblings, 1 reply; 19+ messages in thread
From: Nishad Kamdar @ 2018-11-09 7:36 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio, devel,
linux-kernel
AD7818 does not support busy_pin functionality as per datasheet.
Hence drop busy_pin when AD7818 is used.
Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 3/4] staging: iio: ad7816: Set RD/WR pin and CONVST pin as outputs.
2018-11-09 7:34 [PATCH v3 0/4] staging: iio: ad7816: Switch to the gpio descriptor interface Nishad Kamdar
2018-11-09 7:35 ` [PATCH v3 1/4] " Nishad Kamdar
2018-11-09 7:36 ` [PATCH v3 2/4] staging: iio: ad7816: Do not use busy_pin in case of AD7818 Nishad Kamdar
@ 2018-11-09 7:37 ` Nishad Kamdar
2018-11-11 12:36 ` Jonathan Cameron
2018-11-09 7:38 ` [PATCH v3 4/4] staging: iio: ad7816: Add device tree table Nishad Kamdar
3 siblings, 1 reply; 19+ messages in thread
From: Nishad Kamdar @ 2018-11-09 7:37 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio, devel,
linux-kernel
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 <nishadkamdar@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 4/4] staging: iio: ad7816: Add device tree table.
2018-11-09 7:34 [PATCH v3 0/4] staging: iio: ad7816: Switch to the gpio descriptor interface Nishad Kamdar
` (2 preceding siblings ...)
2018-11-09 7:37 ` [PATCH v3 3/4] staging: iio: ad7816: Set RD/WR pin and CONVST pin as outputs Nishad Kamdar
@ 2018-11-09 7:38 ` Nishad Kamdar
2018-11-09 8:11 ` Ardelean, Alexandru
3 siblings, 1 reply; 19+ messages in thread
From: Nishad Kamdar @ 2018-11-09 7:38 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio, devel,
linux-kernel
Add device tree table for matching vendor ID.
Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/4] staging: iio: ad7816: Switch to the gpio descriptor interface
2018-11-09 7:35 ` [PATCH v3 1/4] " Nishad Kamdar
@ 2018-11-09 8:05 ` Ardelean, Alexandru
2018-11-11 12:24 ` Jonathan Cameron
1 sibling, 0 replies; 19+ messages in thread
From: Ardelean, Alexandru @ 2018-11-09 8:05 UTC (permalink / raw)
To: lars, nishadkamdar
Cc: linux-kernel, knaack.h, jic23, Hennerich, Michael, devel,
linux-iio, pmeerw, gregkh
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 <nishadkamdar@gmail.com>
> ---
> 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;
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/4] staging: iio: ad7816: Switch to the gpio descriptor interface
@ 2018-11-09 8:05 ` Ardelean, Alexandru
0 siblings, 0 replies; 19+ messages in thread
From: Ardelean, Alexandru @ 2018-11-09 8:05 UTC (permalink / raw)
To: lars, nishadkamdar
Cc: linux-kernel, knaack.h, jic23, Hennerich, Michael, devel,
linux-iio, pmeerw, gregkh
T24gRnJpLCAyMDE4LTExLTA5IGF0IDEzOjA1ICswNTMwLCBOaXNoYWQgS2FtZGFyIHdyb3RlOg0K
PiBVc2UgdGhlIGdwaW9kIGludGVyZmFjZSBmb3IgcmR3cl9waW4sIGNvbnZlcnRfcGluIGFuZCBi
dXN5X3Bpbg0KPiBpbnN0ZWFkIG9mIHRoZSBkZXByZWNhdGVkIG9sZCBub24tZGVzY3JpcHRvciBp
bnRlcmZhY2UuDQo+IA0KDQpQYXRjaCBsb29rcyBnb29kLg0KSSBkbyBoYXZlIHNvbWUgdGhvdWdo
dHMgYWJvdXQgaXQuIFNlZSBpbmxpbmUuDQoNCg0KPiBTaWduZWQtb2ZmLWJ5OiBOaXNoYWQgS2Ft
ZGFyIDxuaXNoYWRrYW1kYXJAZ21haWwuY29tPg0KPiAtLS0NCj4gIGRyaXZlcnMvc3RhZ2luZy9p
aW8vYWRjL2FkNzgxNi5jIHwgODAgKysrKysrKysrKysrKystLS0tLS0tLS0tLS0tLS0tLS0NCj4g
IDEgZmlsZSBjaGFuZ2VkLCAzNCBpbnNlcnRpb25zKCspLCA0NiBkZWxldGlvbnMoLSkNCj4gDQo+
IGRpZmYgLS1naXQgYS9kcml2ZXJzL3N0YWdpbmcvaWlvL2FkYy9hZDc4MTYuYw0KPiBiL2RyaXZl
cnMvc3RhZ2luZy9paW8vYWRjL2FkNzgxNi5jDQo+IGluZGV4IGJmNzZhODYyMGJkYi4uMTJjNGUw
YWI0NzEzIDEwMDY0NA0KPiAtLS0gYS9kcml2ZXJzL3N0YWdpbmcvaWlvL2FkYy9hZDc4MTYuYw0K
PiArKysgYi9kcml2ZXJzL3N0YWdpbmcvaWlvL2FkYy9hZDc4MTYuYw0KPiBAQCAtNyw3ICs3LDcg
QEANCj4gICAqLw0KPiAgDQo+ICAjaW5jbHVkZSA8bGludXgvaW50ZXJydXB0Lmg+DQo+IC0jaW5j
bHVkZSA8bGludXgvZ3Bpby5oPg0KPiArI2luY2x1ZGUgPGxpbnV4L2dwaW8vY29uc3VtZXIuaD4N
Cj4gICNpbmNsdWRlIDxsaW51eC9kZXZpY2UuaD4NCj4gICNpbmNsdWRlIDxsaW51eC9rZXJuZWwu
aD4NCj4gICNpbmNsdWRlIDxsaW51eC9zbGFiLmg+DQo+IEBAIC00NCw5ICs0NCw5IEBADQo+ICAN
Cj4gIHN0cnVjdCBhZDc4MTZfY2hpcF9pbmZvIHsNCj4gIAlzdHJ1Y3Qgc3BpX2RldmljZSAqc3Bp
X2RldjsNCj4gLQl1MTYgcmR3cl9waW47DQo+IC0JdTE2IGNvbnZlcnRfcGluOw0KPiAtCXUxNiBi
dXN5X3BpbjsNCj4gKwlzdHJ1Y3QgZ3Bpb19kZXNjICpyZHdyX3BpbjsNCj4gKwlzdHJ1Y3QgZ3Bp
b19kZXNjICpjb252ZXJ0X3BpbjsNCj4gKwlzdHJ1Y3QgZ3Bpb19kZXNjICpidXN5X3BpbjsNCg0K
T25seSBpZiB5b3Ugd2FudCB0byBkbyBhIHJlLXNwaW4sIGhlcmUncyBhbiBpZGVhLg0KSWYgeW91
IGRvbid0IHdhbnQgdG8sIGZlZWwgZnJlZSB0byBkaXNyZWdhcmQsIHNpbmNlIHlvdXIgcGF0Y2gg
aXMgZ29vZC4NCg0KWW91IGNvdWxkIGNvbXBhY3QgdGhpbmdzIGEgYml0OyBJIGtub3cgdGhpcyB3
YXNuJ3Qgd2hhdCB0aGUgaW5pdGlhbCBjb2RlDQp3YXMgZG9pbmcsIGJ1dCBpdCdzIGFuIG9wdGlv
bi4NCg0KU29tZXRoaW5nIGxpa2U6DQoNCmVudW0gew0KCUdQSU9fUldEUiwNCglHUElPX0NPTlZF
UlQsDQoJR1BJT19CVVNZLA0KCV9fR1BJT19NQVgsDQp9DQoNClRoZW4sIHdoYXQgeW91IGVuZCB1
cCBoYXZpbmcgaXM6DQoNCiBzdHJ1Y3QgYWQ3ODE2X2NoaXBfaW5mbyB7DQogIAlzdHJ1Y3Qgc3Bp
X2RldmljZSAqc3BpX2RldjsNCiAtCXUxNiByZHdyX3BpbjsNCiAtCXUxNiBjb252ZXJ0X3BpbjsN
CiAtCXUxNiBidXN5X3BpbjsNCiArCXN0cnVjdCBncGlvX2Rlc2MgKmdwaW9zW19fR1BJT19NQVhd
Ow0KIA0KDQovLyBDb250aW51ZWQgYmVsb3cNCg0KDQo+ICAJdTggIG90aV9kYXRhW0FENzgxNl9D
U19NQVggKyAxXTsNCj4gIAl1OCAgY2hhbm5lbF9pZDsJLyogMCBhbHdheXMgYmUgdGVtcGVyYXR1
cmUgKi8NCj4gIAl1OCAgbW9kZTsNCj4gQEAgLTYxLDI4ICs2MSwyOCBAQCBzdGF0aWMgaW50IGFk
NzgxNl9zcGlfcmVhZChzdHJ1Y3QgYWQ3ODE2X2NoaXBfaW5mbw0KPiAqY2hpcCwgdTE2ICpkYXRh
KQ0KPiAgCWludCByZXQgPSAwOw0KPiAgCV9fYmUxNiBidWY7DQo+ICANCj4gLQlncGlvX3NldF92
YWx1ZShjaGlwLT5yZHdyX3BpbiwgMSk7DQo+IC0JZ3Bpb19zZXRfdmFsdWUoY2hpcC0+cmR3cl9w
aW4sIDApOw0KPiArCWdwaW9kX3NldF92YWx1ZShjaGlwLT5yZHdyX3BpbiwgMSk7DQo+ICsJZ3Bp
b2Rfc2V0X3ZhbHVlKGNoaXAtPnJkd3JfcGluLCAwKTsNCg0KT2J2aW91c2x5LCBpbiB0aGUgYWJv
dmUgY29udGV4dCwgdGhpcyBiZWNvbWVzOg0KICsJZ3Bpb2Rfc2V0X3ZhbHVlKGNoaXAtPmdwaW9z
W0dQSU9fUldEUl0sIDEpOw0KICsJZ3Bpb2Rfc2V0X3ZhbHVlKGNoaXAtPmdwaW9zW0dQSU9fUldE
Ul0sIDApOw0KDQo+ICAJcmV0ID0gc3BpX3dyaXRlKHNwaV9kZXYsICZjaGlwLT5jaGFubmVsX2lk
LCBzaXplb2YoY2hpcC0NCj4gPmNoYW5uZWxfaWQpKTsNCj4gIAlpZiAocmV0IDwgMCkgew0KPiAg
CQlkZXZfZXJyKCZzcGlfZGV2LT5kZXYsICJTUEkgY2hhbm5lbCBzZXR0aW5nIGVycm9yXG4iKTsN
Cj4gIAkJcmV0dXJuIHJldDsNCj4gIAl9DQo+IC0JZ3Bpb19zZXRfdmFsdWUoY2hpcC0+cmR3cl9w
aW4sIDEpOw0KPiArCWdwaW9kX3NldF92YWx1ZShjaGlwLT5yZHdyX3BpbiwgMSk7DQo+ICANCj4g
IAlpZiAoY2hpcC0+bW9kZSA9PSBBRDc4MTZfUEQpIHsgLyogb3BlcmF0aW5nIG1vZGUgMiAqLw0K
PiAtCQlncGlvX3NldF92YWx1ZShjaGlwLT5jb252ZXJ0X3BpbiwgMSk7DQo+IC0JCWdwaW9fc2V0
X3ZhbHVlKGNoaXAtPmNvbnZlcnRfcGluLCAwKTsNCj4gKwkJZ3Bpb2Rfc2V0X3ZhbHVlKGNoaXAt
PmNvbnZlcnRfcGluLCAxKTsNCj4gKwkJZ3Bpb2Rfc2V0X3ZhbHVlKGNoaXAtPmNvbnZlcnRfcGlu
LCAwKTsNCj4gIAl9IGVsc2UgeyAvKiBvcGVyYXRpbmcgbW9kZSAxICovDQo+IC0JCWdwaW9fc2V0
X3ZhbHVlKGNoaXAtPmNvbnZlcnRfcGluLCAwKTsNCj4gLQkJZ3Bpb19zZXRfdmFsdWUoY2hpcC0+
Y29udmVydF9waW4sIDEpOw0KPiArCQlncGlvZF9zZXRfdmFsdWUoY2hpcC0+Y29udmVydF9waW4s
IDApOw0KPiArCQlncGlvZF9zZXRfdmFsdWUoY2hpcC0+Y29udmVydF9waW4sIDEpOw0KPiAgCX0N
Cj4gIA0KPiAtCXdoaWxlIChncGlvX2dldF92YWx1ZShjaGlwLT5idXN5X3BpbikpDQo+ICsJd2hp
bGUgKGdwaW9kX2dldF92YWx1ZShjaGlwLT5idXN5X3BpbikpDQo+ICAJCWNwdV9yZWxheCgpOw0K
PiAgDQo+IC0JZ3Bpb19zZXRfdmFsdWUoY2hpcC0+cmR3cl9waW4sIDApOw0KPiAtCWdwaW9fc2V0
X3ZhbHVlKGNoaXAtPnJkd3JfcGluLCAxKTsNCj4gKwlncGlvZF9zZXRfdmFsdWUoY2hpcC0+cmR3
cl9waW4sIDApOw0KPiArCWdwaW9kX3NldF92YWx1ZShjaGlwLT5yZHdyX3BpbiwgMSk7DQo+ICAJ
cmV0ID0gc3BpX3JlYWQoc3BpX2RldiwgJmJ1Ziwgc2l6ZW9mKCpkYXRhKSk7DQo+ICAJaWYgKHJl
dCA8IDApIHsNCj4gIAkJZGV2X2Vycigmc3BpX2Rldi0+ZGV2LCAiU1BJIGRhdGEgcmVhZCBlcnJv
clxuIik7DQo+IEBAIC05OSw4ICs5OSw4IEBAIHN0YXRpYyBpbnQgYWQ3ODE2X3NwaV93cml0ZShz
dHJ1Y3QgYWQ3ODE2X2NoaXBfaW5mbw0KPiAqY2hpcCwgdTggZGF0YSkNCj4gIAlzdHJ1Y3Qgc3Bp
X2RldmljZSAqc3BpX2RldiA9IGNoaXAtPnNwaV9kZXY7DQo+ICAJaW50IHJldCA9IDA7DQo+ICAN
Cj4gLQlncGlvX3NldF92YWx1ZShjaGlwLT5yZHdyX3BpbiwgMSk7DQo+IC0JZ3Bpb19zZXRfdmFs
dWUoY2hpcC0+cmR3cl9waW4sIDApOw0KPiArCWdwaW9kX3NldF92YWx1ZShjaGlwLT5yZHdyX3Bp
biwgMSk7DQo+ICsJZ3Bpb2Rfc2V0X3ZhbHVlKGNoaXAtPnJkd3JfcGluLCAwKTsNCj4gIAlyZXQg
PSBzcGlfd3JpdGUoc3BpX2RldiwgJmRhdGEsIHNpemVvZihkYXRhKSk7DQo+ICAJaWYgKHJldCA8
IDApDQo+ICAJCWRldl9lcnIoJnNwaV9kZXYtPmRldiwgIlNQSSBvdGkgZGF0YSB3cml0ZSBlcnJv
clxuIik7DQo+IEBAIC0xMjksMTAgKzEyOSwxMCBAQCBzdGF0aWMgc3NpemVfdCBhZDc4MTZfc3Rv
cmVfbW9kZShzdHJ1Y3QgZGV2aWNlDQo+ICpkZXYsDQo+ICAJc3RydWN0IGFkNzgxNl9jaGlwX2lu
Zm8gKmNoaXAgPSBpaW9fcHJpdihpbmRpb19kZXYpOw0KPiAgDQo+ICAJaWYgKHN0cmNtcChidWYs
ICJmdWxsIikpIHsNCj4gLQkJZ3Bpb19zZXRfdmFsdWUoY2hpcC0+cmR3cl9waW4sIDEpOw0KPiAr
CQlncGlvZF9zZXRfdmFsdWUoY2hpcC0+cmR3cl9waW4sIDEpOw0KPiAgCQljaGlwLT5tb2RlID0g
QUQ3ODE2X0ZVTEw7DQo+ICAJfSBlbHNlIHsNCj4gLQkJZ3Bpb19zZXRfdmFsdWUoY2hpcC0+cmR3
cl9waW4sIDApOw0KPiArCQlncGlvZF9zZXRfdmFsdWUoY2hpcC0+cmR3cl9waW4sIDApOw0KPiAg
CQljaGlwLT5tb2RlID0gQUQ3ODE2X1BEOw0KPiAgCX0NCj4gIA0KPiBAQCAtMzQ1LDE1ICszNDUs
OSBAQCBzdGF0aWMgaW50IGFkNzgxNl9wcm9iZShzdHJ1Y3Qgc3BpX2RldmljZSAqc3BpX2RldikN
Cj4gIHsNCj4gIAlzdHJ1Y3QgYWQ3ODE2X2NoaXBfaW5mbyAqY2hpcDsNCj4gIAlzdHJ1Y3QgaWlv
X2RldiAqaW5kaW9fZGV2Ow0KPiAtCXVuc2lnbmVkIHNob3J0ICpwaW5zID0gZGV2X2dldF9wbGF0
ZGF0YSgmc3BpX2Rldi0+ZGV2KTsNCj4gIAlpbnQgcmV0ID0gMDsNCj4gIAlpbnQgaTsNCj4gIA0K
PiAtCWlmICghcGlucykgew0KPiAtCQlkZXZfZXJyKCZzcGlfZGV2LT5kZXYsICJObyBuZWNlc3Nh
cnkgR1BJTyBwbGF0Zm9ybQ0KPiBkYXRhLlxuIik7DQo+IC0JCXJldHVybiAtRUlOVkFMOw0KPiAt
CX0NCj4gLQ0KPiAgCWluZGlvX2RldiA9IGRldm1faWlvX2RldmljZV9hbGxvYygmc3BpX2Rldi0+
ZGV2LCBzaXplb2YoKmNoaXApKTsNCj4gIAlpZiAoIWluZGlvX2RldikNCj4gIAkJcmV0dXJuIC1F
Tk9NRU07DQo+IEBAIC0zNjQsMzQgKzM1OCwyOCBAQCBzdGF0aWMgaW50IGFkNzgxNl9wcm9iZShz
dHJ1Y3Qgc3BpX2RldmljZSAqc3BpX2RldikNCj4gIAljaGlwLT5zcGlfZGV2ID0gc3BpX2RldjsN
Cj4gIAlmb3IgKGkgPSAwOyBpIDw9IEFENzgxNl9DU19NQVg7IGkrKykNCj4gIAkJY2hpcC0+b3Rp
X2RhdGFbaV0gPSAyMDM7DQo+IC0JY2hpcC0+cmR3cl9waW4gPSBwaW5zWzBdOw0KPiAtCWNoaXAt
PmNvbnZlcnRfcGluID0gcGluc1sxXTsNCj4gLQljaGlwLT5idXN5X3BpbiA9IHBpbnNbMl07DQo+
IC0NCj4gLQlyZXQgPSBkZXZtX2dwaW9fcmVxdWVzdCgmc3BpX2Rldi0+ZGV2LCBjaGlwLT5yZHdy
X3BpbiwNCj4gLQkJCQlzcGlfZ2V0X2RldmljZV9pZChzcGlfZGV2KS0+bmFtZSk7DQo+IC0JaWYg
KHJldCkgew0KPiAtCQlkZXZfZXJyKCZzcGlfZGV2LT5kZXYsICJGYWlsIHRvIHJlcXVlc3QgcmR3
ciBncGlvIFBJTg0KPiAlZC5cbiIsDQo+IC0JCQljaGlwLT5yZHdyX3Bpbik7DQo+ICsNCj4gKwlj
aGlwLT5yZHdyX3BpbiA9IGRldm1fZ3Bpb2RfZ2V0KCZzcGlfZGV2LT5kZXYsICJyZHdyIiwgR1BJ
T0RfSU4pOw0KPiArCWlmIChJU19FUlIoY2hpcC0+cmR3cl9waW4pKSB7DQo+ICsJCXJldCA9IFBU
Ul9FUlIoY2hpcC0+cmR3cl9waW4pOw0KPiArCQlkZXZfZXJyKCZzcGlfZGV2LT5kZXYsICJGYWls
ZWQgdG8gcmVxdWVzdCByZHdyIEdQSU86ICVkXG4iLA0KPiArCQkJcmV0KTsNCj4gIAkJcmV0dXJu
IHJldDsNCj4gIAl9DQoNCkJ1dCB0aGUgcmVhbCBiZW5lZml0IGlmIHRoaXMgYXBwcm9hY2ggY29t
ZXMgaGVyZS4NCllvdSBjb3VsZCBkbzoNCg0Kc3RhdGljIGNvbnN0IGNoYXIgKmdwaW9fbmFtZXNb
X19HUElPX01BWF0gPSB7DQoJW0dQSU9fUldEUl0gPSAicndkciIsDQoJW0dQSU9fQ09OVkVSVF0g
PSAiY29udmVydCIsDQoJW0dQSU9fQlVTWV0gPSAiYnVzeSIsDQp9Ow0KDQovLyBjb2RlIGZvciBw
YXRjaDIJDQpib29sIGhhdmVfYnVzeV9waW4gPSAoY2hpcC0+aWQgPT0gSURfQUQ3ODE2IHx8IGNo
aXAtPmlkID09IElEX0FENzgxNyk7DQovLyBlbmQgY29kZSBmb3IgcGF0Y2gyCQ0KDQoNCmZvciAo
aSA9IDA7IGkgPCBfX0dQSU9fTUFYOyBpKyspIHsNCi8vIGNvZGUgZm9yIHBhdGNoMgkNCglpZiAo
aSA9PSBHUElPX0JVU1kgJiYgIWhhdmVfYnVzeV9waW4pDQoJCWNvbnRpbnVlOw0KLy8gZW5kIGNv
ZGUgZm9yIHBhdGNoMgkNCg0KCWNoaXAtPmdwaW9zW2ldID0gZGV2bV9ncGlvZF9nZXQoJnNwaV9k
ZXYtPmRldiwgZ3Bpb19uYW1lc1tpXSwgDQoJCQkJCUdQSU9EX0lOKTsNCglpZiAoSVNfRVJSKGNo
aXAtPmdwaW9zW2ldKSkgew0KCQlyZXQgPSBQVFJfRVJSKGNoaXAtPmdwaW9zW2ldKTsNCgkJZGV2
X2Vycigmc3BpX2Rldi0+ZGV2LCAiRmFpbGVkIHRvIHJlcXVlc3QgcmR3ciBHUElPOiAlZFxuIiwN
CgkJCXJldDsNCgkJcmV0dXJuIHJldDsNCgl9DQp9DQoNClRoZW4sIGluIHlvdXIgcGF0Y2gyIChp
LmUgIiBzdGFnaW5nOiBpaW86IGFkNzgxNjogRG8gbm90IHVzZSBidXN5X3BpbiBpbg0KY2FzZSBv
ZiBBRDc4MTgiKSwgd2hhdCB5b3UgY291bGQgZG8sIGlzIHRvIGFkZCB0aGF0IGNvZGUgKGFsb25n
IHdpdGggdGhlDQpvdGhlciBJRCBjb2RlKS4NCg0KQW5kIGZpbmFseSB3aGF0IHlvdSBjb3VsZCBk
byBpbiBwYXRjaDIsIGlzOg0KDQoJd2hpbGUgKGNoaXAtPmdwaW9zW0dQSU9fQlVTWV0gJiYNCgkg
ICAgICAgZ3Bpb2RfZ2V0X3ZhbHVlKGNoaXAtPmJ1c3lfcGluKSkNCgkJY3B1X3JlbGF4KCk7DQoN
Cg0KDQo+IC0JZ3Bpb19kaXJlY3Rpb25faW5wdXQoY2hpcC0+cmR3cl9waW4pOw0KPiAtCXJldCA9
IGRldm1fZ3Bpb19yZXF1ZXN0KCZzcGlfZGV2LT5kZXYsIGNoaXAtPmNvbnZlcnRfcGluLA0KPiAt
CQkJCXNwaV9nZXRfZGV2aWNlX2lkKHNwaV9kZXYpLT5uYW1lKTsNCj4gLQlpZiAocmV0KSB7DQo+
IC0JCWRldl9lcnIoJnNwaV9kZXYtPmRldiwgIkZhaWwgdG8gcmVxdWVzdCBjb252ZXJ0IGdwaW8g
UElODQo+ICVkLlxuIiwNCj4gLQkJCWNoaXAtPmNvbnZlcnRfcGluKTsNCj4gKwljaGlwLT5jb252
ZXJ0X3BpbiA9IGRldm1fZ3Bpb2RfZ2V0KCZzcGlfZGV2LT5kZXYsICJjb252ZXJ0IiwNCj4gR1BJ
T0RfSU4pOw0KPiArCWlmIChJU19FUlIoY2hpcC0+Y29udmVydF9waW4pKSB7DQo+ICsJCXJldCA9
IFBUUl9FUlIoY2hpcC0+Y29udmVydF9waW4pOw0KPiArCQlkZXZfZXJyKCZzcGlfZGV2LT5kZXYs
ICJGYWlsZWQgdG8gcmVxdWVzdCBjb252ZXJ0IEdQSU86DQo+ICVkXG4iLA0KPiArCQkJcmV0KTsN
Cj4gIAkJcmV0dXJuIHJldDsNCj4gIAl9DQo+IC0JZ3Bpb19kaXJlY3Rpb25faW5wdXQoY2hpcC0+
Y29udmVydF9waW4pOw0KPiAtCXJldCA9IGRldm1fZ3Bpb19yZXF1ZXN0KCZzcGlfZGV2LT5kZXYs
IGNoaXAtPmJ1c3lfcGluLA0KPiAtCQkJCXNwaV9nZXRfZGV2aWNlX2lkKHNwaV9kZXYpLT5uYW1l
KTsNCj4gLQlpZiAocmV0KSB7DQo+IC0JCWRldl9lcnIoJnNwaV9kZXYtPmRldiwgIkZhaWwgdG8g
cmVxdWVzdCBidXN5IGdwaW8gUElODQo+ICVkLlxuIiwNCj4gLQkJCWNoaXAtPmJ1c3lfcGluKTsN
Cj4gKwljaGlwLT5idXN5X3BpbiA9IGRldm1fZ3Bpb2RfZ2V0KCZzcGlfZGV2LT5kZXYsICJidXN5
IiwgR1BJT0RfSU4pOw0KPiArCWlmIChJU19FUlIoY2hpcC0+YnVzeV9waW4pKSB7DQo+ICsJCXJl
dCA9IFBUUl9FUlIoY2hpcC0+YnVzeV9waW4pOw0KPiArCQlkZXZfZXJyKCZzcGlfZGV2LT5kZXYs
ICJGYWlsZWQgdG8gcmVxdWVzdCBidXN5IEdQSU86ICVkXG4iLA0KPiArCQkJcmV0KTsNCj4gIAkJ
cmV0dXJuIHJldDsNCj4gIAl9DQo+IC0JZ3Bpb19kaXJlY3Rpb25faW5wdXQoY2hpcC0+YnVzeV9w
aW4pOw0KPiAgDQo+ICAJaW5kaW9fZGV2LT5uYW1lID0gc3BpX2dldF9kZXZpY2VfaWQoc3BpX2Rl
diktPm5hbWU7DQo+ICAJaW5kaW9fZGV2LT5kZXYucGFyZW50ID0gJnNwaV9kZXYtPmRldjsNCg==
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/4] staging: iio: ad7816: Switch to the gpio descriptor interface
2018-11-09 8:05 ` Ardelean, Alexandru
@ 2018-11-09 8:08 ` Ardelean, Alexandru
-1 siblings, 0 replies; 19+ messages in thread
From: Ardelean, Alexandru @ 2018-11-09 8:08 UTC (permalink / raw)
To: lars, nishadkamdar
Cc: linux-kernel, knaack.h, jic23, Hennerich, Michael, linux-iio,
devel, pmeerw, gregkh
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 <nishadkamdar@gmail.com>
> > ---
> > 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;
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/4] staging: iio: ad7816: Switch to the gpio descriptor interface
@ 2018-11-09 8:08 ` Ardelean, Alexandru
0 siblings, 0 replies; 19+ messages in thread
From: Ardelean, Alexandru @ 2018-11-09 8:08 UTC (permalink / raw)
To: lars, nishadkamdar
Cc: linux-kernel, knaack.h, jic23, Hennerich, Michael, linux-iio,
devel, pmeerw, gregkh
T24gRnJpLCAyMDE4LTExLTA5IGF0IDA4OjA1ICswMDAwLCBBcmRlbGVhbiwgQWxleGFuZHJ1IHdy
b3RlOg0KPiBPbiBGcmksIDIwMTgtMTEtMDkgYXQgMTM6MDUgKzA1MzAsIE5pc2hhZCBLYW1kYXIg
d3JvdGU6DQo+ID4gVXNlIHRoZSBncGlvZCBpbnRlcmZhY2UgZm9yIHJkd3JfcGluLCBjb252ZXJ0
X3BpbiBhbmQgYnVzeV9waW4NCj4gPiBpbnN0ZWFkIG9mIHRoZSBkZXByZWNhdGVkIG9sZCBub24t
ZGVzY3JpcHRvciBpbnRlcmZhY2UuDQo+ID4gDQo+IA0KPiBQYXRjaCBsb29rcyBnb29kLg0KPiBJ
IGRvIGhhdmUgc29tZSB0aG91Z2h0cyBhYm91dCBpdC4gU2VlIGlubGluZS4NCj4gDQoNCk5ldmVy
bWluZCB3aGF0IEkganVzdCBzYWlkIGhlcmU7IEkganVzdCBzYXcgcGF0Y2gzOiBgaWlvOiBhZDc4
MTY6IFNldCBSRC9XUg0KcGluIGFuZCBDT05WU1QgcGluIGFzIG91dHB1dHMuYA0KDQpUaGlzIGxv
b2tzIGdvb2QgYXMgaXMgaW4gdGhlIGNvbnRleHQgb2YgdGhlIHBhdGNoIHNlcmllcy4NCg0KPiAN
Cj4gPiBTaWduZWQtb2ZmLWJ5OiBOaXNoYWQgS2FtZGFyIDxuaXNoYWRrYW1kYXJAZ21haWwuY29t
Pg0KPiA+IC0tLQ0KPiA+ICBkcml2ZXJzL3N0YWdpbmcvaWlvL2FkYy9hZDc4MTYuYyB8IDgwICsr
KysrKysrKysrKysrLS0tLS0tLS0tLS0tLS0tLS0tDQo+ID4gIDEgZmlsZSBjaGFuZ2VkLCAzNCBp
bnNlcnRpb25zKCspLCA0NiBkZWxldGlvbnMoLSkNCj4gPiANCj4gPiBkaWZmIC0tZ2l0IGEvZHJp
dmVycy9zdGFnaW5nL2lpby9hZGMvYWQ3ODE2LmMNCj4gPiBiL2RyaXZlcnMvc3RhZ2luZy9paW8v
YWRjL2FkNzgxNi5jDQo+ID4gaW5kZXggYmY3NmE4NjIwYmRiLi4xMmM0ZTBhYjQ3MTMgMTAwNjQ0
DQo+ID4gLS0tIGEvZHJpdmVycy9zdGFnaW5nL2lpby9hZGMvYWQ3ODE2LmMNCj4gPiArKysgYi9k
cml2ZXJzL3N0YWdpbmcvaWlvL2FkYy9hZDc4MTYuYw0KPiA+IEBAIC03LDcgKzcsNyBAQA0KPiA+
ICAgKi8NCj4gPiAgDQo+ID4gICNpbmNsdWRlIDxsaW51eC9pbnRlcnJ1cHQuaD4NCj4gPiAtI2lu
Y2x1ZGUgPGxpbnV4L2dwaW8uaD4NCj4gPiArI2luY2x1ZGUgPGxpbnV4L2dwaW8vY29uc3VtZXIu
aD4NCj4gPiAgI2luY2x1ZGUgPGxpbnV4L2RldmljZS5oPg0KPiA+ICAjaW5jbHVkZSA8bGludXgv
a2VybmVsLmg+DQo+ID4gICNpbmNsdWRlIDxsaW51eC9zbGFiLmg+DQo+ID4gQEAgLTQ0LDkgKzQ0
LDkgQEANCj4gPiAgDQo+ID4gIHN0cnVjdCBhZDc4MTZfY2hpcF9pbmZvIHsNCj4gPiAgCXN0cnVj
dCBzcGlfZGV2aWNlICpzcGlfZGV2Ow0KPiA+IC0JdTE2IHJkd3JfcGluOw0KPiA+IC0JdTE2IGNv
bnZlcnRfcGluOw0KPiA+IC0JdTE2IGJ1c3lfcGluOw0KPiA+ICsJc3RydWN0IGdwaW9fZGVzYyAq
cmR3cl9waW47DQo+ID4gKwlzdHJ1Y3QgZ3Bpb19kZXNjICpjb252ZXJ0X3BpbjsNCj4gPiArCXN0
cnVjdCBncGlvX2Rlc2MgKmJ1c3lfcGluOw0KPiANCj4gT25seSBpZiB5b3Ugd2FudCB0byBkbyBh
IHJlLXNwaW4sIGhlcmUncyBhbiBpZGVhLg0KPiBJZiB5b3UgZG9uJ3Qgd2FudCB0bywgZmVlbCBm
cmVlIHRvIGRpc3JlZ2FyZCwgc2luY2UgeW91ciBwYXRjaCBpcyBnb29kLg0KPiANCj4gWW91IGNv
dWxkIGNvbXBhY3QgdGhpbmdzIGEgYml0OyBJIGtub3cgdGhpcyB3YXNuJ3Qgd2hhdCB0aGUgaW5p
dGlhbCBjb2RlDQo+IHdhcyBkb2luZywgYnV0IGl0J3MgYW4gb3B0aW9uLg0KPiANCj4gU29tZXRo
aW5nIGxpa2U6DQo+IA0KPiBlbnVtIHsNCj4gCUdQSU9fUldEUiwNCj4gCUdQSU9fQ09OVkVSVCwN
Cj4gCUdQSU9fQlVTWSwNCj4gCV9fR1BJT19NQVgsDQo+IH0NCj4gDQo+IFRoZW4sIHdoYXQgeW91
IGVuZCB1cCBoYXZpbmcgaXM6DQo+IA0KPiAgc3RydWN0IGFkNzgxNl9jaGlwX2luZm8gew0KPiAg
IAlzdHJ1Y3Qgc3BpX2RldmljZSAqc3BpX2RldjsNCj4gIC0JdTE2IHJkd3JfcGluOw0KPiAgLQl1
MTYgY29udmVydF9waW47DQo+ICAtCXUxNiBidXN5X3BpbjsNCj4gICsJc3RydWN0IGdwaW9fZGVz
YyAqZ3Bpb3NbX19HUElPX01BWF07DQo+ICANCj4gDQo+IC8vIENvbnRpbnVlZCBiZWxvdw0KPiAN
Cj4gDQo+ID4gIAl1OCAgb3RpX2RhdGFbQUQ3ODE2X0NTX01BWCArIDFdOw0KPiA+ICAJdTggIGNo
YW5uZWxfaWQ7CS8qIDAgYWx3YXlzIGJlIHRlbXBlcmF0dXJlICovDQo+ID4gIAl1OCAgbW9kZTsN
Cj4gPiBAQCAtNjEsMjggKzYxLDI4IEBAIHN0YXRpYyBpbnQgYWQ3ODE2X3NwaV9yZWFkKHN0cnVj
dCBhZDc4MTZfY2hpcF9pbmZvDQo+ID4gKmNoaXAsIHUxNiAqZGF0YSkNCj4gPiAgCWludCByZXQg
PSAwOw0KPiA+ICAJX19iZTE2IGJ1ZjsNCj4gPiAgDQo+ID4gLQlncGlvX3NldF92YWx1ZShjaGlw
LT5yZHdyX3BpbiwgMSk7DQo+ID4gLQlncGlvX3NldF92YWx1ZShjaGlwLT5yZHdyX3BpbiwgMCk7
DQo+ID4gKwlncGlvZF9zZXRfdmFsdWUoY2hpcC0+cmR3cl9waW4sIDEpOw0KPiA+ICsJZ3Bpb2Rf
c2V0X3ZhbHVlKGNoaXAtPnJkd3JfcGluLCAwKTsNCj4gDQo+IE9idmlvdXNseSwgaW4gdGhlIGFi
b3ZlIGNvbnRleHQsIHRoaXMgYmVjb21lczoNCj4gICsJZ3Bpb2Rfc2V0X3ZhbHVlKGNoaXAtPmdw
aW9zW0dQSU9fUldEUl0sIDEpOw0KPiAgKwlncGlvZF9zZXRfdmFsdWUoY2hpcC0+Z3Bpb3NbR1BJ
T19SV0RSXSwgMCk7DQo+IA0KPiA+ICAJcmV0ID0gc3BpX3dyaXRlKHNwaV9kZXYsICZjaGlwLT5j
aGFubmVsX2lkLCBzaXplb2YoY2hpcC0NCj4gPiA+IGNoYW5uZWxfaWQpKTsNCj4gPiANCj4gPiAg
CWlmIChyZXQgPCAwKSB7DQo+ID4gIAkJZGV2X2Vycigmc3BpX2Rldi0+ZGV2LCAiU1BJIGNoYW5u
ZWwgc2V0dGluZyBlcnJvclxuIik7DQo+ID4gIAkJcmV0dXJuIHJldDsNCj4gPiAgCX0NCj4gPiAt
CWdwaW9fc2V0X3ZhbHVlKGNoaXAtPnJkd3JfcGluLCAxKTsNCj4gPiArCWdwaW9kX3NldF92YWx1
ZShjaGlwLT5yZHdyX3BpbiwgMSk7DQo+ID4gIA0KPiA+ICAJaWYgKGNoaXAtPm1vZGUgPT0gQUQ3
ODE2X1BEKSB7IC8qIG9wZXJhdGluZyBtb2RlIDIgKi8NCj4gPiAtCQlncGlvX3NldF92YWx1ZShj
aGlwLT5jb252ZXJ0X3BpbiwgMSk7DQo+ID4gLQkJZ3Bpb19zZXRfdmFsdWUoY2hpcC0+Y29udmVy
dF9waW4sIDApOw0KPiA+ICsJCWdwaW9kX3NldF92YWx1ZShjaGlwLT5jb252ZXJ0X3BpbiwgMSk7
DQo+ID4gKwkJZ3Bpb2Rfc2V0X3ZhbHVlKGNoaXAtPmNvbnZlcnRfcGluLCAwKTsNCj4gPiAgCX0g
ZWxzZSB7IC8qIG9wZXJhdGluZyBtb2RlIDEgKi8NCj4gPiAtCQlncGlvX3NldF92YWx1ZShjaGlw
LT5jb252ZXJ0X3BpbiwgMCk7DQo+ID4gLQkJZ3Bpb19zZXRfdmFsdWUoY2hpcC0+Y29udmVydF9w
aW4sIDEpOw0KPiA+ICsJCWdwaW9kX3NldF92YWx1ZShjaGlwLT5jb252ZXJ0X3BpbiwgMCk7DQo+
ID4gKwkJZ3Bpb2Rfc2V0X3ZhbHVlKGNoaXAtPmNvbnZlcnRfcGluLCAxKTsNCj4gPiAgCX0NCj4g
PiAgDQo+ID4gLQl3aGlsZSAoZ3Bpb19nZXRfdmFsdWUoY2hpcC0+YnVzeV9waW4pKQ0KPiA+ICsJ
d2hpbGUgKGdwaW9kX2dldF92YWx1ZShjaGlwLT5idXN5X3BpbikpDQo+ID4gIAkJY3B1X3JlbGF4
KCk7DQo+ID4gIA0KPiA+IC0JZ3Bpb19zZXRfdmFsdWUoY2hpcC0+cmR3cl9waW4sIDApOw0KPiA+
IC0JZ3Bpb19zZXRfdmFsdWUoY2hpcC0+cmR3cl9waW4sIDEpOw0KPiA+ICsJZ3Bpb2Rfc2V0X3Zh
bHVlKGNoaXAtPnJkd3JfcGluLCAwKTsNCj4gPiArCWdwaW9kX3NldF92YWx1ZShjaGlwLT5yZHdy
X3BpbiwgMSk7DQo+ID4gIAlyZXQgPSBzcGlfcmVhZChzcGlfZGV2LCAmYnVmLCBzaXplb2YoKmRh
dGEpKTsNCj4gPiAgCWlmIChyZXQgPCAwKSB7DQo+ID4gIAkJZGV2X2Vycigmc3BpX2Rldi0+ZGV2
LCAiU1BJIGRhdGEgcmVhZCBlcnJvclxuIik7DQo+ID4gQEAgLTk5LDggKzk5LDggQEAgc3RhdGlj
IGludCBhZDc4MTZfc3BpX3dyaXRlKHN0cnVjdCBhZDc4MTZfY2hpcF9pbmZvDQo+ID4gKmNoaXAs
IHU4IGRhdGEpDQo+ID4gIAlzdHJ1Y3Qgc3BpX2RldmljZSAqc3BpX2RldiA9IGNoaXAtPnNwaV9k
ZXY7DQo+ID4gIAlpbnQgcmV0ID0gMDsNCj4gPiAgDQo+ID4gLQlncGlvX3NldF92YWx1ZShjaGlw
LT5yZHdyX3BpbiwgMSk7DQo+ID4gLQlncGlvX3NldF92YWx1ZShjaGlwLT5yZHdyX3BpbiwgMCk7
DQo+ID4gKwlncGlvZF9zZXRfdmFsdWUoY2hpcC0+cmR3cl9waW4sIDEpOw0KPiA+ICsJZ3Bpb2Rf
c2V0X3ZhbHVlKGNoaXAtPnJkd3JfcGluLCAwKTsNCj4gPiAgCXJldCA9IHNwaV93cml0ZShzcGlf
ZGV2LCAmZGF0YSwgc2l6ZW9mKGRhdGEpKTsNCj4gPiAgCWlmIChyZXQgPCAwKQ0KPiA+ICAJCWRl
dl9lcnIoJnNwaV9kZXYtPmRldiwgIlNQSSBvdGkgZGF0YSB3cml0ZSBlcnJvclxuIik7DQo+ID4g
QEAgLTEyOSwxMCArMTI5LDEwIEBAIHN0YXRpYyBzc2l6ZV90IGFkNzgxNl9zdG9yZV9tb2RlKHN0
cnVjdCBkZXZpY2UNCj4gPiAqZGV2LA0KPiA+ICAJc3RydWN0IGFkNzgxNl9jaGlwX2luZm8gKmNo
aXAgPSBpaW9fcHJpdihpbmRpb19kZXYpOw0KPiA+ICANCj4gPiAgCWlmIChzdHJjbXAoYnVmLCAi
ZnVsbCIpKSB7DQo+ID4gLQkJZ3Bpb19zZXRfdmFsdWUoY2hpcC0+cmR3cl9waW4sIDEpOw0KPiA+
ICsJCWdwaW9kX3NldF92YWx1ZShjaGlwLT5yZHdyX3BpbiwgMSk7DQo+ID4gIAkJY2hpcC0+bW9k
ZSA9IEFENzgxNl9GVUxMOw0KPiA+ICAJfSBlbHNlIHsNCj4gPiAtCQlncGlvX3NldF92YWx1ZShj
aGlwLT5yZHdyX3BpbiwgMCk7DQo+ID4gKwkJZ3Bpb2Rfc2V0X3ZhbHVlKGNoaXAtPnJkd3JfcGlu
LCAwKTsNCj4gPiAgCQljaGlwLT5tb2RlID0gQUQ3ODE2X1BEOw0KPiA+ICAJfQ0KPiA+ICANCj4g
PiBAQCAtMzQ1LDE1ICszNDUsOSBAQCBzdGF0aWMgaW50IGFkNzgxNl9wcm9iZShzdHJ1Y3Qgc3Bp
X2RldmljZQ0KPiA+ICpzcGlfZGV2KQ0KPiA+ICB7DQo+ID4gIAlzdHJ1Y3QgYWQ3ODE2X2NoaXBf
aW5mbyAqY2hpcDsNCj4gPiAgCXN0cnVjdCBpaW9fZGV2ICppbmRpb19kZXY7DQo+ID4gLQl1bnNp
Z25lZCBzaG9ydCAqcGlucyA9IGRldl9nZXRfcGxhdGRhdGEoJnNwaV9kZXYtPmRldik7DQo+ID4g
IAlpbnQgcmV0ID0gMDsNCj4gPiAgCWludCBpOw0KPiA+ICANCj4gPiAtCWlmICghcGlucykgew0K
PiA+IC0JCWRldl9lcnIoJnNwaV9kZXYtPmRldiwgIk5vIG5lY2Vzc2FyeSBHUElPIHBsYXRmb3Jt
DQo+ID4gZGF0YS5cbiIpOw0KPiA+IC0JCXJldHVybiAtRUlOVkFMOw0KPiA+IC0JfQ0KPiA+IC0N
Cj4gPiAgCWluZGlvX2RldiA9IGRldm1faWlvX2RldmljZV9hbGxvYygmc3BpX2Rldi0+ZGV2LCBz
aXplb2YoKmNoaXApKTsNCj4gPiAgCWlmICghaW5kaW9fZGV2KQ0KPiA+ICAJCXJldHVybiAtRU5P
TUVNOw0KPiA+IEBAIC0zNjQsMzQgKzM1OCwyOCBAQCBzdGF0aWMgaW50IGFkNzgxNl9wcm9iZShz
dHJ1Y3Qgc3BpX2RldmljZQ0KPiA+ICpzcGlfZGV2KQ0KPiA+ICAJY2hpcC0+c3BpX2RldiA9IHNw
aV9kZXY7DQo+ID4gIAlmb3IgKGkgPSAwOyBpIDw9IEFENzgxNl9DU19NQVg7IGkrKykNCj4gPiAg
CQljaGlwLT5vdGlfZGF0YVtpXSA9IDIwMzsNCj4gPiAtCWNoaXAtPnJkd3JfcGluID0gcGluc1sw
XTsNCj4gPiAtCWNoaXAtPmNvbnZlcnRfcGluID0gcGluc1sxXTsNCj4gPiAtCWNoaXAtPmJ1c3lf
cGluID0gcGluc1syXTsNCj4gPiAtDQo+ID4gLQlyZXQgPSBkZXZtX2dwaW9fcmVxdWVzdCgmc3Bp
X2Rldi0+ZGV2LCBjaGlwLT5yZHdyX3BpbiwNCj4gPiAtCQkJCXNwaV9nZXRfZGV2aWNlX2lkKHNw
aV9kZXYpLT5uYW1lKTsNCj4gPiAtCWlmIChyZXQpIHsNCj4gPiAtCQlkZXZfZXJyKCZzcGlfZGV2
LT5kZXYsICJGYWlsIHRvIHJlcXVlc3QgcmR3ciBncGlvIFBJTg0KPiA+ICVkLlxuIiwNCj4gPiAt
CQkJY2hpcC0+cmR3cl9waW4pOw0KPiA+ICsNCj4gPiArCWNoaXAtPnJkd3JfcGluID0gZGV2bV9n
cGlvZF9nZXQoJnNwaV9kZXYtPmRldiwgInJkd3IiLCBHUElPRF9JTik7DQo+ID4gKwlpZiAoSVNf
RVJSKGNoaXAtPnJkd3JfcGluKSkgew0KPiA+ICsJCXJldCA9IFBUUl9FUlIoY2hpcC0+cmR3cl9w
aW4pOw0KPiA+ICsJCWRldl9lcnIoJnNwaV9kZXYtPmRldiwgIkZhaWxlZCB0byByZXF1ZXN0IHJk
d3IgR1BJTzogJWRcbiIsDQo+ID4gKwkJCXJldCk7DQo+ID4gIAkJcmV0dXJuIHJldDsNCj4gPiAg
CX0NCj4gDQo+IEJ1dCB0aGUgcmVhbCBiZW5lZml0IGlmIHRoaXMgYXBwcm9hY2ggY29tZXMgaGVy
ZS4NCj4gWW91IGNvdWxkIGRvOg0KPiANCj4gc3RhdGljIGNvbnN0IGNoYXIgKmdwaW9fbmFtZXNb
X19HUElPX01BWF0gPSB7DQo+IAlbR1BJT19SV0RSXSA9ICJyd2RyIiwNCj4gCVtHUElPX0NPTlZF
UlRdID0gImNvbnZlcnQiLA0KPiAJW0dQSU9fQlVTWV0gPSAiYnVzeSIsDQo+IH07DQo+IA0KPiAv
LyBjb2RlIGZvciBwYXRjaDIJDQo+IGJvb2wgaGF2ZV9idXN5X3BpbiA9IChjaGlwLT5pZCA9PSBJ
RF9BRDc4MTYgfHwgY2hpcC0+aWQgPT0gSURfQUQ3ODE3KTsNCj4gLy8gZW5kIGNvZGUgZm9yIHBh
dGNoMgkNCj4gDQo+IA0KPiBmb3IgKGkgPSAwOyBpIDwgX19HUElPX01BWDsgaSsrKSB7DQo+IC8v
IGNvZGUgZm9yIHBhdGNoMgkNCj4gCWlmIChpID09IEdQSU9fQlVTWSAmJiAhaGF2ZV9idXN5X3Bp
bikNCj4gCQljb250aW51ZTsNCj4gLy8gZW5kIGNvZGUgZm9yIHBhdGNoMgkNCj4gDQo+IAljaGlw
LT5ncGlvc1tpXSA9IGRldm1fZ3Bpb2RfZ2V0KCZzcGlfZGV2LT5kZXYsIGdwaW9fbmFtZXNbaV0s
IA0KPiAJCQkJCUdQSU9EX0lOKTsNCj4gCWlmIChJU19FUlIoY2hpcC0+Z3Bpb3NbaV0pKSB7DQo+
IAkJcmV0ID0gUFRSX0VSUihjaGlwLT5ncGlvc1tpXSk7DQo+IAkJZGV2X2Vycigmc3BpX2Rldi0+
ZGV2LCAiRmFpbGVkIHRvIHJlcXVlc3QgcmR3ciBHUElPOiAlZFxuIiwNCj4gCQkJcmV0Ow0KPiAJ
CXJldHVybiByZXQ7DQo+IAl9DQo+IH0NCj4gDQo+IFRoZW4sIGluIHlvdXIgcGF0Y2gyIChpLmUg
IiBzdGFnaW5nOiBpaW86IGFkNzgxNjogRG8gbm90IHVzZSBidXN5X3BpbiBpbg0KPiBjYXNlIG9m
IEFENzgxOCIpLCB3aGF0IHlvdSBjb3VsZCBkbywgaXMgdG8gYWRkIHRoYXQgY29kZSAoYWxvbmcg
d2l0aCB0aGUNCj4gb3RoZXIgSUQgY29kZSkuDQo+IA0KPiBBbmQgZmluYWx5IHdoYXQgeW91IGNv
dWxkIGRvIGluIHBhdGNoMiwgaXM6DQo+IA0KPiAJd2hpbGUgKGNoaXAtPmdwaW9zW0dQSU9fQlVT
WV0gJiYNCj4gCSAgICAgICBncGlvZF9nZXRfdmFsdWUoY2hpcC0+YnVzeV9waW4pKQ0KPiAJCWNw
dV9yZWxheCgpOw0KPiANCj4gDQo+IA0KPiA+IC0JZ3Bpb19kaXJlY3Rpb25faW5wdXQoY2hpcC0+
cmR3cl9waW4pOw0KPiA+IC0JcmV0ID0gZGV2bV9ncGlvX3JlcXVlc3QoJnNwaV9kZXYtPmRldiwg
Y2hpcC0+Y29udmVydF9waW4sDQo+ID4gLQkJCQlzcGlfZ2V0X2RldmljZV9pZChzcGlfZGV2KS0+
bmFtZSk7DQo+ID4gLQlpZiAocmV0KSB7DQo+ID4gLQkJZGV2X2Vycigmc3BpX2Rldi0+ZGV2LCAi
RmFpbCB0byByZXF1ZXN0IGNvbnZlcnQgZ3BpbyBQSU4NCj4gPiAlZC5cbiIsDQo+ID4gLQkJCWNo
aXAtPmNvbnZlcnRfcGluKTsNCj4gPiArCWNoaXAtPmNvbnZlcnRfcGluID0gZGV2bV9ncGlvZF9n
ZXQoJnNwaV9kZXYtPmRldiwgImNvbnZlcnQiLA0KPiA+IEdQSU9EX0lOKTsNCj4gPiArCWlmIChJ
U19FUlIoY2hpcC0+Y29udmVydF9waW4pKSB7DQo+ID4gKwkJcmV0ID0gUFRSX0VSUihjaGlwLT5j
b252ZXJ0X3Bpbik7DQo+ID4gKwkJZGV2X2Vycigmc3BpX2Rldi0+ZGV2LCAiRmFpbGVkIHRvIHJl
cXVlc3QgY29udmVydCBHUElPOg0KPiA+ICVkXG4iLA0KPiA+ICsJCQlyZXQpOw0KPiA+ICAJCXJl
dHVybiByZXQ7DQo+ID4gIAl9DQo+ID4gLQlncGlvX2RpcmVjdGlvbl9pbnB1dChjaGlwLT5jb252
ZXJ0X3Bpbik7DQo+ID4gLQlyZXQgPSBkZXZtX2dwaW9fcmVxdWVzdCgmc3BpX2Rldi0+ZGV2LCBj
aGlwLT5idXN5X3BpbiwNCj4gPiAtCQkJCXNwaV9nZXRfZGV2aWNlX2lkKHNwaV9kZXYpLT5uYW1l
KTsNCj4gPiAtCWlmIChyZXQpIHsNCj4gPiAtCQlkZXZfZXJyKCZzcGlfZGV2LT5kZXYsICJGYWls
IHRvIHJlcXVlc3QgYnVzeSBncGlvIFBJTg0KPiA+ICVkLlxuIiwNCj4gPiAtCQkJY2hpcC0+YnVz
eV9waW4pOw0KPiA+ICsJY2hpcC0+YnVzeV9waW4gPSBkZXZtX2dwaW9kX2dldCgmc3BpX2Rldi0+
ZGV2LCAiYnVzeSIsIEdQSU9EX0lOKTsNCj4gPiArCWlmIChJU19FUlIoY2hpcC0+YnVzeV9waW4p
KSB7DQo+ID4gKwkJcmV0ID0gUFRSX0VSUihjaGlwLT5idXN5X3Bpbik7DQo+ID4gKwkJZGV2X2Vy
cigmc3BpX2Rldi0+ZGV2LCAiRmFpbGVkIHRvIHJlcXVlc3QgYnVzeSBHUElPOiAlZFxuIiwNCj4g
PiArCQkJcmV0KTsNCj4gPiAgCQlyZXR1cm4gcmV0Ow0KPiA+ICAJfQ0KPiA+IC0JZ3Bpb19kaXJl
Y3Rpb25faW5wdXQoY2hpcC0+YnVzeV9waW4pOw0KPiA+ICANCj4gPiAgCWluZGlvX2Rldi0+bmFt
ZSA9IHNwaV9nZXRfZGV2aWNlX2lkKHNwaV9kZXYpLT5uYW1lOw0KPiA+ICAJaW5kaW9fZGV2LT5k
ZXYucGFyZW50ID0gJnNwaV9kZXYtPmRldjsNCg==
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 4/4] staging: iio: ad7816: Add device tree table.
2018-11-09 7:38 ` [PATCH v3 4/4] staging: iio: ad7816: Add device tree table Nishad Kamdar
@ 2018-11-09 8:11 ` Ardelean, Alexandru
0 siblings, 0 replies; 19+ messages in thread
From: Ardelean, Alexandru @ 2018-11-09 8:11 UTC (permalink / raw)
To: lars, nishadkamdar
Cc: linux-kernel, knaack.h, jic23, Hennerich, Michael, devel,
linux-iio, pmeerw, gregkh
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 <nishadkamdar@gmail.com>
> ---
> 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,
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 4/4] staging: iio: ad7816: Add device tree table.
@ 2018-11-09 8:11 ` Ardelean, Alexandru
0 siblings, 0 replies; 19+ messages in thread
From: Ardelean, Alexandru @ 2018-11-09 8:11 UTC (permalink / raw)
To: lars, nishadkamdar
Cc: linux-kernel, knaack.h, jic23, Hennerich, Michael, devel,
linux-iio, pmeerw, gregkh
T24gRnJpLCAyMDE4LTExLTA5IGF0IDEzOjA4ICswNTMwLCBOaXNoYWQgS2FtZGFyIHdyb3RlOg0K
PiBBZGQgZGV2aWNlIHRyZWUgdGFibGUgZm9yIG1hdGNoaW5nIHZlbmRvciBJRC4NCg0KT25lIGNv
bW1lbnQgaW5saW5lIGZvciB0aGlzLg0KDQpUaGFua3MNCkFsZXgNCg0KPiANCj4gU2lnbmVkLW9m
Zi1ieTogTmlzaGFkIEthbWRhciA8bmlzaGFka2FtZGFyQGdtYWlsLmNvbT4NCj4gLS0tDQo+ICBk
cml2ZXJzL3N0YWdpbmcvaWlvL2FkYy9hZDc4MTYuYyB8IDcgKysrKysrKw0KPiAgMSBmaWxlIGNo
YW5nZWQsIDcgaW5zZXJ0aW9ucygrKQ0KPiANCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvc3RhZ2lu
Zy9paW8vYWRjL2FkNzgxNi5jDQo+IGIvZHJpdmVycy9zdGFnaW5nL2lpby9hZGMvYWQ3ODE2LmMN
Cj4gaW5kZXggYTJmZWFkODVjZDQ2Li5iOGE5MTQ5ZmJhYzEgMTAwNjQ0DQo+IC0tLSBhL2RyaXZl
cnMvc3RhZ2luZy9paW8vYWRjL2FkNzgxNi5jDQo+ICsrKyBiL2RyaXZlcnMvc3RhZ2luZy9paW8v
YWRjL2FkNzgxNi5jDQo+IEBAIC00MjIsNiArNDIyLDEyIEBAIHN0YXRpYyBpbnQgYWQ3ODE2X3By
b2JlKHN0cnVjdCBzcGlfZGV2aWNlICpzcGlfZGV2KQ0KPiAgCXJldHVybiAwOw0KPiAgfQ0KPiAg
DQo+ICtzdGF0aWMgY29uc3Qgc3RydWN0IG9mX2RldmljZV9pZCBhZDc4MTZfb2ZfbWF0Y2hbXSA9
IHsNCj4gKwl7IC5jb21wYXRpYmxlID0gImFkaSxhZDc4MTYiLCB9LA0KDQpJIHRoaW5rIHlvdSBh
bHNvIG5lZWQgdG8gYWRkOg0KKwl7IC5jb21wYXRpYmxlID0gImFkaSxhZDc4MTciLCB9LA0KKwl7
IC5jb21wYXRpYmxlID0gImFkaSxhZDc4MTgiLCB9LA0KDQo+ICsJeyB9DQo+ICt9Ow0KPiArTU9E
VUxFX0RFVklDRV9UQUJMRShvZiwgYWQ3ODE2X29mX21hdGNoKTsNCj4gKw0KPiAgc3RhdGljIGNv
bnN0IHN0cnVjdCBzcGlfZGV2aWNlX2lkIGFkNzgxNl9pZFtdID0gew0KPiAgCXsgImFkNzgxNiIs
IElEX0FENzgxNiB9LA0KPiAgCXsgImFkNzgxNyIsIElEX0FENzgxNyB9LA0KPiBAQCAtNDM0LDYg
KzQ0MCw3IEBAIE1PRFVMRV9ERVZJQ0VfVEFCTEUoc3BpLCBhZDc4MTZfaWQpOw0KPiAgc3RhdGlj
IHN0cnVjdCBzcGlfZHJpdmVyIGFkNzgxNl9kcml2ZXIgPSB7DQo+ICAJLmRyaXZlciA9IHsNCj4g
IAkJLm5hbWUgPSAiYWQ3ODE2IiwNCj4gKwkJLm9mX21hdGNoX3RhYmxlID0gb2ZfbWF0Y2hfcHRy
KGFkNzgxNl9vZl9tYXRjaCksDQo+ICAJfSwNCj4gIAkucHJvYmUgPSBhZDc4MTZfcHJvYmUsDQo+
ICAJLmlkX3RhYmxlID0gYWQ3ODE2X2lkLA0K
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/4] staging: iio: ad7816: Switch to the gpio descriptor interface
2018-11-09 7:35 ` [PATCH v3 1/4] " Nishad Kamdar
2018-11-09 8:05 ` Ardelean, Alexandru
@ 2018-11-11 12:24 ` Jonathan Cameron
2018-11-11 12:31 ` Jonathan Cameron
1 sibling, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2018-11-11 12:24 UTC (permalink / raw)
To: Nishad Kamdar
Cc: Lars-Peter Clausen, Michael Hennerich, Hartmut Knaack,
Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio, devel,
linux-kernel
On Fri, 9 Nov 2018 13:05:17 +0530
Nishad Kamdar <nishadkamdar@gmail.com> 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 <nishadkamdar@gmail.com>
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;
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/4] staging: iio: ad7816: Switch to the gpio descriptor interface
2018-11-11 12:24 ` Jonathan Cameron
@ 2018-11-11 12:31 ` Jonathan Cameron
2018-11-14 17:06 ` Nishad Kamdar
0 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2018-11-11 12:31 UTC (permalink / raw)
To: Nishad Kamdar
Cc: Lars-Peter Clausen, Michael Hennerich, Hartmut Knaack,
Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio, devel,
linux-kernel
On Sun, 11 Nov 2018 12:24:05 +0000
Jonathan Cameron <jic23@kernel.org> wrote:
> On Fri, 9 Nov 2018 13:05:17 +0530
> Nishad Kamdar <nishadkamdar@gmail.com> 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 <nishadkamdar@gmail.com>
> 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;
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/4] staging: iio: ad7816: Do not use busy_pin in case of AD7818
2018-11-09 7:36 ` [PATCH v3 2/4] staging: iio: ad7816: Do not use busy_pin in case of AD7818 Nishad Kamdar
@ 2018-11-11 12:35 ` Jonathan Cameron
0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2018-11-11 12:35 UTC (permalink / raw)
To: Nishad Kamdar
Cc: Lars-Peter Clausen, Michael Hennerich, Hartmut Knaack,
Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio, devel,
linux-kernel
On Fri, 9 Nov 2018 13:06:24 +0530
Nishad Kamdar <nishadkamdar@gmail.com> wrote:
> AD7818 does not support busy_pin functionality as per datasheet.
> Hence drop busy_pin when AD7818 is used.
>
> Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
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 },
> {}
> };
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 3/4] staging: iio: ad7816: Set RD/WR pin and CONVST pin as outputs.
2018-11-09 7:37 ` [PATCH v3 3/4] staging: iio: ad7816: Set RD/WR pin and CONVST pin as outputs Nishad Kamdar
@ 2018-11-11 12:36 ` Jonathan Cameron
0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2018-11-11 12:36 UTC (permalink / raw)
To: Nishad Kamdar
Cc: Lars-Peter Clausen, Michael Hennerich, Hartmut Knaack,
Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio, devel,
linux-kernel
On Fri, 9 Nov 2018 13:07:18 +0530
Nishad Kamdar <nishadkamdar@gmail.com> 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 <nishadkamdar@gmail.com>
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",
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 4/4] staging: iio: ad7816: Add device tree table.
2018-11-09 8:11 ` Ardelean, Alexandru
(?)
@ 2018-11-11 12:38 ` Jonathan Cameron
2018-11-14 16:54 ` Nishad Kamdar
-1 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2018-11-11 12:38 UTC (permalink / raw)
To: Ardelean, Alexandru
Cc: lars, nishadkamdar, linux-kernel, knaack.h, Hennerich, Michael,
devel, linux-iio, pmeerw, gregkh
On Fri, 9 Nov 2018 08:11:57 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> 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 <nishadkamdar@gmail.com>
> > ---
> > 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,
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 4/4] staging: iio: ad7816: Add device tree table.
2018-11-09 8:11 ` Ardelean, Alexandru
(?)
(?)
@ 2018-11-14 16:29 ` Nishad Kamdar
-1 siblings, 0 replies; 19+ messages in thread
From: Nishad Kamdar @ 2018-11-14 16:29 UTC (permalink / raw)
To: Ardelean, Alexandru
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Hartmut Knaack, Peter Meerwald-Stadler, Greg Kroah-Hartman,
linux-iio, devel, linux-kernel
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 <nishadkamdar@gmail.com>
> > ---
> > 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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 4/4] staging: iio: ad7816: Add device tree table.
2018-11-11 12:38 ` Jonathan Cameron
@ 2018-11-14 16:54 ` Nishad Kamdar
0 siblings, 0 replies; 19+ messages in thread
From: Nishad Kamdar @ 2018-11-14 16:54 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Ardelean, Alexandru, Lars-Peter Clausen, Michael Hennerich,
Hartmut Knaack, Peter Meerwald-Stadler, Greg Kroah-Hartman,
linux-iio, devel, linux-kernel
On Sun, Nov 11, 2018 at 12:38:02PM +0000, Jonathan Cameron wrote:
> On Fri, 9 Nov 2018 08:11:57 +0000
> "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> 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 <nishadkamdar@gmail.com>
> > > ---
> > > 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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/4] staging: iio: ad7816: Switch to the gpio descriptor interface
2018-11-11 12:31 ` Jonathan Cameron
@ 2018-11-14 17:06 ` Nishad Kamdar
0 siblings, 0 replies; 19+ messages in thread
From: Nishad Kamdar @ 2018-11-14 17:06 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Michael Hennerich, Hartmut Knaack,
Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio, devel,
linux-kernel
On Sun, Nov 11, 2018 at 12:31:54PM +0000, Jonathan Cameron wrote:
> On Sun, 11 Nov 2018 12:24:05 +0000
> Jonathan Cameron <jic23@kernel.org> wrote:
>
> > On Fri, 9 Nov 2018 13:05:17 +0530
> > Nishad Kamdar <nishadkamdar@gmail.com> 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 <nishadkamdar@gmail.com>
> > 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;
> >
>
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2018-11-14 17:06 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-09 7:34 [PATCH v3 0/4] staging: iio: ad7816: Switch to the gpio descriptor interface Nishad Kamdar
2018-11-09 7:35 ` [PATCH v3 1/4] " Nishad Kamdar
2018-11-09 8:05 ` Ardelean, Alexandru
2018-11-09 8:05 ` Ardelean, Alexandru
2018-11-09 8:08 ` Ardelean, Alexandru
2018-11-09 8:08 ` Ardelean, Alexandru
2018-11-11 12:24 ` Jonathan Cameron
2018-11-11 12:31 ` Jonathan Cameron
2018-11-14 17:06 ` Nishad Kamdar
2018-11-09 7:36 ` [PATCH v3 2/4] staging: iio: ad7816: Do not use busy_pin in case of AD7818 Nishad Kamdar
2018-11-11 12:35 ` Jonathan Cameron
2018-11-09 7:37 ` [PATCH v3 3/4] staging: iio: ad7816: Set RD/WR pin and CONVST pin as outputs Nishad Kamdar
2018-11-11 12:36 ` Jonathan Cameron
2018-11-09 7:38 ` [PATCH v3 4/4] staging: iio: ad7816: Add device tree table Nishad Kamdar
2018-11-09 8:11 ` Ardelean, Alexandru
2018-11-09 8:11 ` Ardelean, Alexandru
2018-11-11 12:38 ` Jonathan Cameron
2018-11-14 16:54 ` Nishad Kamdar
2018-11-14 16:29 ` Nishad Kamdar
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.