Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1738203 > unrolled thread

[PATCH 0/8] GPIO extcon modernization

Started byLinus Walleij <linus.walleij@linaro.org>
First post2017-09-24 17:00 +0200
Last post2017-09-26 04:30 +0200
Articles 12 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/8] GPIO extcon modernization Linus Walleij <linus.walleij@linaro.org> - 2017-09-24 17:00 +0200
    [PATCH 3/8] extcon: gpio: Move platform data into state container Linus Walleij <linus.walleij@linaro.org> - 2017-09-24 17:00 +0200
      Re: [PATCH 3/8] extcon: gpio: Move platform data into state  container Chanwoo Choi <cw00.choi@samsung.com> - 2017-09-26 04:10 +0200
    [PATCH 4/8] extcon: gpio: Convert to fully use GPIO descriptor Linus Walleij <linus.walleij@linaro.org> - 2017-09-24 17:10 +0200
      Re: [PATCH 4/8] extcon: gpio: Convert to fully use GPIO descriptor Chanwoo Choi <cw00.choi@samsung.com> - 2017-09-26 04:20 +0200
    [PATCH 7/8] extcon: gpio: Get connector type from device property Linus Walleij <linus.walleij@linaro.org> - 2017-09-24 17:10 +0200
    [PATCH 8/8] extcon: gpio: Always check state on resume Linus Walleij <linus.walleij@linaro.org> - 2017-09-24 17:10 +0200
      Re: [PATCH 8/8] extcon: gpio: Always check state on resume Chanwoo Choi <cw00.choi@samsung.com> - 2017-09-26 04:30 +0200
    [PATCH 5/8] extcon: gpio: Request reasonable interrupts Linus Walleij <linus.walleij@linaro.org> - 2017-09-24 17:10 +0200
      Re: [PATCH 5/8] extcon: gpio: Request reasonable interrupts Chanwoo Choi <cw00.choi@samsung.com> - 2017-09-26 04:30 +0200
    [PATCH 6/8] extcon: gpio: Get debounce setting from device property Linus Walleij <linus.walleij@linaro.org> - 2017-09-24 17:10 +0200
      Re: [PATCH 6/8] extcon: gpio: Get debounce setting from device  property Chanwoo Choi <cw00.choi@samsung.com> - 2017-09-26 04:30 +0200

#1738203 — [PATCH 0/8] GPIO extcon modernization

FromLinus Walleij <linus.walleij@linaro.org>
Date2017-09-24 17:00 +0200
Subject[PATCH 0/8] GPIO extcon modernization
Message-ID<utgxX-1yO-3@gated-at.bofh.it>
Like with the GPIO mouse input, I found that this driver has no in-tree
users at all.

I *could* just propose to delete it. But the driver seems generally useful,
so I made a patch series shaping it up to get configuration from device
tree or ACPI DSDT using device properties instead.

We start off the series by defining a set of device tree bindings for it.

It would be great to have some input from the initial authors on this
rewrite. I think we would not merge any board file using the platform
data anymore, so making it an all-in device property (device tree || ACPI)
driver seems reasonable.

Linus Walleij (8):
  extcon: gpio: Add DT bindings
  extcon: gpio: Localize platform data
  extcon: gpio: Move platform data into state container
  extcon: gpio: Convert to fully use GPIO descriptor
  extcon: gpio: Request reasonable interrupts
  extcon: gpio: Get debounce setting from device property
  extcon: gpio: Get connector type from device property
  extcon: gpio: Always check state on resume

 .../devicetree/bindings/extcon/extcon-gpio.txt     |  24 ++++
 drivers/extcon/extcon-gpio.c                       | 132 ++++++++++++---------
 include/dt-bindings/extcon/connectors.h            |  38 ++++++
 include/linux/extcon/extcon-gpio.h                 |  47 --------
 4 files changed, 135 insertions(+), 106 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/extcon/extcon-gpio.txt
 create mode 100644 include/dt-bindings/extcon/connectors.h
 delete mode 100644 include/linux/extcon/extcon-gpio.h

-- 
2.13.5

[toc] | [next] | [standalone]


#1738204 — [PATCH 3/8] extcon: gpio: Move platform data into state container

FromLinus Walleij <linus.walleij@linaro.org>
Date2017-09-24 17:00 +0200
Subject[PATCH 3/8] extcon: gpio: Move platform data into state container
Message-ID<utgxY-1yO-7@gated-at.bofh.it>
In reply to#1738203
This moves the platform data settings from the platform data
struct and into the state container, saving some unnecessary
references and simplifying things a bit.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/extcon/extcon-gpio.c | 56 ++++++++++++++++++++------------------------
 1 file changed, 26 insertions(+), 30 deletions(-)

diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
index 6abf5f70fdbf..9c4094edd123 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -29,7 +29,13 @@
 #include <linux/workqueue.h>
 
 /**
- * struct gpio_extcon_pdata - A simple GPIO-controlled extcon device.
+ * struct gpio_extcon_data - A simple GPIO-controlled extcon device state container.
+ * @edev:		Extcon device.
+ * @irq:		Interrupt line for the external connector.
+ * @work:		Work fired by the interrupt.
+ * @debounce_jiffies:	Number of jiffies to wait for the GPIO to stabilize, from the debounce
+ *			value.
+ * @id_gpiod:		GPIO descriptor for this external connector.
  * @extcon_id:		The unique id of specific external connector.
  * @gpio:		Corresponding GPIO.
  * @gpio_active_low:	Boolean describing whether gpio active state is 1 or 0
@@ -40,23 +46,18 @@
  * @check_on_resume:	Boolean describing whether to check the state of gpio
  *			while resuming from sleep.
  */
-struct gpio_extcon_pdata {
-	unsigned int extcon_id;
-	unsigned gpio;
-	bool gpio_active_low;
-	unsigned long debounce;
-	unsigned long irq_flags;
-	bool check_on_resume;
-};
-
 struct gpio_extcon_data {
 	struct extcon_dev *edev;
 	int irq;
 	struct delayed_work work;
 	unsigned long debounce_jiffies;
-
 	struct gpio_desc *id_gpiod;
-	struct gpio_extcon_pdata *pdata;
+	unsigned int extcon_id;
+	unsigned gpio;
+	bool gpio_active_low;
+	unsigned long debounce;
+	unsigned long irq_flags;
+	bool check_on_resume;
 };
 
 static void gpio_extcon_work(struct work_struct *work)
@@ -67,10 +68,10 @@ static void gpio_extcon_work(struct work_struct *work)
 			     work);
 
 	state = gpiod_get_value_cansleep(data->id_gpiod);
-	if (data->pdata->gpio_active_low)
+	if (data->gpio_active_low)
 		state = !state;
 
-	extcon_set_state_sync(data->edev, data->pdata->extcon_id, state);
+	extcon_set_state_sync(data->edev, data->extcon_id, state);
 }
 
 static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
@@ -84,24 +85,23 @@ static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
 
 static int gpio_extcon_init(struct device *dev, struct gpio_extcon_data *data)
 {
-	struct gpio_extcon_pdata *pdata = data->pdata;
 	int ret;
 
-	ret = devm_gpio_request_one(dev, pdata->gpio, GPIOF_DIR_IN,
+	ret = devm_gpio_request_one(dev, data->gpio, GPIOF_DIR_IN,
 				dev_name(dev));
 	if (ret < 0)
 		return ret;
 
-	data->id_gpiod = gpio_to_desc(pdata->gpio);
+	data->id_gpiod = gpio_to_desc(data->gpio);
 	if (!data->id_gpiod)
 		return -EINVAL;
 
-	if (pdata->debounce) {
+	if (data->debounce) {
 		ret = gpiod_set_debounce(data->id_gpiod,
-					pdata->debounce * 1000);
+					 data->debounce * 1000);
 		if (ret < 0)
 			data->debounce_jiffies =
-				msecs_to_jiffies(pdata->debounce);
+				msecs_to_jiffies(data->debounce);
 	}
 
 	data->irq = gpiod_to_irq(data->id_gpiod);
@@ -113,20 +113,16 @@ static int gpio_extcon_init(struct device *dev, struct gpio_extcon_data *data)
 
 static int gpio_extcon_probe(struct platform_device *pdev)
 {
-	struct gpio_extcon_pdata *pdata = dev_get_platdata(&pdev->dev);
 	struct gpio_extcon_data *data;
 	int ret;
 
-	if (!pdata)
-		return -EBUSY;
-	if (!pdata->irq_flags || pdata->extcon_id > EXTCON_NONE)
-		return -EINVAL;
-
 	data = devm_kzalloc(&pdev->dev, sizeof(struct gpio_extcon_data),
 				   GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
-	data->pdata = pdata;
+
+	if (!data->irq_flags || data->extcon_id > EXTCON_NONE)
+		return -EINVAL;
 
 	/* Initialize the gpio */
 	ret = gpio_extcon_init(&pdev->dev, data);
@@ -134,7 +130,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 		return ret;
 
 	/* Allocate the memory of extcon devie and register extcon device */
-	data->edev = devm_extcon_dev_allocate(&pdev->dev, &pdata->extcon_id);
+	data->edev = devm_extcon_dev_allocate(&pdev->dev, &data->extcon_id);
 	if (IS_ERR(data->edev)) {
 		dev_err(&pdev->dev, "failed to allocate extcon device\n");
 		return -ENOMEM;
@@ -151,7 +147,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 	 * is attached or detached.
 	 */
 	ret = devm_request_any_context_irq(&pdev->dev, data->irq,
-					gpio_irq_handler, pdata->irq_flags,
+					gpio_irq_handler, data->irq_flags,
 					pdev->name, data);
 	if (ret < 0)
 		return ret;
@@ -178,7 +174,7 @@ static int gpio_extcon_resume(struct device *dev)
 	struct gpio_extcon_data *data;
 
 	data = dev_get_drvdata(dev);
-	if (data->pdata->check_on_resume)
+	if (data->check_on_resume)
 		queue_delayed_work(system_power_efficient_wq,
 			&data->work, data->debounce_jiffies);
 
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]


#1739430 — Re: [PATCH 3/8] extcon: gpio: Move platform data into state container

FromChanwoo Choi <cw00.choi@samsung.com>
Date2017-09-26 04:10 +0200
SubjectRe: [PATCH 3/8] extcon: gpio: Move platform data into state container
Message-ID<utNtU-6AN-3@gated-at.bofh.it>
In reply to#1738204
Hi Linus,

On 2017년 09월 24일 23:56, Linus Walleij wrote:
> This moves the platform data settings from the platform data
> struct and into the state container, saving some unnecessary
> references and simplifying things a bit.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/extcon/extcon-gpio.c | 56 ++++++++++++++++++++------------------------
>  1 file changed, 26 insertions(+), 30 deletions(-)

Looks good to me.
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>


> 
> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
> index 6abf5f70fdbf..9c4094edd123 100644
> --- a/drivers/extcon/extcon-gpio.c
> +++ b/drivers/extcon/extcon-gpio.c
> @@ -29,7 +29,13 @@
>  #include <linux/workqueue.h>
>  
>  /**
> - * struct gpio_extcon_pdata - A simple GPIO-controlled extcon device.
> + * struct gpio_extcon_data - A simple GPIO-controlled extcon device state container.
> + * @edev:		Extcon device.
> + * @irq:		Interrupt line for the external connector.
> + * @work:		Work fired by the interrupt.
> + * @debounce_jiffies:	Number of jiffies to wait for the GPIO to stabilize, from the debounce
> + *			value.
> + * @id_gpiod:		GPIO descriptor for this external connector.
>   * @extcon_id:		The unique id of specific external connector.
>   * @gpio:		Corresponding GPIO.
>   * @gpio_active_low:	Boolean describing whether gpio active state is 1 or 0
> @@ -40,23 +46,18 @@
>   * @check_on_resume:	Boolean describing whether to check the state of gpio
>   *			while resuming from sleep.
>   */
> -struct gpio_extcon_pdata {
> -	unsigned int extcon_id;
> -	unsigned gpio;
> -	bool gpio_active_low;
> -	unsigned long debounce;
> -	unsigned long irq_flags;
> -	bool check_on_resume;
> -};
> -
>  struct gpio_extcon_data {
>  	struct extcon_dev *edev;
>  	int irq;
>  	struct delayed_work work;
>  	unsigned long debounce_jiffies;
> -
>  	struct gpio_desc *id_gpiod;
> -	struct gpio_extcon_pdata *pdata;
> +	unsigned int extcon_id;
> +	unsigned gpio;
> +	bool gpio_active_low;
> +	unsigned long debounce;
> +	unsigned long irq_flags;
> +	bool check_on_resume;
>  };
>  
>  static void gpio_extcon_work(struct work_struct *work)
> @@ -67,10 +68,10 @@ static void gpio_extcon_work(struct work_struct *work)
>  			     work);
>  
>  	state = gpiod_get_value_cansleep(data->id_gpiod);
> -	if (data->pdata->gpio_active_low)
> +	if (data->gpio_active_low)
>  		state = !state;
>  
> -	extcon_set_state_sync(data->edev, data->pdata->extcon_id, state);
> +	extcon_set_state_sync(data->edev, data->extcon_id, state);
>  }
>  
>  static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
> @@ -84,24 +85,23 @@ static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
>  
>  static int gpio_extcon_init(struct device *dev, struct gpio_extcon_data *data)
>  {
> -	struct gpio_extcon_pdata *pdata = data->pdata;
>  	int ret;
>  
> -	ret = devm_gpio_request_one(dev, pdata->gpio, GPIOF_DIR_IN,
> +	ret = devm_gpio_request_one(dev, data->gpio, GPIOF_DIR_IN,
>  				dev_name(dev));
>  	if (ret < 0)
>  		return ret;
>  
> -	data->id_gpiod = gpio_to_desc(pdata->gpio);
> +	data->id_gpiod = gpio_to_desc(data->gpio);
>  	if (!data->id_gpiod)
>  		return -EINVAL;
>  
> -	if (pdata->debounce) {
> +	if (data->debounce) {
>  		ret = gpiod_set_debounce(data->id_gpiod,
> -					pdata->debounce * 1000);
> +					 data->debounce * 1000);
>  		if (ret < 0)
>  			data->debounce_jiffies =
> -				msecs_to_jiffies(pdata->debounce);
> +				msecs_to_jiffies(data->debounce);
>  	}
>  
>  	data->irq = gpiod_to_irq(data->id_gpiod);
> @@ -113,20 +113,16 @@ static int gpio_extcon_init(struct device *dev, struct gpio_extcon_data *data)
>  
>  static int gpio_extcon_probe(struct platform_device *pdev)
>  {
> -	struct gpio_extcon_pdata *pdata = dev_get_platdata(&pdev->dev);
>  	struct gpio_extcon_data *data;
>  	int ret;
>  
> -	if (!pdata)
> -		return -EBUSY;
> -	if (!pdata->irq_flags || pdata->extcon_id > EXTCON_NONE)
> -		return -EINVAL;
> -
>  	data = devm_kzalloc(&pdev->dev, sizeof(struct gpio_extcon_data),
>  				   GFP_KERNEL);
>  	if (!data)
>  		return -ENOMEM;
> -	data->pdata = pdata;
> +
> +	if (!data->irq_flags || data->extcon_id > EXTCON_NONE)
> +		return -EINVAL;
>  
>  	/* Initialize the gpio */
>  	ret = gpio_extcon_init(&pdev->dev, data);
> @@ -134,7 +130,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
>  		return ret;
>  
>  	/* Allocate the memory of extcon devie and register extcon device */
> -	data->edev = devm_extcon_dev_allocate(&pdev->dev, &pdata->extcon_id);
> +	data->edev = devm_extcon_dev_allocate(&pdev->dev, &data->extcon_id);
>  	if (IS_ERR(data->edev)) {
>  		dev_err(&pdev->dev, "failed to allocate extcon device\n");
>  		return -ENOMEM;
> @@ -151,7 +147,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
>  	 * is attached or detached.
>  	 */
>  	ret = devm_request_any_context_irq(&pdev->dev, data->irq,
> -					gpio_irq_handler, pdata->irq_flags,
> +					gpio_irq_handler, data->irq_flags,
>  					pdev->name, data);
>  	if (ret < 0)
>  		return ret;
> @@ -178,7 +174,7 @@ static int gpio_extcon_resume(struct device *dev)
>  	struct gpio_extcon_data *data;
>  
>  	data = dev_get_drvdata(dev);
> -	if (data->pdata->check_on_resume)
> +	if (data->check_on_resume)
>  		queue_delayed_work(system_power_efficient_wq,
>  			&data->work, data->debounce_jiffies);
>  
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

[toc] | [prev] | [next] | [standalone]


#1738206 — [PATCH 4/8] extcon: gpio: Convert to fully use GPIO descriptor

FromLinus Walleij <linus.walleij@linaro.org>
Date2017-09-24 17:10 +0200
Subject[PATCH 4/8] extcon: gpio: Convert to fully use GPIO descriptor
Message-ID<utgHD-1R2-1@gated-at.bofh.it>
In reply to#1738203
Since we are not getting the GPIO from any platform data and global
GPIO numberspace, we simply get the named "extcon" GPIO directly from
the device. Cut away "active low" since GPIO descriptors already know
if the line is active high or low. Simplify a bit with a
struct device *dev helper variable in probe() and cut the complex
init() function.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/extcon/extcon-gpio.c | 66 ++++++++++----------------------------------
 1 file changed, 15 insertions(+), 51 deletions(-)

diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
index 9c4094edd123..86f3ec6d6014 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -18,7 +18,6 @@
  */
 
 #include <linux/extcon.h>
-#include <linux/gpio.h>
 #include <linux/gpio/consumer.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
@@ -35,12 +34,8 @@
  * @work:		Work fired by the interrupt.
  * @debounce_jiffies:	Number of jiffies to wait for the GPIO to stabilize, from the debounce
  *			value.
- * @id_gpiod:		GPIO descriptor for this external connector.
+ * @gpiod:		GPIO descriptor for this external connector.
  * @extcon_id:		The unique id of specific external connector.
- * @gpio:		Corresponding GPIO.
- * @gpio_active_low:	Boolean describing whether gpio active state is 1 or 0
- *			If true, low state of gpio means active.
- *			If false, high state of gpio means active.
  * @debounce:		Debounce time for GPIO IRQ in ms.
  * @irq_flags:		IRQ Flags (e.g., IRQF_TRIGGER_LOW).
  * @check_on_resume:	Boolean describing whether to check the state of gpio
@@ -51,10 +46,8 @@ struct gpio_extcon_data {
 	int irq;
 	struct delayed_work work;
 	unsigned long debounce_jiffies;
-	struct gpio_desc *id_gpiod;
+	struct gpio_desc *gpiod;
 	unsigned int extcon_id;
-	unsigned gpio;
-	bool gpio_active_low;
 	unsigned long debounce;
 	unsigned long irq_flags;
 	bool check_on_resume;
@@ -67,10 +60,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_cansleep(data->id_gpiod);
-	if (data->gpio_active_low)
-		state = !state;
-
+	state = gpiod_get_value_cansleep(data->gpiod);
 	extcon_set_state_sync(data->edev, data->extcon_id, state);
 }
 
@@ -83,60 +73,34 @@ static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int gpio_extcon_init(struct device *dev, struct gpio_extcon_data *data)
-{
-	int ret;
-
-	ret = devm_gpio_request_one(dev, data->gpio, GPIOF_DIR_IN,
-				dev_name(dev));
-	if (ret < 0)
-		return ret;
-
-	data->id_gpiod = gpio_to_desc(data->gpio);
-	if (!data->id_gpiod)
-		return -EINVAL;
-
-	if (data->debounce) {
-		ret = gpiod_set_debounce(data->id_gpiod,
-					 data->debounce * 1000);
-		if (ret < 0)
-			data->debounce_jiffies =
-				msecs_to_jiffies(data->debounce);
-	}
-
-	data->irq = gpiod_to_irq(data->id_gpiod);
-	if (data->irq < 0)
-		return data->irq;
-
-	return 0;
-}
-
 static int gpio_extcon_probe(struct platform_device *pdev)
 {
 	struct gpio_extcon_data *data;
+	struct device *dev = &pdev->dev;
 	int ret;
 
-	data = devm_kzalloc(&pdev->dev, sizeof(struct gpio_extcon_data),
-				   GFP_KERNEL);
+	data = devm_kzalloc(dev, sizeof(struct gpio_extcon_data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
 	if (!data->irq_flags || data->extcon_id > EXTCON_NONE)
 		return -EINVAL;
 
-	/* Initialize the gpio */
-	ret = gpio_extcon_init(&pdev->dev, data);
-	if (ret < 0)
-		return ret;
+	data->gpiod = devm_gpiod_get(dev, "extcon", GPIOD_IN);
+	if (IS_ERR(data->gpiod))
+		return PTR_ERR(data->gpiod);
+	data->irq = gpiod_to_irq(data->gpiod);
+	if (data->irq <= 0)
+		return data->irq;
 
 	/* Allocate the memory of extcon devie and register extcon device */
-	data->edev = devm_extcon_dev_allocate(&pdev->dev, &data->extcon_id);
+	data->edev = devm_extcon_dev_allocate(dev, &data->extcon_id);
 	if (IS_ERR(data->edev)) {
-		dev_err(&pdev->dev, "failed to allocate extcon device\n");
+		dev_err(dev, "failed to allocate extcon device\n");
 		return -ENOMEM;
 	}
 
-	ret = devm_extcon_dev_register(&pdev->dev, data->edev);
+	ret = devm_extcon_dev_register(dev, data->edev);
 	if (ret < 0)
 		return ret;
 
@@ -146,7 +110,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 	 * Request the interrupt of gpio to detect whether external connector
 	 * is attached or detached.
 	 */
-	ret = devm_request_any_context_irq(&pdev->dev, data->irq,
+	ret = devm_request_any_context_irq(dev, data->irq,
 					gpio_irq_handler, data->irq_flags,
 					pdev->name, data);
 	if (ret < 0)
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]


#1739446 — Re: [PATCH 4/8] extcon: gpio: Convert to fully use GPIO descriptor

FromChanwoo Choi <cw00.choi@samsung.com>
Date2017-09-26 04:20 +0200
SubjectRe: [PATCH 4/8] extcon: gpio: Convert to fully use GPIO descriptor
Message-ID<utNDz-6M2-3@gated-at.bofh.it>
In reply to#1738206
Hi Linus,

Looks good to me. But, there is one comment
of gpiod_to_irq()'s return value.

If you modify it, feel free to add my tag:
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

On 2017년 09월 24일 23:56, Linus Walleij wrote:
> Since we are not getting the GPIO from any platform data and global
> GPIO numberspace, we simply get the named "extcon" GPIO directly from
> the device. Cut away "active low" since GPIO descriptors already know
> if the line is active high or low. Simplify a bit with a
> struct device *dev helper variable in probe() and cut the complex
> init() function.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/extcon/extcon-gpio.c | 66 ++++++++++----------------------------------
>  1 file changed, 15 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
> index 9c4094edd123..86f3ec6d6014 100644
> --- a/drivers/extcon/extcon-gpio.c
> +++ b/drivers/extcon/extcon-gpio.c
> @@ -18,7 +18,6 @@
>   */
>  
>  #include <linux/extcon.h>
> -#include <linux/gpio.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/init.h>
>  #include <linux/interrupt.h>
> @@ -35,12 +34,8 @@
>   * @work:		Work fired by the interrupt.
>   * @debounce_jiffies:	Number of jiffies to wait for the GPIO to stabilize, from the debounce
>   *			value.
> - * @id_gpiod:		GPIO descriptor for this external connector.
> + * @gpiod:		GPIO descriptor for this external connector.
>   * @extcon_id:		The unique id of specific external connector.
> - * @gpio:		Corresponding GPIO.
> - * @gpio_active_low:	Boolean describing whether gpio active state is 1 or 0
> - *			If true, low state of gpio means active.
> - *			If false, high state of gpio means active.
>   * @debounce:		Debounce time for GPIO IRQ in ms.
>   * @irq_flags:		IRQ Flags (e.g., IRQF_TRIGGER_LOW).
>   * @check_on_resume:	Boolean describing whether to check the state of gpio
> @@ -51,10 +46,8 @@ struct gpio_extcon_data {
>  	int irq;
>  	struct delayed_work work;
>  	unsigned long debounce_jiffies;
> -	struct gpio_desc *id_gpiod;
> +	struct gpio_desc *gpiod;
>  	unsigned int extcon_id;
> -	unsigned gpio;
> -	bool gpio_active_low;
>  	unsigned long debounce;
>  	unsigned long irq_flags;
>  	bool check_on_resume;
> @@ -67,10 +60,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_cansleep(data->id_gpiod);
> -	if (data->gpio_active_low)
> -		state = !state;
> -
> +	state = gpiod_get_value_cansleep(data->gpiod);
>  	extcon_set_state_sync(data->edev, data->extcon_id, state);
>  }
>  
> @@ -83,60 +73,34 @@ static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>  
> -static int gpio_extcon_init(struct device *dev, struct gpio_extcon_data *data)
> -{
> -	int ret;
> -
> -	ret = devm_gpio_request_one(dev, data->gpio, GPIOF_DIR_IN,
> -				dev_name(dev));
> -	if (ret < 0)
> -		return ret;
> -
> -	data->id_gpiod = gpio_to_desc(data->gpio);
> -	if (!data->id_gpiod)
> -		return -EINVAL;
> -
> -	if (data->debounce) {
> -		ret = gpiod_set_debounce(data->id_gpiod,
> -					 data->debounce * 1000);
> -		if (ret < 0)
> -			data->debounce_jiffies =
> -				msecs_to_jiffies(data->debounce);
> -	}
> -
> -	data->irq = gpiod_to_irq(data->id_gpiod);
> -	if (data->irq < 0)
> -		return data->irq;
> -
> -	return 0;
> -}
> -
>  static int gpio_extcon_probe(struct platform_device *pdev)
>  {
>  	struct gpio_extcon_data *data;
> +	struct device *dev = &pdev->dev;
>  	int ret;
>  
> -	data = devm_kzalloc(&pdev->dev, sizeof(struct gpio_extcon_data),
> -				   GFP_KERNEL);
> +	data = devm_kzalloc(dev, sizeof(struct gpio_extcon_data), GFP_KERNEL);
>  	if (!data)
>  		return -ENOMEM;
>  
>  	if (!data->irq_flags || data->extcon_id > EXTCON_NONE)
>  		return -EINVAL;
>  
> -	/* Initialize the gpio */
> -	ret = gpio_extcon_init(&pdev->dev, data);
> -	if (ret < 0)
> -		return ret;
> +	data->gpiod = devm_gpiod_get(dev, "extcon", GPIOD_IN);
> +	if (IS_ERR(data->gpiod))
> +		return PTR_ERR(data->gpiod);
> +	data->irq = gpiod_to_irq(data->gpiod);
> +	if (data->irq <= 0)

"if (data->irq < 0)" is enough. If irq is zero, gpiod_to_irq()
returns the -ENXIO.

> +		return data->irq;
>  
>  	/* Allocate the memory of extcon devie and register extcon device */
> -	data->edev = devm_extcon_dev_allocate(&pdev->dev, &data->extcon_id);
> +	data->edev = devm_extcon_dev_allocate(dev, &data->extcon_id);
>  	if (IS_ERR(data->edev)) {
> -		dev_err(&pdev->dev, "failed to allocate extcon device\n");
> +		dev_err(dev, "failed to allocate extcon device\n");
>  		return -ENOMEM;
>  	}
>  
> -	ret = devm_extcon_dev_register(&pdev->dev, data->edev);
> +	ret = devm_extcon_dev_register(dev, data->edev);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -146,7 +110,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
>  	 * Request the interrupt of gpio to detect whether external connector
>  	 * is attached or detached.
>  	 */
> -	ret = devm_request_any_context_irq(&pdev->dev, data->irq,
> +	ret = devm_request_any_context_irq(dev, data->irq,
>  					gpio_irq_handler, data->irq_flags,
>  					pdev->name, data);
>  	if (ret < 0)
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

[toc] | [prev] | [next] | [standalone]


#1738207 — [PATCH 7/8] extcon: gpio: Get connector type from device property

FromLinus Walleij <linus.walleij@linaro.org>
Date2017-09-24 17:10 +0200
Subject[PATCH 7/8] extcon: gpio: Get connector type from device property
Message-ID<utgHD-1R2-3@gated-at.bofh.it>
In reply to#1738203
We do not use the "EXTCON_NONE" type to report this as before, use
the connector type defined in the device property, from device tree or
ACPI DSDT.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/extcon/extcon-gpio.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
index 8fc52631c8a2..b7353f5018b5 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -35,7 +35,8 @@
  * @debounce_jiffies:	Number of jiffies to wait for the GPIO to stabilize, from the debounce
  *			value.
  * @gpiod:		GPIO descriptor for this external connector.
- * @extcon_id:		The unique id of specific external connector.
+ * @connector_type:	The connector type we're detecting on this extcon, terminated with EXTCON_NONE
+ *			One GPIO is one cable, so one type only.
  * @check_on_resume:	Boolean describing whether to check the state of gpio
  *			while resuming from sleep.
  */
@@ -44,7 +45,7 @@ struct gpio_extcon_data {
 	struct delayed_work work;
 	unsigned long debounce_jiffies;
 	struct gpio_desc *gpiod;
-	unsigned int extcon_id;
+	unsigned int connector_type[2];
 	bool check_on_resume;
 };
 
@@ -56,7 +57,7 @@ static void gpio_extcon_work(struct work_struct *work)
 			     work);
 
 	state = gpiod_get_value_cansleep(data->gpiod);
-	extcon_set_state_sync(data->edev, data->extcon_id, state);
+	extcon_set_state_sync(data->edev, data->connector_type[0], state);
 }
 
 static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
@@ -74,6 +75,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	unsigned long irq_flags;
 	u32 debounce_usecs;
+	u32 connector_type;
 	int irq;
 	int ret;
 
@@ -81,9 +83,6 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 	if (!data)
 		return -ENOMEM;
 
-	if (data->extcon_id > EXTCON_NONE)
-		return -EINVAL;
-
 	data->gpiod = devm_gpiod_get(dev, "extcon", GPIOD_IN);
 	if (IS_ERR(data->gpiod))
 		return PTR_ERR(data->gpiod);
@@ -102,8 +101,16 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 	else
 		irq_flags = IRQF_TRIGGER_RISING;
 
+	ret = device_property_read_u32(dev, "extcon-connector-types", &connector_type);
+	if (ret || !connector_type) {
+		dev_err(dev, "illegal cable type or undefined cable type\n");
+		return -EINVAL;
+	}
+	data->connector_type[0] = connector_type;
+	data->connector_type[1] = EXTCON_NONE;
+
 	/* Allocate the memory of extcon devie and register extcon device */
-	data->edev = devm_extcon_dev_allocate(dev, &data->extcon_id);
+	data->edev = devm_extcon_dev_allocate(dev, data->connector_type);
 	if (IS_ERR(data->edev)) {
 		dev_err(dev, "failed to allocate extcon device\n");
 		return -ENOMEM;
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]


#1738208 — [PATCH 8/8] extcon: gpio: Always check state on resume

FromLinus Walleij <linus.walleij@linaro.org>
Date2017-09-24 17:10 +0200
Subject[PATCH 8/8] extcon: gpio: Always check state on resume
Message-ID<utgHD-1R2-7@gated-at.bofh.it>
In reply to#1738203
It makes most sense to always check the state of the GPIO external
connector at system resume so just do this by default. Add a TODO
if people turn out to desire to parameterize this.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/extcon/extcon-gpio.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
index b7353f5018b5..4f0ad5ad2722 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -83,6 +83,15 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 	if (!data)
 		return -ENOMEM;
 
+	/*
+	 * Always checking connector state on resume makes most sense so do this
+	 * by default.
+	 *
+	 * TODO: if parameterization is needed, augment this to use proper device
+	 * properties or set it up from PM core.
+	 */
+	data->check_on_resume = true;
+
 	data->gpiod = devm_gpiod_get(dev, "extcon", GPIOD_IN);
 	if (IS_ERR(data->gpiod))
 		return PTR_ERR(data->gpiod);
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]


#1739456 — Re: [PATCH 8/8] extcon: gpio: Always check state on resume

FromChanwoo Choi <cw00.choi@samsung.com>
Date2017-09-26 04:30 +0200
SubjectRe: [PATCH 8/8] extcon: gpio: Always check state on resume
Message-ID<utNNg-6PT-23@gated-at.bofh.it>
In reply to#1738208
Hi Linus,

On 2017년 09월 24일 23:56, Linus Walleij wrote:
> It makes most sense to always check the state of the GPIO external
> connector at system resume so just do this by default. Add a TODO
> if people turn out to desire to parameterize this.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/extcon/extcon-gpio.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

Looks good to me.
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

> 
> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
> index b7353f5018b5..4f0ad5ad2722 100644
> --- a/drivers/extcon/extcon-gpio.c
> +++ b/drivers/extcon/extcon-gpio.c
> @@ -83,6 +83,15 @@ static int gpio_extcon_probe(struct platform_device *pdev)
>  	if (!data)
>  		return -ENOMEM;
>  
> +	/*
> +	 * Always checking connector state on resume makes most sense so do this
> +	 * by default.
> +	 *
> +	 * TODO: if parameterization is needed, augment this to use proper device
> +	 * properties or set it up from PM core.
> +	 */
> +	data->check_on_resume = true;
> +
>  	data->gpiod = devm_gpiod_get(dev, "extcon", GPIOD_IN);
>  	if (IS_ERR(data->gpiod))
>  		return PTR_ERR(data->gpiod);
> 

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

[toc] | [prev] | [next] | [standalone]


#1738210 — [PATCH 5/8] extcon: gpio: Request reasonable interrupts

FromLinus Walleij <linus.walleij@linaro.org>
Date2017-09-24 17:10 +0200
Subject[PATCH 5/8] extcon: gpio: Request reasonable interrupts
Message-ID<utgHE-1R2-17@gated-at.bofh.it>
In reply to#1738203
The only thing that makes sense is to request a falling edge interrupt
if the line is active low and a rising edge interrupt if the line is
active high, so just do that and get rid of the assignment from
platform data. The GPIO descriptor knows if the line is active high
or low.

Also make irq a local variable in probe(), it's not used anywhere else.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/extcon/extcon-gpio.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
index 86f3ec6d6014..6d9cb4ed11c2 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -30,26 +30,22 @@
 /**
  * struct gpio_extcon_data - A simple GPIO-controlled extcon device state container.
  * @edev:		Extcon device.
- * @irq:		Interrupt line for the external connector.
  * @work:		Work fired by the interrupt.
  * @debounce_jiffies:	Number of jiffies to wait for the GPIO to stabilize, from the debounce
  *			value.
  * @gpiod:		GPIO descriptor for this external connector.
  * @extcon_id:		The unique id of specific external connector.
  * @debounce:		Debounce time for GPIO IRQ in ms.
- * @irq_flags:		IRQ Flags (e.g., IRQF_TRIGGER_LOW).
  * @check_on_resume:	Boolean describing whether to check the state of gpio
  *			while resuming from sleep.
  */
 struct gpio_extcon_data {
 	struct extcon_dev *edev;
-	int irq;
 	struct delayed_work work;
 	unsigned long debounce_jiffies;
 	struct gpio_desc *gpiod;
 	unsigned int extcon_id;
 	unsigned long debounce;
-	unsigned long irq_flags;
 	bool check_on_resume;
 };
 
@@ -77,21 +73,34 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 {
 	struct gpio_extcon_data *data;
 	struct device *dev = &pdev->dev;
+	unsigned long irq_flags;
+	int irq;
 	int ret;
 
 	data = devm_kzalloc(dev, sizeof(struct gpio_extcon_data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
-	if (!data->irq_flags || data->extcon_id > EXTCON_NONE)
+	if (data->extcon_id > EXTCON_NONE)
 		return -EINVAL;
 
 	data->gpiod = devm_gpiod_get(dev, "extcon", GPIOD_IN);
 	if (IS_ERR(data->gpiod))
 		return PTR_ERR(data->gpiod);
-	data->irq = gpiod_to_irq(data->gpiod);
-	if (data->irq <= 0)
-		return data->irq;
+	irq = gpiod_to_irq(data->gpiod);
+	if (irq <= 0)
+		return irq;
+
+	/*
+	 * It is unlikely that this is an acknowledged interrupt that goes
+	 * away after handling, what we are looking for are falling edges
+	 * if the signal is active low, and rising edges if the signal is
+	 * active high.
+	 */
+	if (gpiod_is_active_low(data->gpiod))
+		irq_flags = IRQF_TRIGGER_FALLING;
+	else
+		irq_flags = IRQF_TRIGGER_RISING;
 
 	/* Allocate the memory of extcon devie and register extcon device */
 	data->edev = devm_extcon_dev_allocate(dev, &data->extcon_id);
@@ -110,8 +119,8 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 	 * Request the interrupt of gpio to detect whether external connector
 	 * is attached or detached.
 	 */
-	ret = devm_request_any_context_irq(dev, data->irq,
-					gpio_irq_handler, data->irq_flags,
+	ret = devm_request_any_context_irq(dev, irq,
+					gpio_irq_handler, irq_flags,
 					pdev->name, data);
 	if (ret < 0)
 		return ret;
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]


#1739447 — Re: [PATCH 5/8] extcon: gpio: Request reasonable interrupts

FromChanwoo Choi <cw00.choi@samsung.com>
Date2017-09-26 04:30 +0200
SubjectRe: [PATCH 5/8] extcon: gpio: Request reasonable interrupts
Message-ID<utNNf-6PT-5@gated-at.bofh.it>
In reply to#1738210
Hi Linus,

On 2017년 09월 24일 23:56, Linus Walleij wrote:
> The only thing that makes sense is to request a falling edge interrupt
> if the line is active low and a rising edge interrupt if the line is
> active high, so just do that and get rid of the assignment from
> platform data. The GPIO descriptor knows if the line is active high
> or low.
> 
> Also make irq a local variable in probe(), it's not used anywhere else.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/extcon/extcon-gpio.c | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)

Looks good to me.
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

> 
> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
> index 86f3ec6d6014..6d9cb4ed11c2 100644
> --- a/drivers/extcon/extcon-gpio.c
> +++ b/drivers/extcon/extcon-gpio.c
> @@ -30,26 +30,22 @@
>  /**
>   * struct gpio_extcon_data - A simple GPIO-controlled extcon device state container.
>   * @edev:		Extcon device.
> - * @irq:		Interrupt line for the external connector.
>   * @work:		Work fired by the interrupt.
>   * @debounce_jiffies:	Number of jiffies to wait for the GPIO to stabilize, from the debounce
>   *			value.
>   * @gpiod:		GPIO descriptor for this external connector.
>   * @extcon_id:		The unique id of specific external connector.
>   * @debounce:		Debounce time for GPIO IRQ in ms.
> - * @irq_flags:		IRQ Flags (e.g., IRQF_TRIGGER_LOW).
>   * @check_on_resume:	Boolean describing whether to check the state of gpio
>   *			while resuming from sleep.
>   */
>  struct gpio_extcon_data {
>  	struct extcon_dev *edev;
> -	int irq;
>  	struct delayed_work work;
>  	unsigned long debounce_jiffies;
>  	struct gpio_desc *gpiod;
>  	unsigned int extcon_id;
>  	unsigned long debounce;
> -	unsigned long irq_flags;
>  	bool check_on_resume;
>  };
>  
> @@ -77,21 +73,34 @@ static int gpio_extcon_probe(struct platform_device *pdev)
>  {
>  	struct gpio_extcon_data *data;
>  	struct device *dev = &pdev->dev;
> +	unsigned long irq_flags;
> +	int irq;
>  	int ret;
>  
>  	data = devm_kzalloc(dev, sizeof(struct gpio_extcon_data), GFP_KERNEL);
>  	if (!data)
>  		return -ENOMEM;
>  
> -	if (!data->irq_flags || data->extcon_id > EXTCON_NONE)
> +	if (data->extcon_id > EXTCON_NONE)
>  		return -EINVAL;
>  
>  	data->gpiod = devm_gpiod_get(dev, "extcon", GPIOD_IN);
>  	if (IS_ERR(data->gpiod))
>  		return PTR_ERR(data->gpiod);
> -	data->irq = gpiod_to_irq(data->gpiod);
> -	if (data->irq <= 0)
> -		return data->irq;
> +	irq = gpiod_to_irq(data->gpiod);
> +	if (irq <= 0)
> +		return irq;
> +
> +	/*
> +	 * It is unlikely that this is an acknowledged interrupt that goes
> +	 * away after handling, what we are looking for are falling edges
> +	 * if the signal is active low, and rising edges if the signal is
> +	 * active high.
> +	 */
> +	if (gpiod_is_active_low(data->gpiod))
> +		irq_flags = IRQF_TRIGGER_FALLING;
> +	else
> +		irq_flags = IRQF_TRIGGER_RISING;
>  
>  	/* Allocate the memory of extcon devie and register extcon device */
>  	data->edev = devm_extcon_dev_allocate(dev, &data->extcon_id);
> @@ -110,8 +119,8 @@ static int gpio_extcon_probe(struct platform_device *pdev)
>  	 * Request the interrupt of gpio to detect whether external connector
>  	 * is attached or detached.
>  	 */
> -	ret = devm_request_any_context_irq(dev, data->irq,
> -					gpio_irq_handler, data->irq_flags,
> +	ret = devm_request_any_context_irq(dev, irq,
> +					gpio_irq_handler, irq_flags,
>  					pdev->name, data);
>  	if (ret < 0)
>  		return ret;
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

[toc] | [prev] | [next] | [standalone]


#1738211 — [PATCH 6/8] extcon: gpio: Get debounce setting from device property

FromLinus Walleij <linus.walleij@linaro.org>
Date2017-09-24 17:10 +0200
Subject[PATCH 6/8] extcon: gpio: Get debounce setting from device property
Message-ID<utgHE-1R2-23@gated-at.bofh.it>
In reply to#1738203
Look up the debouncing value using the device property that will
pick it from device tree or ACPI DSDT or whatever is available.

Reintroduce the debounce handling previously deleted in the
series, setting the delayed worker to delay 0 ms if the GPIO
driver supports debouncing for us, else just delay the reading
of the value delayed by jiffies.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/extcon/extcon-gpio.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
index 6d9cb4ed11c2..8fc52631c8a2 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -24,6 +24,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/workqueue.h>
 
@@ -35,7 +36,6 @@
  *			value.
  * @gpiod:		GPIO descriptor for this external connector.
  * @extcon_id:		The unique id of specific external connector.
- * @debounce:		Debounce time for GPIO IRQ in ms.
  * @check_on_resume:	Boolean describing whether to check the state of gpio
  *			while resuming from sleep.
  */
@@ -45,7 +45,6 @@ struct gpio_extcon_data {
 	unsigned long debounce_jiffies;
 	struct gpio_desc *gpiod;
 	unsigned int extcon_id;
-	unsigned long debounce;
 	bool check_on_resume;
 };
 
@@ -74,6 +73,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 	struct gpio_extcon_data *data;
 	struct device *dev = &pdev->dev;
 	unsigned long irq_flags;
+	u32 debounce_usecs;
 	int irq;
 	int ret;
 
@@ -109,6 +109,15 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	ret = device_property_read_u32(dev, "input-debounce", &debounce_usecs);
+	if (ret || !debounce_usecs) {
+		dev_err(dev, "illegal debounce value, set to 20 ms\n");
+		debounce_usecs = 20000;
+	}
+	ret = gpiod_set_debounce(data->gpiod, debounce_usecs);
+	if (ret)
+		data->debounce_jiffies = msecs_to_jiffies(debounce_usecs * 1000);
+
 	ret = devm_extcon_dev_register(dev, data->edev);
 	if (ret < 0)
 		return ret;
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]


#1739451 — Re: [PATCH 6/8] extcon: gpio: Get debounce setting from device property

FromChanwoo Choi <cw00.choi@samsung.com>
Date2017-09-26 04:30 +0200
SubjectRe: [PATCH 6/8] extcon: gpio: Get debounce setting from device property
Message-ID<utNNf-6PT-15@gated-at.bofh.it>
In reply to#1738211
Hi Linus,

On 2017년 09월 24일 23:56, Linus Walleij wrote:
> Look up the debouncing value using the device property that will
> pick it from device tree or ACPI DSDT or whatever is available.
> 
> Reintroduce the debounce handling previously deleted in the
> series, setting the delayed worker to delay 0 ms if the GPIO
> driver supports debouncing for us, else just delay the reading
> of the value delayed by jiffies.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/extcon/extcon-gpio.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)

I checked the pinctrl-bindings.txt[1] in order to check
the property name of 'input-debounce'. It looks good to me.
[1] Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

> 
> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
> index 6d9cb4ed11c2..8fc52631c8a2 100644
> --- a/drivers/extcon/extcon-gpio.c
> +++ b/drivers/extcon/extcon-gpio.c
> @@ -24,6 +24,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/property.h>
>  #include <linux/slab.h>
>  #include <linux/workqueue.h>
>  
> @@ -35,7 +36,6 @@
>   *			value.
>   * @gpiod:		GPIO descriptor for this external connector.
>   * @extcon_id:		The unique id of specific external connector.
> - * @debounce:		Debounce time for GPIO IRQ in ms.
>   * @check_on_resume:	Boolean describing whether to check the state of gpio
>   *			while resuming from sleep.
>   */
> @@ -45,7 +45,6 @@ struct gpio_extcon_data {
>  	unsigned long debounce_jiffies;
>  	struct gpio_desc *gpiod;
>  	unsigned int extcon_id;
> -	unsigned long debounce;
>  	bool check_on_resume;
>  };
>  
> @@ -74,6 +73,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
>  	struct gpio_extcon_data *data;
>  	struct device *dev = &pdev->dev;
>  	unsigned long irq_flags;
> +	u32 debounce_usecs;
>  	int irq;
>  	int ret;
>  
> @@ -109,6 +109,15 @@ static int gpio_extcon_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
>  
> +	ret = device_property_read_u32(dev, "input-debounce", &debounce_usecs);
> +	if (ret || !debounce_usecs) {
> +		dev_err(dev, "illegal debounce value, set to 20 ms\n");
> +		debounce_usecs = 20000;
> +	}
> +	ret = gpiod_set_debounce(data->gpiod, debounce_usecs);
> +	if (ret)
> +		data->debounce_jiffies = msecs_to_jiffies(debounce_usecs * 1000);
> +
>  	ret = devm_extcon_dev_register(dev, data->edev);
>  	if (ret < 0)
>  		return ret;
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web