* [PATCH v2] staging: iio: ad7816: Switch to the gpio descriptor interface
@ 2018-10-17 14:47 Nishad Kamdar
2018-10-17 14:49 ` Lars-Peter Clausen
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Nishad Kamdar @ 2018-10-17 14:47 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, outreachy-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>
---
Changes in v2:
- Correct the error messages as pin number being showed
has now been replaced by error code.
---
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] 7+ messages in thread* Re: [PATCH v2] staging: iio: ad7816: Switch to the gpio descriptor interface
2018-10-17 14:47 [PATCH v2] staging: iio: ad7816: Switch to the gpio descriptor interface Nishad Kamdar
@ 2018-10-17 14:49 ` Lars-Peter Clausen
2018-10-17 14:58 ` Sasha Levin
2018-10-18 7:28 ` Phil Reid
2 siblings, 0 replies; 7+ messages in thread
From: Lars-Peter Clausen @ 2018-10-17 14:49 UTC (permalink / raw)
To: Nishad Kamdar
Cc: Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio, devel,
linux-kernel, outreachy-kernel
On 10/17/2018 04:47 PM, Nishad Kamdar 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>
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
Thanks.
> ---
> Changes in v2:
> - Correct the error messages as pin number being showed
> has now been replaced by error code.
> ---
> drivers/staging/iio/adc/ad7816.c | 80 ++++++++++++++------------------
> 1 file changed, 34 insertions(+), 46 deletions(-)
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] staging: iio: ad7816: Switch to the gpio descriptor interface
2018-10-17 14:47 [PATCH v2] staging: iio: ad7816: Switch to the gpio descriptor interface Nishad Kamdar
2018-10-17 14:49 ` Lars-Peter Clausen
@ 2018-10-17 14:58 ` Sasha Levin
2018-10-17 19:56 ` Dan Carpenter
2018-10-18 7:28 ` Phil Reid
2 siblings, 1 reply; 7+ messages in thread
From: Sasha Levin @ 2018-10-17 14:58 UTC (permalink / raw)
To: Nishad Kamdar
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Hartmut Knaack, Peter Meerwald-Stadler, Greg Kroah-Hartman,
linux-iio, devel, linux-kernel, outreachy-kernel
On Wed, Oct 17, 2018 at 08:17:20PM +0530, Nishad Kamdar wrote:
>+ 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;
> }
Hm, from what I can tell devm_gpio_request() is allocating some memory,
which makes this a series of 4 allocations.
What happens if the fourth allocation fails? Do we leak the first three?
--
Thanks,
Sasha
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] staging: iio: ad7816: Switch to the gpio descriptor interface
2018-10-17 14:58 ` Sasha Levin
@ 2018-10-17 19:56 ` Dan Carpenter
0 siblings, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2018-10-17 19:56 UTC (permalink / raw)
To: Sasha Levin
Cc: Nishad Kamdar, devel, Lars-Peter Clausen, Michael Hennerich,
linux-iio, Greg Kroah-Hartman, linux-kernel, outreachy-kernel,
Peter Meerwald-Stadler, Hartmut Knaack, Jonathan Cameron
On Wed, Oct 17, 2018 at 10:58:23AM -0400, Sasha Levin wrote:
> On Wed, Oct 17, 2018 at 08:17:20PM +0530, Nishad Kamdar wrote:
> > + 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;
> > }
>
> Hm, from what I can tell devm_gpio_request() is allocating some memory,
> which makes this a series of 4 allocations.
>
> What happens if the fourth allocation fails? Do we leak the first three?
These are devm_ allocations. They are freed when &spi_dev->dev is
released.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] staging: iio: ad7816: Switch to the gpio descriptor interface
2018-10-17 14:47 [PATCH v2] staging: iio: ad7816: Switch to the gpio descriptor interface Nishad Kamdar
2018-10-17 14:49 ` Lars-Peter Clausen
2018-10-17 14:58 ` Sasha Levin
@ 2018-10-18 7:28 ` Phil Reid
2018-10-18 7:40 ` Lars-Peter Clausen
2 siblings, 1 reply; 7+ messages in thread
From: Phil Reid @ 2018-10-18 7:28 UTC (permalink / raw)
To: Nishad Kamdar, Lars-Peter Clausen
Cc: Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio, devel,
linux-kernel, outreachy-kernel
On 17/10/2018 10:47 PM, Nishad Kamdar 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>
> ---
> Changes in v2:
> - Correct the error messages as pin number being showed
> has now been replaced by error code.
> ---
> 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);
The RD/WR pin is an input to the AD78xx. So this doesn't make sense being GPIOD_IN.
> - 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);
ditto, the CONVST pin is an input to the AD78xx.
> - 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);
The busy pin doesn't exist on the ad7818.
Which the driver claims to support in the id table:
> static const struct spi_device_id ad7816_id[] = {
> { "ad7816", 0 },
> { "ad7817", 0 },
> { "ad7818", 0 },
> {}
> };
See:
https://www.analog.com/media/en/technical-documentation/data-sheets/AD7817_7818.pdf
Page 9.
>
> indio_dev->name = spi_get_device_id(spi_dev)->name;
> indio_dev->dev.parent = &spi_dev->dev;
>
Also should the pin names be documented in a device tree binding doc?
--
Regards
Phil Reid
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] staging: iio: ad7816: Switch to the gpio descriptor interface
2018-10-18 7:28 ` Phil Reid
@ 2018-10-18 7:40 ` Lars-Peter Clausen
2018-10-21 14:52 ` Jonathan Cameron
0 siblings, 1 reply; 7+ messages in thread
From: Lars-Peter Clausen @ 2018-10-18 7:40 UTC (permalink / raw)
To: Phil Reid, Nishad Kamdar
Cc: Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio, devel,
linux-kernel, outreachy-kernel
On 10/18/2018 09:28 AM, Phil Reid wrote:
[...]
>> + 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);
>
> The RD/WR pin is an input to the AD78xx. So this doesn't make sense being
> GPIOD_IN.
One thing at a time. This patch is a straight forward conversion to the GPIO
descriptor interface. It keeps the existing semantics of the driver as they are.
Now these semantics are obviously wrong and should be fixed but that should
be a separate patch from changing the interface.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] staging: iio: ad7816: Switch to the gpio descriptor interface
2018-10-18 7:40 ` Lars-Peter Clausen
@ 2018-10-21 14:52 ` Jonathan Cameron
0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2018-10-21 14:52 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Phil Reid, Nishad Kamdar, Michael Hennerich, Hartmut Knaack,
Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio, devel,
linux-kernel, outreachy-kernel
On Thu, 18 Oct 2018 09:40:00 +0200
Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 10/18/2018 09:28 AM, Phil Reid wrote:
> [...]
> >> + 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);
> >
> > The RD/WR pin is an input to the AD78xx. So this doesn't make sense being
> > GPIOD_IN.
>
> One thing at a time. This patch is a straight forward conversion to the GPIO
> descriptor interface. It keeps the existing semantics of the driver as they are.
>
> Now these semantics are obviously wrong and should be fixed but that should
> be a separate patch from changing the interface.
Agreed. Useful to raise these issues however, and I've added a note to the
patch to bring this to anyone's attention should they be interesting.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-10-21 14:52 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-17 14:47 [PATCH v2] staging: iio: ad7816: Switch to the gpio descriptor interface Nishad Kamdar
2018-10-17 14:49 ` Lars-Peter Clausen
2018-10-17 14:58 ` Sasha Levin
2018-10-17 19:56 ` Dan Carpenter
2018-10-18 7:28 ` Phil Reid
2018-10-18 7:40 ` Lars-Peter Clausen
2018-10-21 14:52 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).