2014-11-05 15:32:49

by George Cherian

[permalink] [raw]
Subject: [RESEND PATCH 0/2] Add dt support for extcon gpio driver

This series
- Adds dt support to extcon-gpio driver.
- Add cable name support in case of dt.

George Cherian (2):
extcon: gpio: Add dt support for the driver
extcon: gpio: Add support for using cable names

drivers/extcon/extcon-gpio.c | 87 +++++++++++++-------------------------
include/linux/extcon/extcon-gpio.h | 59 --------------------------
2 files changed, 29 insertions(+), 117 deletions(-)
delete mode 100644 include/linux/extcon/extcon-gpio.h

--
1.8.3.1


2014-11-05 15:32:52

by George Cherian

[permalink] [raw]
Subject: [RESEND PATCH 1/2] extcon: gpio: Add dt support for the driver

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

While at that
- Cleanup the pdata as there are no users for the same.
- Convert the driver to use gpiod_* API's.
- 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 <[email protected]>
---
.../devicetree/bindings/extcon/extcon-gpio.txt | 21 ++++++
drivers/extcon/extcon-gpio.c | 83 +++++++---------------
include/linux/extcon/extcon-gpio.h | 59 ---------------
3 files changed, 46 insertions(+), 117 deletions(-)
create mode 100644 Documentation/devicetree/bindings/extcon/extcon-gpio.txt
delete mode 100644 include/linux/extcon/extcon-gpio.h

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 72f19a3..85795de 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -21,26 +21,23 @@
*/

#include <linux/extcon.h>
-#include <linux/extcon/extcon-gpio.h>
#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>

struct gpio_extcon_data {
struct extcon_dev *edev;
- unsigned gpio;
- bool gpio_active_low;
- const char *state_on;
- const char *state_off;
+ struct gpio_desc *gpiod;
int irq;
struct delayed_work work;
unsigned long debounce_jiffies;
- bool check_on_resume;
};

static void gpio_extcon_work(struct work_struct *work)
@@ -50,9 +47,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_cansleep(data->gpiod);
extcon_set_state(data->edev, state);
}

@@ -65,34 +60,16 @@ static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
return IRQ_HANDLED;
}

-static ssize_t extcon_gpio_print_state(struct extcon_dev *edev, char *buf)
-{
- struct device *dev = edev->dev.parent;
- struct gpio_extcon_data *extcon_data = dev_get_drvdata(dev);
- const char *state;
-
- if (extcon_get_state(edev))
- state = extcon_data->state_on;
- else
- state = extcon_data->state_off;
-
- if (state)
- return sprintf(buf, "%s\n", state);
- return -EINVAL;
-}
-
static int gpio_extcon_probe(struct platform_device *pdev)
{
- struct gpio_extcon_platform_data *pdata = dev_get_platdata(&pdev->dev);
+ struct device_node *np = pdev->dev.of_node;
struct gpio_extcon_data *extcon_data;
+ unsigned int irq_flags;
+ unsigned int debounce = 0;
int ret;

- if (!pdata)
- return -EBUSY;
- if (!pdata->irq_flags) {
- dev_err(&pdev->dev, "IRQ flag is not specified.\n");
+ if (!np)
return -EINVAL;
- }

extcon_data = devm_kzalloc(&pdev->dev, sizeof(struct gpio_extcon_data),
GFP_KERNEL);
@@ -104,27 +81,23 @@ 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->gpio = pdata->gpio;
- extcon_data->gpio_active_low = pdata->gpio_active_low;
- 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,
- pdev->name);
- if (ret < 0)
- return ret;
+ extcon_data->edev->name = np->name;
+ extcon_data->gpiod = gpiod_get(&pdev->dev, NULL);
+ if (IS_ERR(extcon_data->gpiod))
+ return PTR_ERR(extcon_data->gpiod);

- if (pdata->debounce) {
- ret = gpio_set_debounce(extcon_data->gpio,
- pdata->debounce * 1000);
+ extcon_data->irq = gpiod_to_irq(extcon_data->gpiod);
+ if (extcon_data->irq < 0)
+ return extcon_data->irq;
+
+ irq_flags = irq_get_trigger_type(extcon_data->irq);
+ of_property_read_u32(np, "debounce", &debounce);
+ if (debounce) {
+ ret = gpiod_set_debounce(extcon_data->gpiod,
+ 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);
@@ -132,13 +105,8 @@ static int gpio_extcon_probe(struct platform_device *pdev)
return ret;

INIT_DELAYED_WORK(&extcon_data->work, gpio_extcon_work);
-
- extcon_data->irq = gpio_to_irq(extcon_data->gpio);
- if (extcon_data->irq < 0)
- 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;
@@ -166,9 +134,8 @@ static int gpio_extcon_resume(struct device *dev)
struct gpio_extcon_data *extcon_data;

extcon_data = dev_get_drvdata(dev);
- if (extcon_data->check_on_resume)
- queue_delayed_work(system_power_efficient_wq,
- &extcon_data->work, extcon_data->debounce_jiffies);
+ queue_delayed_work(system_power_efficient_wq,
+ &extcon_data->work, extcon_data->debounce_jiffies);

return 0;
}
diff --git a/include/linux/extcon/extcon-gpio.h b/include/linux/extcon/extcon-gpio.h
deleted file mode 100644
index 0b17ad4..0000000
--- a/include/linux/extcon/extcon-gpio.h
+++ /dev/null
@@ -1,59 +0,0 @@
-/*
- * External connector (extcon) class generic GPIO driver
- *
- * Copyright (C) 2012 Samsung Electronics
- * Author: MyungJoo Ham <[email protected]>
- *
- * based on switch class driver
- * Copyright (C) 2008 Google, Inc.
- * Author: Mike Lockwood <[email protected]>
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
-*/
-#ifndef __EXTCON_GPIO_H__
-#define __EXTCON_GPIO_H__ __FILE__
-
-#include <linux/extcon.h>
-
-/**
- * struct gpio_extcon_platform_data - A simple GPIO-controlled extcon device.
- * @name: The name of this GPIO extcon device.
- * @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).
- * @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_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,
- * the print_state is not overriden.
- */
-struct gpio_extcon_platform_data {
- const char *name;
- unsigned gpio;
- bool gpio_active_low;
- unsigned long debounce;
- unsigned long irq_flags;
-
- /* if NULL, "0" or "1" will be printed */
- const char *state_on;
- const char *state_off;
- bool check_on_resume;
-};
-
-#endif /* __EXTCON_GPIO_H__ */
--
1.8.3.1

2014-11-05 15:32:47

by George Cherian

[permalink] [raw]
Subject: [RESEND PATCH 2/2] extcon: gpio: Add support for using cable names

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 <[email protected]>
---
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 85795de..0e1b3e8 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -38,6 +38,7 @@ struct gpio_extcon_data {
int irq;
struct delayed_work work;
unsigned long debounce_jiffies;
+ const char *cable_name[1];
};

static void gpio_extcon_work(struct work_struct *work)
@@ -100,6 +101,9 @@ static int gpio_extcon_probe(struct platform_device *pdev)
msecs_to_jiffies(debounce);
}

+ of_property_read_string_index(np, "cable-name", 0,
+ extcon_data->cable_name);
+ extcon_data->edev->supported_cable = extcon_data->cable_name;
ret = devm_extcon_dev_register(&pdev->dev, extcon_data->edev);
if (ret < 0)
return ret;
--
1.8.3.1

2014-11-05 18:30:15

by Felipe Balbi

[permalink] [raw]
Subject: Re: [RESEND PATCH 1/2] extcon: gpio: Add dt support for the driver

On Wed, Nov 05, 2014 at 08:59:47PM +0530, George Cherian wrote:
> Add device tree support to extcon-gpio driver.
> Add devicetree binding documentation
>
> While at that
> - Cleanup the pdata as there are no users for the same.

should be a patch of its own.

> - Convert the driver to use gpiod_* API's.

should be a patch of its own.

> - Some gpio's can sleep while reading, so always use gpio_get_value_cansleep

doesn't explain the fact that you have such a platform. Make it clear
that you have a platform supported in mainline which uses a GPIO from an
external i2c I/O expander and because of that you need to use can_sleep.

Should also be a patch of its own, as it was before.

--
balbi


Attachments:
(No filename) (702.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-11-06 01:15:00

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [RESEND PATCH 0/2] Add dt support for extcon gpio driver

Hi George,

On 11/06/2014 12:29 AM, George Cherian wrote:
> This series
> - Adds dt support to extcon-gpio driver.
> - Add cable name support in case of dt.
>
> George Cherian (2):
> extcon: gpio: Add dt support for the driver
> extcon: gpio: Add support for using cable names
>
> drivers/extcon/extcon-gpio.c | 87 +++++++++++++-------------------------
> include/linux/extcon/extcon-gpio.h | 59 --------------------------
> 2 files changed, 29 insertions(+), 117 deletions(-)
> delete mode 100644 include/linux/extcon/extcon-gpio.h
>

I'm so sorry for late reply because I'm busy for other jobs.

But, I will review this patchset and test it on my board
within 3~4 days.

Best Regards,
Chanwoo Choi

2014-11-12 01:10:14

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [RESEND PATCH 1/2] extcon: gpio: Add dt support for the driver

Hi George,

How do you test this patch? This patch cannot test it
because this patch has not compatible string for binding through DT.

You have to add following of_device_id for extcon-gpio:

+#if defined(CONFIG_OF)
+static const struct of_device_id gpio_extcon_of_match[] = {
+ { .compatible = "extcon-gpio", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, gpio_extcon_of_match);
+#endif
+
static struct platform_driver gpio_extcon_driver = {
.probe = gpio_extcon_probe,
.remove = gpio_extcon_remove,
.driver = {
.name = "extcon-gpio",
.owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(gpio_extcon_of_match),
.pm = &gpio_extcon_pm_ops,
},
};


On 11/06/2014 12:29 AM, George Cherian wrote:
> Add device tree support to extcon-gpio driver.
> Add devicetree binding documentation
>
> While at that
> - Cleanup the pdata as there are no users for the same.
> - Convert the driver to use gpiod_* API's.
> - 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 <[email protected]>
> ---
> .../devicetree/bindings/extcon/extcon-gpio.txt | 21 ++++++
> drivers/extcon/extcon-gpio.c | 83 +++++++---------------
> include/linux/extcon/extcon-gpio.h | 59 ---------------
> 3 files changed, 46 insertions(+), 117 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/extcon/extcon-gpio.txt
> delete mode 100644 include/linux/extcon/extcon-gpio.h
>
> 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"

I prefer to use simply "extcon-gpio" compatible" instead of "linux,extcon-gpio".

> + - gpios: specifies the gpio pin used.
> +
> +Optional Properties:
> + - debounce: Debounce time for GPIO IRQ in ms
> +
> +

Remove unneeded blank line.

> +Eg:
> +
> + extcon1: am43_usbid_extcon1 {

I want to change the example of extcon-gpio as following:
"extcon1: am43_usbid_extcon1" -> "usb_extcon: extcon-gpio0"

> + compatible = "linux,extcon-gpio";

ditto.

> + gpios = <&gpio3 12 GPIO_ACTIVE_HIGH>;
> + debounce = <20>;
> + };

You have to fix indentation of brace.

> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
> index 72f19a3..85795de 100644
> --- a/drivers/extcon/extcon-gpio.c
> +++ b/drivers/extcon/extcon-gpio.c
> @@ -21,26 +21,23 @@
> */
>
> #include <linux/extcon.h>
> -#include <linux/extcon/extcon-gpio.h>
> #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>
>
> struct gpio_extcon_data {
> struct extcon_dev *edev;
> - unsigned gpio;
> - bool gpio_active_low;
> - const char *state_on;
> - const char *state_off;
> + struct gpio_desc *gpiod;
> int irq;
> struct delayed_work work;
> unsigned long debounce_jiffies;
> - bool check_on_resume;
> };
>
> static void gpio_extcon_work(struct work_struct *work)
> @@ -50,9 +47,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_cansleep(data->gpiod);
> extcon_set_state(data->edev, state);
> }
>
> @@ -65,34 +60,16 @@ static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> -static ssize_t extcon_gpio_print_state(struct extcon_dev *edev, char *buf)
> -{
> - struct device *dev = edev->dev.parent;
> - struct gpio_extcon_data *extcon_data = dev_get_drvdata(dev);
> - const char *state;
> -
> - if (extcon_get_state(edev))
> - state = extcon_data->state_on;
> - else
> - state = extcon_data->state_off;
> -
> - if (state)
> - return sprintf(buf, "%s\n", state);
> - return -EINVAL;
> -}
> -
> static int gpio_extcon_probe(struct platform_device *pdev)
> {
> - struct gpio_extcon_platform_data *pdata = dev_get_platdata(&pdev->dev);
> + struct device_node *np = pdev->dev.of_node;
> struct gpio_extcon_data *extcon_data;
> + unsigned int irq_flags;
> + unsigned int debounce = 0;
> int ret;
>
> - if (!pdata)
> - return -EBUSY;
> - if (!pdata->irq_flags) {
> - dev_err(&pdev->dev, "IRQ flag is not specified.\n");
> + if (!np)
> return -EINVAL;
> - }
>
> extcon_data = devm_kzalloc(&pdev->dev, sizeof(struct gpio_extcon_data),
> GFP_KERNEL);
> @@ -104,27 +81,23 @@ 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->gpio = pdata->gpio;
> - extcon_data->gpio_active_low = pdata->gpio_active_low;
> - 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,
> - pdev->name);
> - if (ret < 0)
> - return ret;
> + extcon_data->edev->name = np->name;
> + extcon_data->gpiod = gpiod_get(&pdev->dev, NULL);
> + if (IS_ERR(extcon_data->gpiod))
> + return PTR_ERR(extcon_data->gpiod);
>
> - if (pdata->debounce) {
> - ret = gpio_set_debounce(extcon_data->gpio,
> - pdata->debounce * 1000);
> + extcon_data->irq = gpiod_to_irq(extcon_data->gpiod);
> + if (extcon_data->irq < 0)
> + return extcon_data->irq;
> +
> + irq_flags = irq_get_trigger_type(extcon_data->irq);

I recommend to use interrupt flags as following:
irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;

> + of_property_read_u32(np, "debounce", &debounce);

I don't want to use additional variable ('debounce').
You better to use 'data->debounce_jiffies' direclty instead of 'debounce'
Also, I prefer to check the return value of of_property_read_u32().

> + if (debounce) {
> + ret = gpiod_set_debounce(extcon_data->gpiod,
> + debounce * 1000);
> if (ret < 0)
> extcon_data->debounce_jiffies =
> - msecs_to_jiffies(pdata->debounce);
> + msecs_to_jiffies(debounce);
> }

How about following codes?

if (of_property_read_u32(np, "debounce", &data->debounce_jiffies))
data->debounce_jiffies = 0;

ret = gpiod_set_debounce(data->gpiod, data->debounce_jiffies * 1000);
if (ret < 0)
data->debounce_jiffies
= msecs_to_jiffies(data->debounce_jiffies);


>
> ret = devm_extcon_dev_register(&pdev->dev, extcon_data->edev);
> @@ -132,13 +105,8 @@ static int gpio_extcon_probe(struct platform_device *pdev)
> return ret;
>
> INIT_DELAYED_WORK(&extcon_data->work, gpio_extcon_work);
> -
> - extcon_data->irq = gpio_to_irq(extcon_data->gpio);
> - if (extcon_data->irq < 0)
> - 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;
> @@ -166,9 +134,8 @@ static int gpio_extcon_resume(struct device *dev)
> struct gpio_extcon_data *extcon_data;
>
> extcon_data = dev_get_drvdata(dev);
> - if (extcon_data->check_on_resume)

Why did you remove 'check_on_resume' flag without any description?

> - queue_delayed_work(system_power_efficient_wq,
> - &extcon_data->work, extcon_data->debounce_jiffies);
> + queue_delayed_work(system_power_efficient_wq,
> + &extcon_data->work, extcon_data->debounce_jiffies);
>
> return 0;
> }
> diff --git a/include/linux/extcon/extcon-gpio.h b/include/linux/extcon/extcon-gpio.h
> deleted file mode 100644
> index 0b17ad4..0000000
> --- a/include/linux/extcon/extcon-gpio.h
> +++ /dev/null
> @@ -1,59 +0,0 @@
> -/*
> - * External connector (extcon) class generic GPIO driver
> - *
> - * Copyright (C) 2012 Samsung Electronics
> - * Author: MyungJoo Ham <[email protected]>
> - *
> - * based on switch class driver
> - * Copyright (C) 2008 Google, Inc.
> - * Author: Mike Lockwood <[email protected]>
> - *
> - * This software is licensed under the terms of the GNU General Public
> - * License version 2, as published by the Free Software Foundation, and
> - * may be copied, distributed, and modified under those terms.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - *
> -*/
> -#ifndef __EXTCON_GPIO_H__
> -#define __EXTCON_GPIO_H__ __FILE__
> -
> -#include <linux/extcon.h>
> -
> -/**
> - * struct gpio_extcon_platform_data - A simple GPIO-controlled extcon device.
> - * @name: The name of this GPIO extcon device.
> - * @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).
> - * @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_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,
> - * the print_state is not overriden.
> - */
> -struct gpio_extcon_platform_data {
> - const char *name;
> - unsigned gpio;
> - bool gpio_active_low;
> - unsigned long debounce;
> - unsigned long irq_flags;
> -
> - /* if NULL, "0" or "1" will be printed */
> - const char *state_on;
> - const char *state_off;
> - bool check_on_resume;
> -};
> -
> -#endif /* __EXTCON_GPIO_H__ */
>

Thanks,
Chanwoo Choi

2014-11-12 01:12:56

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [RESEND PATCH 2/2] extcon: gpio: Add support for using cable names

Hi George,

On 11/06/2014 12:29 AM, George Cherian wrote:
> 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 <[email protected]>
> ---
> 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 85795de..0e1b3e8 100644
> --- a/drivers/extcon/extcon-gpio.c
> +++ b/drivers/extcon/extcon-gpio.c
> @@ -38,6 +38,7 @@ struct gpio_extcon_data {
> int irq;
> struct delayed_work work;
> unsigned long debounce_jiffies;
> + const char *cable_name[1];

*cable_name[1] -> **cable_name

> };
>
> static void gpio_extcon_work(struct work_struct *work)
> @@ -100,6 +101,9 @@ static int gpio_extcon_probe(struct platform_device *pdev)
> msecs_to_jiffies(debounce);
> }
>
> + of_property_read_string_index(np, "cable-name", 0,
> + extcon_data->cable_name);
> + extcon_data->edev->supported_cable = extcon_data->cable_name;

It is wrong. I don't want to allocate ables to supported_cable directly.
For consistency of extcon driver, you have to add the array of supported cables
by using devm_extcon_dev_allocate().

Thanks,
Chanwoo Choi

> ret = devm_extcon_dev_register(&pdev->dev, extcon_data->edev);
> if (ret < 0)
> return ret;
>