Hi Choi, Sorry for changing author, will update author field with your name. Regarding Rob Herring comments, You had already replied. I felt separate compatible for each external connector is not required, as client driver can detect the type of external cable(sdp,dcp, microphone) on receiving notification from extcon provider, I have also added more description for wakeup-source. Do you see any other changes required on top of v4 patch? Regards, Venkat > -----Original Message----- > From: Chanwoo Choi [mailto:cwchoi00@xxxxxxxxx] > Sent: Thursday, May 26, 2016 6:52 PM > To: Venkat Reddy Talla > Cc: MyungJoo Ham; Rob Herring; Pawel Moll; Mark Rutland; Ian Campbell; > devicetree; Kumar Gala; linux-kernel; Laxman Dewangan > Subject: Re: [PATCH v4] extcon: gpio: Add the support for Device tree > bindings > > Hi Venkat, > > There is some miscommunication. On previous my reply, I don't mean that > the author of the patch[1] is changed from me to you. > > I'd like you to remain the original author(me) for the patch[1] without > changing the author. > [1] https://lkml.org/lkml/2015/10/21/8 > - [PATCH v3] extcon: gpio: Add the support for Device tree bindings > > You can use the patch[1] as based patch and you can add new feature on > base patch[1]. Also, If you ok, you can modify the extccon-gpio.c driver as > Rob comment[2]. > > But, Rob Herring gave me the some comment[2]. > [2] https://lkml.org/lkml/2015/10/21/906 > > > Thanks, > Chanwoo Choi > > > On Thu, May 26, 2016 at 8:47 PM, Venkat Reddy Talla > <vreddytalla@xxxxxxxxxx> wrote: > > Add the support for Device tree bindings of extcon-gpio driver. > > The extcon-gpio device tree node must include the both 'extcon-id' and > > 'gpios' property. > > > > For example: > > usb_cable: extcon-gpio-0 { > > compatible = "extcon-gpio"; > > extcon-id = <EXTCON_USB>; > > gpios = <&gpio6 1 GPIO_ACTIVE_HIGH>; > > } > > ta_cable: extcon-gpio-1 { > > compatible = "extcon-gpio"; > > extcon-id = <EXTCON_CHG_USB_DCP>; > > gpios = <&gpio3 2 GPIO_ACTIVE_LOW>; > > debounce-ms = <50>; /* 50 millisecond */ > > wakeup-source; > > } > > &dwc3_usb { > > extcon = <&usb_cable>; > > }; > > &charger { > > extcon = <&ta_cable>; > > }; > > > > Signed-off-by: Venkat Reddy Talla <vreddytalla@xxxxxxxxxx> > > Signed-off-by: Chanwoo Choi <cw00.choi@xxxxxxxxxxx> > > Signed-off-by: MyungJoo Ham <myungjoo.ham@xxxxxxxxxxx> > > > > --- > > changes from v3: > > - add description for wakeup-source in documentation > > - change dts property extcon-gpio name to gpios > > - use of_get_named_gpio_flags to get gpio number and flags Changes > > from v2: > > - Add the more description for 'extcon-id' property in documentation > > Changes from v1: > > - Create the include/dt-bindings/extcon/extcon.h including the > > identification of external connector. These definitions are used in dts file. > > - Fix error if CONFIG_OF is disabled. > > - Add signed-off tag by Myungjoo Ham > > --- > > --- > > .../devicetree/bindings/extcon/extcon-gpio.txt | 48 +++++++++ > > drivers/extcon/extcon-gpio.c | 109 +++++++++++++++++---- > > include/dt-bindings/extcon/extcon.h | 47 +++++++++ > > include/linux/extcon/extcon-gpio.h | 8 +- > > 4 files changed, 189 insertions(+), 23 deletions(-) create mode > > 100644 Documentation/devicetree/bindings/extcon/extcon-gpio.txt > > create mode 100644 include/dt-bindings/extcon/extcon.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..81f7932 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt > > @@ -0,0 +1,48 @@ > > +GPIO Extcon device > > + > > +This is a virtual device used to generate the specific cable states > > +from the GPIO pin. > > + > > +Required properties: > > +- compatible: Must be "extcon-gpio". > > +- extcon-id: The extcon support the various type of external > > +connector to check > > + whether connector is attached or detached. The each external > > +connector has > > + the unique number to identify it. So this property includes the > > +unique number > > + which indicates the specific external connector. When external > > +connector is > > + attached or detached, GPIO pin detect the changed state. See > > +include/ > > + dt-bindings/extcon/extcon.h which defines the unique number for > > +supported > > + external connector from extcon framework. > > +- gpios: GPIO pin to detect the external connector. See gpio binding. > > + > > +Optional properties: > > +- debounce-ms: the debounce dealy for GPIO pin in millisecond. > > +- wakeup-source: Boolean, extcon can wake-up the system from > suspend. > > + if gpio provided in extcon-gpio DT node is registered as interrupt, > > + then extcon can wake-up the system from suspend if wakeup-source > > +property > > + is available in DT node, if gpio registered as interrupt but > > +wakeup-source > > + is not available in DT node, then system wake-up due to extcon > > +events > > + not supported. > > + > > +Example: Examples of extcon-gpio node as listed below: > > + > > + usb_cable: extcon-gpio-0 { > > + compatible = "extcon-gpio"; > > + extcon-id = <EXTCON_USB>; > > + extcon-gpio = <&gpio6 1 GPIO_ACTIVE_HIGH>; > > + } > > + > > + ta_cable: extcon-gpio-1 { > > + compatible = "extcon-gpio"; > > + extcon-id = <EXTCON_CHG_USB_DCP>; > > + extcon-gpio = <&gpio3 2 GPIO_ACTIVE_LOW>; > > + debounce-ms = <50>; /* 50 millisecond */ > > + wakeup-source; > > + } > > + > > + &dwc3_usb { > > + extcon = <&usb_cable>; > > + }; > > + > > + &charger { > > + extcon = <&ta_cable>; > > + }; > > diff --git a/drivers/extcon/extcon-gpio.c > > b/drivers/extcon/extcon-gpio.c index d023789..592f395 100644 > > --- a/drivers/extcon/extcon-gpio.c > > +++ b/drivers/extcon/extcon-gpio.c > > @@ -1,11 +1,9 @@ > > /* > > * extcon_gpio.c - Single-state GPIO extcon driver based on extcon class > > * > > - * Copyright (C) 2008 Google, Inc. > > - * Author: Mike Lockwood <lockwood@xxxxxxxxxxx> > > - * > > - * Modified by MyungJoo Ham <myungjoo.ham@xxxxxxxxxxx> to > support > > extcon > > - * (originally switch class is supported) > > + * Copyright (C) 2016 Chanwoo Choi <cw00.choi@xxxxxxxxxxx>, > Samsung > > + Electronics > > + * Copyright (C) 2012 MyungJoo Ham <myungjoo.ham@xxxxxxxxxxx>, > > + Samsung Electronics > > + * Copyright (C) 2008 Mike Lockwood <lockwood@xxxxxxxxxxx>, Google, > Inc. > > * > > * This software is licensed under the terms of the GNU General Public > > * License version 2, as published by the Free Software Foundation, > > and @@ -25,13 +23,17 @@ #include <linux/interrupt.h> #include > > <linux/kernel.h> #include <linux/module.h> > > +#include <linux/of.h> > > +#include <linux/of_gpio.h> > > #include <linux/platform_device.h> > > +#include <linux/property.h> > > #include <linux/slab.h> > > #include <linux/workqueue.h> > > > > struct gpio_extcon_data { > > struct extcon_dev *edev; > > int irq; > > + bool irq_wakeup; > > struct delayed_work work; > > unsigned long debounce_jiffies; > > > > @@ -49,7 +51,7 @@ static void gpio_extcon_work(struct work_struct > *work) > > state = gpiod_get_value_cansleep(data->id_gpiod); > > if (data->pdata->gpio_active_low) > > state = !state; > > - extcon_set_state(data->edev, state); > > + extcon_set_cable_state_(data->edev, data->pdata->extcon_id, > > + state); > > } > > > > static irqreturn_t gpio_irq_handler(int irq, void *dev_id) @@ -61,6 > > +63,42 @@ static irqreturn_t gpio_irq_handler(int irq, void *dev_id) > > return IRQ_HANDLED; > > } > > > > +static int gpio_extcon_parse_of(struct platform_device *pdev, > > + struct gpio_extcon_data *data) { > > + struct gpio_extcon_pdata *pdata; > > + struct device_node *np = pdev->dev.of_node; > > + enum of_gpio_flags flags; > > + int ret; > > + > > + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); > > + if (!pdata) > > + return -ENOMEM; > > + > > + ret = device_property_read_u32(&pdev->dev, "extcon-id", > > + &pdata->extcon_id); > > + if (ret < 0) > > + return -EINVAL; > > + > > + pdata->gpio = of_get_named_gpio_flags(np, "gpios", 0, &flags); > > + if (pdata->gpio < 0) > > + return -EINVAL; > > + > > + if (flags & OF_GPIO_ACTIVE_LOW) > > + pdata->gpio_active_low = true; > > + > > + data->irq_wakeup = device_property_read_bool(&pdev->dev, > > + "wakeup-source"); > > + > > + device_property_read_u32(&pdev->dev, "debounce-ms", > > + &pdata->debounce); > > + > > + pdata->irq_flags = (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING > > + | IRQF_ONESHOT); > > + > > + data->pdata = pdata; > > + return 0; > > +} > > + > > static int gpio_extcon_init(struct device *dev, struct > > gpio_extcon_data *data) { > > struct gpio_extcon_pdata *pdata = data->pdata; @@ -96,16 > > +134,20 @@ static int gpio_extcon_probe(struct platform_device *pdev) > > 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); > > + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); > > if (!data) > > return -ENOMEM; > > - data->pdata = pdata; > > + > > + if (!pdata) { > > + ret = gpio_extcon_parse_of(pdev, data); > > + if (ret < 0) > > + return ret; > > + } else { > > + data->pdata = pdata; > > + } > > + > > + if (!data->pdata->irq_flags || data->pdata->extcon_id == > EXTCON_NONE) > > + return -EINVAL; > > > > /* Initialize the gpio */ > > ret = gpio_extcon_init(&pdev->dev, data); @@ -113,7 +155,8 @@ > > 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->pdata->extcon_id); > > if (IS_ERR(data->edev)) { > > dev_err(&pdev->dev, "failed to allocate extcon device\n"); > > return -ENOMEM; > > @@ -130,7 +173,8 @@ 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->pdata->irq_flags, > > pdev->name, data); > > if (ret < 0) > > return ret; > > @@ -139,6 +183,8 @@ static int gpio_extcon_probe(struct > platform_device *pdev) > > /* Perform initial detection */ > > gpio_extcon_work(&data->work.work); > > > > + if (data->irq_wakeup) > > + device_init_wakeup(&pdev->dev, data->irq_wakeup); > > return 0; > > } > > > > @@ -152,11 +198,23 @@ static int gpio_extcon_remove(struct > > platform_device *pdev) } > > > > #ifdef CONFIG_PM_SLEEP > > +static int gpio_extcon_suspend(struct device *dev) { > > + struct gpio_extcon_data *data = dev_get_drvdata(dev); > > + > > + if (data->irq_wakeup) > > + enable_irq_wake(data->irq); > > + > > + return 0; > > +} > > + > > static int gpio_extcon_resume(struct device *dev) { > > - struct gpio_extcon_data *data; > > + struct gpio_extcon_data *data = dev_get_drvdata(dev); > > + > > + if (data->irq_wakeup) > > + disable_irq_wake(data->irq); > > > > - data = dev_get_drvdata(dev); > > if (data->pdata->check_on_resume) > > queue_delayed_work(system_power_efficient_wq, > > &data->work, data->debounce_jiffies); @@ > > -165,7 +223,18 @@ static int gpio_extcon_resume(struct device *dev) } > > #endif > > > > -static SIMPLE_DEV_PM_OPS(gpio_extcon_pm_ops, NULL, > > gpio_extcon_resume); > > +#if defined(CONFIG_OF) > > +static const struct of_device_id gpio_extcon_of_match[] = { > > + { .compatible = "extcon-gpio", }, > > + { /* sentinel */ }, > > +}; > > +MODULE_DEVICE_TABLE(of, gpio_extcon_of_match); #else > > +#define gpio_extcon_of_match NULL > > +#endif > > + > > +static SIMPLE_DEV_PM_OPS(gpio_extcon_pm_ops, > > + gpio_extcon_suspend, gpio_extcon_resume); > > > > static struct platform_driver gpio_extcon_driver = { > > .probe = gpio_extcon_probe, > > @@ -173,11 +242,13 @@ static struct platform_driver gpio_extcon_driver = > { > > .driver = { > > .name = "extcon-gpio", > > .pm = &gpio_extcon_pm_ops, > > + .of_match_table = gpio_extcon_of_match, > > }, > > }; > > > > module_platform_driver(gpio_extcon_driver); > > > > +MODULE_AUTHOR("Chanwoo Choi <cw00.choi@xxxxxxxxxxx>"); > > MODULE_AUTHOR("Mike Lockwood <lockwood@xxxxxxxxxxx>"); > > MODULE_DESCRIPTION("GPIO extcon driver"); MODULE_LICENSE("GPL"); > diff > > --git a/include/dt-bindings/extcon/extcon.h > > b/include/dt-bindings/extcon/extcon.h > > new file mode 100644 > > index 0000000..dbba588 > > --- /dev/null > > +++ b/include/dt-bindings/extcon/extcon.h > > @@ -0,0 +1,47 @@ > > +/* > > + * This header provides constants for most extcon bindings. > > + * > > + * Most extcon bindings include the unique identification format. > > + * In most cases, the unique id format uses the standard values/macro > > + * defined in this header. > > + */ > > + > > +#ifndef _DT_BINDINGS_EXTCON_EXTCON_H > > +#define _DT_BINDINGS_EXTCON_EXTCON_H > > + > > +/* USB external connector */ > > +#define EXTCON_USB 1 > > +#define EXTCON_USB_HOST 2 > > + > > +/* Charging external connector */ > > +#define EXTCON_CHG_USB_SDP 5 /* Standard Downstream Port */ > > +#define EXTCON_CHG_USB_DCP 6 /* Dedicated Charging Port */ > > +#define EXTCON_CHG_USB_CDP 7 /* Charging Downstream Port */ > > +#define EXTCON_CHG_USB_ACA 8 /* Accessory Charger Adapter */ > > +#define EXTCON_CHG_USB_FAST 9 > > +#define EXTCON_CHG_USB_SLOW 10 > > + > > +/* Jack external connector */ > > +#define EXTCON_JACK_MICROPHONE 20 > > +#define EXTCON_JACK_HEADPHONE 21 > > +#define EXTCON_JACK_LINE_IN 22 > > +#define EXTCON_JACK_LINE_OUT 23 > > +#define EXTCON_JACK_VIDEO_IN 24 > > +#define EXTCON_JACK_VIDEO_OUT 25 > > +#define EXTCON_JACK_SPDIF_IN 26 /* Sony Philips Digital InterFace > */ > > +#define EXTCON_JACK_SPDIF_OUT 27 > > + > > +/* Display external connector */ > > +#define EXTCON_DISP_HDMI 40 /* High-Definition Multimedia > Interface */ > > +#define EXTCON_DISP_MHL 41 /* Mobile High-Definition Link > */ > > +#define EXTCON_DISP_DVI 42 /* Digital Visual Interface */ > > +#define EXTCON_DISP_VGA 43 /* Video Graphics Array */ > > + > > +/* Miscellaneous external connector */ > > +#define EXTCON_DOCK 60 > > +#define EXTCON_JIG 61 > > +#define EXTCON_MECHANICAL 62 > > + > > +#define EXTCON_NUM 63 > > + > > +#endif /* _DT_BINDINGS_EXTCON_EXTCON_H */ > > diff --git a/include/linux/extcon/extcon-gpio.h > > b/include/linux/extcon/extcon-gpio.h > > index 7cacafb..f7e7673 100644 > > --- a/include/linux/extcon/extcon-gpio.h > > +++ b/include/linux/extcon/extcon-gpio.h > > @@ -1,8 +1,8 @@ > > /* > > * Single-state GPIO extcon driver based on extcon class > > * > > - * Copyright (C) 2012 Samsung Electronics > > - * Author: MyungJoo Ham <myungjoo.ham@xxxxxxxxxxx> > > + * Copyright (C) 2016 Chanwoo Choi <cw00.choi@xxxxxxxxxxx>, > Samsung > > + Electronics > > + * Copyright (C) 2012 MyungJoo Ham <myungjoo.ham@xxxxxxxxxxx>, > > + Samsung Electronics > > * > > * based on switch class driver > > * Copyright (C) 2008 Google, Inc. > > @@ -35,10 +35,10 @@ > > * while resuming from sleep. > > */ > > struct gpio_extcon_pdata { > > - unsigned int extcon_id; > > + u32 extcon_id; > > unsigned gpio; > > bool gpio_active_low; > > - unsigned long debounce; > > + u32 debounce; > > unsigned long irq_flags; > > > > bool check_on_resume; > > -- > > 2.1.4 > > ��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
