[PATCH 0/5] Add dt support for extcon gpio driver

[PATCH 0/5] Add dt support for extcon gpio driver

[George Cherian]

This series
        - Convert extcon-gpio driver to use gpiod_* API's
        - Adds dt support to extcon-gpio driver.
	- Add cable name support in case of dt.

George Cherian (5):
  extcon: gpio: Minor cleanups
  extcon: gpio: Convert the driver to use gpio desc API's
  extcon: gpio: Add dt support for the driver.
  extcon: gpio: Always use  gpio_get_value_cansleep
  extcon: gpio: Add support for using cable names

 .../devicetree/bindings/extcon/extcon-gpio.txt     |  23 +++++
 drivers/extcon/extcon-gpio.c                       | 100 +++++++++++++--------
 include/linux/extcon/extcon-gpio.h                 |   4 +-
 3 files changed, 89 insertions(+), 38 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/extcon/extcon-gpio.txt

-- 
1.8.3.1

[PATCH 1/5] extcon: gpio: Minor cleanups

[George Cherian]

Minor Cleanups
 - Order the include files in alphabetical order.
 - Fix description of state_off in extcon_gpio.h
 - Add a descrition for check_on_resume in extcon_gpio.h

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/extcon/extcon-gpio.c       | 10 +++++-----
 include/linux/extcon/extcon-gpio.h |  4 +++-
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
index 5b7ec27..72f19a3 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -20,16 +20,16 @@
  *
 */
 
-#include <linux/module.h>
-#include <linux/kernel.h>
+#include <linux/extcon.h>
+#include <linux/extcon/extcon-gpio.h>
+#include <linux/gpio.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/workqueue.h>
-#include <linux/gpio.h>
-#include <linux/extcon.h>
-#include <linux/extcon/extcon-gpio.h>
 
 struct gpio_extcon_data {
 	struct extcon_dev *edev;
diff --git a/include/linux/extcon/extcon-gpio.h b/include/linux/extcon/extcon-gpio.h
index 8900fdf..0b17ad4 100644
--- a/include/linux/extcon/extcon-gpio.h
+++ b/include/linux/extcon/extcon-gpio.h
@@ -34,8 +34,10 @@
  * @irq_flags:		IRQ Flags (e.g., IRQF_TRIGGER_LOW).
  * @state_on:		print_state is overriden with state_on if attached.
  *			If NULL, default method of extcon class is used.
- * @state_off:		print_state is overriden with state_on if detached.
+ * @state_off:		print_state is overriden with state_off if detached.
  *			If NUll, default method of extcon class is used.
+ * @check_on_resume:	Boolean describing whether to check the state of gpio
+ *			while resuming from sleep.
  *
  * Note that in order for state_on or state_off to be valid, both state_on
  * and state_off should be not NULL. If at least one of them is NULL,
-- 
1.8.3.1

[PATCH 2/5] extcon: gpio: Convert the driver to use gpio desc API's

[George Cherian]

Convert the driver to use gpiod_* API's.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/extcon/extcon-gpio.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
index 72f19a3..25269f6 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -33,8 +33,7 @@
 
 struct gpio_extcon_data {
 	struct extcon_dev *edev;
-	unsigned gpio;
-	bool gpio_active_low;
+	struct gpio_desc *gpiod;
 	const char *state_on;
 	const char *state_off;
 	int irq;
@@ -50,9 +49,7 @@ static void gpio_extcon_work(struct work_struct *work)
 		container_of(to_delayed_work(work), struct gpio_extcon_data,
 			     work);
 
-	state = gpio_get_value(data->gpio);
-	if (data->gpio_active_low)
-		state = !state;
+	state = gpiod_get_value(data->gpiod);
 	extcon_set_state(data->edev, state);
 }
 
@@ -106,22 +103,21 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 	}
 	extcon_data->edev->name = pdata->name;
 
-	extcon_data->gpio = pdata->gpio;
-	extcon_data->gpio_active_low = pdata->gpio_active_low;
+	extcon_data->gpiod = gpio_to_desc(pdata->gpio);
 	extcon_data->state_on = pdata->state_on;
 	extcon_data->state_off = pdata->state_off;
 	extcon_data->check_on_resume = pdata->check_on_resume;
 	if (pdata->state_on && pdata->state_off)
 		extcon_data->edev->print_state = extcon_gpio_print_state;
 
-	ret = devm_gpio_request_one(&pdev->dev, extcon_data->gpio, GPIOF_DIR_IN,
+	ret = devm_gpio_request_one(&pdev->dev, pdata->gpio, GPIOF_DIR_IN,
 				    pdev->name);
 	if (ret < 0)
 		return ret;
 
 	if (pdata->debounce) {
-		ret = gpio_set_debounce(extcon_data->gpio,
-					pdata->debounce * 1000);
+		ret = gpiod_set_debounce(extcon_data->gpiod,
+					 pdata->debounce * 1000);
 		if (ret < 0)
 			extcon_data->debounce_jiffies =
 				msecs_to_jiffies(pdata->debounce);
@@ -133,7 +129,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 
 	INIT_DELAYED_WORK(&extcon_data->work, gpio_extcon_work);
 
-	extcon_data->irq = gpio_to_irq(extcon_data->gpio);
+	extcon_data->irq = gpiod_to_irq(extcon_data->gpiod);
 	if (extcon_data->irq < 0)
 		return extcon_data->irq;
 
-- 
1.8.3.1

[PATCH 3/5] extcon: gpio: Add dt support for the driver.

[George Cherian]

Add device tree support to extcon-gpio driver.
Add devicetree binding documentation

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 .../devicetree/bindings/extcon/extcon-gpio.txt     | 21 +++++++
 drivers/extcon/extcon-gpio.c                       | 70 +++++++++++++++-------
 2 files changed, 69 insertions(+), 22 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/extcon/extcon-gpio.txt

diff --git a/Documentation/devicetree/bindings/extcon/extcon-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
new file mode 100644
index 0000000..30aa2e1
--- /dev/null
+++ b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
@@ -0,0 +1,21 @@
+GPIO based EXTCON
+
+EXTCON GPIO
+-----------
+
+Required Properties:
+ - compatible: should be:
+	* "linux,extcon-gpio"
+ - gpios: specifies the gpio pin used.
+
+Optional Properties:
+ - debounce: Debounce time for GPIO IRQ in ms
+
+
+Eg:
+
+	extcon1: am43_usbid_extcon1 {
+		compatible = "linux,extcon-gpio";
+		gpios = <&gpio3 12 GPIO_ACTIVE_HIGH>;
+		debounce = <20>;
+		};
diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
index 25269f6..2bfbd2e 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -25,8 +25,10 @@
 #include <linux/gpio.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of_gpio.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/workqueue.h>
@@ -80,16 +82,12 @@ static ssize_t extcon_gpio_print_state(struct extcon_dev *edev, char *buf)
 
 static int gpio_extcon_probe(struct platform_device *pdev)
 {
+	struct device_node *np = pdev->dev.of_node;
 	struct gpio_extcon_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	struct gpio_extcon_data *extcon_data;
 	int ret;
-
-	if (!pdata)
-		return -EBUSY;
-	if (!pdata->irq_flags) {
-		dev_err(&pdev->dev, "IRQ flag is not specified.\n");
-		return -EINVAL;
-	}
+	unsigned int irq_flags;
+	unsigned int debounce = 0;
 
 	extcon_data = devm_kzalloc(&pdev->dev, sizeof(struct gpio_extcon_data),
 				   GFP_KERNEL);
@@ -101,26 +99,48 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "failed to allocate extcon device\n");
 		return -ENOMEM;
 	}
-	extcon_data->edev->name = pdata->name;
 
-	extcon_data->gpiod = gpio_to_desc(pdata->gpio);
-	extcon_data->state_on = pdata->state_on;
-	extcon_data->state_off = pdata->state_off;
-	extcon_data->check_on_resume = pdata->check_on_resume;
-	if (pdata->state_on && pdata->state_off)
-		extcon_data->edev->print_state = extcon_gpio_print_state;
+	if (np) {
+		int irq;
+
+		extcon_data->gpiod = gpiod_get(&pdev->dev, NULL);
+		if (IS_ERR(extcon_data->gpiod))
+			return PTR_ERR(extcon_data->gpiod);
+
+		extcon_data->edev->name = np->name;
+		of_property_read_u32(np, "debounce", &debounce);
+		irq = gpiod_to_irq(extcon_data->gpiod);
+		irq_flags = irq_get_trigger_type(irq);
+	} else {
+		if (!pdata)
+			return -EBUSY;
+		if (!pdata->irq_flags) {
+			dev_err(&pdev->dev, "IRQ flag is not specified.\n");
+			return -EINVAL;
+		}
+		extcon_data->edev->name = pdata->name;
+		extcon_data->state_on = pdata->state_on;
+		extcon_data->state_off = pdata->state_off;
+		extcon_data->check_on_resume = pdata->check_on_resume;
+		if (pdata->state_on && pdata->state_off)
+			extcon_data->edev->print_state = extcon_gpio_print_state;
+
+		extcon_data->gpiod = gpio_to_desc(pdata->gpio);
+		ret = devm_gpio_request_one(&pdev->dev, pdata->gpio,
+					    GPIOF_DIR_IN, pdev->name);
+		if (ret < 0)
+			return ret;
+		irq_flags = pdata->irq_flags;
+		debounce = pdata->debounce;
+	}
 
-	ret = devm_gpio_request_one(&pdev->dev, pdata->gpio, GPIOF_DIR_IN,
-				    pdev->name);
-	if (ret < 0)
-		return ret;
 
-	if (pdata->debounce) {
+	if (debounce) {
 		ret = gpiod_set_debounce(extcon_data->gpiod,
-					 pdata->debounce * 1000);
+					 debounce * 1000);
 		if (ret < 0)
 			extcon_data->debounce_jiffies =
-				msecs_to_jiffies(pdata->debounce);
+				msecs_to_jiffies(debounce);
 	}
 
 	ret = devm_extcon_dev_register(&pdev->dev, extcon_data->edev);
@@ -134,7 +154,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 		return extcon_data->irq;
 
 	ret = request_any_context_irq(extcon_data->irq, gpio_irq_handler,
-				      pdata->irq_flags, pdev->name,
+				      irq_flags, pdev->name,
 				      extcon_data);
 	if (ret < 0)
 		return ret;
@@ -172,6 +192,11 @@ static int gpio_extcon_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(gpio_extcon_pm_ops, NULL, gpio_extcon_resume);
 
+static struct of_device_id of_extcon_gpio_match_tbl[] = {
+	{ .compatible = "linux,extcon-gpio", },
+	{ /* end */ }
+};
+
 static struct platform_driver gpio_extcon_driver = {
 	.probe		= gpio_extcon_probe,
 	.remove		= gpio_extcon_remove,
@@ -179,6 +204,7 @@ static struct platform_driver gpio_extcon_driver = {
 		.name	= "extcon-gpio",
 		.owner	= THIS_MODULE,
 		.pm	= &gpio_extcon_pm_ops,
+		.of_match_table = of_extcon_gpio_match_tbl,
 	},
 };
 
-- 
1.8.3.1

[PATCH 4/5] extcon: gpio: Always use gpio_get_value_cansleep

[George Cherian]

Some gpio's can sleep while reading, so always use gpio_get_value_cansleep
to get data. This fixes warning from gpiolib due to wrong API usage.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/extcon/extcon-gpio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
index 2bfbd2e..3839749 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -51,7 +51,7 @@ static void gpio_extcon_work(struct work_struct *work)
 		container_of(to_delayed_work(work), struct gpio_extcon_data,
 			     work);
 
-	state = gpiod_get_value(data->gpiod);
+	state = gpiod_get_value_cansleep(data->gpiod);
 	extcon_set_state(data->edev, state);
 }
 
-- 
1.8.3.1

[PATCH 5/5] extcon: gpio: Add support for using cable names

[George Cherian]

Add support for using cable names. Enables other drivers to register interest
and get notified using extcon provided notifier call backs.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 Documentation/devicetree/bindings/extcon/extcon-gpio.txt | 2 ++
 drivers/extcon/extcon-gpio.c                             | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/extcon/extcon-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
index 30aa2e1..2c9d29f 100644
--- a/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
+++ b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
@@ -7,6 +7,7 @@ Required Properties:
  - compatible: should be:
 	* "linux,extcon-gpio"
  - gpios: specifies the gpio pin used.
+ - cable-name: Name of the cable used.
 
 Optional Properties:
  - debounce: Debounce time for GPIO IRQ in ms
@@ -18,4 +19,5 @@ Eg:
 		compatible = "linux,extcon-gpio";
 		gpios = <&gpio3 12 GPIO_ACTIVE_HIGH>;
 		debounce = <20>;
+		cable-name = "USB-HOST";
 		};
diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
index 3839749..c0ab8e7 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -42,6 +42,7 @@ struct gpio_extcon_data {
 	struct delayed_work work;
 	unsigned long debounce_jiffies;
 	bool check_on_resume;
+	const char *cable_name[1];
 };
 
 static void gpio_extcon_work(struct work_struct *work)
@@ -111,6 +112,9 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 		of_property_read_u32(np, "debounce", &debounce);
 		irq = gpiod_to_irq(extcon_data->gpiod);
 		irq_flags = irq_get_trigger_type(irq);
+		of_property_read_string_index(np, "cable-name", 0,
+					      extcon_data->cable_name);
+		extcon_data->edev->supported_cable = extcon_data->cable_name;
 	} else {
 		if (!pdata)
 			return -EBUSY;
-- 
1.8.3.1

Re: [PATCH 0/5] Add dt support for extcon gpio driver

[George Cherian]

Hi Chanwoo,

Could you please look into this series?

On 09/09/2014 09:44 AM, George Cherian wrote:
> This series
>          - Convert extcon-gpio driver to use gpiod_* API's
>          - Adds dt support to extcon-gpio driver.
> 	- Add cable name support in case of dt.
>
> George Cherian (5):
>    extcon: gpio: Minor cleanups
>    extcon: gpio: Convert the driver to use gpio desc API's
>    extcon: gpio: Add dt support for the driver.
>    extcon: gpio: Always use  gpio_get_value_cansleep
>    extcon: gpio: Add support for using cable names
>
>   .../devicetree/bindings/extcon/extcon-gpio.txt     |  23 +++++
>   drivers/extcon/extcon-gpio.c                       | 100 +++++++++++++--------
>   include/linux/extcon/extcon-gpio.h                 |   4 +-
>   3 files changed, 89 insertions(+), 38 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/extcon/extcon-gpio.txt
>
Regards
-George

Re: [PATCH 1/5] extcon: gpio: Minor cleanups

[Chanwoo Choi]

Hi George,

On 09/09/2014 01:14 PM, George Cherian wrote:
> Minor Cleanups
>  - Order the include files in alphabetical order.
>  - Fix description of state_off in extcon_gpio.h
>  - Add a descrition for check_on_resume in extcon_gpio.h
> 
> Signed-off-by: George Cherian <george.cherian@ti.com>

I modify the name/description of patch to keep standary coding style
and applied it.

Thanks,
Chanwoo Choi

> ---
>  drivers/extcon/extcon-gpio.c       | 10 +++++-----
>  include/linux/extcon/extcon-gpio.h |  4 +++-
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
> index 5b7ec27..72f19a3 100644
> --- a/drivers/extcon/extcon-gpio.c
> +++ b/drivers/extcon/extcon-gpio.c
> @@ -20,16 +20,16 @@
>   *
>  */
>  
> -#include <linux/module.h>
> -#include <linux/kernel.h>
> +#include <linux/extcon.h>
> +#include <linux/extcon/extcon-gpio.h>
> +#include <linux/gpio.h>
>  #include <linux/init.h>
>  #include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/workqueue.h>
> -#include <linux/gpio.h>
> -#include <linux/extcon.h>
> -#include <linux/extcon/extcon-gpio.h>
>  
>  struct gpio_extcon_data {
>  	struct extcon_dev *edev;
> diff --git a/include/linux/extcon/extcon-gpio.h b/include/linux/extcon/extcon-gpio.h
> index 8900fdf..0b17ad4 100644
> --- a/include/linux/extcon/extcon-gpio.h
> +++ b/include/linux/extcon/extcon-gpio.h
> @@ -34,8 +34,10 @@
>   * @irq_flags:		IRQ Flags (e.g., IRQF_TRIGGER_LOW).
>   * @state_on:		print_state is overriden with state_on if attached.
>   *			If NULL, default method of extcon class is used.
> - * @state_off:		print_state is overriden with state_on if detached.
> + * @state_off:		print_state is overriden with state_off if detached.
>   *			If NUll, default method of extcon class is used.
> + * @check_on_resume:	Boolean describing whether to check the state of gpio
> + *			while resuming from sleep.
>   *
>   * Note that in order for state_on or state_off to be valid, both state_on
>   * and state_off should be not NULL. If at least one of them is NULL,
> 

Re: [PATCH 2/5] extcon: gpio: Convert the driver to use gpio desc API's

[Chanwoo Choi]

Hi George,

This patch removes 'gpio_active_low' field of struct gpio_extcon_data.
But, include/linux/extcon-gpio.h has the description of 'gpio_active_low' field.

Also,
This patch has not included the any description/comment of removing 'gpio_active_low'.

Also,
How to set 'FLAG_ACTIVE_LOW' bit for gpio when using platform data?
This patch don't call 'set_bit()' function to set FLAG_ACTIVE_LOW flag.

Thanks,
Chanwoo Choi

On 09/09/2014 01:14 PM, George Cherian wrote:
> Convert the driver to use gpiod_* API's.
> 
> Signed-off-by: George Cherian <george.cherian@ti.com>
> ---
>  drivers/extcon/extcon-gpio.c | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
> index 72f19a3..25269f6 100644
> --- a/drivers/extcon/extcon-gpio.c
> +++ b/drivers/extcon/extcon-gpio.c
> @@ -33,8 +33,7 @@
>  
>  struct gpio_extcon_data {
>  	struct extcon_dev *edev;
> -	unsigned gpio;
> -	bool gpio_active_low;
> +	struct gpio_desc *gpiod;
>  	const char *state_on;
>  	const char *state_off;
>  	int irq;
> @@ -50,9 +49,7 @@ static void gpio_extcon_work(struct work_struct *work)
>  		container_of(to_delayed_work(work), struct gpio_extcon_data,
>  			     work);
>  
> -	state = gpio_get_value(data->gpio);
> -	if (data->gpio_active_low)
> -		state = !state;
> +	state = gpiod_get_value(data->gpiod);
>  	extcon_set_state(data->edev, state);
>  }
>  
> @@ -106,22 +103,21 @@ static int gpio_extcon_probe(struct platform_device *pdev)
>  	}
>  	extcon_data->edev->name = pdata->name;
>  
> -	extcon_data->gpio = pdata->gpio;
> -	extcon_data->gpio_active_low = pdata->gpio_active_low;
> +	extcon_data->gpiod = gpio_to_desc(pdata->gpio);
>  	extcon_data->state_on = pdata->state_on;
>  	extcon_data->state_off = pdata->state_off;
>  	extcon_data->check_on_resume = pdata->check_on_resume;
>  	if (pdata->state_on && pdata->state_off)
>  		extcon_data->edev->print_state = extcon_gpio_print_state;
>  
> -	ret = devm_gpio_request_one(&pdev->dev, extcon_data->gpio, GPIOF_DIR_IN,
> +	ret = devm_gpio_request_one(&pdev->dev, pdata->gpio, GPIOF_DIR_IN,
>  				    pdev->name);
>  	if (ret < 0)
>  		return ret;
>  
>  	if (pdata->debounce) {
> -		ret = gpio_set_debounce(extcon_data->gpio,
> -					pdata->debounce * 1000);
> +		ret = gpiod_set_debounce(extcon_data->gpiod,
> +					 pdata->debounce * 1000);
>  		if (ret < 0)
>  			extcon_data->debounce_jiffies =
>  				msecs_to_jiffies(pdata->debounce);
> @@ -133,7 +129,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
>  
>  	INIT_DELAYED_WORK(&extcon_data->work, gpio_extcon_work);
>  
> -	extcon_data->irq = gpio_to_irq(extcon_data->gpio);
> +	extcon_data->irq = gpiod_to_irq(extcon_data->gpiod);
>  	if (extcon_data->irq < 0)
>  		return extcon_data->irq;
>  
> 

Re: [PATCH 2/5] extcon: gpio: Convert the driver to use gpio desc API's

[George Cherian]

On 09/22/2014 01:37 PM, Chanwoo Choi wrote:
> Hi George,
>
> This patch removes 'gpio_active_low' field of struct gpio_extcon_data.
> But, include/linux/extcon-gpio.h has the description of 'gpio_active_low' field.
Yes didn't want the platform data users to break.
Actually I couldn't find any platform users for this driver. Could you 
please point me to
one if in case I missed it. If non present then why cant we get rid of 
platform data altogether.
>
> Also,
> This patch has not included the any description/comment of removing 'gpio_active_low'.
>
> Also,
> How to set 'FLAG_ACTIVE_LOW' bit for gpio when using platform data?
Now that we are using gpiod_* API's  we need not check for 
gpio_active_low from this driver.

> This patch don't call 'set_bit()' function to set FLAG_ACTIVE_LOW flag.
>
> Thanks,
> Chanwoo Choi
>
> On 09/09/2014 01:14 PM, George Cherian wrote:
>> Convert the driver to use gpiod_* API's.
>>
>> Signed-off-by: George Cherian <george.cherian@ti.com>
>> ---
>>   drivers/extcon/extcon-gpio.c | 18 +++++++-----------
>>   1 file changed, 7 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
>> index 72f19a3..25269f6 100644
>> --- a/drivers/extcon/extcon-gpio.c
>> +++ b/drivers/extcon/extcon-gpio.c
>> @@ -33,8 +33,7 @@
>>   
>>   struct gpio_extcon_data {
>>   	struct extcon_dev *edev;
>> -	unsigned gpio;
>> -	bool gpio_active_low;
>> +	struct gpio_desc *gpiod;
>>   	const char *state_on;
>>   	const char *state_off;
>>   	int irq;
>> @@ -50,9 +49,7 @@ static void gpio_extcon_work(struct work_struct *work)
>>   		container_of(to_delayed_work(work), struct gpio_extcon_data,
>>   			     work);
>>   
>> -	state = gpio_get_value(data->gpio);
>> -	if (data->gpio_active_low)
>> -		state = !state;
>> +	state = gpiod_get_value(data->gpiod);
>>   	extcon_set_state(data->edev, state);
>>   }
>>   
>> @@ -106,22 +103,21 @@ static int gpio_extcon_probe(struct platform_device *pdev)
>>   	}
>>   	extcon_data->edev->name = pdata->name;
>>   
>> -	extcon_data->gpio = pdata->gpio;
>> -	extcon_data->gpio_active_low = pdata->gpio_active_low;
>> +	extcon_data->gpiod = gpio_to_desc(pdata->gpio);
>>   	extcon_data->state_on = pdata->state_on;
>>   	extcon_data->state_off = pdata->state_off;
>>   	extcon_data->check_on_resume = pdata->check_on_resume;
>>   	if (pdata->state_on && pdata->state_off)
>>   		extcon_data->edev->print_state = extcon_gpio_print_state;
>>   
>> -	ret = devm_gpio_request_one(&pdev->dev, extcon_data->gpio, GPIOF_DIR_IN,
>> +	ret = devm_gpio_request_one(&pdev->dev, pdata->gpio, GPIOF_DIR_IN,
>>   				    pdev->name);
>>   	if (ret < 0)
>>   		return ret;
>>   
>>   	if (pdata->debounce) {
>> -		ret = gpio_set_debounce(extcon_data->gpio,
>> -					pdata->debounce * 1000);
>> +		ret = gpiod_set_debounce(extcon_data->gpiod,
>> +					 pdata->debounce * 1000);
>>   		if (ret < 0)
>>   			extcon_data->debounce_jiffies =
>>   				msecs_to_jiffies(pdata->debounce);
>> @@ -133,7 +129,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
>>   
>>   	INIT_DELAYED_WORK(&extcon_data->work, gpio_extcon_work);
>>   
>> -	extcon_data->irq = gpio_to_irq(extcon_data->gpio);
>> +	extcon_data->irq = gpiod_to_irq(extcon_data->gpiod);
>>   	if (extcon_data->irq < 0)
>>   		return extcon_data->irq;
>>   
>>

Re: [PATCH 2/5] extcon: gpio: Convert the driver to use gpio desc API's

[Chanwoo Choi]

On 09/22/2014 06:51 PM, George Cherian wrote:
> 
> On 09/22/2014 01:37 PM, Chanwoo Choi wrote:
>> Hi George,
>>
>> This patch removes 'gpio_active_low' field of struct gpio_extcon_data.
>> But, include/linux/extcon-gpio.h has the description of 'gpio_active_low' field.
> Yes didn't want the platform data users to break.
> Actually I couldn't find any platform users for this driver. Could you please point me to
> one if in case I missed it. If non present then why cant we get rid of platform data altogether.

Right,
But, Why do you support platform data on as following your patch?
- [PATCH 3/5] extcon: gpio: Add dt support for the driver.
According to your comment, you had to remove the support for platform data.

IMO,
I think this patchset must need to reorder the sequence of patchset.
Also, this patchset is more detailed description.

>>
>> Also,
>> This patch has not included the any description/comment of removing 'gpio_active_low'.
>>
>> Also,
>> How to set 'FLAG_ACTIVE_LOW' bit for gpio when using platform data?
> Now that we are using gpiod_* API's  we need not check for gpio_active_low from this driver.

This patch just use gpiod API instead of legacy gpio API.

I think that if extcon-gpio don't need to check gpio_activ_low field,
you have to implement dt support patch before this patch.

> 
>> This patch don't call 'set_bit()' function to set FLAG_ACTIVE_LOW flag.
>>
>> Thanks,
>> Chanwoo Choi
>>
>> On 09/09/2014 01:14 PM, George Cherian wrote:
>>> Convert the driver to use gpiod_* API's.
>>>
>>> Signed-off-by: George Cherian <george.cherian@ti.com>
>>> ---
>>>   drivers/extcon/extcon-gpio.c | 18 +++++++-----------
>>>   1 file changed, 7 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
>>> index 72f19a3..25269f6 100644
>>> --- a/drivers/extcon/extcon-gpio.c
>>> +++ b/drivers/extcon/extcon-gpio.c
>>> @@ -33,8 +33,7 @@
>>>     struct gpio_extcon_data {
>>>       struct extcon_dev *edev;
>>> -    unsigned gpio;
>>> -    bool gpio_active_low;
>>> +    struct gpio_desc *gpiod;
>>>       const char *state_on;
>>>       const char *state_off;
>>>       int irq;
>>> @@ -50,9 +49,7 @@ static void gpio_extcon_work(struct work_struct *work)
>>>           container_of(to_delayed_work(work), struct gpio_extcon_data,
>>>                    work);
>>>   -    state = gpio_get_value(data->gpio);
>>> -    if (data->gpio_active_low)
>>> -        state = !state;
>>> +    state = gpiod_get_value(data->gpiod);
>>>       extcon_set_state(data->edev, state);
>>>   }
>>>   @@ -106,22 +103,21 @@ static int gpio_extcon_probe(struct platform_device *pdev)
>>>       }
>>>       extcon_data->edev->name = pdata->name;
>>>   -    extcon_data->gpio = pdata->gpio;
>>> -    extcon_data->gpio_active_low = pdata->gpio_active_low;
>>> +    extcon_data->gpiod = gpio_to_desc(pdata->gpio);
>>>       extcon_data->state_on = pdata->state_on;
>>>       extcon_data->state_off = pdata->state_off;
>>>       extcon_data->check_on_resume = pdata->check_on_resume;
>>>       if (pdata->state_on && pdata->state_off)
>>>           extcon_data->edev->print_state = extcon_gpio_print_state;
>>>   -    ret = devm_gpio_request_one(&pdev->dev, extcon_data->gpio, GPIOF_DIR_IN,
>>> +    ret = devm_gpio_request_one(&pdev->dev, pdata->gpio, GPIOF_DIR_IN,
>>>                       pdev->name);
>>>       if (ret < 0)
>>>           return ret;
>>>         if (pdata->debounce) {
>>> -        ret = gpio_set_debounce(extcon_data->gpio,
>>> -                    pdata->debounce * 1000);
>>> +        ret = gpiod_set_debounce(extcon_data->gpiod,
>>> +                     pdata->debounce * 1000);
>>>           if (ret < 0)
>>>               extcon_data->debounce_jiffies =
>>>                   msecs_to_jiffies(pdata->debounce);
>>> @@ -133,7 +129,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
>>>         INIT_DELAYED_WORK(&extcon_data->work, gpio_extcon_work);
>>>   -    extcon_data->irq = gpio_to_irq(extcon_data->gpio);
>>> +    extcon_data->irq = gpiod_to_irq(extcon_data->gpiod);
>>>       if (extcon_data->irq < 0)
>>>           return extcon_data->irq;
>>>  
> 
> 

Re: [PATCH 2/5] extcon: gpio: Convert the driver to use gpio desc API's

[George Cherian]

On 09/23/2014 04:44 AM, Chanwoo Choi wrote:
> On 09/22/2014 06:51 PM, George Cherian wrote:
>> On 09/22/2014 01:37 PM, Chanwoo Choi wrote:
>>> Hi George,
>>>
>>> This patch removes 'gpio_active_low' field of struct gpio_extcon_data.
>>> But, include/linux/extcon-gpio.h has the description of 'gpio_active_low' field.
>> Yes didn't want the platform data users to break.
>> Actually I couldn't find any platform users for this driver. Could you please point me to
>> one if in case I missed it. If non present then why cant we get rid of platform data altogether.
> Right,
> But, Why do you support platform data on as following your patch?
> - [PATCH 3/5] extcon: gpio: Add dt support for the driver.
> According to your comment, you had to remove the support for platform data.
My intention with this series was to add dt support by keeping the 
existing platform data.
Now that we know there are no platform data users I will rework on this 
and keep only dt
support.
>
> IMO,
> I think this patchset must need to reorder the sequence of patchset.
> Also, this patchset is more detailed description.
I will rework and submit a v2.
>>> Also,
>>> This patch has not included the any description/comment of removing 'gpio_active_low'.
>>>
>>> Also,
>>> How to set 'FLAG_ACTIVE_LOW' bit for gpio when using platform data?
>> Now that we are using gpiod_* API's  we need not check for gpio_active_low from this driver.
> This patch just use gpiod API instead of legacy gpio API.
>
> I think that if extcon-gpio don't need to check gpio_activ_low field,
> you have to implement dt support patch before this patch.
yes will do in v2

Thanks for your review.
>>> This patch don't call 'set_bit()' function to set FLAG_ACTIVE_LOW flag.
>>>
>>> Thanks,
>>> Chanwoo Choi
>>>
>>> On 09/09/2014 01:14 PM, George Cherian wrote:
>>>> Convert the driver to use gpiod_* API's.
>>>>
>>>> Signed-off-by: George Cherian <george.cherian@ti.com>
>>>> ---
>>>>    drivers/extcon/extcon-gpio.c | 18 +++++++-----------
>>>>    1 file changed, 7 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
>>>> index 72f19a3..25269f6 100644
>>>> --- a/drivers/extcon/extcon-gpio.c
>>>> +++ b/drivers/extcon/extcon-gpio.c
>>>> @@ -33,8 +33,7 @@
>>>>      struct gpio_extcon_data {
>>>>        struct extcon_dev *edev;
>>>> -    unsigned gpio;
>>>> -    bool gpio_active_low;
>>>> +    struct gpio_desc *gpiod;
>>>>        const char *state_on;
>>>>        const char *state_off;
>>>>        int irq;
>>>> @@ -50,9 +49,7 @@ static void gpio_extcon_work(struct work_struct *work)
>>>>            container_of(to_delayed_work(work), struct gpio_extcon_data,
>>>>                     work);
>>>>    -    state = gpio_get_value(data->gpio);
>>>> -    if (data->gpio_active_low)
>>>> -        state = !state;
>>>> +    state = gpiod_get_value(data->gpiod);
>>>>        extcon_set_state(data->edev, state);
>>>>    }
>>>>    @@ -106,22 +103,21 @@ static int gpio_extcon_probe(struct platform_device *pdev)
>>>>        }
>>>>        extcon_data->edev->name = pdata->name;
>>>>    -    extcon_data->gpio = pdata->gpio;
>>>> -    extcon_data->gpio_active_low = pdata->gpio_active_low;
>>>> +    extcon_data->gpiod = gpio_to_desc(pdata->gpio);
>>>>        extcon_data->state_on = pdata->state_on;
>>>>        extcon_data->state_off = pdata->state_off;
>>>>        extcon_data->check_on_resume = pdata->check_on_resume;
>>>>        if (pdata->state_on && pdata->state_off)
>>>>            extcon_data->edev->print_state = extcon_gpio_print_state;
>>>>    -    ret = devm_gpio_request_one(&pdev->dev, extcon_data->gpio, GPIOF_DIR_IN,
>>>> +    ret = devm_gpio_request_one(&pdev->dev, pdata->gpio, GPIOF_DIR_IN,
>>>>                        pdev->name);
>>>>        if (ret < 0)
>>>>            return ret;
>>>>          if (pdata->debounce) {
>>>> -        ret = gpio_set_debounce(extcon_data->gpio,
>>>> -                    pdata->debounce * 1000);
>>>> +        ret = gpiod_set_debounce(extcon_data->gpiod,
>>>> +                     pdata->debounce * 1000);
>>>>            if (ret < 0)
>>>>                extcon_data->debounce_jiffies =
>>>>                    msecs_to_jiffies(pdata->debounce);
>>>> @@ -133,7 +129,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
>>>>          INIT_DELAYED_WORK(&extcon_data->work, gpio_extcon_work);
>>>>    -    extcon_data->irq = gpio_to_irq(extcon_data->gpio);
>>>> +    extcon_data->irq = gpiod_to_irq(extcon_data->gpiod);
>>>>        if (extcon_data->irq < 0)
>>>>            return extcon_data->irq;
>>>>   
>>

-George