[RFC v4 0/7] extcon: usb-gpio: fixes and improvements

[RFC v4 0/7] extcon: usb-gpio: fixes and improvements

[Krzysztof Kozlowski]

Hi,


Some time ago, Robert tried to add VBUS detection to extcon-usb-gpio
driver [1].  There was a discussion about patch #2 ("extcon: usb-gpio:
add support for VBUS detection").

The final conclusion was that Chanwoo will add VBUS/ID notifiers [2].
That unfortunately never happened so this patchset is a follow up.

1. Add VBUS/ID cable state notifiers to extcon, so USB controllers
   could use it.
2. Add VBUS detection to extcon-usb-gpio driver.

Some parts are based on old Robert's work, some are new, some are
reworked.


Best regards,
Krzysztof


[1] http://thread.gmane.org/gmane.linux.kernel/1923192/focus=1923193
[2] http://thread.gmane.org/gmane.linux.kernel/1923192/focus=1941152


Krzysztof Kozlowski (5):
  Revert "extcon: usb-gpio: switch to use pm wakeirq apis"
  extcon: Add raw VBUS and ID cable states
  extcon: usb-gpio: Add support for VBUS detection
  ARM: exynos_defconfig: Enable EXTCON_USB_GPIO for Odroid XU3 USB OTG
  ARM: dts: exynos: Add extcon-usb-gpio node for Odroid XU3

Robert Baldyga (2):
  Documentation: extcon: usb-gpio: update usb-gpio binding description
  extcon: usb-gpio: make debounce value configurable in devicetree

 .../devicetree/bindings/extcon/extcon-usb-gpio.txt |  28 ++++-
 arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts    |  21 ++++
 arch/arm/boot/dts/exynos5422-odroidxu3.dts         |  21 ++++
 arch/arm/configs/exynos_defconfig                  |   1 +
 drivers/extcon/extcon-usb-gpio.c                   | 138 +++++++++++++++++----
 drivers/extcon/extcon.c                            |   3 +
 include/linux/extcon.h                             |   8 +-
 7 files changed, 190 insertions(+), 30 deletions(-)

-- 
1.9.1

[RFC v4 1/7] Documentation: extcon: usb-gpio: update usb-gpio binding description

[Krzysztof Kozlowski]

From: Robert Baldyga <r.baldyga@samsung.com>

Add information about VBUS pin detection support, 'debounce' property
and some other details.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
Acked-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 .../devicetree/bindings/extcon/extcon-usb-gpio.txt | 28 ++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
index af0b903de293..7096f399b771 100644
--- a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
+++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
@@ -1,16 +1,40 @@
 USB GPIO Extcon device
 
-This is a virtual device used to generate USB cable states from the USB ID pin
-connected to a GPIO pin.
+This is a virtual device used to generate USB cable states from the USB
+ID and VBUS signals connected to GPIO pins.
+
+The extcon cable states USB and USB_HOST are actually VBUS and !ID
+pin states and do not indicate what mode the USB needs to operate in.
+That decision is done by the USB stack.
+
+Some devices have only one of these GPIO pins, so we support cases when
+only one of them is present. Hence properties 'id-gpio' and 'vbus-gpio'
+are described as optional, but at least one of them has to be present
+in extcon-usb-gpio node.
+
+In general we have three cases:
+1. If VBUS and ID gpios are present we pass them as is
+	USB-HOST = !ID, USB = VBUS
+2. If only VBUS gpio is present we assume that ID pin is always High.
+	USB-HOST = false, USB = VBUS.
+3. If only ID pin is available we infer the VBUS pin states based on ID.
+	USB-HOST = !ID, USB = ID
 
 Required properties:
 - compatible: Should be "linux,extcon-usb-gpio"
+
+Optional properties
 - id-gpio: gpio for USB ID pin. See gpio binding.
+- vbus-gpio: gpio for USB VBUS pin. See gpio binding.
+- debounce: gpio debounce time in milliseconds (u32).
+
 
 Example: Examples of extcon-usb-gpio node in dra7-evm.dts as listed below:
 	extcon_usb1 {
 		compatible = "linux,extcon-usb-gpio";
 		id-gpio = <&gpio6 1 GPIO_ACTIVE_HIGH>;
+		vbus-gpio = <&gpio6 2 GPIO_ACTIVE_HIGH>;
+		debounce = <25>;
 	}
 
 	&omap_dwc3_1 {
-- 
1.9.1

Re: [RFC v4 1/7] Documentation: extcon: usb-gpio: update usb-gpio binding description

[Rob Herring]

On Wed, Jun 08, 2016 at 03:48:00PM +0200, Krzysztof Kozlowski wrote:
> From: Robert Baldyga <r.baldyga@samsung.com>
> 
> Add information about VBUS pin detection support, 'debounce' property
> and some other details.

The extcon bindings are a complete mess if you've seen my recent 
comments. I'm inclined to NAK anything that amends broken bindings. This 
one is headed in the right direction though...


> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
> Acked-by: Roger Quadros <rogerq@ti.com>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
>  .../devicetree/bindings/extcon/extcon-usb-gpio.txt | 28 ++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
> index af0b903de293..7096f399b771 100644
> --- a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
> +++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
> @@ -1,16 +1,40 @@
>  USB GPIO Extcon device
>  
> -This is a virtual device used to generate USB cable states from the USB ID pin
> -connected to a GPIO pin.
> +This is a virtual device used to generate USB cable states from the USB
> +ID and VBUS signals connected to GPIO pins.

It is not a virtual device, but a connector and connector states.

> +
> +The extcon cable states USB and USB_HOST are actually VBUS and !ID
> +pin states and do not indicate what mode the USB needs to operate in.
> +That decision is done by the USB stack.
> +
> +Some devices have only one of these GPIO pins, so we support cases when
> +only one of them is present. Hence properties 'id-gpio' and 'vbus-gpio'
> +are described as optional, but at least one of them has to be present
> +in extcon-usb-gpio node.
> +
> +In general we have three cases:
> +1. If VBUS and ID gpios are present we pass them as is
> +	USB-HOST = !ID, USB = VBUS
> +2. If only VBUS gpio is present we assume that ID pin is always High.
> +	USB-HOST = false, USB = VBUS.
> +3. If only ID pin is available we infer the VBUS pin states based on ID.
> +	USB-HOST = !ID, USB = ID

USB and USB-HOST look like driver details. Shouldn't this just be in 
terms of host and device?

>  
>  Required properties:
>  - compatible: Should be "linux,extcon-usb-gpio"

I'd prefer to see this deprecated in favor of a string that defines the 
type of connector (usb-a-connector, usb-microab-connector, etc.)

> +
> +Optional properties
>  - id-gpio: gpio for USB ID pin. See gpio binding.
> +- vbus-gpio: gpio for USB VBUS pin. See gpio binding.

vbus-gpios

> +- debounce: gpio debounce time in milliseconds (u32).

debounce of which gpio? Needs a unit suffix.

> +
>  
>  Example: Examples of extcon-usb-gpio node in dra7-evm.dts as listed below:
>  	extcon_usb1 {
>  		compatible = "linux,extcon-usb-gpio";
>  		id-gpio = <&gpio6 1 GPIO_ACTIVE_HIGH>;
> +		vbus-gpio = <&gpio6 2 GPIO_ACTIVE_HIGH>;
> +		debounce = <25>;
>  	}
>  
>  	&omap_dwc3_1 {
> -- 
> 1.9.1
> 

[RFC v4 2/7] Revert "extcon: usb-gpio: switch to use pm wakeirq apis"

[Krzysztof Kozlowski]

This reverts commit 8106e404174253639731cc30a44f5b3ab764c5b7.

When using PM wakeirq API only one wakeup IRQ can be set. However the
driver will support also VBUS GPIO so we need two wakeup interrupts.
---
 drivers/extcon/extcon-usb-gpio.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
index 2512660dc4b9..a36aab007022 100644
--- a/drivers/extcon/extcon-usb-gpio.c
+++ b/drivers/extcon/extcon-usb-gpio.c
@@ -24,7 +24,6 @@
 #include <linux/module.h>
 #include <linux/of_gpio.h>
 #include <linux/platform_device.h>
-#include <linux/pm_wakeirq.h>
 #include <linux/slab.h>
 #include <linux/workqueue.h>
 #include <linux/acpi.h>
@@ -143,8 +142,7 @@ static int usb_extcon_probe(struct platform_device *pdev)
 	}
 
 	platform_set_drvdata(pdev, info);
-	device_init_wakeup(dev, true);
-	dev_pm_set_wake_irq(dev, info->id_irq);
+	device_init_wakeup(dev, 1);
 
 	/* Perform initial detection */
 	usb_extcon_detect_cable(&info->wq_detcable.work);
@@ -158,9 +156,6 @@ static int usb_extcon_remove(struct platform_device *pdev)
 
 	cancel_delayed_work_sync(&info->wq_detcable);
 
-	dev_pm_clear_wake_irq(&pdev->dev);
-	device_init_wakeup(&pdev->dev, false);
-
 	return 0;
 }
 
@@ -170,6 +165,12 @@ static int usb_extcon_suspend(struct device *dev)
 	struct usb_extcon_info *info = dev_get_drvdata(dev);
 	int ret = 0;
 
+	if (device_may_wakeup(dev)) {
+		ret = enable_irq_wake(info->id_irq);
+		if (ret)
+			return ret;
+	}
+
 	/*
 	 * We don't want to process any IRQs after this point
 	 * as GPIOs used behind I2C subsystem might not be
@@ -185,6 +186,12 @@ static int usb_extcon_resume(struct device *dev)
 	struct usb_extcon_info *info = dev_get_drvdata(dev);
 	int ret = 0;
 
+	if (device_may_wakeup(dev)) {
+		ret = disable_irq_wake(info->id_irq);
+		if (ret)
+			return ret;
+	}
+
 	enable_irq(info->id_irq);
 	if (!device_may_wakeup(dev))
 		queue_delayed_work(system_power_efficient_wq,
-- 
1.9.1

Re: [RFC v4 2/7] Revert "extcon: usb-gpio: switch to use pm wakeirq apis"

[Roger Quadros]

-balbi@ti
+balbi@kernel

Hi,

On 08/06/16 16:48, Krzysztof Kozlowski wrote:
> This reverts commit 8106e404174253639731cc30a44f5b3ab764c5b7.

The above commit id is not present in v4.7-rc1 and this patch doesn't apply properly.
Can you please rebase this series on v4.7-rc1?

> 
> When using PM wakeirq API only one wakeup IRQ can be set. However the
> driver will support also VBUS GPIO so we need two wakeup interrupts.
> ---
>  drivers/extcon/extcon-usb-gpio.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
> index 2512660dc4b9..a36aab007022 100644
> --- a/drivers/extcon/extcon-usb-gpio.c
> +++ b/drivers/extcon/extcon-usb-gpio.c
> @@ -24,7 +24,6 @@
>  #include <linux/module.h>
>  #include <linux/of_gpio.h>
>  #include <linux/platform_device.h>
> -#include <linux/pm_wakeirq.h>
>  #include <linux/slab.h>
>  #include <linux/workqueue.h>
>  #include <linux/acpi.h>
> @@ -143,8 +142,7 @@ static int usb_extcon_probe(struct platform_device *pdev)
>  	}
>  
>  	platform_set_drvdata(pdev, info);
> -	device_init_wakeup(dev, true);
> -	dev_pm_set_wake_irq(dev, info->id_irq);
> +	device_init_wakeup(dev, 1);
>  
>  	/* Perform initial detection */
>  	usb_extcon_detect_cable(&info->wq_detcable.work);
> @@ -158,9 +156,6 @@ static int usb_extcon_remove(struct platform_device *pdev)
>  
>  	cancel_delayed_work_sync(&info->wq_detcable);
>  
> -	dev_pm_clear_wake_irq(&pdev->dev);
> -	device_init_wakeup(&pdev->dev, false);
> -
>  	return 0;
>  }
>  
> @@ -170,6 +165,12 @@ static int usb_extcon_suspend(struct device *dev)
>  	struct usb_extcon_info *info = dev_get_drvdata(dev);
>  	int ret = 0;
>  
> +	if (device_may_wakeup(dev)) {
> +		ret = enable_irq_wake(info->id_irq);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	/*
>  	 * We don't want to process any IRQs after this point
>  	 * as GPIOs used behind I2C subsystem might not be
> @@ -185,6 +186,12 @@ static int usb_extcon_resume(struct device *dev)
>  	struct usb_extcon_info *info = dev_get_drvdata(dev);
>  	int ret = 0;
>  
> +	if (device_may_wakeup(dev)) {
> +		ret = disable_irq_wake(info->id_irq);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	enable_irq(info->id_irq);
>  	if (!device_may_wakeup(dev))
>  		queue_delayed_work(system_power_efficient_wq,
> 

--
cheers,
-roger

Re: [RFC v4 2/7] Revert "extcon: usb-gpio: switch to use pm wakeirq apis"

[Krzysztof Kozlowski]

On 06/09/2016 10:00 AM, Roger Quadros wrote:
> -balbi@ti
> +balbi@kernel
> 
> Hi,
> 
> On 08/06/16 16:48, Krzysztof Kozlowski wrote:
>> This reverts commit 8106e404174253639731cc30a44f5b3ab764c5b7.
> 
> The above commit id is not present in v4.7-rc1 and this patch doesn't apply properly.
> Can you please rebase this series on v4.7-rc1?

The commit is present in extcon tree for v4.8. The extcon patches will
be applied (probably) to that tree so what is the point of rebasing on
v4.7-rc1?

Also the point of this particular patch is to show that previous change
should be just dropped. Don't replace things to complicated devm-like
functions just to simplify one path in the driver...

Best regards,
Krzysztof

Re: [RFC v4 2/7] Revert "extcon: usb-gpio: switch to use pm wakeirq apis"

[Roger Quadros]

On 09/06/16 11:10, Krzysztof Kozlowski wrote:
> On 06/09/2016 10:00 AM, Roger Quadros wrote:
>> -balbi@ti
>> +balbi@kernel
>>
>> Hi,
>>
>> On 08/06/16 16:48, Krzysztof Kozlowski wrote:
>>> This reverts commit 8106e404174253639731cc30a44f5b3ab764c5b7.
>>
>> The above commit id is not present in v4.7-rc1 and this patch doesn't apply properly.
>> Can you please rebase this series on v4.7-rc1?
> 
> The commit is present in extcon tree for v4.8. The extcon patches will
> be applied (probably) to that tree so what is the point of rebasing on
> v4.7-rc1?

OK, please ignore my comment. Would be nice if you mention in the cover letter
which tree the series is based on.

> 
> Also the point of this particular patch is to show that previous change
> should be just dropped. Don't replace things to complicated devm-like
> functions just to simplify one path in the driver...
> 

--
cheers,
0roger

[RFC v4 3/7] extcon: Add raw VBUS and ID cable states

[Krzysztof Kozlowski]

The USB controller driver might want to receive the changes for VBUS and
ID pins so it could properly handle USB OTG. Add new cable states for
that purpose.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

---

Previous discussion:
http://lkml.kernel.org/g/<1427980385-21285-3-git-send-email-r.baldyga@samsung.com>
---
 drivers/extcon/extcon.c | 3 +++
 include/linux/extcon.h  | 8 +++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 21a123cadf78..aa02d94e8a78 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -74,6 +74,9 @@ static const char *extcon_name[] =  {
 	[EXTCON_JIG]			= "JIG",
 	[EXTCON_MECHANICAL]		= "MECHANICAL",
 
+	[EXTCON_USB_ID]			= "USB-ID",
+	[EXTCON_USB_VBUS]		= "USB-VBUS",
+
 	NULL,
 };
 
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index 7abf674c388c..126ae41b84fd 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -66,7 +66,13 @@
 #define EXTCON_JIG		61
 #define EXTCON_MECHANICAL	62
 
-#define EXTCON_NUM		63
+/* Raw status, useful for USB controllers */
+#define EXTCON_USB_ID		63
+#define EXTCON_USB_VBUS		64
+
+#define EXTCON_NUM		65
+
+
 
 struct extcon_cable;
 
-- 
1.9.1

[RFC v4 4/7] extcon: usb-gpio: Add support for VBUS detection

[Krzysztof Kozlowski]

Add VBUS pin detection support to extcon-usb-gpio driver for boards
which have both VBUS and ID pins, or only one of them.

The logic behind reporting USB and USB-HOST extcon cables is not
affected. The driver however will report extcon changes for USB-VBUS and
USB-ID.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

---

Some parts base on old Robert's patchset.
---
 drivers/extcon/extcon-usb-gpio.c | 125 +++++++++++++++++++++++++++++++--------
 1 file changed, 99 insertions(+), 26 deletions(-)

diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
index a36aab007022..85b8a0ce5731 100644
--- a/drivers/extcon/extcon-usb-gpio.c
+++ b/drivers/extcon/extcon-usb-gpio.c
@@ -35,7 +35,9 @@ struct usb_extcon_info {
 	struct extcon_dev *edev;
 
 	struct gpio_desc *id_gpiod;
+	struct gpio_desc *vbus_gpiod;
 	int id_irq;
+	int vbus_irq;
 
 	unsigned long debounce_jiffies;
 	struct delayed_work wq_detcable;
@@ -44,6 +46,8 @@ struct usb_extcon_info {
 static const unsigned int usb_extcon_cable[] = {
 	EXTCON_USB,
 	EXTCON_USB_HOST,
+	EXTCON_USB_ID,
+	EXTCON_USB_VBUS,
 	EXTCON_NONE,
 };
 
@@ -55,7 +59,8 @@ static void usb_extcon_detect_cable(struct work_struct *work)
 						    wq_detcable);
 
 	/* check ID and update cable state */
-	id = gpiod_get_value_cansleep(info->id_gpiod);
+	id = info->id_gpiod ? gpiod_get_value_cansleep(info->id_gpiod) : 1;
+
 	if (id) {
 		/*
 		 * ID = 1 means USB HOST cable detached.
@@ -73,6 +78,14 @@ static void usb_extcon_detect_cable(struct work_struct *work)
 		extcon_set_cable_state_(info->edev, EXTCON_USB, false);
 		extcon_set_cable_state_(info->edev, EXTCON_USB_HOST, true);
 	}
+
+	if (info->id_gpiod)
+		extcon_set_cable_state_(info->edev, EXTCON_USB_ID, id);
+	if (info->vbus_gpiod) {
+		int vbus = gpiod_get_value_cansleep(info->vbus_gpiod);
+
+		extcon_set_cable_state_(info->edev, EXTCON_USB_VBUS, vbus);
+	}
 }
 
 static irqreturn_t usb_irq_handler(int irq, void *dev_id)
@@ -100,9 +113,11 @@ static int usb_extcon_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	info->dev = dev;
-	info->id_gpiod = devm_gpiod_get(&pdev->dev, "id", GPIOD_IN);
-	if (IS_ERR(info->id_gpiod)) {
-		dev_err(dev, "failed to get ID GPIO\n");
+	info->id_gpiod = devm_gpiod_get_optional(&pdev->dev, "id", GPIOD_IN);
+	info->vbus_gpiod = devm_gpiod_get_optional(&pdev->dev, "vbus",
+						   GPIOD_IN);
+	if (!info->id_gpiod && !info->vbus_gpiod) {
+		dev_err(dev, "failed to get GPIOs\n");
 		return PTR_ERR(info->id_gpiod);
 	}
 
@@ -118,27 +133,54 @@ static int usb_extcon_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = gpiod_set_debounce(info->id_gpiod,
-				 USB_GPIO_DEBOUNCE_MS * 1000);
+	if (info->id_gpiod)
+		ret = gpiod_set_debounce(info->id_gpiod,
+			USB_GPIO_DEBOUNCE_MS * 1000);
+	if (!ret && info->vbus_gpiod) {
+		ret = gpiod_set_debounce(info->vbus_gpiod,
+			USB_GPIO_DEBOUNCE_MS * 1000);
+		if (ret < 0 && info->id_gpiod)
+			gpiod_set_debounce(info->vbus_gpiod, 0);
+	}
 	if (ret < 0)
 		info->debounce_jiffies = msecs_to_jiffies(USB_GPIO_DEBOUNCE_MS);
 
 	INIT_DELAYED_WORK(&info->wq_detcable, usb_extcon_detect_cable);
 
-	info->id_irq = gpiod_to_irq(info->id_gpiod);
-	if (info->id_irq < 0) {
-		dev_err(dev, "failed to get ID IRQ\n");
-		return info->id_irq;
+	if (info->id_gpiod) {
+		info->id_irq = gpiod_to_irq(info->id_gpiod);
+		if (info->id_irq < 0) {
+			dev_err(dev, "failed to get ID IRQ\n");
+			return info->id_irq;
+		}
+		ret = devm_request_threaded_irq(dev, info->id_irq, NULL,
+						usb_irq_handler,
+						IRQF_TRIGGER_RISING |
+						IRQF_TRIGGER_FALLING |
+						IRQF_ONESHOT,
+						pdev->name, info);
+		if (ret < 0) {
+			dev_err(dev, "failed to request handler for ID IRQ\n");
+			return ret;
+		}
 	}
 
-	ret = devm_request_threaded_irq(dev, info->id_irq, NULL,
-					usb_irq_handler,
-					IRQF_TRIGGER_RISING |
-					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-					pdev->name, info);
-	if (ret < 0) {
-		dev_err(dev, "failed to request handler for ID IRQ\n");
-		return ret;
+	if (info->vbus_gpiod) {
+		info->vbus_irq = gpiod_to_irq(info->vbus_gpiod);
+		if (info->vbus_irq < 0) {
+			dev_err(dev, "failed to get VBUS IRQ\n");
+			return info->vbus_irq;
+		}
+		ret = devm_request_threaded_irq(dev, info->vbus_irq, NULL,
+						usb_irq_handler,
+						IRQF_TRIGGER_RISING |
+						IRQF_TRIGGER_FALLING |
+						IRQF_ONESHOT,
+						pdev->name, info);
+		if (ret < 0) {
+			dev_err(dev, "failed to request handler for VBUS IRQ\n");
+			return ret;
+		}
 	}
 
 	platform_set_drvdata(pdev, info);
@@ -166,9 +208,16 @@ static int usb_extcon_suspend(struct device *dev)
 	int ret = 0;
 
 	if (device_may_wakeup(dev)) {
-		ret = enable_irq_wake(info->id_irq);
-		if (ret)
-			return ret;
+		if (info->id_gpiod) {
+			ret = enable_irq_wake(info->id_irq);
+			if (ret)
+				return ret;
+		}
+		if (info->vbus_gpiod) {
+			ret = enable_irq_wake(info->vbus_irq);
+			if (ret)
+				goto err;
+		}
 	}
 
 	/*
@@ -176,8 +225,16 @@ static int usb_extcon_suspend(struct device *dev)
 	 * as GPIOs used behind I2C subsystem might not be
 	 * accessible until resume completes. So disable IRQ.
 	 */
-	disable_irq(info->id_irq);
+	if (info->id_gpiod)
+		disable_irq(info->id_irq);
+	if (info->vbus_gpiod)
+		disable_irq(info->vbus_irq);
+
+	return ret;
 
+err:
+	if (info->id_gpiod)
+		disable_irq_wake(info->id_irq);
 	return ret;
 }
 
@@ -187,17 +244,33 @@ static int usb_extcon_resume(struct device *dev)
 	int ret = 0;
 
 	if (device_may_wakeup(dev)) {
-		ret = disable_irq_wake(info->id_irq);
-		if (ret)
-			return ret;
+		if (info->id_gpiod) {
+			ret = disable_irq_wake(info->id_irq);
+			if (ret)
+				return ret;
+		}
+		if (info->vbus_gpiod) {
+			ret = disable_irq_wake(info->vbus_irq);
+			if (ret)
+				goto err;
+		}
 	}
 
-	enable_irq(info->id_irq);
+	if (info->id_gpiod)
+		enable_irq(info->id_irq);
+	if (info->vbus_gpiod)
+		enable_irq(info->vbus_irq);
+
 	if (!device_may_wakeup(dev))
 		queue_delayed_work(system_power_efficient_wq,
 				   &info->wq_detcable, 0);
 
 	return ret;
+
+err:
+	if (info->id_gpiod)
+		enable_irq_wake(info->id_irq);
+	return ret;
 }
 #endif
 
-- 
1.9.1

Re: [RFC v4 4/7] extcon: usb-gpio: Add support for VBUS detection

[Roger Quadros]

Hi,

On 08/06/16 16:48, Krzysztof Kozlowski wrote:
> Add VBUS pin detection support to extcon-usb-gpio driver for boards
> which have both VBUS and ID pins, or only one of them.
> 
> The logic behind reporting USB and USB-HOST extcon cables is not
> affected. The driver however will report extcon changes for USB-VBUS and
> USB-ID.
> 
> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> 
> ---
> 
> Some parts base on old Robert's patchset.
> ---
>  drivers/extcon/extcon-usb-gpio.c | 125 +++++++++++++++++++++++++++++++--------
>  1 file changed, 99 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
> index a36aab007022..85b8a0ce5731 100644
> --- a/drivers/extcon/extcon-usb-gpio.c
> +++ b/drivers/extcon/extcon-usb-gpio.c
> @@ -35,7 +35,9 @@ struct usb_extcon_info {
>  	struct extcon_dev *edev;
>  
>  	struct gpio_desc *id_gpiod;
> +	struct gpio_desc *vbus_gpiod;
>  	int id_irq;
> +	int vbus_irq;
>  
>  	unsigned long debounce_jiffies;
>  	struct delayed_work wq_detcable;
> @@ -44,6 +46,8 @@ struct usb_extcon_info {
>  static const unsigned int usb_extcon_cable[] = {
>  	EXTCON_USB,
>  	EXTCON_USB_HOST,
> +	EXTCON_USB_ID,
> +	EXTCON_USB_VBUS,
>  	EXTCON_NONE,
>  };
>  
> @@ -55,7 +59,8 @@ static void usb_extcon_detect_cable(struct work_struct *work)
>  						    wq_detcable);
>  
>  	/* check ID and update cable state */
> -	id = gpiod_get_value_cansleep(info->id_gpiod);
> +	id = info->id_gpiod ? gpiod_get_value_cansleep(info->id_gpiod) : 1;
> +
>  	if (id) {
>  		/*
>  		 * ID = 1 means USB HOST cable detached.
> @@ -73,6 +78,14 @@ static void usb_extcon_detect_cable(struct work_struct *work)
>  		extcon_set_cable_state_(info->edev, EXTCON_USB, false);
>  		extcon_set_cable_state_(info->edev, EXTCON_USB_HOST, true);
>  	}
> +
> +	if (info->id_gpiod)
> +		extcon_set_cable_state_(info->edev, EXTCON_USB_ID, id);
> +	if (info->vbus_gpiod) {
> +		int vbus = gpiod_get_value_cansleep(info->vbus_gpiod);
> +
> +		extcon_set_cable_state_(info->edev, EXTCON_USB_VBUS, vbus);
> +	}

What happens if either id_gpiod or vbus_gpiod is present?

As per the DT binding document
"In general we have three cases:
1. If VBUS and ID gpios are present we pass them as is
USB-HOST = !ID, USB = VBUS
2. If only VBUS gpio is present we assume that ID pin is always High.
USB-HOST = false, USB = VBUS.
3. If only ID pin is available we infer the VBUS pin states based on ID.
USB-HOST = !ID, USB = ID"

So do you want to be consistent and infer VBUS and ID states based on the other?

>  }
>  
>  static irqreturn_t usb_irq_handler(int irq, void *dev_id)
> @@ -100,9 +113,11 @@ static int usb_extcon_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	info->dev = dev;
> -	info->id_gpiod = devm_gpiod_get(&pdev->dev, "id", GPIOD_IN);
> -	if (IS_ERR(info->id_gpiod)) {
> -		dev_err(dev, "failed to get ID GPIO\n");
> +	info->id_gpiod = devm_gpiod_get_optional(&pdev->dev, "id", GPIOD_IN);
> +	info->vbus_gpiod = devm_gpiod_get_optional(&pdev->dev, "vbus",
> +						   GPIOD_IN);
> +	if (!info->id_gpiod && !info->vbus_gpiod) {
> +		dev_err(dev, "failed to get GPIOs\n");
>  		return PTR_ERR(info->id_gpiod);
>  	}
>  
> @@ -118,27 +133,54 @@ static int usb_extcon_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	ret = gpiod_set_debounce(info->id_gpiod,
> -				 USB_GPIO_DEBOUNCE_MS * 1000);
> +	if (info->id_gpiod)
> +		ret = gpiod_set_debounce(info->id_gpiod,
> +			USB_GPIO_DEBOUNCE_MS * 1000);
> +	if (!ret && info->vbus_gpiod) {
> +		ret = gpiod_set_debounce(info->vbus_gpiod,
> +			USB_GPIO_DEBOUNCE_MS * 1000);
> +		if (ret < 0 && info->id_gpiod)
> +			gpiod_set_debounce(info->vbus_gpiod, 0);
> +	}
>  	if (ret < 0)
>  		info->debounce_jiffies = msecs_to_jiffies(USB_GPIO_DEBOUNCE_MS);
>  
>  	INIT_DELAYED_WORK(&info->wq_detcable, usb_extcon_detect_cable);
>  
> -	info->id_irq = gpiod_to_irq(info->id_gpiod);
> -	if (info->id_irq < 0) {
> -		dev_err(dev, "failed to get ID IRQ\n");
> -		return info->id_irq;
> +	if (info->id_gpiod) {
> +		info->id_irq = gpiod_to_irq(info->id_gpiod);
> +		if (info->id_irq < 0) {
> +			dev_err(dev, "failed to get ID IRQ\n");
> +			return info->id_irq;
> +		}
> +		ret = devm_request_threaded_irq(dev, info->id_irq, NULL,
> +						usb_irq_handler,
> +						IRQF_TRIGGER_RISING |
> +						IRQF_TRIGGER_FALLING |
> +						IRQF_ONESHOT,
> +						pdev->name, info);
> +		if (ret < 0) {
> +			dev_err(dev, "failed to request handler for ID IRQ\n");
> +			return ret;
> +		}
>  	}
>  
> -	ret = devm_request_threaded_irq(dev, info->id_irq, NULL,
> -					usb_irq_handler,
> -					IRQF_TRIGGER_RISING |
> -					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> -					pdev->name, info);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to request handler for ID IRQ\n");
> -		return ret;
> +	if (info->vbus_gpiod) {
> +		info->vbus_irq = gpiod_to_irq(info->vbus_gpiod);
> +		if (info->vbus_irq < 0) {
> +			dev_err(dev, "failed to get VBUS IRQ\n");
> +			return info->vbus_irq;
> +		}
> +		ret = devm_request_threaded_irq(dev, info->vbus_irq, NULL,
> +						usb_irq_handler,
> +						IRQF_TRIGGER_RISING |
> +						IRQF_TRIGGER_FALLING |
> +						IRQF_ONESHOT,
> +						pdev->name, info);
> +		if (ret < 0) {
> +			dev_err(dev, "failed to request handler for VBUS IRQ\n");
> +			return ret;
> +		}
>  	}
>  
>  	platform_set_drvdata(pdev, info);
> @@ -166,9 +208,16 @@ static int usb_extcon_suspend(struct device *dev)
>  	int ret = 0;
>  
>  	if (device_may_wakeup(dev)) {
> -		ret = enable_irq_wake(info->id_irq);
> -		if (ret)
> -			return ret;
> +		if (info->id_gpiod) {
> +			ret = enable_irq_wake(info->id_irq);
> +			if (ret)
> +				return ret;
> +		}
> +		if (info->vbus_gpiod) {
> +			ret = enable_irq_wake(info->vbus_irq);
> +			if (ret)
> +				goto err;
> +		}
>  	}
>  
>  	/*
> @@ -176,8 +225,16 @@ static int usb_extcon_suspend(struct device *dev)
>  	 * as GPIOs used behind I2C subsystem might not be
>  	 * accessible until resume completes. So disable IRQ.
>  	 */
> -	disable_irq(info->id_irq);
> +	if (info->id_gpiod)
> +		disable_irq(info->id_irq);
> +	if (info->vbus_gpiod)
> +		disable_irq(info->vbus_irq);
> +
> +	return ret;
>  
> +err:
> +	if (info->id_gpiod)
> +		disable_irq_wake(info->id_irq);
>  	return ret;
>  }
>  
> @@ -187,17 +244,33 @@ static int usb_extcon_resume(struct device *dev)
>  	int ret = 0;
>  
>  	if (device_may_wakeup(dev)) {
> -		ret = disable_irq_wake(info->id_irq);
> -		if (ret)
> -			return ret;
> +		if (info->id_gpiod) {
> +			ret = disable_irq_wake(info->id_irq);
> +			if (ret)
> +				return ret;
> +		}
> +		if (info->vbus_gpiod) {
> +			ret = disable_irq_wake(info->vbus_irq);
> +			if (ret)
> +				goto err;
> +		}
>  	}
>  
> -	enable_irq(info->id_irq);
> +	if (info->id_gpiod)
> +		enable_irq(info->id_irq);
> +	if (info->vbus_gpiod)
> +		enable_irq(info->vbus_irq);
> +
>  	if (!device_may_wakeup(dev))
>  		queue_delayed_work(system_power_efficient_wq,
>  				   &info->wq_detcable, 0);
>  
>  	return ret;
> +
> +err:
> +	if (info->id_gpiod)
> +		enable_irq_wake(info->id_irq);
> +	return ret;
>  }
>  #endif
>  
> 

--
cheers,
-roger

Re: [RFC v4 4/7] extcon: usb-gpio: Add support for VBUS detection

[Roger Quadros]

Fixed Felipe's id.

On 09/06/16 11:38, Roger Quadros wrote:
> Hi,
> 
> On 08/06/16 16:48, Krzysztof Kozlowski wrote:
>> Add VBUS pin detection support to extcon-usb-gpio driver for boards
>> which have both VBUS and ID pins, or only one of them.
>>
>> The logic behind reporting USB and USB-HOST extcon cables is not
>> affected. The driver however will report extcon changes for USB-VBUS and
>> USB-ID.
>>
>> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>
>> ---
>>
>> Some parts base on old Robert's patchset.
>> ---
>>  drivers/extcon/extcon-usb-gpio.c | 125 +++++++++++++++++++++++++++++++--------
>>  1 file changed, 99 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
>> index a36aab007022..85b8a0ce5731 100644
>> --- a/drivers/extcon/extcon-usb-gpio.c
>> +++ b/drivers/extcon/extcon-usb-gpio.c
>> @@ -35,7 +35,9 @@ struct usb_extcon_info {
>>  	struct extcon_dev *edev;
>>  
>>  	struct gpio_desc *id_gpiod;
>> +	struct gpio_desc *vbus_gpiod;
>>  	int id_irq;
>> +	int vbus_irq;
>>  
>>  	unsigned long debounce_jiffies;
>>  	struct delayed_work wq_detcable;
>> @@ -44,6 +46,8 @@ struct usb_extcon_info {
>>  static const unsigned int usb_extcon_cable[] = {
>>  	EXTCON_USB,
>>  	EXTCON_USB_HOST,
>> +	EXTCON_USB_ID,
>> +	EXTCON_USB_VBUS,
>>  	EXTCON_NONE,
>>  };
>>  
>> @@ -55,7 +59,8 @@ static void usb_extcon_detect_cable(struct work_struct *work)
>>  						    wq_detcable);
>>  
>>  	/* check ID and update cable state */
>> -	id = gpiod_get_value_cansleep(info->id_gpiod);
>> +	id = info->id_gpiod ? gpiod_get_value_cansleep(info->id_gpiod) : 1;
>> +
>>  	if (id) {
>>  		/*
>>  		 * ID = 1 means USB HOST cable detached.
>> @@ -73,6 +78,14 @@ static void usb_extcon_detect_cable(struct work_struct *work)
>>  		extcon_set_cable_state_(info->edev, EXTCON_USB, false);
>>  		extcon_set_cable_state_(info->edev, EXTCON_USB_HOST, true);
>>  	}
>> +
>> +	if (info->id_gpiod)
>> +		extcon_set_cable_state_(info->edev, EXTCON_USB_ID, id);
>> +	if (info->vbus_gpiod) {
>> +		int vbus = gpiod_get_value_cansleep(info->vbus_gpiod);
>> +
>> +		extcon_set_cable_state_(info->edev, EXTCON_USB_VBUS, vbus);
>> +	}
> 
> What happens if either id_gpiod or vbus_gpiod is present?
> 
> As per the DT binding document
> "In general we have three cases:
> 1. If VBUS and ID gpios are present we pass them as is
> USB-HOST = !ID, USB = VBUS
> 2. If only VBUS gpio is present we assume that ID pin is always High.
> USB-HOST = false, USB = VBUS.
> 3. If only ID pin is available we infer the VBUS pin states based on ID.
> USB-HOST = !ID, USB = ID"
> 
> So do you want to be consistent and infer VBUS and ID states based on the other?
> 
>>  }
>>  
>>  static irqreturn_t usb_irq_handler(int irq, void *dev_id)
>> @@ -100,9 +113,11 @@ static int usb_extcon_probe(struct platform_device *pdev)
>>  		return -ENOMEM;
>>  
>>  	info->dev = dev;
>> -	info->id_gpiod = devm_gpiod_get(&pdev->dev, "id", GPIOD_IN);
>> -	if (IS_ERR(info->id_gpiod)) {
>> -		dev_err(dev, "failed to get ID GPIO\n");
>> +	info->id_gpiod = devm_gpiod_get_optional(&pdev->dev, "id", GPIOD_IN);
>> +	info->vbus_gpiod = devm_gpiod_get_optional(&pdev->dev, "vbus",
>> +						   GPIOD_IN);
>> +	if (!info->id_gpiod && !info->vbus_gpiod) {
>> +		dev_err(dev, "failed to get GPIOs\n");
>>  		return PTR_ERR(info->id_gpiod);
>>  	}
>>  
>> @@ -118,27 +133,54 @@ static int usb_extcon_probe(struct platform_device *pdev)
>>  		return ret;
>>  	}
>>  
>> -	ret = gpiod_set_debounce(info->id_gpiod,
>> -				 USB_GPIO_DEBOUNCE_MS * 1000);
>> +	if (info->id_gpiod)
>> +		ret = gpiod_set_debounce(info->id_gpiod,
>> +			USB_GPIO_DEBOUNCE_MS * 1000);
>> +	if (!ret && info->vbus_gpiod) {
>> +		ret = gpiod_set_debounce(info->vbus_gpiod,
>> +			USB_GPIO_DEBOUNCE_MS * 1000);
>> +		if (ret < 0 && info->id_gpiod)
>> +			gpiod_set_debounce(info->vbus_gpiod, 0);
>> +	}
>>  	if (ret < 0)
>>  		info->debounce_jiffies = msecs_to_jiffies(USB_GPIO_DEBOUNCE_MS);
>>  
>>  	INIT_DELAYED_WORK(&info->wq_detcable, usb_extcon_detect_cable);
>>  
>> -	info->id_irq = gpiod_to_irq(info->id_gpiod);
>> -	if (info->id_irq < 0) {
>> -		dev_err(dev, "failed to get ID IRQ\n");
>> -		return info->id_irq;
>> +	if (info->id_gpiod) {
>> +		info->id_irq = gpiod_to_irq(info->id_gpiod);
>> +		if (info->id_irq < 0) {
>> +			dev_err(dev, "failed to get ID IRQ\n");
>> +			return info->id_irq;
>> +		}
>> +		ret = devm_request_threaded_irq(dev, info->id_irq, NULL,
>> +						usb_irq_handler,
>> +						IRQF_TRIGGER_RISING |
>> +						IRQF_TRIGGER_FALLING |
>> +						IRQF_ONESHOT,
>> +						pdev->name, info);
>> +		if (ret < 0) {
>> +			dev_err(dev, "failed to request handler for ID IRQ\n");
>> +			return ret;
>> +		}
>>  	}
>>  
>> -	ret = devm_request_threaded_irq(dev, info->id_irq, NULL,
>> -					usb_irq_handler,
>> -					IRQF_TRIGGER_RISING |
>> -					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
>> -					pdev->name, info);
>> -	if (ret < 0) {
>> -		dev_err(dev, "failed to request handler for ID IRQ\n");
>> -		return ret;
>> +	if (info->vbus_gpiod) {
>> +		info->vbus_irq = gpiod_to_irq(info->vbus_gpiod);
>> +		if (info->vbus_irq < 0) {
>> +			dev_err(dev, "failed to get VBUS IRQ\n");
>> +			return info->vbus_irq;
>> +		}
>> +		ret = devm_request_threaded_irq(dev, info->vbus_irq, NULL,
>> +						usb_irq_handler,
>> +						IRQF_TRIGGER_RISING |
>> +						IRQF_TRIGGER_FALLING |
>> +						IRQF_ONESHOT,
>> +						pdev->name, info);
>> +		if (ret < 0) {
>> +			dev_err(dev, "failed to request handler for VBUS IRQ\n");
>> +			return ret;
>> +		}
>>  	}
>>  
>>  	platform_set_drvdata(pdev, info);
>> @@ -166,9 +208,16 @@ static int usb_extcon_suspend(struct device *dev)
>>  	int ret = 0;
>>  
>>  	if (device_may_wakeup(dev)) {
>> -		ret = enable_irq_wake(info->id_irq);
>> -		if (ret)
>> -			return ret;
>> +		if (info->id_gpiod) {
>> +			ret = enable_irq_wake(info->id_irq);
>> +			if (ret)
>> +				return ret;
>> +		}
>> +		if (info->vbus_gpiod) {
>> +			ret = enable_irq_wake(info->vbus_irq);
>> +			if (ret)
>> +				goto err;
>> +		}
>>  	}
>>  
>>  	/*
>> @@ -176,8 +225,16 @@ static int usb_extcon_suspend(struct device *dev)
>>  	 * as GPIOs used behind I2C subsystem might not be
>>  	 * accessible until resume completes. So disable IRQ.
>>  	 */
>> -	disable_irq(info->id_irq);
>> +	if (info->id_gpiod)
>> +		disable_irq(info->id_irq);
>> +	if (info->vbus_gpiod)
>> +		disable_irq(info->vbus_irq);
>> +
>> +	return ret;
>>  
>> +err:
>> +	if (info->id_gpiod)
>> +		disable_irq_wake(info->id_irq);
>>  	return ret;
>>  }
>>  
>> @@ -187,17 +244,33 @@ static int usb_extcon_resume(struct device *dev)
>>  	int ret = 0;
>>  
>>  	if (device_may_wakeup(dev)) {
>> -		ret = disable_irq_wake(info->id_irq);
>> -		if (ret)
>> -			return ret;
>> +		if (info->id_gpiod) {
>> +			ret = disable_irq_wake(info->id_irq);
>> +			if (ret)
>> +				return ret;
>> +		}
>> +		if (info->vbus_gpiod) {
>> +			ret = disable_irq_wake(info->vbus_irq);
>> +			if (ret)
>> +				goto err;
>> +		}
>>  	}
>>  
>> -	enable_irq(info->id_irq);
>> +	if (info->id_gpiod)
>> +		enable_irq(info->id_irq);
>> +	if (info->vbus_gpiod)
>> +		enable_irq(info->vbus_irq);
>> +
>>  	if (!device_may_wakeup(dev))
>>  		queue_delayed_work(system_power_efficient_wq,
>>  				   &info->wq_detcable, 0);
>>  
>>  	return ret;
>> +
>> +err:
>> +	if (info->id_gpiod)
>> +		enable_irq_wake(info->id_irq);
>> +	return ret;
>>  }
>>  #endif
>>  
>>
> 
> --
> cheers,
> -roger
> 

Re: [RFC v4 4/7] extcon: usb-gpio: Add support for VBUS detection

[Krzysztof Kozlowski]

On 06/09/2016 10:38 AM, Roger Quadros wrote:
> Hi,
> 
> On 08/06/16 16:48, Krzysztof Kozlowski wrote:
>> Add VBUS pin detection support to extcon-usb-gpio driver for boards
>> which have both VBUS and ID pins, or only one of them.
>>
>> The logic behind reporting USB and USB-HOST extcon cables is not
>> affected. The driver however will report extcon changes for USB-VBUS and
>> USB-ID.
>>
>> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>
>> ---
>>
>> Some parts base on old Robert's patchset.
>> ---
>>  drivers/extcon/extcon-usb-gpio.c | 125 +++++++++++++++++++++++++++++++--------
>>  1 file changed, 99 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
>> index a36aab007022..85b8a0ce5731 100644
>> --- a/drivers/extcon/extcon-usb-gpio.c
>> +++ b/drivers/extcon/extcon-usb-gpio.c
>> @@ -35,7 +35,9 @@ struct usb_extcon_info {
>>  	struct extcon_dev *edev;
>>  
>>  	struct gpio_desc *id_gpiod;
>> +	struct gpio_desc *vbus_gpiod;
>>  	int id_irq;
>> +	int vbus_irq;
>>  
>>  	unsigned long debounce_jiffies;
>>  	struct delayed_work wq_detcable;
>> @@ -44,6 +46,8 @@ struct usb_extcon_info {
>>  static const unsigned int usb_extcon_cable[] = {
>>  	EXTCON_USB,
>>  	EXTCON_USB_HOST,
>> +	EXTCON_USB_ID,
>> +	EXTCON_USB_VBUS,
>>  	EXTCON_NONE,
>>  };
>>  
>> @@ -55,7 +59,8 @@ static void usb_extcon_detect_cable(struct work_struct *work)
>>  						    wq_detcable);
>>  
>>  	/* check ID and update cable state */
>> -	id = gpiod_get_value_cansleep(info->id_gpiod);
>> +	id = info->id_gpiod ? gpiod_get_value_cansleep(info->id_gpiod) : 1;
>> +
>>  	if (id) {
>>  		/*
>>  		 * ID = 1 means USB HOST cable detached.
>> @@ -73,6 +78,14 @@ static void usb_extcon_detect_cable(struct work_struct *work)
>>  		extcon_set_cable_state_(info->edev, EXTCON_USB, false);
>>  		extcon_set_cable_state_(info->edev, EXTCON_USB_HOST, true);
>>  	}
>> +
>> +	if (info->id_gpiod)
>> +		extcon_set_cable_state_(info->edev, EXTCON_USB_ID, id);
>> +	if (info->vbus_gpiod) {
>> +		int vbus = gpiod_get_value_cansleep(info->vbus_gpiod);
>> +
>> +		extcon_set_cable_state_(info->edev, EXTCON_USB_VBUS, vbus);
>> +	}
> 
> What happens if either id_gpiod or vbus_gpiod is present?
> 
> As per the DT binding document
> "In general we have three cases:
> 1. If VBUS and ID gpios are present we pass them as is
> USB-HOST = !ID, USB = VBUS
> 2. If only VBUS gpio is present we assume that ID pin is always High.
> USB-HOST = false, USB = VBUS.
> 3. If only ID pin is available we infer the VBUS pin states based on ID.
> USB-HOST = !ID, USB = ID"
> 
> So do you want to be consistent and infer VBUS and ID states based on the other?

Right, I couldn't make up my mind whether I want to change/improve
existing logic or just add VBUS/ID raw notifying. Finally I left
original code for USB/USB-HOST cables alone.

Best regards,
Krzysztof

Re: [RFC v4 4/7] extcon: usb-gpio: Add support for VBUS detection

[Roger Quadros]

On 09/06/16 11:43, Krzysztof Kozlowski wrote:
> On 06/09/2016 10:38 AM, Roger Quadros wrote:
>> Hi,
>>
>> On 08/06/16 16:48, Krzysztof Kozlowski wrote:
>>> Add VBUS pin detection support to extcon-usb-gpio driver for boards
>>> which have both VBUS and ID pins, or only one of them.
>>>
>>> The logic behind reporting USB and USB-HOST extcon cables is not
>>> affected. The driver however will report extcon changes for USB-VBUS and
>>> USB-ID.
>>>
>>> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>>
>>> ---
>>>
>>> Some parts base on old Robert's patchset.
>>> ---
>>>  drivers/extcon/extcon-usb-gpio.c | 125 +++++++++++++++++++++++++++++++--------
>>>  1 file changed, 99 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
>>> index a36aab007022..85b8a0ce5731 100644
>>> --- a/drivers/extcon/extcon-usb-gpio.c
>>> +++ b/drivers/extcon/extcon-usb-gpio.c
>>> @@ -35,7 +35,9 @@ struct usb_extcon_info {
>>>  	struct extcon_dev *edev;
>>>  
>>>  	struct gpio_desc *id_gpiod;
>>> +	struct gpio_desc *vbus_gpiod;
>>>  	int id_irq;
>>> +	int vbus_irq;
>>>  
>>>  	unsigned long debounce_jiffies;
>>>  	struct delayed_work wq_detcable;
>>> @@ -44,6 +46,8 @@ struct usb_extcon_info {
>>>  static const unsigned int usb_extcon_cable[] = {
>>>  	EXTCON_USB,
>>>  	EXTCON_USB_HOST,
>>> +	EXTCON_USB_ID,
>>> +	EXTCON_USB_VBUS,
>>>  	EXTCON_NONE,
>>>  };
>>>  
>>> @@ -55,7 +59,8 @@ static void usb_extcon_detect_cable(struct work_struct *work)
>>>  						    wq_detcable);
>>>  
>>>  	/* check ID and update cable state */
>>> -	id = gpiod_get_value_cansleep(info->id_gpiod);
>>> +	id = info->id_gpiod ? gpiod_get_value_cansleep(info->id_gpiod) : 1;
>>> +
>>>  	if (id) {
>>>  		/*
>>>  		 * ID = 1 means USB HOST cable detached.
>>> @@ -73,6 +78,14 @@ static void usb_extcon_detect_cable(struct work_struct *work)
>>>  		extcon_set_cable_state_(info->edev, EXTCON_USB, false);
>>>  		extcon_set_cable_state_(info->edev, EXTCON_USB_HOST, true);
>>>  	}
>>> +
>>> +	if (info->id_gpiod)
>>> +		extcon_set_cable_state_(info->edev, EXTCON_USB_ID, id);
>>> +	if (info->vbus_gpiod) {
>>> +		int vbus = gpiod_get_value_cansleep(info->vbus_gpiod);
>>> +
>>> +		extcon_set_cable_state_(info->edev, EXTCON_USB_VBUS, vbus);
>>> +	}
>>
>> What happens if either id_gpiod or vbus_gpiod is present?
>>
>> As per the DT binding document
>> "In general we have three cases:
>> 1. If VBUS and ID gpios are present we pass them as is
>> USB-HOST = !ID, USB = VBUS
>> 2. If only VBUS gpio is present we assume that ID pin is always High.
>> USB-HOST = false, USB = VBUS.
>> 3. If only ID pin is available we infer the VBUS pin states based on ID.
>> USB-HOST = !ID, USB = ID"
>>
>> So do you want to be consistent and infer VBUS and ID states based on the other?
> 
> Right, I couldn't make up my mind whether I want to change/improve
> existing logic or just add VBUS/ID raw notifying. Finally I left
> original code for USB/USB-HOST cables alone.
> 

I think it is better to stick to raw notifying so nothing needs to be changed.
If there is some mechanism to get the pin state and if the pin is not available
it should return error.

cheers,
-roger

[RFC v4 5/7] extcon: usb-gpio: make debounce value configurable in devicetree

[Krzysztof Kozlowski]

From: Robert Baldyga <r.baldyga@samsung.com>

This patch adds devicetree property for setting debounce value. It allows
to set debounce time shorter or longer depending on the needs of given
platform.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
Acked-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/extcon/extcon-usb-gpio.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
index 85b8a0ce5731..e1d5929bca81 100644
--- a/drivers/extcon/extcon-usb-gpio.c
+++ b/drivers/extcon/extcon-usb-gpio.c
@@ -103,6 +103,7 @@ static int usb_extcon_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
 	struct usb_extcon_info *info;
+	u32 debounce;
 	int ret;
 
 	if (!np && !ACPI_HANDLE(dev))
@@ -113,6 +114,11 @@ static int usb_extcon_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	info->dev = dev;
+
+	ret = of_property_read_u32(np, "debounce", &debounce);
+	if (ret < 0)
+		debounce = USB_GPIO_DEBOUNCE_MS;
+
 	info->id_gpiod = devm_gpiod_get_optional(&pdev->dev, "id", GPIOD_IN);
 	info->vbus_gpiod = devm_gpiod_get_optional(&pdev->dev, "vbus",
 						   GPIOD_IN);
@@ -134,16 +140,14 @@ static int usb_extcon_probe(struct platform_device *pdev)
 	}
 
 	if (info->id_gpiod)
-		ret = gpiod_set_debounce(info->id_gpiod,
-			USB_GPIO_DEBOUNCE_MS * 1000);
+		ret = gpiod_set_debounce(info->id_gpiod, debounce * 1000);
 	if (!ret && info->vbus_gpiod) {
-		ret = gpiod_set_debounce(info->vbus_gpiod,
-			USB_GPIO_DEBOUNCE_MS * 1000);
+		ret = gpiod_set_debounce(info->vbus_gpiod, debounce * 1000);
 		if (ret < 0 && info->id_gpiod)
 			gpiod_set_debounce(info->vbus_gpiod, 0);
 	}
 	if (ret < 0)
-		info->debounce_jiffies = msecs_to_jiffies(USB_GPIO_DEBOUNCE_MS);
+		info->debounce_jiffies = msecs_to_jiffies(debounce);
 
 	INIT_DELAYED_WORK(&info->wq_detcable, usb_extcon_detect_cable);
 
-- 
1.9.1

[RFC v4 6/7] ARM: exynos_defconfig: Enable EXTCON_USB_GPIO for Odroid XU3 USB OTG

[Krzysztof Kozlowski]

The USB 3.0 OTG requires working extcon for reporting VBUS/ID changes.
This is provided by extcon-usb-gpio driver.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 arch/arm/configs/exynos_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig
index daf9762ec794..755c4de94eca 100644
--- a/arch/arm/configs/exynos_defconfig
+++ b/arch/arm/configs/exynos_defconfig
@@ -219,6 +219,7 @@ CONFIG_COMMON_CLK_MAX77686=y
 CONFIG_COMMON_CLK_MAX77802=y
 CONFIG_COMMON_CLK_S2MPS11=y
 CONFIG_EXTCON=y
+CONFIG_EXTCON_USB_GPIO=y
 CONFIG_EXTCON_MAX14577=y
 CONFIG_EXTCON_MAX77693=y
 CONFIG_EXTCON_MAX8997=y
-- 
1.9.1

[RFC v4 7/7] ARM: dts: exynos: Add extcon-usb-gpio node for Odroid XU3

[Krzysztof Kozlowski]

Add node for extcon-usb-gpio driver which will be providing
notifications about VBUS/ID changes to the dwc3 driver.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts | 21 +++++++++++++++++++++
 arch/arm/boot/dts/exynos5422-odroidxu3.dts      | 21 +++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts
index 03fa88c45426..3d4d51bc2e3d 100644
--- a/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts
@@ -19,6 +19,27 @@
 / {
 	model = "Hardkernel Odroid XU3 Lite";
 	compatible = "hardkernel,odroid-xu3-lite", "samsung,exynos5800", "samsung,exynos5";
+
+	extcon_usb: extcon_usb {
+		compatible = "linux,extcon-usb-gpio";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&usb_otg_id &usb_vbus>;
+		id-gpio = <&gpx1 1 GPIO_ACTIVE_HIGH>;
+		vbus-gpio = <&gpx3 4 GPIO_ACTIVE_HIGH>;
+	};
+};
+
+&pinctrl_0 {
+	usb_otg_id: usb-otg-id {
+		samsung,pins = "gpx1-1";
+		samsung,pin-pud = <0>;
+	};
+
+	usb_vbus: usb-vbus {
+		samsung,pins = "gpx3-4";
+		samsung,pin-pud = <0>;
+	};
 };
 
 &pwm {
diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3.dts b/arch/arm/boot/dts/exynos5422-odroidxu3.dts
index 9ed6564acfb0..020eb097c9c6 100644
--- a/arch/arm/boot/dts/exynos5422-odroidxu3.dts
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3.dts
@@ -18,6 +18,15 @@
 / {
 	model = "Hardkernel Odroid XU3";
 	compatible = "hardkernel,odroid-xu3", "samsung,exynos5800", "samsung,exynos5";
+
+	extcon_usb: extcon_usb {
+		compatible = "linux,extcon-usb-gpio";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&usb_otg_id &usb_vbus>;
+		id-gpio = <&gpx1 1 GPIO_ACTIVE_HIGH>;
+		vbus-gpio = <&gpx3 4 GPIO_ACTIVE_HIGH>;
+	};
 };
 
 &i2c_0 {
@@ -52,6 +61,18 @@
 	};
 };
 
+&pinctrl_0 {
+	usb_otg_id: usb-otg-id {
+		samsung,pins = "gpx1-1";
+		samsung,pin-pud = <0>;
+	};
+
+	usb_vbus: usb-vbus {
+		samsung,pins = "gpx3-4";
+		samsung,pin-pud = <0>;
+	};
+};
+
 &pwm {
 	/*
 	 * PWM 0 -- fan
-- 
1.9.1

Re: [RFC v4 0/7] extcon: usb-gpio: fixes and improvements

[Chanwoo Choi]

Hi,

It is good to support USB_ID and USB_VBUS by extcon.

But,
there is some issue about adding the new cable type for
both EXTCON_USB_ID and EXTCON_USB_VBUS

I think that the ID and VBUS state are not cable type
Instead, ID and VBUS state are the property of USB cable.

So, I'd like to add the following function to support
the property of each cable as following:
The client driver can get the state of property by using
the extcon_get_cable_property_state().

- int extcon_get_cable_property_state(struct extcon_dev *edev,
				unsigned int id,
				enum extcon_property property);
- int extcon_set_cable_property_state(struct extcon_dev *edev,
				unsigned int id,
				enum extcon_property property,
				unsigned int state);

For example,
In extcon-usb-gpio.c, set state of property as follwoing:
	extcon_set_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_ID, 1);
	extcon_set_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_VBUS, 1);


In the extcon client driver, get state of property as following:
	id_state = extcon_get_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_ID);
	vbus_state = extcon_get_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_VUBS);

Regards,
Chanwoo Choi


On 2016년 06월 08일 22:47, Krzysztof Kozlowski wrote:
> Hi,
> 
> 
> Some time ago, Robert tried to add VBUS detection to extcon-usb-gpio
> driver [1].  There was a discussion about patch #2 ("extcon: usb-gpio:
> add support for VBUS detection").
> 
> The final conclusion was that Chanwoo will add VBUS/ID notifiers [2].
> That unfortunately never happened so this patchset is a follow up.
> 
> 1. Add VBUS/ID cable state notifiers to extcon, so USB controllers
>    could use it.
> 2. Add VBUS detection to extcon-usb-gpio driver.
> 
> Some parts are based on old Robert's work, some are new, some are
> reworked.
> 
> 
> Best regards,
> Krzysztof
> 
> 
> [1] http://thread.gmane.org/gmane.linux.kernel/1923192/focus=1923193
> [2] http://thread.gmane.org/gmane.linux.kernel/1923192/focus=1941152
> 
> 
> Krzysztof Kozlowski (5):
>   Revert "extcon: usb-gpio: switch to use pm wakeirq apis"
>   extcon: Add raw VBUS and ID cable states
>   extcon: usb-gpio: Add support for VBUS detection
>   ARM: exynos_defconfig: Enable EXTCON_USB_GPIO for Odroid XU3 USB OTG
>   ARM: dts: exynos: Add extcon-usb-gpio node for Odroid XU3
> 
> Robert Baldyga (2):
>   Documentation: extcon: usb-gpio: update usb-gpio binding description
>   extcon: usb-gpio: make debounce value configurable in devicetree
> 
>  .../devicetree/bindings/extcon/extcon-usb-gpio.txt |  28 ++++-
>  arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts    |  21 ++++
>  arch/arm/boot/dts/exynos5422-odroidxu3.dts         |  21 ++++
>  arch/arm/configs/exynos_defconfig                  |   1 +
>  drivers/extcon/extcon-usb-gpio.c                   | 138 +++++++++++++++++----
>  drivers/extcon/extcon.c                            |   3 +
>  include/linux/extcon.h                             |   8 +-
>  7 files changed, 190 insertions(+), 30 deletions(-)
> 

Re: [RFC v4 0/7] extcon: usb-gpio: fixes and improvements

[Krzysztof Kozlowski]

Hi,


On 06/09/2016 10:35 AM, Chanwoo Choi wrote:
> Hi,
> 
> It is good to support USB_ID and USB_VBUS by extcon.
> 
> But,
> there is some issue about adding the new cable type for
> both EXTCON_USB_ID and EXTCON_USB_VBUS
> 
> I think that the ID and VBUS state are not cable type
> Instead, ID and VBUS state are the property of USB cable.
> 
> So, I'd like to add the following function to support
> the property of each cable as following:
> The client driver can get the state of property by using
> the extcon_get_cable_property_state().
> 
> - int extcon_get_cable_property_state(struct extcon_dev *edev,
> 				unsigned int id,
> 				enum extcon_property property);
> - int extcon_set_cable_property_state(struct extcon_dev *edev,
> 				unsigned int id,
> 				enum extcon_property property,
> 				unsigned int state);
> 
> For example,
> In extcon-usb-gpio.c, set state of property as follwoing:
> 	extcon_set_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_ID, 1);
> 	extcon_set_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_VBUS, 1);
> 
> 
> In the extcon client driver, get state of property as following:
> 	id_state = extcon_get_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_ID);
> 	vbus_state = extcon_get_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_VUBS);

How one can receive notifications with this API?

Last time you wrote, at the end of discussion:
http://thread.gmane.org/gmane.linux.kernel/1923192/focus=1923193
> IMO, if some usb driver check both id and vbus pin at the same time,
> the usb driver can know the both id and vbus pin state through only
one notifier event.
>
> Also,
> If some usb driver want to know the state of id pin except of vbus state,
> when receiving following events, id pin is low.
> 	#define EXTCON_USB_ID_L_VBUS_L0
> 	#define EXTCON_USB_ID_L_VBUS_H1
> when receiving following events, id pin is high.
> 	#define EXTCON_USB_ID_H_VBUS_L2
> 	#define EXTCON_USB_ID_H_VBUS_H3
> Also, some usb driver would catch the vbus pin state with same method.
>
> But, it is just my opinion. We may use following notifier events for
each pin.
> We need to discuss it.
> 	#define EXTCON_USB_ID_LOW
> 	#define EXTCON_USB_ID_HIGH	
> 	#define EXTCON_USB_VBUS_LOW
> 	#define EXTCON_USB_VBUS_HIGH

... all other participants agreed on that conclusion. So why change of
view point now?

Best regards,
Krzysztof

Re: [RFC v4 0/7] extcon: usb-gpio: fixes and improvements

[Chanwoo Choi]

Hi,

On 2016년 06월 09일 17:39, Krzysztof Kozlowski wrote:
> 
> Hi,
> 
> 
> On 06/09/2016 10:35 AM, Chanwoo Choi wrote:
>> Hi,
>>
>> It is good to support USB_ID and USB_VBUS by extcon.
>>
>> But,
>> there is some issue about adding the new cable type for
>> both EXTCON_USB_ID and EXTCON_USB_VBUS
>>
>> I think that the ID and VBUS state are not cable type
>> Instead, ID and VBUS state are the property of USB cable.
>>
>> So, I'd like to add the following function to support
>> the property of each cable as following:
>> The client driver can get the state of property by using
>> the extcon_get_cable_property_state().
>>
>> - int extcon_get_cable_property_state(struct extcon_dev *edev,
>> 				unsigned int id,
>> 				enum extcon_property property);
>> - int extcon_set_cable_property_state(struct extcon_dev *edev,
>> 				unsigned int id,
>> 				enum extcon_property property,
>> 				unsigned int state);
>>
>> For example,
>> In extcon-usb-gpio.c, set state of property as follwoing:
>> 	extcon_set_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_ID, 1);
>> 	extcon_set_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_VBUS, 1);
>>
>>
>> In the extcon client driver, get state of property as following:
>> 	id_state = extcon_get_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_ID);
>> 	vbus_state = extcon_get_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_VUBS);
> 
> How one can receive notifications with this API?

I think that the extcon don't support the notification only for property.
When USB or USB_HOST cable state is changed, the client driver
call the extcon_get_cable_property_state() to read the state of property.

Namely, the property depend on the specific external connector.

> 
> Last time you wrote, at the end of discussion:
> http://thread.gmane.org/gmane.linux.kernel/1923192/focus=1923193
>> IMO, if some usb driver check both id and vbus pin at the same time,
>> the usb driver can know the both id and vbus pin state through only
> one notifier event.
>>
>> Also,
>> If some usb driver want to know the state of id pin except of vbus state,
>> when receiving following events, id pin is low.
>> 	#define EXTCON_USB_ID_L_VBUS_L0
>> 	#define EXTCON_USB_ID_L_VBUS_H1
>> when receiving following events, id pin is high.
>> 	#define EXTCON_USB_ID_H_VBUS_L2
>> 	#define EXTCON_USB_ID_H_VBUS_H3
>> Also, some usb driver would catch the vbus pin state with same method.
>>
>> But, it is just my opinion. We may use following notifier events for
> each pin.
>> We need to discuss it.
>> 	#define EXTCON_USB_ID_LOW
>> 	#define EXTCON_USB_ID_HIGH	
>> 	#define EXTCON_USB_VBUS_LOW
>> 	#define EXTCON_USB_VBUS_HIGH
> 
> ... all other participants agreed on that conclusion. So why change of
> view point now?

Unitl now, the extcon framework only handle the state of external connector(cable).
- The state of external connector is either attached or detached.

I think that ID and VBUS are not external connector(cable).
The ID and VBUS are more appropriate as property than new type of external connector.

Regards,
Chanwoo Choi

Re: [RFC v4 0/7] extcon: usb-gpio: fixes and improvements

[Roger Quadros]

Hi,

On 09/06/16 12:32, Chanwoo Choi wrote:
> Hi,
> 
> On 2016년 06월 09일 17:39, Krzysztof Kozlowski wrote:
>>
>> Hi,
>>
>>
>> On 06/09/2016 10:35 AM, Chanwoo Choi wrote:
>>> Hi,
>>>
>>> It is good to support USB_ID and USB_VBUS by extcon.
>>>
>>> But,
>>> there is some issue about adding the new cable type for
>>> both EXTCON_USB_ID and EXTCON_USB_VBUS
>>>
>>> I think that the ID and VBUS state are not cable type
>>> Instead, ID and VBUS state are the property of USB cable.
>>>
>>> So, I'd like to add the following function to support
>>> the property of each cable as following:
>>> The client driver can get the state of property by using
>>> the extcon_get_cable_property_state().
>>>
>>> - int extcon_get_cable_property_state(struct extcon_dev *edev,
>>> 				unsigned int id,
>>> 				enum extcon_property property);
>>> - int extcon_set_cable_property_state(struct extcon_dev *edev,
>>> 				unsigned int id,
>>> 				enum extcon_property property,
>>> 				unsigned int state);
>>>
>>> For example,
>>> In extcon-usb-gpio.c, set state of property as follwoing:
>>> 	extcon_set_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_ID, 1);
>>> 	extcon_set_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_VBUS, 1);
>>>
>>>
>>> In the extcon client driver, get state of property as following:
>>> 	id_state = extcon_get_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_ID);
>>> 	vbus_state = extcon_get_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_VUBS);
>>
>> How one can receive notifications with this API?
> 
> I think that the extcon don't support the notification only for property.
> When USB or USB_HOST cable state is changed, the client driver
> call the extcon_get_cable_property_state() to read the state of property.
> 
> Namely, the property depend on the specific external connector.
> 
>>
>> Last time you wrote, at the end of discussion:
>> http://thread.gmane.org/gmane.linux.kernel/1923192/focus=1923193
>>> IMO, if some usb driver check both id and vbus pin at the same time,
>>> the usb driver can know the both id and vbus pin state through only
>> one notifier event.
>>>
>>> Also,
>>> If some usb driver want to know the state of id pin except of vbus state,
>>> when receiving following events, id pin is low.
>>> 	#define EXTCON_USB_ID_L_VBUS_L0
>>> 	#define EXTCON_USB_ID_L_VBUS_H1
>>> when receiving following events, id pin is high.
>>> 	#define EXTCON_USB_ID_H_VBUS_L2
>>> 	#define EXTCON_USB_ID_H_VBUS_H3
>>> Also, some usb driver would catch the vbus pin state with same method.
>>>
>>> But, it is just my opinion. We may use following notifier events for
>> each pin.
>>> We need to discuss it.
>>> 	#define EXTCON_USB_ID_LOW
>>> 	#define EXTCON_USB_ID_HIGH	
>>> 	#define EXTCON_USB_VBUS_LOW
>>> 	#define EXTCON_USB_VBUS_HIGH
>>
>> ... all other participants agreed on that conclusion. So why change of
>> view point now?
> 
> Unitl now, the extcon framework only handle the state of external connector(cable).
> - The state of external connector is either attached or detached.
> 
> I think that ID and VBUS are not external connector(cable).
> The ID and VBUS are more appropriate as property than new type of external connector.
> 

Was there any conclusion on how we can support raw ID and VBUS events with extcon?

cheers,
-roger

Re: [RFC v4 0/7] extcon: usb-gpio: fixes and improvements

[Chanwoo Choi]

Hi Roger,

2016-08-01 21:23 GMT+09:00 Roger Quadros <rogerq@ti.com>:
> Hi,
>
> On 09/06/16 12:32, Chanwoo Choi wrote:
>> Hi,
>>
>> On 2016년 06월 09일 17:39, Krzysztof Kozlowski wrote:
>>>
>>> Hi,
>>>
>>>
>>> On 06/09/2016 10:35 AM, Chanwoo Choi wrote:
>>>> Hi,
>>>>
>>>> It is good to support USB_ID and USB_VBUS by extcon.
>>>>
>>>> But,
>>>> there is some issue about adding the new cable type for
>>>> both EXTCON_USB_ID and EXTCON_USB_VBUS
>>>>
>>>> I think that the ID and VBUS state are not cable type
>>>> Instead, ID and VBUS state are the property of USB cable.
>>>>
>>>> So, I'd like to add the following function to support
>>>> the property of each cable as following:
>>>> The client driver can get the state of property by using
>>>> the extcon_get_cable_property_state().
>>>>
>>>> - int extcon_get_cable_property_state(struct extcon_dev *edev,
>>>>                             unsigned int id,
>>>>                             enum extcon_property property);
>>>> - int extcon_set_cable_property_state(struct extcon_dev *edev,
>>>>                             unsigned int id,
>>>>                             enum extcon_property property,
>>>>                             unsigned int state);
>>>>
>>>> For example,
>>>> In extcon-usb-gpio.c, set state of property as follwoing:
>>>>     extcon_set_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_ID, 1);
>>>>     extcon_set_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_VBUS, 1);
>>>>
>>>>
>>>> In the extcon client driver, get state of property as following:
>>>>     id_state = extcon_get_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_ID);
>>>>     vbus_state = extcon_get_cable_property_state(edev, EXTCON_USB, EXTCON_USB_PROP_VUBS);
>>>
>>> How one can receive notifications with this API?
>>
>> I think that the extcon don't support the notification only for property.
>> When USB or USB_HOST cable state is changed, the client driver
>> call the extcon_get_cable_property_state() to read the state of property.
>>
>> Namely, the property depend on the specific external connector.
>>
>>>
>>> Last time you wrote, at the end of discussion:
>>> http://thread.gmane.org/gmane.linux.kernel/1923192/focus=1923193
>>>> IMO, if some usb driver check both id and vbus pin at the same time,
>>>> the usb driver can know the both id and vbus pin state through only
>>> one notifier event.
>>>>
>>>> Also,
>>>> If some usb driver want to know the state of id pin except of vbus state,
>>>> when receiving following events, id pin is low.
>>>>     #define EXTCON_USB_ID_L_VBUS_L0
>>>>     #define EXTCON_USB_ID_L_VBUS_H1
>>>> when receiving following events, id pin is high.
>>>>     #define EXTCON_USB_ID_H_VBUS_L2
>>>>     #define EXTCON_USB_ID_H_VBUS_H3
>>>> Also, some usb driver would catch the vbus pin state with same method.
>>>>
>>>> But, it is just my opinion. We may use following notifier events for
>>> each pin.
>>>> We need to discuss it.
>>>>     #define EXTCON_USB_ID_LOW
>>>>     #define EXTCON_USB_ID_HIGH
>>>>     #define EXTCON_USB_VBUS_LOW
>>>>     #define EXTCON_USB_VBUS_HIGH
>>>
>>> ... all other participants agreed on that conclusion. So why change of
>>> view point now?
>>
>> Unitl now, the extcon framework only handle the state of external connector(cable).
>> - The state of external connector is either attached or detached.
>>
>> I think that ID and VBUS are not external connector(cable).
>> The ID and VBUS are more appropriate as property than new type of external connector.
>>
>
> Was there any conclusion on how we can support raw ID and VBUS events with extcon?

The extcon will support the USB_ID and USB_VBUS as extcon property.
I'm sending the patches[1] for extcon property.

[1] https://lkml.org/lkml/2016/8/1/25
: [PATCH v2 0/6] extcon: Add the support for extcon type and property

Thanks,
Chanwoo Choi

Re: [RFC v4 0/7] extcon: usb-gpio: fixes and improvements

[Tobias Jakobi]

Hello Krzysztof,

just wanted to ask on which kernel branch the patchset is based on. At
least for me the set doesn't apply cleanly to 4.7-rc4.

With best wishes,
Tobias


Krzysztof Kozlowski wrote:
> Hi,
> 
> 
> Some time ago, Robert tried to add VBUS detection to extcon-usb-gpio
> driver [1].  There was a discussion about patch #2 ("extcon: usb-gpio:
> add support for VBUS detection").
> 
> The final conclusion was that Chanwoo will add VBUS/ID notifiers [2].
> That unfortunately never happened so this patchset is a follow up.
> 
> 1. Add VBUS/ID cable state notifiers to extcon, so USB controllers
>    could use it.
> 2. Add VBUS detection to extcon-usb-gpio driver.
> 
> Some parts are based on old Robert's work, some are new, some are
> reworked.
> 
> 
> Best regards,
> Krzysztof
> 
> 
> [1] http://thread.gmane.org/gmane.linux.kernel/1923192/focus=1923193
> [2] http://thread.gmane.org/gmane.linux.kernel/1923192/focus=1941152
> 
> 
> Krzysztof Kozlowski (5):
>   Revert "extcon: usb-gpio: switch to use pm wakeirq apis"
>   extcon: Add raw VBUS and ID cable states
>   extcon: usb-gpio: Add support for VBUS detection
>   ARM: exynos_defconfig: Enable EXTCON_USB_GPIO for Odroid XU3 USB OTG
>   ARM: dts: exynos: Add extcon-usb-gpio node for Odroid XU3
> 
> Robert Baldyga (2):
>   Documentation: extcon: usb-gpio: update usb-gpio binding description
>   extcon: usb-gpio: make debounce value configurable in devicetree
> 
>  .../devicetree/bindings/extcon/extcon-usb-gpio.txt |  28 ++++-
>  arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts    |  21 ++++
>  arch/arm/boot/dts/exynos5422-odroidxu3.dts         |  21 ++++
>  arch/arm/configs/exynos_defconfig                  |   1 +
>  drivers/extcon/extcon-usb-gpio.c                   | 138 +++++++++++++++++----
>  drivers/extcon/extcon.c                            |   3 +
>  include/linux/extcon.h                             |   8 +-
>  7 files changed, 190 insertions(+), 30 deletions(-)
> 

Re: [RFC v4 0/7] extcon: usb-gpio: fixes and improvements

[Krzysztof Kozlowski]

On 06/26/2016 06:39 PM, Tobias Jakobi wrote:
> Hello Krzysztof,
> 
> just wanted to ask on which kernel branch the patchset is based on. At
> least for me the set doesn't apply cleanly to 4.7-rc4.

Hi,

It was based on next-20160608.

Best regards,
Krzysztof