staging: iio: adt7316: Switch to the gpio descriptor interface

PATCH In review Nishad Kamdar 4 participants 5 messages Oct 22, 2018 – Oct 28, 2018

Subsystems (4)

Staging IIO Analog Devices IIO drivers Staging - industrial io

lore

Share & tools

lore

Nishad Kamdar

author

2018-10-22 17:14 UTC #0 lore raw

PATCH 1/1 staging: iio: adt7316: Switch to the gpio descriptor interface

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

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

---
 drivers/staging/iio/addac/adt7316.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

drivers/staging/iio/addac/adt7316.c +12 -5

index 3f22d1088713..94f945ba0097 100644
@@ -8,7 +8,7 @@
  */
 
 #include <linux/interrupt.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/workqueue.h>
 #include <linux/device.h>
 #include <linux/kernel.h>

@@ -177,7 +177,7 @@
 
 struct adt7316_chip_info {
 	struct adt7316_bus	bus;
-	u16			ldac_pin;
+	struct gpio_desc	*ldac_pin;
 	u16			int_mask;	/* 0x2f */
 	u8			config1;
 	u8			config2;

@@ -950,8 +950,8 @@ static ssize_t adt7316_store_update_DAC(struct device *dev,
 		if (ret)
 			return -EIO;
 	} else {
-		gpio_set_value(chip->ldac_pin, 0);
-		gpio_set_value(chip->ldac_pin, 1);
+		gpiod_set_value(chip->ldac_pin, 0);
+		gpiod_set_value(chip->ldac_pin, 1);
 	}
 
 	return len;

@@ -2120,7 +2120,14 @@ int adt7316_probe(struct device *dev, struct adt7316_bus *bus,
 	else
 		return -ENODEV;
 
-	chip->ldac_pin = adt7316_platform_data[1];
+	chip->ldac_pin = devm_gpiod_get(dev, "ldac", GPIOD_OUT_HIGH);
+	if (IS_ERR(chip->ldac_pin)) {
+		ret = PTR_ERR(chip->ldac_pin);
+		dev_err(dev, "Failed to request ldac GPIO: %d\n",
+			ret);
+		return ret;
+	}
+
 	if (chip->ldac_pin) {
 		chip->config3 |= ADT7316_DA_EN_VIA_DAC_LDCA;
 		if ((chip->id & ID_FAMILY_MASK) == ID_ADT75XX)

Signature · 1 line

2.17.1

Shreeya Patel

2018-10-22 17:52 UTC #1 lore raw

Quoted 5 lines · Nishad Kamdar

Use the gpiod interface instead of the deprecated old non-descriptor interface for ldac_pin.
Signed-off-by Nishad Kamdar <[email protected]>
---

Hi Nishad,

I have been working on implementing device tree bindings for this driver and removing platform data from it.

I've sent a series of patches for it to Jonathan. I didn't send it on the mailing list as I wanted to confirm from Jonathan if we wants something more to be done in the series. That series also includes the change which you are proposing here.
I am really sorry that you didn't know about this.
If you are planning to send any other patches for this driver then let me know before you start implementing it so that we can co-ordinate before doing any work.

Thanks

Quoted 56 lines

 drivers/staging/iio/addac/adt7316.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

drivers/staging/iio/addac/adt7316.c +4 -4

index 3f22d1088713..94f945ba0097 100644
@@ -8,7 +8,7 @@
  */
 
 #include <linux/interrupt.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/workqueue.h>
 #include <linux/device.h>
 #include <linux/kernel.h>

@@ -177,7 +177,7 @@
 
 struct adt7316_chip_info {
 	struct adt7316_bus	bus;
-	u16			ldac_pin;
+	struct gpio_desc	*ldac_pin;
 	u16			int_mask;	/* 0x2f */
 	u8			config1;
 	u8			config2;

@@ -950,8 +950,8 @@ static ssize_t adt7316_store_update_DAC(struct
 device *dev,
 		if (ret)
 			return -EIO;
 	} else {
-		gpio_set_value(chip->ldac_pin, 0);
-		gpio_set_value(chip->ldac_pin, 1);
+		gpiod_set_value(chip->ldac_pin, 0);
+		gpiod_set_value(chip->ldac_pin, 1);
 	}

drivers/staging/iio/addac/adt7316.c +8 -1

 	return len;

@@ -2120,7 +2120,14 @@ int adt7316_probe(struct device *dev, struct
 adt7316_bus *bus,
 	else
 		return -ENODEV;
 
-	chip->ldac_pin = adt7316_platform_data[1];
+	chip->ldac_pin = devm_gpiod_get(dev, "ldac",
 GPIOD_OUT_HIGH);
+	if (IS_ERR(chip->ldac_pin)) {
+		ret = PTR_ERR(chip->ldac_pin);
+		dev_err(dev, "Failed to request ldac GPIO: %d\n",
+			ret);
+		return ret;
+	}
+
 	if (chip->ldac_pin) {

drivers/staging/iio/addac/adt7316.c +0 -0

 		chip->config3 |= ADT7316_DA_EN_VIA_DAC_LDCA;
 		if ((chip->id & ID_FAMILY_MASK) == ID_ADT75XX)

Nishad Kamdar

author

2018-10-24 16:33 UTC #2 lore raw

Quoted 67 lines · Shreeya Patel

Quoted 5 lines · Nishad Kamdar
Use the gpiod interface instead of the deprecated old non-descriptor interface for ldac_pin.
Signed-off-by Nishad Kamdar <[email protected]>
---
Hi Nishad,
I have been working on implementing device tree bindings for this driver and removing platform data from it.
I've sent a series of patches for it to Jonathan. I didn't send it on the mailing list as I wanted to confirm from Jonathan if we wants something more to be done in the series. That series also includes the change which you are proposing here.
I am really sorry that you didn't know about this.
If you are planning to send any other patches for this driver then let me know before you start implementing it so that we can co-ordinate before doing any work.
Thanks
Quoted 43 lines
 drivers/staging/iio/addac/adt7316.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)
drivers/staging/iio/addac/adt7316.c +4 -4
index 3f22d1088713..94f945ba0097 100644
@@ -8,7 +8,7 @@
  */
 
 #include <linux/interrupt.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/workqueue.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
@@ -177,7 +177,7 @@
 
 struct adt7316_chip_info {
 	struct adt7316_bus	bus;
-	u16			ldac_pin;
+	struct gpio_desc	*ldac_pin;
 	u16			int_mask;	/* 0x2f */
 	u8			config1;
 	u8			config2;
@@ -950,8 +950,8 @@ static ssize_t adt7316_store_update_DAC(struct
 device *dev,
 		if (ret)
 			return -EIO;
 	} else {
-		gpio_set_value(chip->ldac_pin, 0);
-		gpio_set_value(chip->ldac_pin, 1);
+		gpiod_set_value(chip->ldac_pin, 0);
+		gpiod_set_value(chip->ldac_pin, 1);
 	}
 
drivers/staging/iio/addac/adt7316.c +0 -0
 	return len;
@@ -2120,7 +2120,14 @@ int adt7316_probe(struct device *dev, struct
 adt7316_bus *bus,
 	else
 		return -ENODEV;
 

Quoted 13 lines

drivers/staging/iio/addac/adt7316.c int adt7316_probe(struct device *dev, struct adt7316_bus *bus, · line 2133
Quoted 13 lines
drivers/staging/iio/addac/adt7316.c +8 -1
-	chip->ldac_pin = adt7316_platform_data[1];
+	chip->ldac_pin = devm_gpiod_get(dev, "ldac",
 GPIOD_OUT_HIGH);
+	if (IS_ERR(chip->ldac_pin)) {
+		ret = PTR_ERR(chip->ldac_pin);
+		dev_err(dev, "Failed to request ldac GPIO: %d\n",
+			ret);
+		return ret;
+	}
+
 	if (chip->ldac_pin) {
drivers/staging/iio/addac/adt7316.c +0 -0
 		chip->config3 |= ADT7316_DA_EN_VIA_DAC_LDCA;
 		if ((chip->id & ID_FAMILY_MASK) == ID_ADT75XX)

Hello Shreeya,

Sure no problem.

Thanks and regards,
Nishad

Jonathan Cameron

maintainer

2018-10-28 23:43 UTC #3 lore raw

Quoted 83 lines · On Wed, 24 Oct 2018 22:03:54 +0530 Nishad Kamdar <[email protected]> wrote:

Quoted 67 lines · Shreeya Patel
Quoted 5 lines · Nishad Kamdar
Use the gpiod interface instead of the deprecated old non-descriptor interface for ldac_pin.
Signed-off-by Nishad Kamdar <[email protected]>
---
Hi Nishad,
I have been working on implementing device tree bindings for this driver and removing platform data from it.
I've sent a series of patches for it to Jonathan. I didn't send it on the mailing list as I wanted to confirm from Jonathan if we wants something more to be done in the series. That series also includes the change which you are proposing here.
I am really sorry that you didn't know about this.
If you are planning to send any other patches for this driver then let me know before you start implementing it so that we can co-ordinate before doing any work.
Thanks
Quoted 43 lines
 drivers/staging/iio/addac/adt7316.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)
drivers/staging/iio/addac/adt7316.c +4 -4
index 3f22d1088713..94f945ba0097 100644
@@ -8,7 +8,7 @@
  */
 
 #include <linux/interrupt.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/workqueue.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
@@ -177,7 +177,7 @@
 
 struct adt7316_chip_info {
 	struct adt7316_bus	bus;
-	u16			ldac_pin;
+	struct gpio_desc	*ldac_pin;
 	u16			int_mask;	/* 0x2f */
 	u8			config1;
 	u8			config2;
@@ -950,8 +950,8 @@ static ssize_t adt7316_store_update_DAC(struct
 device *dev,
 		if (ret)
 			return -EIO;
 	} else {
-		gpio_set_value(chip->ldac_pin, 0);
-		gpio_set_value(chip->ldac_pin, 1);
+		gpiod_set_value(chip->ldac_pin, 0);
+		gpiod_set_value(chip->ldac_pin, 1);
 	}
 
drivers/staging/iio/addac/adt7316.c +0 -0
 	return len;
@@ -2120,7 +2120,14 @@ int adt7316_probe(struct device *dev, struct
 adt7316_bus *bus,
 	else
 		return -ENODEV;
 
Quoted 13 lines
drivers/staging/iio/addac/adt7316.c int adt7316_probe(struct device *dev, struct adt7316_bus *bus, · line 2133
Quoted 13 lines
drivers/staging/iio/addac/adt7316.c +8 -1
-	chip->ldac_pin = adt7316_platform_data[1];
+	chip->ldac_pin = devm_gpiod_get(dev, "ldac",
 GPIOD_OUT_HIGH);
+	if (IS_ERR(chip->ldac_pin)) {
+		ret = PTR_ERR(chip->ldac_pin);
+		dev_err(dev, "Failed to request ldac GPIO: %d\n",
+			ret);
+		return ret;
+	}
+
 	if (chip->ldac_pin) {
drivers/staging/iio/addac/adt7316.c +0 -0
 		chip->config3 |= ADT7316_DA_EN_VIA_DAC_LDCA;
 		if ((chip->id & ID_FAMILY_MASK) == ID_ADT75XX)  
Hello Shreeya,

Quoted 4 lines

Sure no problem.
Thanks and regards,
Nishad

Hi Nishad,

Thanks for being so understanding on this. I would normally operate a policy of first to post on the list (assuming they update reasonably quickly) is the on I apply, but as you are fine with it and this will save Shreeya a little time (by avoiding a rebase) I'll wait for Shreeya's public version.

I once discovered I was one of 3 people working on new drivers for a single camera sensor years ago. All 3 got posted in the same week despite the part having been around for years before that. Sometimes we get unlucky - the nature of opensource ;)

Jonathan

Dan Carpenter

2018-10-23 05:40 UTC #4 lore raw

In reply to Nishad Kamdar

drivers/staging/iio/addac/adt7316.c int adt7316_probe(struct device *dev, struct adt7316_bus *bus, · line 2127

Quoted 6 lines · Nishad Kamdar

drivers/staging/iio/addac/adt7316.c +5 -1

-	chip->ldac_pin = adt7316_platform_data[1];
+	chip->ldac_pin = devm_gpiod_get(dev, "ldac", GPIOD_OUT_HIGH);
+	if (IS_ERR(chip->ldac_pin)) {
+		ret = PTR_ERR(chip->ldac_pin);
+		dev_err(dev, "Failed to request ldac GPIO: %d\n",
+			ret);

This can fit on one line:

		dev_err(dev, "Failed to request ldac GPIO: %d\n", ret);

regards,
dan carpenter

Quoted 2 lines

drivers/staging/iio/addac/adt7316.c +2 -0
+		return ret;
+	}