[PATCH v3 0/4] extcon: usb-gpio: fixes and improvements

[PATCH v3 0/4] extcon: usb-gpio: fixes and improvements

[Robert Baldyga]

Hello,

This patch set modifies extcon-usb-gpio driver fixing bugs, and adds
new features - VBUS pin detection support and 'debounce' property in
devicetree node. It also updates documentation with information about
new features.

More detailed description of changes can be found in commit messages.

Best regards,
Robert Baldyga

Changelog:
v3:
- reduced number of redundant calls of extcon_set_cable_state() function

v2: https://lkml.org/lkml/2015/4/1/84
- addressed Roger's comments

v1: https://lkml.org/lkml/2015/3/31/96

Robert Baldyga (4):
  extcon: usb-gpio: register extcon device before IRQ registration
  extcon: usb-gpio: add support for VBUS detection
  extcon: usb-gpio: make debounce value configurable in devicetree
  Documentation: extcon: usb-gpio: update usb-gpio binding description

 .../devicetree/bindings/extcon/extcon-usb-gpio.txt |  28 +++-
 drivers/extcon/extcon-usb-gpio.c                   | 181 +++++++++++++++------
 2 files changed, 153 insertions(+), 56 deletions(-)

-- 
1.9.1

[PATCH v3 1/4] extcon: usb-gpio: register extcon device before IRQ registration

[Robert Baldyga]

IRQ handler touches info->edev, so if interrupt occurs before extcon
device initialization it can cause NULL pointer dereference. Doing extcon
initialization before IRQ handler registration fixes this problem.

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

diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
index 3f0bad3..f6aa99d 100644
--- a/drivers/extcon/extcon-usb-gpio.c
+++ b/drivers/extcon/extcon-usb-gpio.c
@@ -119,6 +119,18 @@ static int usb_extcon_probe(struct platform_device *pdev)
 		return PTR_ERR(info->id_gpiod);
 	}
 
+	info->edev = devm_extcon_dev_allocate(dev, usb_extcon_cable);
+	if (IS_ERR(info->edev)) {
+		dev_err(dev, "failed to allocate extcon device\n");
+		return -ENOMEM;
+	}
+
+	ret = devm_extcon_dev_register(dev, info->edev);
+	if (ret < 0) {
+		dev_err(dev, "failed to register extcon device\n");
+		return ret;
+	}
+
 	ret = gpiod_set_debounce(info->id_gpiod,
 				 USB_GPIO_DEBOUNCE_MS * 1000);
 	if (ret < 0)
@@ -142,18 +154,6 @@ static int usb_extcon_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	info->edev = devm_extcon_dev_allocate(dev, usb_extcon_cable);
-	if (IS_ERR(info->edev)) {
-		dev_err(dev, "failed to allocate extcon device\n");
-		return -ENOMEM;
-	}
-
-	ret = devm_extcon_dev_register(dev, info->edev);
-	if (ret < 0) {
-		dev_err(dev, "failed to register extcon device\n");
-		return ret;
-	}
-
 	platform_set_drvdata(pdev, info);
 	device_init_wakeup(dev, 1);
 
-- 
1.9.1

[PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Robert Baldyga]

This patch adds VBUS pin detection support to extcon-usb-gpio driver.
It allows to use this driver with boards which have both VBUS and ID
pins, or only one of them.

Following table of states presents relationship between this signals
and detected cable type:

 State              |    ID   |   VBUS
----------------------------------------
 [1] USB            |    H    |    H
 [2] none           |    H    |    L
 [3] USB & USB-HOST |    L    |    H
 [4] USB-HOST       |    L    |    L

In case we have only one of these signals:
- VBUS only - we want to distinguish between [1] and [2], so ID is always 1.
- ID only - we want to distinguish between [1] and [4], so VBUS = ID.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/extcon/extcon-usb-gpio.c | 169 +++++++++++++++++++++++++++------------
 1 file changed, 119 insertions(+), 50 deletions(-)

diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
index f6aa99d..baf7add 100644
--- a/drivers/extcon/extcon-usb-gpio.c
+++ b/drivers/extcon/extcon-usb-gpio.c
@@ -32,7 +32,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;
@@ -52,40 +54,51 @@ static const char *usb_extcon_cable[] = {
 	NULL,
 };
 
+/*
+ * "USB" = VBUS and "USB-HOST" = !ID, so we have:
+ *
+ *  State              |    ID   |   VBUS
+ * ----------------------------------------
+ *  [1] USB            |    H    |    H
+ *  [2] none           |    H    |    L
+ *  [3] USB & USB-HOST |    L    |    H
+ *  [4] USB-HOST       |    L    |    L
+ *
+ * In case we have only one of these signals:
+ * - VBUS only - we want to distinguish between [1] and [2], so ID is always 1.
+ * - ID only - we want to distinguish between [1] and [4], so VBUS = ID.
+ */
+
 static void usb_extcon_detect_cable(struct work_struct *work)
 {
 	int id;
+	int vbus;
 	struct usb_extcon_info *info = container_of(to_delayed_work(work),
 						    struct usb_extcon_info,
 						    wq_detcable);
 
-	/* check ID and update cable state */
-	id = gpiod_get_value_cansleep(info->id_gpiod);
-	if (id) {
-		/*
-		 * ID = 1 means USB HOST cable detached.
-		 * As we don't have event for USB peripheral cable attached,
-		 * we simulate USB peripheral attach here.
-		 */
+	/* check ID and VBUS and update cable state */
+
+	id = info->id_gpiod ?
+		gpiod_get_value_cansleep(info->id_gpiod) : 1;
+
+	vbus = info->vbus_gpiod ?
+		gpiod_get_value_cansleep(info->vbus_gpiod) : id;
+
+	/* at first we clean states which are no longer active */
+	if (id)
 		extcon_set_cable_state(info->edev,
-				       usb_extcon_cable[EXTCON_CABLE_USB_HOST],
-				       false);
+			usb_extcon_cable[EXTCON_CABLE_USB_HOST], false);
+	if (!vbus)
 		extcon_set_cable_state(info->edev,
-				       usb_extcon_cable[EXTCON_CABLE_USB],
-				       true);
-	} else {
-		/*
-		 * ID = 0 means USB HOST cable attached.
-		 * As we don't have event for USB peripheral cable detached,
-		 * we simulate USB peripheral detach here.
-		 */
+			usb_extcon_cable[EXTCON_CABLE_USB], false);
+
+	if (!id)
 		extcon_set_cable_state(info->edev,
-				       usb_extcon_cable[EXTCON_CABLE_USB],
-				       false);
+				usb_extcon_cable[EXTCON_CABLE_USB_HOST], true);
+	if (vbus)
 		extcon_set_cable_state(info->edev,
-				       usb_extcon_cable[EXTCON_CABLE_USB_HOST],
-				       true);
-	}
+			usb_extcon_cable[EXTCON_CABLE_USB], true);
 }
 
 static irqreturn_t usb_irq_handler(int irq, void *dev_id)
@@ -113,10 +126,12 @@ static int usb_extcon_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	info->dev = dev;
-	info->id_gpiod = devm_gpiod_get(&pdev->dev, "id");
-	if (IS_ERR(info->id_gpiod)) {
-		dev_err(dev, "failed to get ID GPIO\n");
-		return PTR_ERR(info->id_gpiod);
+	info->id_gpiod = devm_gpiod_get_optional(&pdev->dev, "id");
+	info->vbus_gpiod = devm_gpiod_get_optional(&pdev->dev, "vbus");
+
+	if (!info->id_gpiod && !info->vbus_gpiod) {
+		dev_err(dev, "failed to get gpios\n");
+		return -ENODEV;
 	}
 
 	info->edev = devm_extcon_dev_allocate(dev, usb_extcon_cable);
@@ -131,27 +146,51 @@ 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);
@@ -179,9 +218,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;
+		}
 	}
 
 	/*
@@ -189,9 +235,17 @@ 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;
 }
 
 static int usb_extcon_resume(struct device *dev)
@@ -200,13 +254,28 @@ 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);
+
+	return ret;
 
+err:
+	if (info->id_gpiod)
+		enable_irq_wake(info->id_irq);
 	return ret;
 }
 #endif
-- 
1.9.1

[PATCH v3 3/4] extcon: usb-gpio: make debounce value configurable in devicetree

[Robert Baldyga]

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>
---
 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 baf7add..fe223bf 100644
--- a/drivers/extcon/extcon-usb-gpio.c
+++ b/drivers/extcon/extcon-usb-gpio.c
@@ -116,6 +116,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)
@@ -126,6 +127,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");
 	info->vbus_gpiod = devm_gpiod_get_optional(&pdev->dev, "vbus");
 
@@ -147,16 +153,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

[PATCH v3 4/4] Documentation: extcon: usb-gpio: update usb-gpio binding description

[Robert Baldyga]

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>
---
 .../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 af0b903..7096f39 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: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Roger Quadros]

On 02/04/15 16:13, Robert Baldyga wrote:
> This patch adds VBUS pin detection support to extcon-usb-gpio driver.
> It allows to use this driver with boards which have both VBUS and ID
> pins, or only one of them.
> 
> Following table of states presents relationship between this signals
> and detected cable type:
> 
>  State              |    ID   |   VBUS
> ----------------------------------------
>  [1] USB            |    H    |    H
>  [2] none           |    H    |    L
>  [3] USB & USB-HOST |    L    |    H
>  [4] USB-HOST       |    L    |    L
> 
> In case we have only one of these signals:
> - VBUS only - we want to distinguish between [1] and [2], so ID is always 1.
> - ID only - we want to distinguish between [1] and [4], so VBUS = ID.
> 
> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>

Acked-by: Roger Quadros <rogerq@ti.com>

cheers,
-roger

> ---
>  drivers/extcon/extcon-usb-gpio.c | 169 +++++++++++++++++++++++++++------------
>  1 file changed, 119 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
> index f6aa99d..baf7add 100644
> --- a/drivers/extcon/extcon-usb-gpio.c
> +++ b/drivers/extcon/extcon-usb-gpio.c
> @@ -32,7 +32,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;
> @@ -52,40 +54,51 @@ static const char *usb_extcon_cable[] = {
>  	NULL,
>  };
>  
> +/*
> + * "USB" = VBUS and "USB-HOST" = !ID, so we have:
> + *
> + *  State              |    ID   |   VBUS
> + * ----------------------------------------
> + *  [1] USB            |    H    |    H
> + *  [2] none           |    H    |    L
> + *  [3] USB & USB-HOST |    L    |    H
> + *  [4] USB-HOST       |    L    |    L
> + *
> + * In case we have only one of these signals:
> + * - VBUS only - we want to distinguish between [1] and [2], so ID is always 1.
> + * - ID only - we want to distinguish between [1] and [4], so VBUS = ID.
> + */
> +
>  static void usb_extcon_detect_cable(struct work_struct *work)
>  {
>  	int id;
> +	int vbus;
>  	struct usb_extcon_info *info = container_of(to_delayed_work(work),
>  						    struct usb_extcon_info,
>  						    wq_detcable);
>  
> -	/* check ID and update cable state */
> -	id = gpiod_get_value_cansleep(info->id_gpiod);
> -	if (id) {
> -		/*
> -		 * ID = 1 means USB HOST cable detached.
> -		 * As we don't have event for USB peripheral cable attached,
> -		 * we simulate USB peripheral attach here.
> -		 */
> +	/* check ID and VBUS and update cable state */
> +
> +	id = info->id_gpiod ?
> +		gpiod_get_value_cansleep(info->id_gpiod) : 1;
> +
> +	vbus = info->vbus_gpiod ?
> +		gpiod_get_value_cansleep(info->vbus_gpiod) : id;
> +
> +	/* at first we clean states which are no longer active */
> +	if (id)
>  		extcon_set_cable_state(info->edev,
> -				       usb_extcon_cable[EXTCON_CABLE_USB_HOST],
> -				       false);
> +			usb_extcon_cable[EXTCON_CABLE_USB_HOST], false);
> +	if (!vbus)
>  		extcon_set_cable_state(info->edev,
> -				       usb_extcon_cable[EXTCON_CABLE_USB],
> -				       true);
> -	} else {
> -		/*
> -		 * ID = 0 means USB HOST cable attached.
> -		 * As we don't have event for USB peripheral cable detached,
> -		 * we simulate USB peripheral detach here.
> -		 */
> +			usb_extcon_cable[EXTCON_CABLE_USB], false);
> +
> +	if (!id)
>  		extcon_set_cable_state(info->edev,
> -				       usb_extcon_cable[EXTCON_CABLE_USB],
> -				       false);
> +				usb_extcon_cable[EXTCON_CABLE_USB_HOST], true);
> +	if (vbus)
>  		extcon_set_cable_state(info->edev,
> -				       usb_extcon_cable[EXTCON_CABLE_USB_HOST],
> -				       true);
> -	}
> +			usb_extcon_cable[EXTCON_CABLE_USB], true);
>  }
>  
>  static irqreturn_t usb_irq_handler(int irq, void *dev_id)
> @@ -113,10 +126,12 @@ static int usb_extcon_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	info->dev = dev;
> -	info->id_gpiod = devm_gpiod_get(&pdev->dev, "id");
> -	if (IS_ERR(info->id_gpiod)) {
> -		dev_err(dev, "failed to get ID GPIO\n");
> -		return PTR_ERR(info->id_gpiod);
> +	info->id_gpiod = devm_gpiod_get_optional(&pdev->dev, "id");
> +	info->vbus_gpiod = devm_gpiod_get_optional(&pdev->dev, "vbus");
> +
> +	if (!info->id_gpiod && !info->vbus_gpiod) {
> +		dev_err(dev, "failed to get gpios\n");
> +		return -ENODEV;
>  	}
>  
>  	info->edev = devm_extcon_dev_allocate(dev, usb_extcon_cable);
> @@ -131,27 +146,51 @@ 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);
> @@ -179,9 +218,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;
> +		}
>  	}
>  
>  	/*
> @@ -189,9 +235,17 @@ 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;
>  }
>  
>  static int usb_extcon_resume(struct device *dev)
> @@ -200,13 +254,28 @@ 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);
> +
> +	return ret;
>  
> +err:
> +	if (info->id_gpiod)
> +		enable_irq_wake(info->id_irq);
>  	return ret;
>  }
>  #endif
> 

Re: [PATCH v3 1/4] extcon: usb-gpio: register extcon device before IRQ registration

[Chanwoo Choi]

Hi Robert,

On 04/02/2015 10:13 PM, Robert Baldyga wrote:
> IRQ handler touches info->edev, so if interrupt occurs before extcon
> device initialization it can cause NULL pointer dereference. Doing extcon
> initialization before IRQ handler registration fixes this problem.
> 
> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
> Acked-by: Roger Quadros <rogerq@ti.com>
> ---
>  drivers/extcon/extcon-usb-gpio.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
> index 3f0bad3..f6aa99d 100644
> --- a/drivers/extcon/extcon-usb-gpio.c
> +++ b/drivers/extcon/extcon-usb-gpio.c
> @@ -119,6 +119,18 @@ static int usb_extcon_probe(struct platform_device *pdev)
>  		return PTR_ERR(info->id_gpiod);
>  	}
>  
> +	info->edev = devm_extcon_dev_allocate(dev, usb_extcon_cable);
> +	if (IS_ERR(info->edev)) {
> +		dev_err(dev, "failed to allocate extcon device\n");
> +		return -ENOMEM;
> +	}
> +
> +	ret = devm_extcon_dev_register(dev, info->edev);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to register extcon device\n");
> +		return ret;
> +	}
> +
>  	ret = gpiod_set_debounce(info->id_gpiod,
>  				 USB_GPIO_DEBOUNCE_MS * 1000);
>  	if (ret < 0)
> @@ -142,18 +154,6 @@ static int usb_extcon_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	info->edev = devm_extcon_dev_allocate(dev, usb_extcon_cable);
> -	if (IS_ERR(info->edev)) {
> -		dev_err(dev, "failed to allocate extcon device\n");
> -		return -ENOMEM;
> -	}
> -
> -	ret = devm_extcon_dev_register(dev, info->edev);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to register extcon device\n");
> -		return ret;
> -	}
> -
>  	platform_set_drvdata(pdev, info);
>  	device_init_wakeup(dev, 1);
>  
> 

Applied this patch for v4.1-rc.

Thanks,
Chanwoo Choi

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Chanwoo Choi]

Hi Robert,

On 04/02/2015 10:13 PM, Robert Baldyga wrote:
> This patch adds VBUS pin detection support to extcon-usb-gpio driver.
> It allows to use this driver with boards which have both VBUS and ID
> pins, or only one of them.
> 
> Following table of states presents relationship between this signals
> and detected cable type:
> 
>  State              |    ID   |   VBUS
> ----------------------------------------
>  [1] USB            |    H    |    H
>  [2] none           |    H    |    L
>  [3] USB & USB-HOST |    L    |    H
>  [4] USB-HOST       |    L    |    L
> 
> In case we have only one of these signals:
> - VBUS only - we want to distinguish between [1] and [2], so ID is always 1.
> - ID only - we want to distinguish between [1] and [4], so VBUS = ID.
> 
> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
> ---
>  drivers/extcon/extcon-usb-gpio.c | 169 +++++++++++++++++++++++++++------------
>  1 file changed, 119 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
> index f6aa99d..baf7add 100644
> --- a/drivers/extcon/extcon-usb-gpio.c
> +++ b/drivers/extcon/extcon-usb-gpio.c
> @@ -32,7 +32,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;
> @@ -52,40 +54,51 @@ static const char *usb_extcon_cable[] = {
>  	NULL,
>  };
>  
> +/*
> + * "USB" = VBUS and "USB-HOST" = !ID, so we have:
> + *
> + *  State              |    ID   |   VBUS
> + * ----------------------------------------
> + *  [1] USB            |    H    |    H
> + *  [2] none           |    H    |    L
> + *  [3] USB & USB-HOST |    L    |    H
> + *  [4] USB-HOST       |    L    |    L
> + *
> + * In case we have only one of these signals:
> + * - VBUS only - we want to distinguish between [1] and [2], so ID is always 1.
> + * - ID only - we want to distinguish between [1] and [4], so VBUS = ID.
> + */
> +
>  static void usb_extcon_detect_cable(struct work_struct *work)
>  {
>  	int id;
> +	int vbus;
>  	struct usb_extcon_info *info = container_of(to_delayed_work(work),
>  						    struct usb_extcon_info,
>  						    wq_detcable);
>  
> -	/* check ID and update cable state */
> -	id = gpiod_get_value_cansleep(info->id_gpiod);
> -	if (id) {
> -		/*
> -		 * ID = 1 means USB HOST cable detached.
> -		 * As we don't have event for USB peripheral cable attached,
> -		 * we simulate USB peripheral attach here.
> -		 */
> +	/* check ID and VBUS and update cable state */
> +
> +	id = info->id_gpiod ?
> +		gpiod_get_value_cansleep(info->id_gpiod) : 1;
> +
> +	vbus = info->vbus_gpiod ?
> +		gpiod_get_value_cansleep(info->vbus_gpiod) : id;
> +
> +	/* at first we clean states which are no longer active */
> +	if (id)
>  		extcon_set_cable_state(info->edev,
> -				       usb_extcon_cable[EXTCON_CABLE_USB_HOST],
> -				       false);
> +			usb_extcon_cable[EXTCON_CABLE_USB_HOST], false);
> +	if (!vbus)
>  		extcon_set_cable_state(info->edev,
> -				       usb_extcon_cable[EXTCON_CABLE_USB],
> -				       true);
> -	} else {
> -		/*
> -		 * ID = 0 means USB HOST cable attached.
> -		 * As we don't have event for USB peripheral cable detached,
> -		 * we simulate USB peripheral detach here.
> -		 */
> +			usb_extcon_cable[EXTCON_CABLE_USB], false);
> +
> +	if (!id)
>  		extcon_set_cable_state(info->edev,
> -				       usb_extcon_cable[EXTCON_CABLE_USB],
> -				       false);
> +				usb_extcon_cable[EXTCON_CABLE_USB_HOST], true);
> +	if (vbus)
>  		extcon_set_cable_state(info->edev,
> -				       usb_extcon_cable[EXTCON_CABLE_USB_HOST],
> -				       true);
> -	}
> +			usb_extcon_cable[EXTCON_CABLE_USB], true);
>  }

Looks good to me of this patch.

But, I have one question about case[3]

If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?


> + * "USB" = VBUS and "USB-HOST" = !ID, so we have:
> + *
> + *  State              |    ID   |   VBUS
> + * ----------------------------------------
> + *  [1] USB            |    H    |    H
> + *  [2] none           |    H    |    L
> + *  [3] USB & USB-HOST |    L    |    H
> + *  [4] USB-HOST       |    L    |    L
> + *

[snip]

Thanks,
Chanwoo Choi

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Robert Baldyga]

Hi Chanwoo,

On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
> Hi Robert,
> 
> On 04/02/2015 10:13 PM, Robert Baldyga wrote:
>> This patch adds VBUS pin detection support to extcon-usb-gpio driver.
>> It allows to use this driver with boards which have both VBUS and ID
>> pins, or only one of them.
>>
>> Following table of states presents relationship between this signals
>> and detected cable type:
>>
>>  State              |    ID   |   VBUS
>> ----------------------------------------
>>  [1] USB            |    H    |    H
>>  [2] none           |    H    |    L
>>  [3] USB & USB-HOST |    L    |    H
>>  [4] USB-HOST       |    L    |    L
>>
>> In case we have only one of these signals:
>> - VBUS only - we want to distinguish between [1] and [2], so ID is always 1.
>> - ID only - we want to distinguish between [1] and [4], so VBUS = ID.
>>
>> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
>> ---
>>  drivers/extcon/extcon-usb-gpio.c | 169 +++++++++++++++++++++++++++------------
>>  1 file changed, 119 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
>> index f6aa99d..baf7add 100644
>> --- a/drivers/extcon/extcon-usb-gpio.c
>> +++ b/drivers/extcon/extcon-usb-gpio.c
>> @@ -32,7 +32,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;
>> @@ -52,40 +54,51 @@ static const char *usb_extcon_cable[] = {
>>  	NULL,
>>  };
>>  
>> +/*
>> + * "USB" = VBUS and "USB-HOST" = !ID, so we have:
>> + *
>> + *  State              |    ID   |   VBUS
>> + * ----------------------------------------
>> + *  [1] USB            |    H    |    H
>> + *  [2] none           |    H    |    L
>> + *  [3] USB & USB-HOST |    L    |    H
>> + *  [4] USB-HOST       |    L    |    L
>> + *
>> + * In case we have only one of these signals:
>> + * - VBUS only - we want to distinguish between [1] and [2], so ID is always 1.
>> + * - ID only - we want to distinguish between [1] and [4], so VBUS = ID.
>> + */
>> +
>>  static void usb_extcon_detect_cable(struct work_struct *work)
>>  {
>>  	int id;
>> +	int vbus;
>>  	struct usb_extcon_info *info = container_of(to_delayed_work(work),
>>  						    struct usb_extcon_info,
>>  						    wq_detcable);
>>  
>> -	/* check ID and update cable state */
>> -	id = gpiod_get_value_cansleep(info->id_gpiod);
>> -	if (id) {
>> -		/*
>> -		 * ID = 1 means USB HOST cable detached.
>> -		 * As we don't have event for USB peripheral cable attached,
>> -		 * we simulate USB peripheral attach here.
>> -		 */
>> +	/* check ID and VBUS and update cable state */
>> +
>> +	id = info->id_gpiod ?
>> +		gpiod_get_value_cansleep(info->id_gpiod) : 1;
>> +
>> +	vbus = info->vbus_gpiod ?
>> +		gpiod_get_value_cansleep(info->vbus_gpiod) : id;
>> +
>> +	/* at first we clean states which are no longer active */
>> +	if (id)
>>  		extcon_set_cable_state(info->edev,
>> -				       usb_extcon_cable[EXTCON_CABLE_USB_HOST],
>> -				       false);
>> +			usb_extcon_cable[EXTCON_CABLE_USB_HOST], false);
>> +	if (!vbus)
>>  		extcon_set_cable_state(info->edev,
>> -				       usb_extcon_cable[EXTCON_CABLE_USB],
>> -				       true);
>> -	} else {
>> -		/*
>> -		 * ID = 0 means USB HOST cable attached.
>> -		 * As we don't have event for USB peripheral cable detached,
>> -		 * we simulate USB peripheral detach here.
>> -		 */
>> +			usb_extcon_cable[EXTCON_CABLE_USB], false);
>> +
>> +	if (!id)
>>  		extcon_set_cable_state(info->edev,
>> -				       usb_extcon_cable[EXTCON_CABLE_USB],
>> -				       false);
>> +				usb_extcon_cable[EXTCON_CABLE_USB_HOST], true);
>> +	if (vbus)
>>  		extcon_set_cable_state(info->edev,
>> -				       usb_extcon_cable[EXTCON_CABLE_USB_HOST],
>> -				       true);
>> -	}
>> +			usb_extcon_cable[EXTCON_CABLE_USB], true);
>>  }
> 
> Looks good to me of this patch.
> 
> But, I have one question about case[3]
> 
> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
> 

It's because state of single USB cable connection cannot be completely
described using single extcon cable. USB cable state has two bits (VBUS
and ID), so we need to use two cables for single cable connection. We
use following convention:
cable "USB" = VBUS
cable "USB-HOST" = !ID.

In fact it would be better to have cables named "USB-VBUS" and "USB-ID"
- in this convention it would be more clear.

> 
>> + * "USB" = VBUS and "USB-HOST" = !ID, so we have:
>> + *
>> + *  State              |    ID   |   VBUS
>> + * ----------------------------------------
>> + *  [1] USB            |    H    |    H
>> + *  [2] none           |    H    |    L
>> + *  [3] USB & USB-HOST |    L    |    H
>> + *  [4] USB-HOST       |    L    |    L
>> + *
> 

Thanks,
Robert Baldyga

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Chanwoo Choi]

Hi Robert,

On 04/09/2015 04:57 PM, Robert Baldyga wrote:
> Hi Chanwoo,
> 
> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>> Hi Robert,
>>

[snip]

>> But, I have one question about case[3]
>>
>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>
> 
> It's because state of single USB cable connection cannot be completely
> described using single extcon cable. USB cable state has two bits (VBUS
> and ID), so we need to use two cables for single cable connection. We
> use following convention:
> cable "USB" = VBUS
> cable "USB-HOST" = !ID.

I think that extcon provider driver have to update the only one cable state
of either USB or USB-HOST because USB and USB-HOST feature can not be used
at the same time through one h/w port.

If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.

> In fact it would be better to have cables named "USB-VBUS" and "USB-ID"
> - in this convention it would be more clear.

Thanks,
Chanwoo Choi

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Robert Baldyga]

Hi Chanwoo,

On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
> Hi Robert,
> 
> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>> Hi Chanwoo,
>>
>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>> Hi Robert,
>>>
> 
> [snip]
> 
>>> But, I have one question about case[3]
>>>
>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>
>>
>> It's because state of single USB cable connection cannot be completely
>> described using single extcon cable. USB cable state has two bits (VBUS
>> and ID), so we need to use two cables for single cable connection. We
>> use following convention:
>> cable "USB" = VBUS
>> cable "USB-HOST" = !ID.
> 
> I think that extcon provider driver have to update the only one cable state
> of either USB or USB-HOST because USB and USB-HOST feature can not be used
> at the same time through one h/w port.
> 
> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
> 

It can. USB OTG allows for that. Moreover device can be host even if
ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
detected cable type is USB host). Devices would need to have complete
information about USB cable connection, because OTG state machine needs
that. As I wrote, current USB cable names are misleading. It would be
better to have "USB-VBUS" and "USB-ID".

>> In fact it would be better to have cables named "USB-VBUS" and "USB-ID"
>> - in this convention it would be more clear.

Thanks,
Robert Baldyga

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Roger Quadros]

Hi,

On 09/04/15 12:24, Robert Baldyga wrote:
> Hi Chanwoo,
> 
> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
>> Hi Robert,
>>
>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>>> Hi Chanwoo,
>>>
>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>>> Hi Robert,
>>>>
>>
>> [snip]
>>
>>>> But, I have one question about case[3]
>>>>
>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>>
>>>
>>> It's because state of single USB cable connection cannot be completely
>>> described using single extcon cable. USB cable state has two bits (VBUS
>>> and ID), so we need to use two cables for single cable connection. We
>>> use following convention:
>>> cable "USB" = VBUS
>>> cable "USB-HOST" = !ID.
>>
>> I think that extcon provider driver have to update the only one cable state
>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
>> at the same time through one h/w port.

At least for the kernel users [1] we are treating USB-HOST as !ID and USB as VBUS.
So it is not an issue for these kernel users if both USB and USB-HOST are attached.
This is a valid USB state.
If we don't do so then extcon with 3 cable states is not sufficient to capture the
entire USB scenario. (we need 4 states for 2 pins).

[1]
- drivers/usb/phy/phy-omap-otg.c
- drivers/usb/dwc3/dwc3-omap.c


>>
>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
>>
> 
> It can. USB OTG allows for that. Moreover device can be host even if
> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
> detected cable type is USB host). Devices would need to have complete
> information about USB cable connection, because OTG state machine needs
> that. As I wrote, current USB cable names are misleading. It would be
> better to have "USB-VBUS" and "USB-ID".

We need to first understand how user space is using "USB" and "USB-HOST" events
and does it cause an issue if both USB and USB-HOST become attached.

What is the "ABI" explanation for "USB" and "USB-HOST" cable states?

cheers,
-roger

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Robert Baldyga]

On 04/09/2015 11:59 AM, Roger Quadros wrote:
> Hi,
> 
> On 09/04/15 12:24, Robert Baldyga wrote:
>> Hi Chanwoo,
>>
>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
>>> Hi Robert,
>>>
>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>>>> Hi Chanwoo,
>>>>
>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>>>> Hi Robert,
>>>>>
>>>
>>> [snip]
>>>
>>>>> But, I have one question about case[3]
>>>>>
>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>>>
>>>>
>>>> It's because state of single USB cable connection cannot be completely
>>>> described using single extcon cable. USB cable state has two bits (VBUS
>>>> and ID), so we need to use two cables for single cable connection. We
>>>> use following convention:
>>>> cable "USB" = VBUS
>>>> cable "USB-HOST" = !ID.
>>>
>>> I think that extcon provider driver have to update the only one cable state
>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
>>> at the same time through one h/w port.
> 
> At least for the kernel users [1] we are treating USB-HOST as !ID and USB as VBUS.
> So it is not an issue for these kernel users if both USB and USB-HOST are attached.
> This is a valid USB state.
> If we don't do so then extcon with 3 cable states is not sufficient to capture the
> entire USB scenario. (we need 4 states for 2 pins).
> 
> [1]
> - drivers/usb/phy/phy-omap-otg.c
> - drivers/usb/dwc3/dwc3-omap.c
> 
> 
>>>
>>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
>>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
>>>
>>
>> It can. USB OTG allows for that. Moreover device can be host even if
>> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
>> detected cable type is USB host). Devices would need to have complete
>> information about USB cable connection, because OTG state machine needs
>> that. As I wrote, current USB cable names are misleading. It would be
>> better to have "USB-VBUS" and "USB-ID".
> 
> We need to first understand how user space is using "USB" and "USB-HOST" events
> and does it cause an issue if both USB and USB-HOST become attached.
> 
> What is the "ABI" explanation for "USB" and "USB-HOST" cable states?
> 

We can also leave USB and USB-HOST as "dummy cable detection states",
like they currently are, and add new USB-VBUS and USB-ID cables without
removing the old ones. It will cause some redundancy, but will make us
sure, that no ABI break can have a place.

Thanks,
Robert Baldyga

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Chanwoo Choi]

Hi Robert,

On 04/09/2015 06:24 PM, Robert Baldyga wrote:
> Hi Chanwoo,
> 
> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
>> Hi Robert,
>>
>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>>> Hi Chanwoo,
>>>
>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>>> Hi Robert,
>>>>
>>
>> [snip]
>>
>>>> But, I have one question about case[3]
>>>>
>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>>
>>>
>>> It's because state of single USB cable connection cannot be completely
>>> described using single extcon cable. USB cable state has two bits (VBUS
>>> and ID), so we need to use two cables for single cable connection. We
>>> use following convention:
>>> cable "USB" = VBUS
>>> cable "USB-HOST" = !ID.
>>
>> I think that extcon provider driver have to update the only one cable state
>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
>> at the same time through one h/w port.
>>
>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
>>
> 
> It can. USB OTG allows for that. Moreover device can be host even if
> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
> detected cable type is USB host). Devices would need to have complete
> information about USB cable connection, because OTG state machine needs

As I knew, USB OTG port don't send the attached cable of both USB and USB-HOST
at the same time. The case3 in your patch update two cable state about one h/w port.

I don't agree. 

> that. As I wrote, current USB cable names are misleading. It would be
> better to have "USB-VBUS" and "USB-ID".

It is strange cable name. I prefer to use only 'USB' cable name.
But, we could support the other method to get the state of whether USB-VBUS or USB-ID
by using helper API or others.

Thanks,
Chanwoo Choi

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Chanwoo Choi]

Hi Roger,

On 04/09/2015 06:59 PM, Roger Quadros wrote:
> Hi,
> 
> On 09/04/15 12:24, Robert Baldyga wrote:
>> Hi Chanwoo,
>>
>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
>>> Hi Robert,
>>>
>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>>>> Hi Chanwoo,
>>>>
>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>>>> Hi Robert,
>>>>>
>>>
>>> [snip]
>>>
>>>>> But, I have one question about case[3]
>>>>>
>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>>>
>>>>
>>>> It's because state of single USB cable connection cannot be completely
>>>> described using single extcon cable. USB cable state has two bits (VBUS
>>>> and ID), so we need to use two cables for single cable connection. We
>>>> use following convention:
>>>> cable "USB" = VBUS
>>>> cable "USB-HOST" = !ID.
>>>
>>> I think that extcon provider driver have to update the only one cable state
>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
>>> at the same time through one h/w port.
> 
> At least for the kernel users [1] we are treating USB-HOST as !ID and USB as VBUS.
> So it is not an issue for these kernel users if both USB and USB-HOST are attached.
> This is a valid USB state.

This case is only valid kernel users[1]. I think that we must
consider all case about extcon consumer driver and some framework.

> If we don't do so then extcon with 3 cable states is not sufficient to capture the
> entire USB scenario. (we need 4 states for 2 pins).
> 
> [1]
> - drivers/usb/phy/phy-omap-otg.c
> - drivers/usb/dwc3/dwc3-omap.c
> 

I think that extcon-usb-gpio.c send the only attached state of USB-HOST
and extcon could provider VBUS state with other solution instead of 
sending the attached state of both USB and USB-HOST cable at the same time.

Thanks,
Chanwoo Choi

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Robert Baldyga]

On 04/10/2015 09:17 AM, Chanwoo Choi wrote:
> Hi Robert,
> 
> On 04/09/2015 06:24 PM, Robert Baldyga wrote:
>> Hi Chanwoo,
>>
>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
>>> Hi Robert,
>>>
>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>>>> Hi Chanwoo,
>>>>
>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>>>> Hi Robert,
>>>>>
>>>
>>> [snip]
>>>
>>>>> But, I have one question about case[3]
>>>>>
>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>>>
>>>>
>>>> It's because state of single USB cable connection cannot be completely
>>>> described using single extcon cable. USB cable state has two bits (VBUS
>>>> and ID), so we need to use two cables for single cable connection. We
>>>> use following convention:
>>>> cable "USB" = VBUS
>>>> cable "USB-HOST" = !ID.
>>>
>>> I think that extcon provider driver have to update the only one cable state
>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
>>> at the same time through one h/w port.
>>>
>>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
>>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
>>>
>>
>> It can. USB OTG allows for that. Moreover device can be host even if
>> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
>> detected cable type is USB host). Devices would need to have complete
>> information about USB cable connection, because OTG state machine needs
> 
> As I knew, USB OTG port don't send the attached cable of both USB and USB-HOST
> at the same time. The case3 in your patch update two cable state about one h/w port.
> 

It's because simple "USB" or "USB-HOST" means nothing for USB OTG
machine. It needs to know exact VBUS and ID states, which cannot be
concluded basing on cable type only. That's why I have used "USB-HOST"
name together with "USB" to pass additional information about USB cable
connection.

> I don't agree. 
> 
>> that. As I wrote, current USB cable names are misleading. It would be
>> better to have "USB-VBUS" and "USB-ID".
> 
> It is strange cable name. I prefer to use only 'USB' cable name.
> But, we could support the other method to get the state of whether USB-VBUS or USB-ID
> by using helper API or others.
> 

Ok, so do you have any idea how to do it? Do we want to supply
additional API for notifying about VBUS and ID changes?

Thanks,
Robert Baldyga

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Chanwoo Choi]

On 04/10/2015 04:45 PM, Robert Baldyga wrote:
> On 04/10/2015 09:17 AM, Chanwoo Choi wrote:
>> Hi Robert,
>>
>> On 04/09/2015 06:24 PM, Robert Baldyga wrote:
>>> Hi Chanwoo,
>>>
>>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
>>>> Hi Robert,
>>>>
>>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>>>>> Hi Chanwoo,
>>>>>
>>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>>>>> Hi Robert,
>>>>>>
>>>>
>>>> [snip]
>>>>
>>>>>> But, I have one question about case[3]
>>>>>>
>>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>>>>
>>>>>
>>>>> It's because state of single USB cable connection cannot be completely
>>>>> described using single extcon cable. USB cable state has two bits (VBUS
>>>>> and ID), so we need to use two cables for single cable connection. We
>>>>> use following convention:
>>>>> cable "USB" = VBUS
>>>>> cable "USB-HOST" = !ID.
>>>>
>>>> I think that extcon provider driver have to update the only one cable state
>>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
>>>> at the same time through one h/w port.
>>>>
>>>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
>>>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
>>>>
>>>
>>> It can. USB OTG allows for that. Moreover device can be host even if
>>> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
>>> detected cable type is USB host). Devices would need to have complete
>>> information about USB cable connection, because OTG state machine needs
>>
>> As I knew, USB OTG port don't send the attached cable of both USB and USB-HOST
>> at the same time. The case3 in your patch update two cable state about one h/w port.
>>
> 
> It's because simple "USB" or "USB-HOST" means nothing for USB OTG
> machine. It needs to know exact VBUS and ID states, which cannot be
> concluded basing on cable type only. That's why I have used "USB-HOST"
> name together with "USB" to pass additional information about USB cable
> connection.

I think this method is not proper to support this case.
It may cause the confusion about other case using USB/USB-HOST cable state
except of you commented case.

> 
>> I don't agree. 
>>
>>> that. As I wrote, current USB cable names are misleading. It would be
>>> better to have "USB-VBUS" and "USB-ID".
>>
>> It is strange cable name. I prefer to use only 'USB' cable name.
>> But, we could support the other method to get the state of whether USB-VBUS or USB-ID
>> by using helper API or others.
>>
> 
> Ok, so do you have any idea how to do it? Do we want to supply
> additional API for notifying about VBUS and ID changes?

No, we need to consider more standard solution to support this case.

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Robert Baldyga]

On 04/10/2015 10:10 AM, Chanwoo Choi wrote:
> On 04/10/2015 04:45 PM, Robert Baldyga wrote:
>> On 04/10/2015 09:17 AM, Chanwoo Choi wrote:
>>> Hi Robert,
>>>
>>> On 04/09/2015 06:24 PM, Robert Baldyga wrote:
>>>> Hi Chanwoo,
>>>>
>>>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
>>>>> Hi Robert,
>>>>>
>>>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>>>>>> Hi Chanwoo,
>>>>>>
>>>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>>>>>> Hi Robert,
>>>>>>>
>>>>>
>>>>> [snip]
>>>>>
>>>>>>> But, I have one question about case[3]
>>>>>>>
>>>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>>>>>
>>>>>>
>>>>>> It's because state of single USB cable connection cannot be completely
>>>>>> described using single extcon cable. USB cable state has two bits (VBUS
>>>>>> and ID), so we need to use two cables for single cable connection. We
>>>>>> use following convention:
>>>>>> cable "USB" = VBUS
>>>>>> cable "USB-HOST" = !ID.
>>>>>
>>>>> I think that extcon provider driver have to update the only one cable state
>>>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
>>>>> at the same time through one h/w port.
>>>>>
>>>>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
>>>>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
>>>>>
>>>>
>>>> It can. USB OTG allows for that. Moreover device can be host even if
>>>> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
>>>> detected cable type is USB host). Devices would need to have complete
>>>> information about USB cable connection, because OTG state machine needs
>>>
>>> As I knew, USB OTG port don't send the attached cable of both USB and USB-HOST
>>> at the same time. The case3 in your patch update two cable state about one h/w port.
>>>
>>
>> It's because simple "USB" or "USB-HOST" means nothing for USB OTG
>> machine. It needs to know exact VBUS and ID states, which cannot be
>> concluded basing on cable type only. That's why I have used "USB-HOST"
>> name together with "USB" to pass additional information about USB cable
>> connection.
> 
> I think this method is not proper to support this case.
> It may cause the confusion about other case using USB/USB-HOST cable state
> except of you commented case.

That's why I finally proposed to use "USB-ID" and "USB-VBUS" in parallel
with old names. It seems to be simpler solution than adding new
mechanism notifying about VBUS and ID states changes.

> 
>>
>>> I don't agree. 
>>>
>>>> that. As I wrote, current USB cable names are misleading. It would be
>>>> better to have "USB-VBUS" and "USB-ID".
>>>
>>> It is strange cable name. I prefer to use only 'USB' cable name.
>>> But, we could support the other method to get the state of whether USB-VBUS or USB-ID
>>> by using helper API or others.
>>>
>>
>> Ok, so do you have any idea how to do it? Do we want to supply
>> additional API for notifying about VBUS and ID changes?
> 
> No, we need to consider more standard solution to support this case.
> 

Thanks,
Robert Baldyga

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Chanwoo Choi]

On 04/10/2015 05:46 PM, Robert Baldyga wrote:
> On 04/10/2015 10:10 AM, Chanwoo Choi wrote:
>> On 04/10/2015 04:45 PM, Robert Baldyga wrote:
>>> On 04/10/2015 09:17 AM, Chanwoo Choi wrote:
>>>> Hi Robert,
>>>>
>>>> On 04/09/2015 06:24 PM, Robert Baldyga wrote:
>>>>> Hi Chanwoo,
>>>>>
>>>>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
>>>>>> Hi Robert,
>>>>>>
>>>>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>>>>>>> Hi Chanwoo,
>>>>>>>
>>>>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>>>>>>> Hi Robert,
>>>>>>>>
>>>>>>
>>>>>> [snip]
>>>>>>
>>>>>>>> But, I have one question about case[3]
>>>>>>>>
>>>>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>>>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>>>>>>
>>>>>>>
>>>>>>> It's because state of single USB cable connection cannot be completely
>>>>>>> described using single extcon cable. USB cable state has two bits (VBUS
>>>>>>> and ID), so we need to use two cables for single cable connection. We
>>>>>>> use following convention:
>>>>>>> cable "USB" = VBUS
>>>>>>> cable "USB-HOST" = !ID.
>>>>>>
>>>>>> I think that extcon provider driver have to update the only one cable state
>>>>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
>>>>>> at the same time through one h/w port.
>>>>>>
>>>>>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
>>>>>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
>>>>>>
>>>>>
>>>>> It can. USB OTG allows for that. Moreover device can be host even if
>>>>> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
>>>>> detected cable type is USB host). Devices would need to have complete
>>>>> information about USB cable connection, because OTG state machine needs
>>>>
>>>> As I knew, USB OTG port don't send the attached cable of both USB and USB-HOST
>>>> at the same time. The case3 in your patch update two cable state about one h/w port.
>>>>
>>>
>>> It's because simple "USB" or "USB-HOST" means nothing for USB OTG
>>> machine. It needs to know exact VBUS and ID states, which cannot be
>>> concluded basing on cable type only. That's why I have used "USB-HOST"
>>> name together with "USB" to pass additional information about USB cable
>>> connection.
>>
>> I think this method is not proper to support this case.
>> It may cause the confusion about other case using USB/USB-HOST cable state
>> except of you commented case.
> 
> That's why I finally proposed to use "USB-ID" and "USB-VBUS" in parallel
> with old names. It seems to be simpler solution than adding new
> mechanism notifying about VBUS and ID states changes.


As I commented on previous reply, I don't agree to use 'USB-ID' and 'USB-VBUS'.
If we add new strange 'USB-ID' and 'USB-VBUS' name, we would add non-general cable
name continuoulsy.

I think that extcon core provide the helper API to get the value of VBUS.
But I need to consider it.

> 
>>
>>>
>>>> I don't agree. 
>>>>
>>>>> that. As I wrote, current USB cable names are misleading. It would be
>>>>> better to have "USB-VBUS" and "USB-ID".
>>>>
>>>> It is strange cable name. I prefer to use only 'USB' cable name.
>>>> But, we could support the other method to get the state of whether USB-VBUS or USB-ID
>>>> by using helper API or others.
>>>>
>>>
>>> Ok, so do you have any idea how to do it? Do we want to supply
>>> additional API for notifying about VBUS and ID changes?
>>
>> No, we need to consider more standard solution to support this case.
>>
> 
> Thanks,
> Robert Baldyga
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Robert Baldyga]

On 04/10/2015 11:18 AM, Chanwoo Choi wrote:
> On 04/10/2015 05:46 PM, Robert Baldyga wrote:
>> On 04/10/2015 10:10 AM, Chanwoo Choi wrote:
>>> On 04/10/2015 04:45 PM, Robert Baldyga wrote:
>>>> On 04/10/2015 09:17 AM, Chanwoo Choi wrote:
>>>>> Hi Robert,
>>>>>
>>>>> On 04/09/2015 06:24 PM, Robert Baldyga wrote:
>>>>>> Hi Chanwoo,
>>>>>>
>>>>>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
>>>>>>> Hi Robert,
>>>>>>>
>>>>>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>>>>>>>> Hi Chanwoo,
>>>>>>>>
>>>>>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>>>>>>>> Hi Robert,
>>>>>>>>>
>>>>>>>
>>>>>>> [snip]
>>>>>>>
>>>>>>>>> But, I have one question about case[3]
>>>>>>>>>
>>>>>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>>>>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>>>>>>>
>>>>>>>>
>>>>>>>> It's because state of single USB cable connection cannot be completely
>>>>>>>> described using single extcon cable. USB cable state has two bits (VBUS
>>>>>>>> and ID), so we need to use two cables for single cable connection. We
>>>>>>>> use following convention:
>>>>>>>> cable "USB" = VBUS
>>>>>>>> cable "USB-HOST" = !ID.
>>>>>>>
>>>>>>> I think that extcon provider driver have to update the only one cable state
>>>>>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
>>>>>>> at the same time through one h/w port.
>>>>>>>
>>>>>>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
>>>>>>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
>>>>>>>
>>>>>>
>>>>>> It can. USB OTG allows for that. Moreover device can be host even if
>>>>>> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
>>>>>> detected cable type is USB host). Devices would need to have complete
>>>>>> information about USB cable connection, because OTG state machine needs
>>>>>
>>>>> As I knew, USB OTG port don't send the attached cable of both USB and USB-HOST
>>>>> at the same time. The case3 in your patch update two cable state about one h/w port.
>>>>>
>>>>
>>>> It's because simple "USB" or "USB-HOST" means nothing for USB OTG
>>>> machine. It needs to know exact VBUS and ID states, which cannot be
>>>> concluded basing on cable type only. That's why I have used "USB-HOST"
>>>> name together with "USB" to pass additional information about USB cable
>>>> connection.
>>>
>>> I think this method is not proper to support this case.
>>> It may cause the confusion about other case using USB/USB-HOST cable state
>>> except of you commented case.
>>
>> That's why I finally proposed to use "USB-ID" and "USB-VBUS" in parallel
>> with old names. It seems to be simpler solution than adding new
>> mechanism notifying about VBUS and ID states changes.
> 
> 
> As I commented on previous reply, I don't agree to use 'USB-ID' and 'USB-VBUS'.
> If we add new strange 'USB-ID' and 'USB-VBUS' name, we would add non-general cable
> name continuoulsy.
> 
> I think that extcon core provide the helper API to get the value of VBUS.
> But I need to consider it.

We need more than API for getting VBUS value - we need to be notified
about its changes, because if we don't distinguish between USB-HOST with
VBUS on, and USB-HOST with VBUS off, then we will not receive
notification from extcon at VBUS state change.

Thanks,
Robert Baldyga

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Roger Quadros]

On 10/04/15 12:18, Chanwoo Choi wrote:
> On 04/10/2015 05:46 PM, Robert Baldyga wrote:
>> On 04/10/2015 10:10 AM, Chanwoo Choi wrote:
>>> On 04/10/2015 04:45 PM, Robert Baldyga wrote:
>>>> On 04/10/2015 09:17 AM, Chanwoo Choi wrote:
>>>>> Hi Robert,
>>>>>
>>>>> On 04/09/2015 06:24 PM, Robert Baldyga wrote:
>>>>>> Hi Chanwoo,
>>>>>>
>>>>>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
>>>>>>> Hi Robert,
>>>>>>>
>>>>>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>>>>>>>> Hi Chanwoo,
>>>>>>>>
>>>>>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>>>>>>>> Hi Robert,
>>>>>>>>>
>>>>>>>
>>>>>>> [snip]
>>>>>>>
>>>>>>>>> But, I have one question about case[3]
>>>>>>>>>
>>>>>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>>>>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>>>>>>>
>>>>>>>>
>>>>>>>> It's because state of single USB cable connection cannot be completely
>>>>>>>> described using single extcon cable. USB cable state has two bits (VBUS
>>>>>>>> and ID), so we need to use two cables for single cable connection. We
>>>>>>>> use following convention:
>>>>>>>> cable "USB" = VBUS
>>>>>>>> cable "USB-HOST" = !ID.
>>>>>>>
>>>>>>> I think that extcon provider driver have to update the only one cable state
>>>>>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
>>>>>>> at the same time through one h/w port.
>>>>>>>
>>>>>>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
>>>>>>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
>>>>>>>
>>>>>>
>>>>>> It can. USB OTG allows for that. Moreover device can be host even if
>>>>>> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
>>>>>> detected cable type is USB host). Devices would need to have complete
>>>>>> information about USB cable connection, because OTG state machine needs
>>>>>
>>>>> As I knew, USB OTG port don't send the attached cable of both USB and USB-HOST
>>>>> at the same time. The case3 in your patch update two cable state about one h/w port.
>>>>>
>>>>
>>>> It's because simple "USB" or "USB-HOST" means nothing for USB OTG
>>>> machine. It needs to know exact VBUS and ID states, which cannot be
>>>> concluded basing on cable type only. That's why I have used "USB-HOST"
>>>> name together with "USB" to pass additional information about USB cable
>>>> connection.
>>>
>>> I think this method is not proper to support this case.
>>> It may cause the confusion about other case using USB/USB-HOST cable state
>>> except of you commented case.
>>
>> That's why I finally proposed to use "USB-ID" and "USB-VBUS" in parallel
>> with old names. It seems to be simpler solution than adding new
>> mechanism notifying about VBUS and ID states changes.
> 
> 
> As I commented on previous reply, I don't agree to use 'USB-ID' and 'USB-VBUS'.
> If we add new strange 'USB-ID' and 'USB-VBUS' name, we would add non-general cable
> name continuoulsy.
> 
> I think that extcon core provide the helper API to get the value of VBUS.
> But I need to consider it.

Now it is starting to look like existing extcon states are not suitable for USB/PHY drivers to deliver
VBUS and ID information reliably.
This is because based on your comments the "USB" and "USB-HOST" states look like some fuzzy states
and have no direct correspondence with "VBUS" and "ID". The fact that they can't become
attached simultaneously makes me conclude that "USB" and "USB-HOST" cable states are really 
capturing only the ID pin state.

I can suggest the following options
a) let "USB" and "USB-HOST" only indicate ID pin status. Add a new cable state for "VBUS" notification.
Maybe call it "USB-POWER" or something.

NOTE: "USB-POWER" can become attached simultaneously with "USB" or "USB-HOST". But "USB-POWER" is now really
a different cable like "Fast-Charger" or "Slow-Charger".

b) stop using extcon framework for USB VBUS and ID notification.

cheers,
-roger

> 
>>
>>>
>>>>
>>>>> I don't agree. 
>>>>>
>>>>>> that. As I wrote, current USB cable names are misleading. It would be
>>>>>> better to have "USB-VBUS" and "USB-ID".
>>>>>
>>>>> It is strange cable name. I prefer to use only 'USB' cable name.
>>>>> But, we could support the other method to get the state of whether USB-VBUS or USB-ID
>>>>> by using helper API or others.
>>>>>
>>>>
>>>> Ok, so do you have any idea how to do it? Do we want to supply
>>>> additional API for notifying about VBUS and ID changes?
>>>
>>> No, we need to consider more standard solution to support this case.
>>>
>>
>> Thanks,
>> Robert Baldyga
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>>
> 

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Roger Quadros]

Fixed Kishon's id.

On 14/04/15 13:01, Roger Quadros wrote:
> On 10/04/15 12:18, Chanwoo Choi wrote:
>> On 04/10/2015 05:46 PM, Robert Baldyga wrote:
>>> On 04/10/2015 10:10 AM, Chanwoo Choi wrote:
>>>> On 04/10/2015 04:45 PM, Robert Baldyga wrote:
>>>>> On 04/10/2015 09:17 AM, Chanwoo Choi wrote:
>>>>>> Hi Robert,
>>>>>>
>>>>>> On 04/09/2015 06:24 PM, Robert Baldyga wrote:
>>>>>>> Hi Chanwoo,
>>>>>>>
>>>>>>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
>>>>>>>> Hi Robert,
>>>>>>>>
>>>>>>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>>>>>>>>> Hi Chanwoo,
>>>>>>>>>
>>>>>>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>>>>>>>>> Hi Robert,
>>>>>>>>>>
>>>>>>>>
>>>>>>>> [snip]
>>>>>>>>
>>>>>>>>>> But, I have one question about case[3]
>>>>>>>>>>
>>>>>>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>>>>>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> It's because state of single USB cable connection cannot be completely
>>>>>>>>> described using single extcon cable. USB cable state has two bits (VBUS
>>>>>>>>> and ID), so we need to use two cables for single cable connection. We
>>>>>>>>> use following convention:
>>>>>>>>> cable "USB" = VBUS
>>>>>>>>> cable "USB-HOST" = !ID.
>>>>>>>>
>>>>>>>> I think that extcon provider driver have to update the only one cable state
>>>>>>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
>>>>>>>> at the same time through one h/w port.
>>>>>>>>
>>>>>>>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
>>>>>>>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
>>>>>>>>
>>>>>>>
>>>>>>> It can. USB OTG allows for that. Moreover device can be host even if
>>>>>>> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
>>>>>>> detected cable type is USB host). Devices would need to have complete
>>>>>>> information about USB cable connection, because OTG state machine needs
>>>>>>
>>>>>> As I knew, USB OTG port don't send the attached cable of both USB and USB-HOST
>>>>>> at the same time. The case3 in your patch update two cable state about one h/w port.
>>>>>>
>>>>>
>>>>> It's because simple "USB" or "USB-HOST" means nothing for USB OTG
>>>>> machine. It needs to know exact VBUS and ID states, which cannot be
>>>>> concluded basing on cable type only. That's why I have used "USB-HOST"
>>>>> name together with "USB" to pass additional information about USB cable
>>>>> connection.
>>>>
>>>> I think this method is not proper to support this case.
>>>> It may cause the confusion about other case using USB/USB-HOST cable state
>>>> except of you commented case.
>>>
>>> That's why I finally proposed to use "USB-ID" and "USB-VBUS" in parallel
>>> with old names. It seems to be simpler solution than adding new
>>> mechanism notifying about VBUS and ID states changes.
>>
>>
>> As I commented on previous reply, I don't agree to use 'USB-ID' and 'USB-VBUS'.
>> If we add new strange 'USB-ID' and 'USB-VBUS' name, we would add non-general cable
>> name continuoulsy.
>>
>> I think that extcon core provide the helper API to get the value of VBUS.
>> But I need to consider it.
> 
> Now it is starting to look like existing extcon states are not suitable for USB/PHY drivers to deliver
> VBUS and ID information reliably.
> This is because based on your comments the "USB" and "USB-HOST" states look like some fuzzy states
> and have no direct correspondence with "VBUS" and "ID". The fact that they can't become
> attached simultaneously makes me conclude that "USB" and "USB-HOST" cable states are really 
> capturing only the ID pin state.
> 
> I can suggest the following options
> a) let "USB" and "USB-HOST" only indicate ID pin status. Add a new cable state for "VBUS" notification.
> Maybe call it "USB-POWER" or something.
> 
> NOTE: "USB-POWER" can become attached simultaneously with "USB" or "USB-HOST". But "USB-POWER" is now really
> a different cable like "Fast-Charger" or "Slow-Charger".
> 
> b) stop using extcon framework for USB VBUS and ID notification.
> 
> cheers,
> -roger
> 
>>
>>>
>>>>
>>>>>
>>>>>> I don't agree. 
>>>>>>
>>>>>>> that. As I wrote, current USB cable names are misleading. It would be
>>>>>>> better to have "USB-VBUS" and "USB-ID".
>>>>>>
>>>>>> It is strange cable name. I prefer to use only 'USB' cable name.
>>>>>> But, we could support the other method to get the state of whether USB-VBUS or USB-ID
>>>>>> by using helper API or others.
>>>>>>
>>>>>
>>>>> Ok, so do you have any idea how to do it? Do we want to supply
>>>>> additional API for notifying about VBUS and ID changes?
>>>>
>>>> No, we need to consider more standard solution to support this case.
>>>>
>>>
>>> Thanks,
>>> Robert Baldyga
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at  http://www.tux.org/lkml/
>>>
>>
> 

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Chanwoo Choi]

On 04/14/2015 07:02 PM, Roger Quadros wrote:
> Fixed Kishon's id.
> 
> On 14/04/15 13:01, Roger Quadros wrote:
>> On 10/04/15 12:18, Chanwoo Choi wrote:
>>> On 04/10/2015 05:46 PM, Robert Baldyga wrote:
>>>> On 04/10/2015 10:10 AM, Chanwoo Choi wrote:
>>>>> On 04/10/2015 04:45 PM, Robert Baldyga wrote:
>>>>>> On 04/10/2015 09:17 AM, Chanwoo Choi wrote:
>>>>>>> Hi Robert,
>>>>>>>
>>>>>>> On 04/09/2015 06:24 PM, Robert Baldyga wrote:
>>>>>>>> Hi Chanwoo,
>>>>>>>>
>>>>>>>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
>>>>>>>>> Hi Robert,
>>>>>>>>>
>>>>>>>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>>>>>>>>>> Hi Chanwoo,
>>>>>>>>>>
>>>>>>>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> [snip]
>>>>>>>>>
>>>>>>>>>>> But, I have one question about case[3]
>>>>>>>>>>>
>>>>>>>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>>>>>>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> It's because state of single USB cable connection cannot be completely
>>>>>>>>>> described using single extcon cable. USB cable state has two bits (VBUS
>>>>>>>>>> and ID), so we need to use two cables for single cable connection. We
>>>>>>>>>> use following convention:
>>>>>>>>>> cable "USB" = VBUS
>>>>>>>>>> cable "USB-HOST" = !ID.
>>>>>>>>>
>>>>>>>>> I think that extcon provider driver have to update the only one cable state
>>>>>>>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
>>>>>>>>> at the same time through one h/w port.
>>>>>>>>>
>>>>>>>>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
>>>>>>>>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
>>>>>>>>>
>>>>>>>>
>>>>>>>> It can. USB OTG allows for that. Moreover device can be host even if
>>>>>>>> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
>>>>>>>> detected cable type is USB host). Devices would need to have complete
>>>>>>>> information about USB cable connection, because OTG state machine needs
>>>>>>>
>>>>>>> As I knew, USB OTG port don't send the attached cable of both USB and USB-HOST
>>>>>>> at the same time. The case3 in your patch update two cable state about one h/w port.
>>>>>>>
>>>>>>
>>>>>> It's because simple "USB" or "USB-HOST" means nothing for USB OTG
>>>>>> machine. It needs to know exact VBUS and ID states, which cannot be
>>>>>> concluded basing on cable type only. That's why I have used "USB-HOST"
>>>>>> name together with "USB" to pass additional information about USB cable
>>>>>> connection.
>>>>>
>>>>> I think this method is not proper to support this case.
>>>>> It may cause the confusion about other case using USB/USB-HOST cable state
>>>>> except of you commented case.
>>>>
>>>> That's why I finally proposed to use "USB-ID" and "USB-VBUS" in parallel
>>>> with old names. It seems to be simpler solution than adding new
>>>> mechanism notifying about VBUS and ID states changes.
>>>
>>>
>>> As I commented on previous reply, I don't agree to use 'USB-ID' and 'USB-VBUS'.
>>> If we add new strange 'USB-ID' and 'USB-VBUS' name, we would add non-general cable
>>> name continuoulsy.
>>>
>>> I think that extcon core provide the helper API to get the value of VBUS.
>>> But I need to consider it.
>>
>> Now it is starting to look like existing extcon states are not suitable for USB/PHY drivers to deliver
>> VBUS and ID information reliably.
>> This is because based on your comments the "USB" and "USB-HOST" states look like some fuzzy states
>> and have no direct correspondence with "VBUS" and "ID". The fact that they can't become
>> attached simultaneously makes me conclude that "USB" and "USB-HOST" cable states are really 
>> capturing only the ID pin state.
>>
>> I can suggest the following options
>> a) let "USB" and "USB-HOST" only indicate ID pin status. Add a new cable state for "VBUS" notification.
>> Maybe call it "USB-POWER" or something.

We must discuss it before using the new cable name
such as "USB-POWER", "USB-ID" and "USB-VBUS".

What is the appropriate method of following two solution?
- Fisrt, use the new cable name "USB-*".
- Second, use the additional API to get the VBUS state.

Cheers,
Chanwoo Choi

>>
>> NOTE: "USB-POWER" can become attached simultaneously with "USB" or "USB-HOST". But "USB-POWER" is now really
>> a different cable like "Fast-Charger" or "Slow-Charger".
>>
>> b) stop using extcon framework for USB VBUS and ID notification.

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Roger Quadros]

On 14/04/15 13:31, Chanwoo Choi wrote:
> On 04/14/2015 07:02 PM, Roger Quadros wrote:
>> Fixed Kishon's id.
>>
>> On 14/04/15 13:01, Roger Quadros wrote:
>>> On 10/04/15 12:18, Chanwoo Choi wrote:
>>>> On 04/10/2015 05:46 PM, Robert Baldyga wrote:
>>>>> On 04/10/2015 10:10 AM, Chanwoo Choi wrote:
>>>>>> On 04/10/2015 04:45 PM, Robert Baldyga wrote:
>>>>>>> On 04/10/2015 09:17 AM, Chanwoo Choi wrote:
>>>>>>>> Hi Robert,
>>>>>>>>
>>>>>>>> On 04/09/2015 06:24 PM, Robert Baldyga wrote:
>>>>>>>>> Hi Chanwoo,
>>>>>>>>>
>>>>>>>>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
>>>>>>>>>> Hi Robert,
>>>>>>>>>>
>>>>>>>>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>>>>>>>>>>> Hi Chanwoo,
>>>>>>>>>>>
>>>>>>>>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> [snip]
>>>>>>>>>>
>>>>>>>>>>>> But, I have one question about case[3]
>>>>>>>>>>>>
>>>>>>>>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>>>>>>>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> It's because state of single USB cable connection cannot be completely
>>>>>>>>>>> described using single extcon cable. USB cable state has two bits (VBUS
>>>>>>>>>>> and ID), so we need to use two cables for single cable connection. We
>>>>>>>>>>> use following convention:
>>>>>>>>>>> cable "USB" = VBUS
>>>>>>>>>>> cable "USB-HOST" = !ID.
>>>>>>>>>>
>>>>>>>>>> I think that extcon provider driver have to update the only one cable state
>>>>>>>>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
>>>>>>>>>> at the same time through one h/w port.
>>>>>>>>>>
>>>>>>>>>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
>>>>>>>>>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> It can. USB OTG allows for that. Moreover device can be host even if
>>>>>>>>> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
>>>>>>>>> detected cable type is USB host). Devices would need to have complete
>>>>>>>>> information about USB cable connection, because OTG state machine needs
>>>>>>>>
>>>>>>>> As I knew, USB OTG port don't send the attached cable of both USB and USB-HOST
>>>>>>>> at the same time. The case3 in your patch update two cable state about one h/w port.
>>>>>>>>
>>>>>>>
>>>>>>> It's because simple "USB" or "USB-HOST" means nothing for USB OTG
>>>>>>> machine. It needs to know exact VBUS and ID states, which cannot be
>>>>>>> concluded basing on cable type only. That's why I have used "USB-HOST"
>>>>>>> name together with "USB" to pass additional information about USB cable
>>>>>>> connection.
>>>>>>
>>>>>> I think this method is not proper to support this case.
>>>>>> It may cause the confusion about other case using USB/USB-HOST cable state
>>>>>> except of you commented case.
>>>>>
>>>>> That's why I finally proposed to use "USB-ID" and "USB-VBUS" in parallel
>>>>> with old names. It seems to be simpler solution than adding new
>>>>> mechanism notifying about VBUS and ID states changes.
>>>>
>>>>
>>>> As I commented on previous reply, I don't agree to use 'USB-ID' and 'USB-VBUS'.
>>>> If we add new strange 'USB-ID' and 'USB-VBUS' name, we would add non-general cable
>>>> name continuoulsy.
>>>>
>>>> I think that extcon core provide the helper API to get the value of VBUS.
>>>> But I need to consider it.
>>>
>>> Now it is starting to look like existing extcon states are not suitable for USB/PHY drivers to deliver
>>> VBUS and ID information reliably.
>>> This is because based on your comments the "USB" and "USB-HOST" states look like some fuzzy states
>>> and have no direct correspondence with "VBUS" and "ID". The fact that they can't become
>>> attached simultaneously makes me conclude that "USB" and "USB-HOST" cable states are really 
>>> capturing only the ID pin state.
>>>
>>> I can suggest the following options
>>> a) let "USB" and "USB-HOST" only indicate ID pin status. Add a new cable state for "VBUS" notification.
>>> Maybe call it "USB-POWER" or something.
> 
> We must discuss it before using the new cable name
> such as "USB-POWER", "USB-ID" and "USB-VBUS".

I didn't say to add "USB-ID" or "USB-VBUS". solution (a) was to have the following
"USB" - attached means ID is high. i.e. Type-B plug attached.
"USB-HOST" - attached means ID is low. i.e. Type-A plug attached.
"USB-POWER" - attached means USB power is present. i.e. VBUS is alive.

This way the definition of USB and USB-HOST remain true to their name and avoid further confusions.
VBUS state is got through the "USB-POWER" cable state.

> 
> What is the appropriate method of following two solution?
> - Fisrt, use the new cable name "USB-*".
I explained this above.

> - Second, use the additional API to get the VBUS state.

You keep mentioning additional API for VBUS. But I don't see any such API. Can you please
suggest what API you are talking about?

cheers,
-roger


> 
> Cheers,
> Chanwoo Choi
> 
>>>
>>> NOTE: "USB-POWER" can become attached simultaneously with "USB" or "USB-HOST". But "USB-POWER" is now really
>>> a different cable like "Fast-Charger" or "Slow-Charger".
>>>
>>> b) stop using extcon framework for USB VBUS and ID notification.
> 
> 

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Chanwoo Choi]

On 04/14/2015 07:38 PM, Roger Quadros wrote:
> On 14/04/15 13:31, Chanwoo Choi wrote:
>> On 04/14/2015 07:02 PM, Roger Quadros wrote:
>>> Fixed Kishon's id.
>>>
>>> On 14/04/15 13:01, Roger Quadros wrote:
>>>> On 10/04/15 12:18, Chanwoo Choi wrote:
>>>>> On 04/10/2015 05:46 PM, Robert Baldyga wrote:
>>>>>> On 04/10/2015 10:10 AM, Chanwoo Choi wrote:
>>>>>>> On 04/10/2015 04:45 PM, Robert Baldyga wrote:
>>>>>>>> On 04/10/2015 09:17 AM, Chanwoo Choi wrote:
>>>>>>>>> Hi Robert,
>>>>>>>>>
>>>>>>>>> On 04/09/2015 06:24 PM, Robert Baldyga wrote:
>>>>>>>>>> Hi Chanwoo,
>>>>>>>>>>
>>>>>>>>>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>
>>>>>>>>>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>>>>>>>>>>>> Hi Chanwoo,
>>>>>>>>>>>>
>>>>>>>>>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> [snip]
>>>>>>>>>>>
>>>>>>>>>>>>> But, I have one question about case[3]
>>>>>>>>>>>>>
>>>>>>>>>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>>>>>>>>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> It's because state of single USB cable connection cannot be completely
>>>>>>>>>>>> described using single extcon cable. USB cable state has two bits (VBUS
>>>>>>>>>>>> and ID), so we need to use two cables for single cable connection. We
>>>>>>>>>>>> use following convention:
>>>>>>>>>>>> cable "USB" = VBUS
>>>>>>>>>>>> cable "USB-HOST" = !ID.
>>>>>>>>>>>
>>>>>>>>>>> I think that extcon provider driver have to update the only one cable state
>>>>>>>>>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
>>>>>>>>>>> at the same time through one h/w port.
>>>>>>>>>>>
>>>>>>>>>>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
>>>>>>>>>>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> It can. USB OTG allows for that. Moreover device can be host even if
>>>>>>>>>> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
>>>>>>>>>> detected cable type is USB host). Devices would need to have complete
>>>>>>>>>> information about USB cable connection, because OTG state machine needs
>>>>>>>>>
>>>>>>>>> As I knew, USB OTG port don't send the attached cable of both USB and USB-HOST
>>>>>>>>> at the same time. The case3 in your patch update two cable state about one h/w port.
>>>>>>>>>
>>>>>>>>
>>>>>>>> It's because simple "USB" or "USB-HOST" means nothing for USB OTG
>>>>>>>> machine. It needs to know exact VBUS and ID states, which cannot be
>>>>>>>> concluded basing on cable type only. That's why I have used "USB-HOST"
>>>>>>>> name together with "USB" to pass additional information about USB cable
>>>>>>>> connection.
>>>>>>>
>>>>>>> I think this method is not proper to support this case.
>>>>>>> It may cause the confusion about other case using USB/USB-HOST cable state
>>>>>>> except of you commented case.
>>>>>>
>>>>>> That's why I finally proposed to use "USB-ID" and "USB-VBUS" in parallel
>>>>>> with old names. It seems to be simpler solution than adding new
>>>>>> mechanism notifying about VBUS and ID states changes.
>>>>>
>>>>>
>>>>> As I commented on previous reply, I don't agree to use 'USB-ID' and 'USB-VBUS'.
>>>>> If we add new strange 'USB-ID' and 'USB-VBUS' name, we would add non-general cable
>>>>> name continuoulsy.
>>>>>
>>>>> I think that extcon core provide the helper API to get the value of VBUS.
>>>>> But I need to consider it.
>>>>
>>>> Now it is starting to look like existing extcon states are not suitable for USB/PHY drivers to deliver
>>>> VBUS and ID information reliably.
>>>> This is because based on your comments the "USB" and "USB-HOST" states look like some fuzzy states
>>>> and have no direct correspondence with "VBUS" and "ID". The fact that they can't become
>>>> attached simultaneously makes me conclude that "USB" and "USB-HOST" cable states are really 
>>>> capturing only the ID pin state.
>>>>
>>>> I can suggest the following options
>>>> a) let "USB" and "USB-HOST" only indicate ID pin status. Add a new cable state for "VBUS" notification.
>>>> Maybe call it "USB-POWER" or something.
>>
>> We must discuss it before using the new cable name
>> such as "USB-POWER", "USB-ID" and "USB-VBUS".
> 
> I didn't say to add "USB-ID" or "USB-VBUS". solution (a) was to have the following

Right. Robert suggested the "USB-ID" and "USB-VBUS" cable name on previous mail in mail thread.

> "USB" - attached means ID is high. i.e. Type-B plug attached.
> "USB-HOST" - attached means ID is low. i.e. Type-A plug attached.
> "USB-POWER" - attached means USB power is present. i.e. VBUS is alive.
> 
> This way the definition of USB and USB-HOST remain true to their name and avoid further confusions.
> VBUS state is got through the "USB-POWER" cable state.

There is the same case for MHL cable.
Also, MHL cable could be connected to VBUS line.
- MHL : attached just MHL cable.
- MHL-POWER : attache MHL cable which is connected with VBUS line.

We must need the opinion of USB/PHY driver's maintainer.

> 
>>
>> What is the appropriate method of following two solution?
>> - Fisrt, use the new cable name "USB-*".
> I explained this above.
> 
>> - Second, use the additional API to get the VBUS state.
> 
> You keep mentioning additional API for VBUS. But I don't see any such API. Can you please
> suggest what API you are talking about?

I'm considering following functions for VBUS state. But it is my opinion,
If USB/PHY drivers's maintainers don't agree the new cable ("USB-POWER"),
We could use the following function to get VBUS state.
Because new cable name will affect the USB/PHY drivers.
- int extcon_{get|set}_vbus_state(struct extcon_dev *edev);

Cheers,
Chanwoo Choi

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Peter Chen]

On Tue, Apr 14, 2015 at 08:29:34PM +0900, Chanwoo Choi wrote:
> On 04/14/2015 07:38 PM, Roger Quadros wrote:
> > On 14/04/15 13:31, Chanwoo Choi wrote:
> >> On 04/14/2015 07:02 PM, Roger Quadros wrote:
> >>> Fixed Kishon's id.
> >>>
> >>> On 14/04/15 13:01, Roger Quadros wrote:
> >>>> On 10/04/15 12:18, Chanwoo Choi wrote:
> >>>>> On 04/10/2015 05:46 PM, Robert Baldyga wrote:
> >>>>>> On 04/10/2015 10:10 AM, Chanwoo Choi wrote:
> >>>>>>> On 04/10/2015 04:45 PM, Robert Baldyga wrote:
> >>>>>>>> On 04/10/2015 09:17 AM, Chanwoo Choi wrote:
> >>>>>>>>> Hi Robert,
> >>>>>>>>>
> >>>>>>>>> On 04/09/2015 06:24 PM, Robert Baldyga wrote:
> >>>>>>>>>> Hi Chanwoo,
> >>>>>>>>>>
> >>>>>>>>>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
> >>>>>>>>>>> Hi Robert,
> >>>>>>>>>>>
> >>>>>>>>>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
> >>>>>>>>>>>> Hi Chanwoo,
> >>>>>>>>>>>>
> >>>>>>>>>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
> >>>>>>>>>>>>> Hi Robert,
> >>>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> [snip]
> >>>>>>>>>>>
> >>>>>>>>>>>>> But, I have one question about case[3]
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
> >>>>>>>>>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
> >>>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> It's because state of single USB cable connection cannot be completely
> >>>>>>>>>>>> described using single extcon cable. USB cable state has two bits (VBUS
> >>>>>>>>>>>> and ID), so we need to use two cables for single cable connection. We
> >>>>>>>>>>>> use following convention:
> >>>>>>>>>>>> cable "USB" = VBUS
> >>>>>>>>>>>> cable "USB-HOST" = !ID.
> >>>>>>>>>>>
> >>>>>>>>>>> I think that extcon provider driver have to update the only one cable state
> >>>>>>>>>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
> >>>>>>>>>>> at the same time through one h/w port.
> >>>>>>>>>>>
> >>>>>>>>>>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
> >>>>>>>>>>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> It can. USB OTG allows for that. Moreover device can be host even if
> >>>>>>>>>> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
> >>>>>>>>>> detected cable type is USB host). Devices would need to have complete
> >>>>>>>>>> information about USB cable connection, because OTG state machine needs
> >>>>>>>>>
> >>>>>>>>> As I knew, USB OTG port don't send the attached cable of both USB and USB-HOST
> >>>>>>>>> at the same time. The case3 in your patch update two cable state about one h/w port.
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> It's because simple "USB" or "USB-HOST" means nothing for USB OTG
> >>>>>>>> machine. It needs to know exact VBUS and ID states, which cannot be
> >>>>>>>> concluded basing on cable type only. That's why I have used "USB-HOST"
> >>>>>>>> name together with "USB" to pass additional information about USB cable
> >>>>>>>> connection.
> >>>>>>>
> >>>>>>> I think this method is not proper to support this case.
> >>>>>>> It may cause the confusion about other case using USB/USB-HOST cable state
> >>>>>>> except of you commented case.
> >>>>>>
> >>>>>> That's why I finally proposed to use "USB-ID" and "USB-VBUS" in parallel
> >>>>>> with old names. It seems to be simpler solution than adding new
> >>>>>> mechanism notifying about VBUS and ID states changes.
> >>>>>
> >>>>>
> >>>>> As I commented on previous reply, I don't agree to use 'USB-ID' and 'USB-VBUS'.
> >>>>> If we add new strange 'USB-ID' and 'USB-VBUS' name, we would add non-general cable
> >>>>> name continuoulsy.
> >>>>>
> >>>>> I think that extcon core provide the helper API to get the value of VBUS.
> >>>>> But I need to consider it.
> >>>>
> >>>> Now it is starting to look like existing extcon states are not suitable for USB/PHY drivers to deliver
> >>>> VBUS and ID information reliably.
> >>>> This is because based on your comments the "USB" and "USB-HOST" states look like some fuzzy states
> >>>> and have no direct correspondence with "VBUS" and "ID". The fact that they can't become
> >>>> attached simultaneously makes me conclude that "USB" and "USB-HOST" cable states are really 
> >>>> capturing only the ID pin state.
> >>>>
> >>>> I can suggest the following options
> >>>> a) let "USB" and "USB-HOST" only indicate ID pin status. Add a new cable state for "VBUS" notification.
> >>>> Maybe call it "USB-POWER" or something.
> >>
> >> We must discuss it before using the new cable name
> >> such as "USB-POWER", "USB-ID" and "USB-VBUS".
> > 
> > I didn't say to add "USB-ID" or "USB-VBUS". solution (a) was to have the following
> 
> Right. Robert suggested the "USB-ID" and "USB-VBUS" cable name on previous mail in mail thread.

>From USB/USB-PHY driver point, it needs to know id and vbus value
for their internal logic, so as extcon users, the cable name
is better to reflect meaning of id and vbus, like "USB-ID" and "USB-VBUS",
if the power is from vbus pin at USB cable, I don't think we need another
cable name "USB-POWER" even the USB/USB-PHY driver don't need it.

> 
> > "USB" - attached means ID is high. i.e. Type-B plug attached.
> > "USB-HOST" - attached means ID is low. i.e. Type-A plug attached.
> > "USB-POWER" - attached means USB power is present. i.e. VBUS is alive.
> > 
> > This way the definition of USB and USB-HOST remain true to their name and avoid further confusions.
> > VBUS state is got through the "USB-POWER" cable state.
> 
> There is the same case for MHL cable.
> Also, MHL cable could be connected to VBUS line.
> - MHL : attached just MHL cable.
> - MHL-POWER : attache MHL cable which is connected with VBUS line.
> 
> We must need the opinion of USB/PHY driver's maintainer.
> 
> > 
> >>
> >> What is the appropriate method of following two solution?
> >> - Fisrt, use the new cable name "USB-*".
> > I explained this above.
> > 
> >> - Second, use the additional API to get the VBUS state.
> > 
> > You keep mentioning additional API for VBUS. But I don't see any such API. Can you please
> > suggest what API you are talking about?
> 
> I'm considering following functions for VBUS state. But it is my opinion,
> If USB/PHY drivers's maintainers don't agree the new cable ("USB-POWER"),
> We could use the following function to get VBUS state.
> Because new cable name will affect the USB/PHY drivers.
> - int extcon_{get|set}_vbus_state(struct extcon_dev *edev);
> 
> Cheers,
> Chanwoo Choi

-- 

Best Regards,
Peter Chen

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Roger Quadros]

On 15/04/15 06:27, Peter Chen wrote:
> On Tue, Apr 14, 2015 at 08:29:34PM +0900, Chanwoo Choi wrote:
>> On 04/14/2015 07:38 PM, Roger Quadros wrote:
>>> On 14/04/15 13:31, Chanwoo Choi wrote:
>>>> On 04/14/2015 07:02 PM, Roger Quadros wrote:
>>>>> Fixed Kishon's id.
>>>>>
>>>>> On 14/04/15 13:01, Roger Quadros wrote:
>>>>>> On 10/04/15 12:18, Chanwoo Choi wrote:
>>>>>>> On 04/10/2015 05:46 PM, Robert Baldyga wrote:
>>>>>>>> On 04/10/2015 10:10 AM, Chanwoo Choi wrote:
>>>>>>>>> On 04/10/2015 04:45 PM, Robert Baldyga wrote:
>>>>>>>>>> On 04/10/2015 09:17 AM, Chanwoo Choi wrote:
>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>
>>>>>>>>>>> On 04/09/2015 06:24 PM, Robert Baldyga wrote:
>>>>>>>>>>>> Hi Chanwoo,
>>>>>>>>>>>>
>>>>>>>>>>>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
>>>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>>>>>>>>>>>>>> Hi Chanwoo,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>>>>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> [snip]
>>>>>>>>>>>>>
>>>>>>>>>>>>>>> But, I have one question about case[3]
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>>>>>>>>>>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> It's because state of single USB cable connection cannot be completely
>>>>>>>>>>>>>> described using single extcon cable. USB cable state has two bits (VBUS
>>>>>>>>>>>>>> and ID), so we need to use two cables for single cable connection. We
>>>>>>>>>>>>>> use following convention:
>>>>>>>>>>>>>> cable "USB" = VBUS
>>>>>>>>>>>>>> cable "USB-HOST" = !ID.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I think that extcon provider driver have to update the only one cable state
>>>>>>>>>>>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
>>>>>>>>>>>>> at the same time through one h/w port.
>>>>>>>>>>>>>
>>>>>>>>>>>>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
>>>>>>>>>>>>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> It can. USB OTG allows for that. Moreover device can be host even if
>>>>>>>>>>>> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
>>>>>>>>>>>> detected cable type is USB host). Devices would need to have complete
>>>>>>>>>>>> information about USB cable connection, because OTG state machine needs
>>>>>>>>>>>
>>>>>>>>>>> As I knew, USB OTG port don't send the attached cable of both USB and USB-HOST
>>>>>>>>>>> at the same time. The case3 in your patch update two cable state about one h/w port.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> It's because simple "USB" or "USB-HOST" means nothing for USB OTG
>>>>>>>>>> machine. It needs to know exact VBUS and ID states, which cannot be
>>>>>>>>>> concluded basing on cable type only. That's why I have used "USB-HOST"
>>>>>>>>>> name together with "USB" to pass additional information about USB cable
>>>>>>>>>> connection.
>>>>>>>>>
>>>>>>>>> I think this method is not proper to support this case.
>>>>>>>>> It may cause the confusion about other case using USB/USB-HOST cable state
>>>>>>>>> except of you commented case.
>>>>>>>>
>>>>>>>> That's why I finally proposed to use "USB-ID" and "USB-VBUS" in parallel
>>>>>>>> with old names. It seems to be simpler solution than adding new
>>>>>>>> mechanism notifying about VBUS and ID states changes.
>>>>>>>
>>>>>>>
>>>>>>> As I commented on previous reply, I don't agree to use 'USB-ID' and 'USB-VBUS'.
>>>>>>> If we add new strange 'USB-ID' and 'USB-VBUS' name, we would add non-general cable
>>>>>>> name continuoulsy.
>>>>>>>
>>>>>>> I think that extcon core provide the helper API to get the value of VBUS.
>>>>>>> But I need to consider it.
>>>>>>
>>>>>> Now it is starting to look like existing extcon states are not suitable for USB/PHY drivers to deliver
>>>>>> VBUS and ID information reliably.
>>>>>> This is because based on your comments the "USB" and "USB-HOST" states look like some fuzzy states
>>>>>> and have no direct correspondence with "VBUS" and "ID". The fact that they can't become
>>>>>> attached simultaneously makes me conclude that "USB" and "USB-HOST" cable states are really 
>>>>>> capturing only the ID pin state.
>>>>>>
>>>>>> I can suggest the following options
>>>>>> a) let "USB" and "USB-HOST" only indicate ID pin status. Add a new cable state for "VBUS" notification.
>>>>>> Maybe call it "USB-POWER" or something.
>>>>
>>>> We must discuss it before using the new cable name
>>>> such as "USB-POWER", "USB-ID" and "USB-VBUS".
>>>
>>> I didn't say to add "USB-ID" or "USB-VBUS". solution (a) was to have the following
>>
>> Right. Robert suggested the "USB-ID" and "USB-VBUS" cable name on previous mail in mail thread.
> 
> From USB/USB-PHY driver point, it needs to know id and vbus value
> for their internal logic, so as extcon users, the cable name
> is better to reflect meaning of id and vbus, like "USB-ID" and "USB-VBUS",
> if the power is from vbus pin at USB cable, I don't think we need another
> cable name "USB-POWER" even the USB/USB-PHY driver don't need it.

I agree as well that this is the *best* option for USB case. Just because Chanwoo was
objecting these names I suggested "USB-POWER".

Chanwoo, can we simply get rid of "USB" and "USB-HOST" cables and move to
"USB-ID" and "USB-VBUS"?

The only reason you objected was saying that it is a strange cable name. Well this is
only what we care about from USB PHY drivers and user space is not interested in it
so what is the concern?

> 
>>
>>> "USB" - attached means ID is high. i.e. Type-B plug attached.
>>> "USB-HOST" - attached means ID is low. i.e. Type-A plug attached.
>>> "USB-POWER" - attached means USB power is present. i.e. VBUS is alive.
>>>
>>> This way the definition of USB and USB-HOST remain true to their name and avoid further confusions.
>>> VBUS state is got through the "USB-POWER" cable state.
>>
>> There is the same case for MHL cable.
>> Also, MHL cable could be connected to VBUS line.
>> - MHL : attached just MHL cable.
>> - MHL-POWER : attache MHL cable which is connected with VBUS line.
>>
>> We must need the opinion of USB/PHY driver's maintainer.
>>
>>>
>>>>
>>>> What is the appropriate method of following two solution?
>>>> - Fisrt, use the new cable name "USB-*".
>>> I explained this above.
>>>
>>>> - Second, use the additional API to get the VBUS state.
>>>
>>> You keep mentioning additional API for VBUS. But I don't see any such API. Can you please
>>> suggest what API you are talking about?
>>
>> I'm considering following functions for VBUS state. But it is my opinion,
>> If USB/PHY drivers's maintainers don't agree the new cable ("USB-POWER"),
>> We could use the following function to get VBUS state.
>> Because new cable name will affect the USB/PHY drivers.
>> - int extcon_{get|set}_vbus_state(struct extcon_dev *edev);

This is not suitable for us as USB drivers need VBUS notification event to come.
They can't keep polling for VBUS state using this API.

cheers,
-roger

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Chanwoo Choi]

Hi Roger and Peter,

On 04/15/2015 04:50 PM, Roger Quadros wrote:
> On 15/04/15 06:27, Peter Chen wrote:
>> On Tue, Apr 14, 2015 at 08:29:34PM +0900, Chanwoo Choi wrote:
>>> On 04/14/2015 07:38 PM, Roger Quadros wrote:
>>>> On 14/04/15 13:31, Chanwoo Choi wrote:
>>>>> On 04/14/2015 07:02 PM, Roger Quadros wrote:
>>>>>> Fixed Kishon's id.
>>>>>>
>>>>>> On 14/04/15 13:01, Roger Quadros wrote:
>>>>>>> On 10/04/15 12:18, Chanwoo Choi wrote:
>>>>>>>> On 04/10/2015 05:46 PM, Robert Baldyga wrote:
>>>>>>>>> On 04/10/2015 10:10 AM, Chanwoo Choi wrote:
>>>>>>>>>> On 04/10/2015 04:45 PM, Robert Baldyga wrote:
>>>>>>>>>>> On 04/10/2015 09:17 AM, Chanwoo Choi wrote:
>>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>>
>>>>>>>>>>>> On 04/09/2015 06:24 PM, Robert Baldyga wrote:
>>>>>>>>>>>>> Hi Chanwoo,
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
>>>>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>>>>>>>>>>>>>>> Hi Chanwoo,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>>>>>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> [snip]
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> But, I have one question about case[3]
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>>>>>>>>>>>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> It's because state of single USB cable connection cannot be completely
>>>>>>>>>>>>>>> described using single extcon cable. USB cable state has two bits (VBUS
>>>>>>>>>>>>>>> and ID), so we need to use two cables for single cable connection. We
>>>>>>>>>>>>>>> use following convention:
>>>>>>>>>>>>>>> cable "USB" = VBUS
>>>>>>>>>>>>>>> cable "USB-HOST" = !ID.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I think that extcon provider driver have to update the only one cable state
>>>>>>>>>>>>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
>>>>>>>>>>>>>> at the same time through one h/w port.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
>>>>>>>>>>>>>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> It can. USB OTG allows for that. Moreover device can be host even if
>>>>>>>>>>>>> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
>>>>>>>>>>>>> detected cable type is USB host). Devices would need to have complete
>>>>>>>>>>>>> information about USB cable connection, because OTG state machine needs
>>>>>>>>>>>>
>>>>>>>>>>>> As I knew, USB OTG port don't send the attached cable of both USB and USB-HOST
>>>>>>>>>>>> at the same time. The case3 in your patch update two cable state about one h/w port.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> It's because simple "USB" or "USB-HOST" means nothing for USB OTG
>>>>>>>>>>> machine. It needs to know exact VBUS and ID states, which cannot be
>>>>>>>>>>> concluded basing on cable type only. That's why I have used "USB-HOST"
>>>>>>>>>>> name together with "USB" to pass additional information about USB cable
>>>>>>>>>>> connection.
>>>>>>>>>>
>>>>>>>>>> I think this method is not proper to support this case.
>>>>>>>>>> It may cause the confusion about other case using USB/USB-HOST cable state
>>>>>>>>>> except of you commented case.
>>>>>>>>>
>>>>>>>>> That's why I finally proposed to use "USB-ID" and "USB-VBUS" in parallel
>>>>>>>>> with old names. It seems to be simpler solution than adding new
>>>>>>>>> mechanism notifying about VBUS and ID states changes.
>>>>>>>>
>>>>>>>>
>>>>>>>> As I commented on previous reply, I don't agree to use 'USB-ID' and 'USB-VBUS'.
>>>>>>>> If we add new strange 'USB-ID' and 'USB-VBUS' name, we would add non-general cable
>>>>>>>> name continuoulsy.
>>>>>>>>
>>>>>>>> I think that extcon core provide the helper API to get the value of VBUS.
>>>>>>>> But I need to consider it.
>>>>>>>
>>>>>>> Now it is starting to look like existing extcon states are not suitable for USB/PHY drivers to deliver
>>>>>>> VBUS and ID information reliably.
>>>>>>> This is because based on your comments the "USB" and "USB-HOST" states look like some fuzzy states
>>>>>>> and have no direct correspondence with "VBUS" and "ID". The fact that they can't become
>>>>>>> attached simultaneously makes me conclude that "USB" and "USB-HOST" cable states are really 
>>>>>>> capturing only the ID pin state.
>>>>>>>
>>>>>>> I can suggest the following options
>>>>>>> a) let "USB" and "USB-HOST" only indicate ID pin status. Add a new cable state for "VBUS" notification.
>>>>>>> Maybe call it "USB-POWER" or something.
>>>>>
>>>>> We must discuss it before using the new cable name
>>>>> such as "USB-POWER", "USB-ID" and "USB-VBUS".
>>>>
>>>> I didn't say to add "USB-ID" or "USB-VBUS". solution (a) was to have the following
>>>
>>> Right. Robert suggested the "USB-ID" and "USB-VBUS" cable name on previous mail in mail thread.
>>
>> From USB/USB-PHY driver point, it needs to know id and vbus value
>> for their internal logic, so as extcon users, the cable name
>> is better to reflect meaning of id and vbus, like "USB-ID" and "USB-VBUS",
>> if the power is from vbus pin at USB cable, I don't think we need another
>> cable name "USB-POWER" even the USB/USB-PHY driver don't need it.
> 
> I agree as well that this is the *best* option for USB case. Just because Chanwoo was
> objecting these names I suggested "USB-POWER".
> 
> Chanwoo, can we simply get rid of "USB" and "USB-HOST" cables and move to
> "USB-ID" and "USB-VBUS"?

I'm wondering about changing the previous cable name from 'USB'/'USB-HOST'
to 'USB-ID/USB-VBUS' because extcon framework update the state of cable by
using uevent and the user-space process would catch the changed state by
using cable name ('USB'/'USB-HOST').

The user-space process may not consider the both id and vbus of USB.
If 'USB-ID'/'USB-VBUS' cable name is used instead of 'USB'/'USB-HOST',
It may cause the confusion about what is meaning of cable name
on user-space process.

So,
I prefer to use existing 'USB' and 'USB-HOST' cable name.
and then want to add additional method to get the vbus state.

I think two following method to get the vbus state.
1) Add the extcon_{get|set}_vbus_state()
- extcon_{get|set}_vbus_state()
- the list of of return value
	#define EXTCON_USB_VBUS_OFF	0
	#define EXTCON_USB_VBUS_ON	1

When USB/USB-HOST is attached and receive the notification onextcon consumer driver
,extcon consumer driver would get the vbus state by extcon_get_vbus_state().

2) Add the notifier chain for vbus state update
- extcon_{register|unregister}_vbus_notifier()
- the list of notifier event
	#define EXTCON_USB_VBUS_OFF	0
	#define EXTCON_USB_VBUS_ON	1


3) add the new cable 'USB-POWER' by Roger suggestion .
- When 'USB-POWER' cable is attached, extcon will update the cable state
'USB-POWER' means only the vbus state. But, 'USB-POWER' is not h/w cable.
The user-space process would handle this uevent of 'USB-POWER'
such as h/w cable's uevent. I think it is not clear on the user-space process aspect.

> 
> The only reason you objected was saying that it is a strange cable name. Well this is
> only what we care about from USB PHY drivers and user space is not interested in it
> so what is the concern?

I added the reason why don't want to change the legacy cable name about USB/USB-HOST.

> 
>>
>>>
>>>> "USB" - attached means ID is high. i.e. Type-B plug attached.
>>>> "USB-HOST" - attached means ID is low. i.e. Type-A plug attached.
>>>> "USB-POWER" - attached means USB power is present. i.e. VBUS is alive.
>>>>
>>>> This way the definition of USB and USB-HOST remain true to their name and avoid further confusions.
>>>> VBUS state is got through the "USB-POWER" cable state.
>>>
>>> There is the same case for MHL cable.
>>> Also, MHL cable could be connected to VBUS line.
>>> - MHL : attached just MHL cable.
>>> - MHL-POWER : attache MHL cable which is connected with VBUS line.
>>>
>>> We must need the opinion of USB/PHY driver's maintainer.
>>>
>>>>
>>>>>
>>>>> What is the appropriate method of following two solution?
>>>>> - Fisrt, use the new cable name "USB-*".
>>>> I explained this above.
>>>>
>>>>> - Second, use the additional API to get the VBUS state.
>>>>
>>>> You keep mentioning additional API for VBUS. But I don't see any such API. Can you please
>>>> suggest what API you are talking about?
>>>
>>> I'm considering following functions for VBUS state. But it is my opinion,
>>> If USB/PHY drivers's maintainers don't agree the new cable ("USB-POWER"),
>>> We could use the following function to get VBUS state.
>>> Because new cable name will affect the USB/PHY drivers.
>>> - int extcon_{get|set}_vbus_state(struct extcon_dev *edev);
> 
> This is not suitable for us as USB drivers need VBUS notification event to come.
> They can't keep polling for VBUS state using this API.

I don't agree the polling method. You refer to upper my description.

Thanks,
Chanwoo Choi

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Peter Chen]

On Wed, Apr 15, 2015 at 06:26:23PM +0900, Chanwoo Choi wrote:
> Hi Roger and Peter,
> 
> On 04/15/2015 04:50 PM, Roger Quadros wrote:
> > On 15/04/15 06:27, Peter Chen wrote:
> >> On Tue, Apr 14, 2015 at 08:29:34PM +0900, Chanwoo Choi wrote:
> >>> On 04/14/2015 07:38 PM, Roger Quadros wrote:
> >>>> On 14/04/15 13:31, Chanwoo Choi wrote:
> >>>>> On 04/14/2015 07:02 PM, Roger Quadros wrote:
> >>>>>> Fixed Kishon's id.
> >>>>>>
> >>>>>> On 14/04/15 13:01, Roger Quadros wrote:
> >>>>>>> On 10/04/15 12:18, Chanwoo Choi wrote:
> >>>>>>>> On 04/10/2015 05:46 PM, Robert Baldyga wrote:
> >>>>>>>>> On 04/10/2015 10:10 AM, Chanwoo Choi wrote:
> >>>>>>>>>> On 04/10/2015 04:45 PM, Robert Baldyga wrote:
> >>>>>>>>>>> On 04/10/2015 09:17 AM, Chanwoo Choi wrote:
> >>>>>>>>>>>> Hi Robert,
> >>>>>>>>>>>>
> >>>>>>>>>>>> On 04/09/2015 06:24 PM, Robert Baldyga wrote:
> >>>>>>>>>>>>> Hi Chanwoo,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
> >>>>>>>>>>>>>> Hi Robert,
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
> >>>>>>>>>>>>>>> Hi Chanwoo,
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
> >>>>>>>>>>>>>>>> Hi Robert,
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> [snip]
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> But, I have one question about case[3]
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
> >>>>>>>>>>>>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> It's because state of single USB cable connection cannot be completely
> >>>>>>>>>>>>>>> described using single extcon cable. USB cable state has two bits (VBUS
> >>>>>>>>>>>>>>> and ID), so we need to use two cables for single cable connection. We
> >>>>>>>>>>>>>>> use following convention:
> >>>>>>>>>>>>>>> cable "USB" = VBUS
> >>>>>>>>>>>>>>> cable "USB-HOST" = !ID.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> I think that extcon provider driver have to update the only one cable state
> >>>>>>>>>>>>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
> >>>>>>>>>>>>>> at the same time through one h/w port.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
> >>>>>>>>>>>>>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> It can. USB OTG allows for that. Moreover device can be host even if
> >>>>>>>>>>>>> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
> >>>>>>>>>>>>> detected cable type is USB host). Devices would need to have complete
> >>>>>>>>>>>>> information about USB cable connection, because OTG state machine needs
> >>>>>>>>>>>>
> >>>>>>>>>>>> As I knew, USB OTG port don't send the attached cable of both USB and USB-HOST
> >>>>>>>>>>>> at the same time. The case3 in your patch update two cable state about one h/w port.
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> It's because simple "USB" or "USB-HOST" means nothing for USB OTG
> >>>>>>>>>>> machine. It needs to know exact VBUS and ID states, which cannot be
> >>>>>>>>>>> concluded basing on cable type only. That's why I have used "USB-HOST"
> >>>>>>>>>>> name together with "USB" to pass additional information about USB cable
> >>>>>>>>>>> connection.
> >>>>>>>>>>
> >>>>>>>>>> I think this method is not proper to support this case.
> >>>>>>>>>> It may cause the confusion about other case using USB/USB-HOST cable state
> >>>>>>>>>> except of you commented case.
> >>>>>>>>>
> >>>>>>>>> That's why I finally proposed to use "USB-ID" and "USB-VBUS" in parallel
> >>>>>>>>> with old names. It seems to be simpler solution than adding new
> >>>>>>>>> mechanism notifying about VBUS and ID states changes.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> As I commented on previous reply, I don't agree to use 'USB-ID' and 'USB-VBUS'.
> >>>>>>>> If we add new strange 'USB-ID' and 'USB-VBUS' name, we would add non-general cable
> >>>>>>>> name continuoulsy.
> >>>>>>>>
> >>>>>>>> I think that extcon core provide the helper API to get the value of VBUS.
> >>>>>>>> But I need to consider it.
> >>>>>>>
> >>>>>>> Now it is starting to look like existing extcon states are not suitable for USB/PHY drivers to deliver
> >>>>>>> VBUS and ID information reliably.
> >>>>>>> This is because based on your comments the "USB" and "USB-HOST" states look like some fuzzy states
> >>>>>>> and have no direct correspondence with "VBUS" and "ID". The fact that they can't become
> >>>>>>> attached simultaneously makes me conclude that "USB" and "USB-HOST" cable states are really 
> >>>>>>> capturing only the ID pin state.
> >>>>>>>
> >>>>>>> I can suggest the following options
> >>>>>>> a) let "USB" and "USB-HOST" only indicate ID pin status. Add a new cable state for "VBUS" notification.
> >>>>>>> Maybe call it "USB-POWER" or something.
> >>>>>
> >>>>> We must discuss it before using the new cable name
> >>>>> such as "USB-POWER", "USB-ID" and "USB-VBUS".
> >>>>
> >>>> I didn't say to add "USB-ID" or "USB-VBUS". solution (a) was to have the following
> >>>
> >>> Right. Robert suggested the "USB-ID" and "USB-VBUS" cable name on previous mail in mail thread.
> >>
> >> From USB/USB-PHY driver point, it needs to know id and vbus value
> >> for their internal logic, so as extcon users, the cable name
> >> is better to reflect meaning of id and vbus, like "USB-ID" and "USB-VBUS",
> >> if the power is from vbus pin at USB cable, I don't think we need another
> >> cable name "USB-POWER" even the USB/USB-PHY driver don't need it.
> > 
> > I agree as well that this is the *best* option for USB case. Just because Chanwoo was
> > objecting these names I suggested "USB-POWER".
> > 
> > Chanwoo, can we simply get rid of "USB" and "USB-HOST" cables and move to
> > "USB-ID" and "USB-VBUS"?
> 
> I'm wondering about changing the previous cable name from 'USB'/'USB-HOST'
> to 'USB-ID/USB-VBUS' because extcon framework update the state of cable by
> using uevent and the user-space process would catch the changed state by
> using cable name ('USB'/'USB-HOST').
> 
> The user-space process may not consider the both id and vbus of USB.
> If 'USB-ID'/'USB-VBUS' cable name is used instead of 'USB'/'USB-HOST',
> It may cause the confusion about what is meaning of cable name
> on user-space process.

>From the user point, maybe the name of 'USB-OTG' is more suitable
due to below reasons:
- The users usually call this Micro-AB cable as 'USB-OTG' cable
- When this Micro-AB cable is inserted, the current port may will work as
host role, but if OTG HNP is supported, this port may be switched to device
role on the fly, eg, use case like Apple Carplay.

> 
> So,
> I prefer to use existing 'USB' and 'USB-HOST' cable name.
> and then want to add additional method to get the vbus state.
> 
> I think two following method to get the vbus state.
> 1) Add the extcon_{get|set}_vbus_state()
> - extcon_{get|set}_vbus_state()
> - the list of of return value
> 	#define EXTCON_USB_VBUS_OFF	0
> 	#define EXTCON_USB_VBUS_ON	1
> 
> When USB/USB-HOST is attached and receive the notification onextcon consumer driver
> ,extcon consumer driver would get the vbus state by extcon_get_vbus_state().
> 
> 2) Add the notifier chain for vbus state update
> - extcon_{register|unregister}_vbus_notifier()
> - the list of notifier event
> 	#define EXTCON_USB_VBUS_OFF	0
> 	#define EXTCON_USB_VBUS_ON	1
> 

Ok, from USB point, external id/vbus value can't decide
which role the controller will be, the controller driver 
will decide role according to many things, eg, user configurations,
id/vbus value, OTG HNP, etc.

So, from USB controller/phy driver, it doesn't care which cable is
inserted, it cares about id/vbus value. Eg, it can get id/vbus value
and it will be notified when the id/vbus value has changed.

> 
> 3) add the new cable 'USB-POWER' by Roger suggestion .
> - When 'USB-POWER' cable is attached, extcon will update the cable state
> 'USB-POWER' means only the vbus state. But, 'USB-POWER' is not h/w cable.
> The user-space process would handle this uevent of 'USB-POWER'
> such as h/w cable's uevent. I think it is not clear on the user-space process aspect.

Would you explain the user for 'USB-POWER', and what it stands for from
user point?

> 
> > 
> > The only reason you objected was saying that it is a strange cable name. Well this is
> > only what we care about from USB PHY drivers and user space is not interested in it
> > so what is the concern?
> 
> I added the reason why don't want to change the legacy cable name about USB/USB-HOST.
> 
> > 
> >>
> >>>
> >>>> "USB" - attached means ID is high. i.e. Type-B plug attached.
> >>>> "USB-HOST" - attached means ID is low. i.e. Type-A plug attached.
> >>>> "USB-POWER" - attached means USB power is present. i.e. VBUS is alive.
> >>>>
> >>>> This way the definition of USB and USB-HOST remain true to their name and avoid further confusions.
> >>>> VBUS state is got through the "USB-POWER" cable state.
> >>>
> >>> There is the same case for MHL cable.
> >>> Also, MHL cable could be connected to VBUS line.
> >>> - MHL : attached just MHL cable.
> >>> - MHL-POWER : attache MHL cable which is connected with VBUS line.
> >>>
> >>> We must need the opinion of USB/PHY driver's maintainer.
> >>>
> >>>>
> >>>>>
> >>>>> What is the appropriate method of following two solution?
> >>>>> - Fisrt, use the new cable name "USB-*".
> >>>> I explained this above.
> >>>>
> >>>>> - Second, use the additional API to get the VBUS state.
> >>>>
> >>>> You keep mentioning additional API for VBUS. But I don't see any such API. Can you please
> >>>> suggest what API you are talking about?
> >>>
> >>> I'm considering following functions for VBUS state. But it is my opinion,
> >>> If USB/PHY drivers's maintainers don't agree the new cable ("USB-POWER"),
> >>> We could use the following function to get VBUS state.
> >>> Because new cable name will affect the USB/PHY drivers.
> >>> - int extcon_{get|set}_vbus_state(struct extcon_dev *edev);
> > 
> > This is not suitable for us as USB drivers need VBUS notification event to come.
> > They can't keep polling for VBUS state using this API.
> 
> I don't agree the polling method. You refer to upper my description.
> 
> Thanks,
> Chanwoo Choi
> 
> 

-- 

Best Regards,
Peter Chen

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Chanwoo Choi]

Hi Peter,

On 04/16/2015 10:59 AM, Peter Chen wrote:
> On Wed, Apr 15, 2015 at 06:26:23PM +0900, Chanwoo Choi wrote:
>> Hi Roger and Peter,
>>
>> On 04/15/2015 04:50 PM, Roger Quadros wrote:
>>> On 15/04/15 06:27, Peter Chen wrote:
>>>> On Tue, Apr 14, 2015 at 08:29:34PM +0900, Chanwoo Choi wrote:
>>>>> On 04/14/2015 07:38 PM, Roger Quadros wrote:
>>>>>> On 14/04/15 13:31, Chanwoo Choi wrote:
>>>>>>> On 04/14/2015 07:02 PM, Roger Quadros wrote:
>>>>>>>> Fixed Kishon's id.
>>>>>>>>
>>>>>>>> On 14/04/15 13:01, Roger Quadros wrote:
>>>>>>>>> On 10/04/15 12:18, Chanwoo Choi wrote:
>>>>>>>>>> On 04/10/2015 05:46 PM, Robert Baldyga wrote:
>>>>>>>>>>> On 04/10/2015 10:10 AM, Chanwoo Choi wrote:
>>>>>>>>>>>> On 04/10/2015 04:45 PM, Robert Baldyga wrote:
>>>>>>>>>>>>> On 04/10/2015 09:17 AM, Chanwoo Choi wrote:
>>>>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 04/09/2015 06:24 PM, Robert Baldyga wrote:
>>>>>>>>>>>>>>> Hi Chanwoo,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
>>>>>>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>>>>>>>>>>>>>>>>> Hi Chanwoo,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>>>>>>>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> [snip]
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> But, I have one question about case[3]
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>>>>>>>>>>>>>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> It's because state of single USB cable connection cannot be completely
>>>>>>>>>>>>>>>>> described using single extcon cable. USB cable state has two bits (VBUS
>>>>>>>>>>>>>>>>> and ID), so we need to use two cables for single cable connection. We
>>>>>>>>>>>>>>>>> use following convention:
>>>>>>>>>>>>>>>>> cable "USB" = VBUS
>>>>>>>>>>>>>>>>> cable "USB-HOST" = !ID.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I think that extcon provider driver have to update the only one cable state
>>>>>>>>>>>>>>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
>>>>>>>>>>>>>>>> at the same time through one h/w port.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
>>>>>>>>>>>>>>>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> It can. USB OTG allows for that. Moreover device can be host even if
>>>>>>>>>>>>>>> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
>>>>>>>>>>>>>>> detected cable type is USB host). Devices would need to have complete
>>>>>>>>>>>>>>> information about USB cable connection, because OTG state machine needs
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> As I knew, USB OTG port don't send the attached cable of both USB and USB-HOST
>>>>>>>>>>>>>> at the same time. The case3 in your patch update two cable state about one h/w port.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> It's because simple "USB" or "USB-HOST" means nothing for USB OTG
>>>>>>>>>>>>> machine. It needs to know exact VBUS and ID states, which cannot be
>>>>>>>>>>>>> concluded basing on cable type only. That's why I have used "USB-HOST"
>>>>>>>>>>>>> name together with "USB" to pass additional information about USB cable
>>>>>>>>>>>>> connection.
>>>>>>>>>>>>
>>>>>>>>>>>> I think this method is not proper to support this case.
>>>>>>>>>>>> It may cause the confusion about other case using USB/USB-HOST cable state
>>>>>>>>>>>> except of you commented case.
>>>>>>>>>>>
>>>>>>>>>>> That's why I finally proposed to use "USB-ID" and "USB-VBUS" in parallel
>>>>>>>>>>> with old names. It seems to be simpler solution than adding new
>>>>>>>>>>> mechanism notifying about VBUS and ID states changes.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> As I commented on previous reply, I don't agree to use 'USB-ID' and 'USB-VBUS'.
>>>>>>>>>> If we add new strange 'USB-ID' and 'USB-VBUS' name, we would add non-general cable
>>>>>>>>>> name continuoulsy.
>>>>>>>>>>
>>>>>>>>>> I think that extcon core provide the helper API to get the value of VBUS.
>>>>>>>>>> But I need to consider it.
>>>>>>>>>
>>>>>>>>> Now it is starting to look like existing extcon states are not suitable for USB/PHY drivers to deliver
>>>>>>>>> VBUS and ID information reliably.
>>>>>>>>> This is because based on your comments the "USB" and "USB-HOST" states look like some fuzzy states
>>>>>>>>> and have no direct correspondence with "VBUS" and "ID". The fact that they can't become
>>>>>>>>> attached simultaneously makes me conclude that "USB" and "USB-HOST" cable states are really 
>>>>>>>>> capturing only the ID pin state.
>>>>>>>>>
>>>>>>>>> I can suggest the following options
>>>>>>>>> a) let "USB" and "USB-HOST" only indicate ID pin status. Add a new cable state for "VBUS" notification.
>>>>>>>>> Maybe call it "USB-POWER" or something.
>>>>>>>
>>>>>>> We must discuss it before using the new cable name
>>>>>>> such as "USB-POWER", "USB-ID" and "USB-VBUS".
>>>>>>
>>>>>> I didn't say to add "USB-ID" or "USB-VBUS". solution (a) was to have the following
>>>>>
>>>>> Right. Robert suggested the "USB-ID" and "USB-VBUS" cable name on previous mail in mail thread.
>>>>
>>>> From USB/USB-PHY driver point, it needs to know id and vbus value
>>>> for their internal logic, so as extcon users, the cable name
>>>> is better to reflect meaning of id and vbus, like "USB-ID" and "USB-VBUS",
>>>> if the power is from vbus pin at USB cable, I don't think we need another
>>>> cable name "USB-POWER" even the USB/USB-PHY driver don't need it.
>>>
>>> I agree as well that this is the *best* option for USB case. Just because Chanwoo was
>>> objecting these names I suggested "USB-POWER".
>>>
>>> Chanwoo, can we simply get rid of "USB" and "USB-HOST" cables and move to
>>> "USB-ID" and "USB-VBUS"?
>>
>> I'm wondering about changing the previous cable name from 'USB'/'USB-HOST'
>> to 'USB-ID/USB-VBUS' because extcon framework update the state of cable by
>> using uevent and the user-space process would catch the changed state by
>> using cable name ('USB'/'USB-HOST').
>>
>> The user-space process may not consider the both id and vbus of USB.
>> If 'USB-ID'/'USB-VBUS' cable name is used instead of 'USB'/'USB-HOST',
>> It may cause the confusion about what is meaning of cable name
>> on user-space process.
> 
>>From the user point, maybe the name of 'USB-OTG' is more suitable
> due to below reasons:
> - The users usually call this Micro-AB cable as 'USB-OTG' cable
> - When this Micro-AB cable is inserted, the current port may will work as
> host role, but if OTG HNP is supported, this port may be switched to device
> role on the fly, eg, use case like Apple Carplay.

OK. I agree that using the 'USB-OTG' cable name instead of 'USB-HOST'.
- 'USB' for usb device
- 'USB-HOST' -> 'USB-OTG' for usb host

> 
>>
>> So,
>> I prefer to use existing 'USB' and 'USB-HOST' cable name.
>> and then want to add additional method to get the vbus state.
>>
>> I think two following method to get the vbus state.
>> 1) Add the extcon_{get|set}_vbus_state()
>> - extcon_{get|set}_vbus_state()
>> - the list of of return value
>> 	#define EXTCON_USB_VBUS_OFF	0
>> 	#define EXTCON_USB_VBUS_ON	1
>>
>> When USB/USB-HOST is attached and receive the notification onextcon consumer driver
>> ,extcon consumer driver would get the vbus state by extcon_get_vbus_state().
>>
>> 2) Add the notifier chain for vbus state update
>> - extcon_{register|unregister}_vbus_notifier()
>> - the list of notifier event
>> 	#define EXTCON_USB_VBUS_OFF	0
>> 	#define EXTCON_USB_VBUS_ON	1
>>
> 
> Ok, from USB point, external id/vbus value can't decide
> which role the controller will be, the controller driver 
> will decide role according to many things, eg, user configurations,
> id/vbus value, OTG HNP, etc.
> 
> So, from USB controller/phy driver, it doesn't care which cable is
> inserted, it cares about id/vbus value. Eg, it can get id/vbus value
> and it will be notified when the id/vbus value has changed.

OK, I change the notifier name and add notifier events as following:

- extcon_{register|unregister}_usb_notifier(struct extcon_dev *edev, struct notifier_block *nb);
- list of notifier events
	#define EXTCON_USB_ID_L_VBUS_L	0	/* ID low  and VBUS low */
	#define EXTCON_USB_ID_L_VBUS_H	1	/* ID low  and VBUS high */
	#define EXTCON_USB_ID_H_VBUS_L	2	/* ID high and VBUS low */
	#define EXTCON_USB_ID_H_VBUS_H	3	/* ID high and VBUS high */

I think that we need the opinion of Felipe and Kishon about this notifier chain.

> 
>>
>> 3) add the new cable 'USB-POWER' by Roger suggestion .
>> - When 'USB-POWER' cable is attached, extcon will update the cable state
>> 'USB-POWER' means only the vbus state. But, 'USB-POWER' is not h/w cable.
>> The user-space process would handle this uevent of 'USB-POWER'
>> such as h/w cable's uevent. I think it is not clear on the user-space process aspect.
> 
> Would you explain the user for 'USB-POWER', and what it stands for from
> user point?

IMO, I think '*-POWER' keyword is not standard cable name on the user-space.
As I commend on upper reply, I agree USB/USB-OTG cable name.

[snip]

Thanks,
Chanwoo Choi

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Ivan T. Ivanov]

Hi,

On Thu, 2015-04-16 at 16:00 +0900, Chanwoo Choi wrote:
> Hi Peter,
> 
> On 04/16/2015 10:59 AM, Peter Chen wrote:
> > 

> > Ok, from USB point, external id/vbus value can't decide
> > which role the controller will be, the controller driver
> > will decide role according to many things, eg, user configurations,
> > id/vbus value, OTG HNP, etc.
> > 
> > So, from USB controller/phy driver, it doesn't care which cable is
> > inserted, it cares about id/vbus value. Eg, it can get id/vbus value
> > and it will be notified when the id/vbus value has changed.
> 
> OK, I change the notifier name and add notifier events as following:
> 
> - extcon_{register|unregister}_usb_notifier(struct extcon_dev *edev, struct notifier_block *nb);
> - list of notifier events
>         #define EXTCON_USB_ID_L_VBUS_L0       /* ID low  and VBUS low */
>         #define EXTCON_USB_ID_L_VBUS_H1       /* ID low  and VBUS high */
>         #define EXTCON_USB_ID_H_VBUS_L2       /* ID high and VBUS low */
>         #define EXTCON_USB_ID_H_VBUS_H3       /* ID high and VBUS high */

I am still confused, why we mix ID and VBUS events into one? 
Those are two lines and they are not necessarily handled by
the same extcon_dev.

Ivan

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Roger Quadros]

On 16/04/15 10:00, Chanwoo Choi wrote:
> Hi Peter,
> 
> On 04/16/2015 10:59 AM, Peter Chen wrote:
>> On Wed, Apr 15, 2015 at 06:26:23PM +0900, Chanwoo Choi wrote:
>>> Hi Roger and Peter,
>>>
>>> On 04/15/2015 04:50 PM, Roger Quadros wrote:
>>>> On 15/04/15 06:27, Peter Chen wrote:
>>>>> On Tue, Apr 14, 2015 at 08:29:34PM +0900, Chanwoo Choi wrote:
>>>>>> On 04/14/2015 07:38 PM, Roger Quadros wrote:
>>>>>>> On 14/04/15 13:31, Chanwoo Choi wrote:
>>>>>>>> On 04/14/2015 07:02 PM, Roger Quadros wrote:
>>>>>>>>> Fixed Kishon's id.
>>>>>>>>>
>>>>>>>>> On 14/04/15 13:01, Roger Quadros wrote:
>>>>>>>>>> On 10/04/15 12:18, Chanwoo Choi wrote:
>>>>>>>>>>> On 04/10/2015 05:46 PM, Robert Baldyga wrote:
>>>>>>>>>>>> On 04/10/2015 10:10 AM, Chanwoo Choi wrote:
>>>>>>>>>>>>> On 04/10/2015 04:45 PM, Robert Baldyga wrote:
>>>>>>>>>>>>>> On 04/10/2015 09:17 AM, Chanwoo Choi wrote:
>>>>>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 04/09/2015 06:24 PM, Robert Baldyga wrote:
>>>>>>>>>>>>>>>> Hi Chanwoo,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote:
>>>>>>>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote:
>>>>>>>>>>>>>>>>>> Hi Chanwoo,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote:
>>>>>>>>>>>>>>>>>>> Hi Robert,
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> [snip]
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> But, I have one question about case[3]
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state.
>>>>>>>>>>>>>>>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously?
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> It's because state of single USB cable connection cannot be completely
>>>>>>>>>>>>>>>>>> described using single extcon cable. USB cable state has two bits (VBUS
>>>>>>>>>>>>>>>>>> and ID), so we need to use two cables for single cable connection. We
>>>>>>>>>>>>>>>>>> use following convention:
>>>>>>>>>>>>>>>>>> cable "USB" = VBUS
>>>>>>>>>>>>>>>>>> cable "USB-HOST" = !ID.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I think that extcon provider driver have to update the only one cable state
>>>>>>>>>>>>>>>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used
>>>>>>>>>>>>>>>>> at the same time through one h/w port.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable
>>>>>>>>>>>>>>>>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> It can. USB OTG allows for that. Moreover device can be host even if
>>>>>>>>>>>>>>>> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so
>>>>>>>>>>>>>>>> detected cable type is USB host). Devices would need to have complete
>>>>>>>>>>>>>>>> information about USB cable connection, because OTG state machine needs
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> As I knew, USB OTG port don't send the attached cable of both USB and USB-HOST
>>>>>>>>>>>>>>> at the same time. The case3 in your patch update two cable state about one h/w port.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> It's because simple "USB" or "USB-HOST" means nothing for USB OTG
>>>>>>>>>>>>>> machine. It needs to know exact VBUS and ID states, which cannot be
>>>>>>>>>>>>>> concluded basing on cable type only. That's why I have used "USB-HOST"
>>>>>>>>>>>>>> name together with "USB" to pass additional information about USB cable
>>>>>>>>>>>>>> connection.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I think this method is not proper to support this case.
>>>>>>>>>>>>> It may cause the confusion about other case using USB/USB-HOST cable state
>>>>>>>>>>>>> except of you commented case.
>>>>>>>>>>>>
>>>>>>>>>>>> That's why I finally proposed to use "USB-ID" and "USB-VBUS" in parallel
>>>>>>>>>>>> with old names. It seems to be simpler solution than adding new
>>>>>>>>>>>> mechanism notifying about VBUS and ID states changes.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> As I commented on previous reply, I don't agree to use 'USB-ID' and 'USB-VBUS'.
>>>>>>>>>>> If we add new strange 'USB-ID' and 'USB-VBUS' name, we would add non-general cable
>>>>>>>>>>> name continuoulsy.
>>>>>>>>>>>
>>>>>>>>>>> I think that extcon core provide the helper API to get the value of VBUS.
>>>>>>>>>>> But I need to consider it.
>>>>>>>>>>
>>>>>>>>>> Now it is starting to look like existing extcon states are not suitable for USB/PHY drivers to deliver
>>>>>>>>>> VBUS and ID information reliably.
>>>>>>>>>> This is because based on your comments the "USB" and "USB-HOST" states look like some fuzzy states
>>>>>>>>>> and have no direct correspondence with "VBUS" and "ID". The fact that they can't become
>>>>>>>>>> attached simultaneously makes me conclude that "USB" and "USB-HOST" cable states are really 
>>>>>>>>>> capturing only the ID pin state.
>>>>>>>>>>
>>>>>>>>>> I can suggest the following options
>>>>>>>>>> a) let "USB" and "USB-HOST" only indicate ID pin status. Add a new cable state for "VBUS" notification.
>>>>>>>>>> Maybe call it "USB-POWER" or something.
>>>>>>>>
>>>>>>>> We must discuss it before using the new cable name
>>>>>>>> such as "USB-POWER", "USB-ID" and "USB-VBUS".
>>>>>>>
>>>>>>> I didn't say to add "USB-ID" or "USB-VBUS". solution (a) was to have the following
>>>>>>
>>>>>> Right. Robert suggested the "USB-ID" and "USB-VBUS" cable name on previous mail in mail thread.
>>>>>
>>>>> From USB/USB-PHY driver point, it needs to know id and vbus value
>>>>> for their internal logic, so as extcon users, the cable name
>>>>> is better to reflect meaning of id and vbus, like "USB-ID" and "USB-VBUS",
>>>>> if the power is from vbus pin at USB cable, I don't think we need another
>>>>> cable name "USB-POWER" even the USB/USB-PHY driver don't need it.
>>>>
>>>> I agree as well that this is the *best* option for USB case. Just because Chanwoo was
>>>> objecting these names I suggested "USB-POWER".
>>>>
>>>> Chanwoo, can we simply get rid of "USB" and "USB-HOST" cables and move to
>>>> "USB-ID" and "USB-VBUS"?
>>>
>>> I'm wondering about changing the previous cable name from 'USB'/'USB-HOST'
>>> to 'USB-ID/USB-VBUS' because extcon framework update the state of cable by
>>> using uevent and the user-space process would catch the changed state by
>>> using cable name ('USB'/'USB-HOST').
>>>
>>> The user-space process may not consider the both id and vbus of USB.
>>> If 'USB-ID'/'USB-VBUS' cable name is used instead of 'USB'/'USB-HOST',
>>> It may cause the confusion about what is meaning of cable name
>>> on user-space process.
>>
>> >From the user point, maybe the name of 'USB-OTG' is more suitable
>> due to below reasons:
>> - The users usually call this Micro-AB cable as 'USB-OTG' cable
>> - When this Micro-AB cable is inserted, the current port may will work as
>> host role, but if OTG HNP is supported, this port may be switched to device
>> role on the fly, eg, use case like Apple Carplay.
> 
> OK. I agree that using the 'USB-OTG' cable name instead of 'USB-HOST'.
> - 'USB' for usb device
> - 'USB-HOST' -> 'USB-OTG' for usb host

I'm lost now.
Can you please explain when and based on what these cables should become attached and detached?

What is user space really interested in?
- Does it only care that USB cable is attached/detached
- does it want to know what type of cable is attached (Type-A, Type-B)
- does it want to know what mode the USB is working on (host, device)

I'm getting a feeling that we have not fully understood the problem and we are already
trying to solve it.

cheers,
-roger

> 
>>
>>>
>>> So,
>>> I prefer to use existing 'USB' and 'USB-HOST' cable name.
>>> and then want to add additional method to get the vbus state.
>>>
>>> I think two following method to get the vbus state.
>>> 1) Add the extcon_{get|set}_vbus_state()
>>> - extcon_{get|set}_vbus_state()
>>> - the list of of return value
>>> 	#define EXTCON_USB_VBUS_OFF	0
>>> 	#define EXTCON_USB_VBUS_ON	1
>>>
>>> When USB/USB-HOST is attached and receive the notification onextcon consumer driver
>>> ,extcon consumer driver would get the vbus state by extcon_get_vbus_state().
>>>
>>> 2) Add the notifier chain for vbus state update
>>> - extcon_{register|unregister}_vbus_notifier()
>>> - the list of notifier event
>>> 	#define EXTCON_USB_VBUS_OFF	0
>>> 	#define EXTCON_USB_VBUS_ON	1
>>>
>>
>> Ok, from USB point, external id/vbus value can't decide
>> which role the controller will be, the controller driver 
>> will decide role according to many things, eg, user configurations,
>> id/vbus value, OTG HNP, etc.
>>
>> So, from USB controller/phy driver, it doesn't care which cable is
>> inserted, it cares about id/vbus value. Eg, it can get id/vbus value
>> and it will be notified when the id/vbus value has changed.
> 
> OK, I change the notifier name and add notifier events as following:
> 
> - extcon_{register|unregister}_usb_notifier(struct extcon_dev *edev, struct notifier_block *nb);
> - list of notifier events
> 	#define EXTCON_USB_ID_L_VBUS_L	0	/* ID low  and VBUS low */
> 	#define EXTCON_USB_ID_L_VBUS_H	1	/* ID low  and VBUS high */
> 	#define EXTCON_USB_ID_H_VBUS_L	2	/* ID high and VBUS low */
> 	#define EXTCON_USB_ID_H_VBUS_H	3	/* ID high and VBUS high */
> 
> I think that we need the opinion of Felipe and Kishon about this notifier chain.
> 
>>
>>>
>>> 3) add the new cable 'USB-POWER' by Roger suggestion .
>>> - When 'USB-POWER' cable is attached, extcon will update the cable state
>>> 'USB-POWER' means only the vbus state. But, 'USB-POWER' is not h/w cable.
>>> The user-space process would handle this uevent of 'USB-POWER'
>>> such as h/w cable's uevent. I think it is not clear on the user-space process aspect.
>>
>> Would you explain the user for 'USB-POWER', and what it stands for from
>> user point?
> 
> IMO, I think '*-POWER' keyword is not standard cable name on the user-space.
> As I commend on upper reply, I agree USB/USB-OTG cable name.
> 
> [snip]
> 
> Thanks,
> Chanwoo Choi
> 

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Chanwoo Choi]

On 04/16/2015 04:13 PM, Ivan T. Ivanov wrote:
> Hi,
> 
> On Thu, 2015-04-16 at 16:00 +0900, Chanwoo Choi wrote:
>> Hi Peter,
>>
>> On 04/16/2015 10:59 AM, Peter Chen wrote:
>>>
> 
>>> Ok, from USB point, external id/vbus value can't decide
>>> which role the controller will be, the controller driver
>>> will decide role according to many things, eg, user configurations,
>>> id/vbus value, OTG HNP, etc.
>>>
>>> So, from USB controller/phy driver, it doesn't care which cable is
>>> inserted, it cares about id/vbus value. Eg, it can get id/vbus value
>>> and it will be notified when the id/vbus value has changed.
>>
>> OK, I change the notifier name and add notifier events as following:
>>
>> - extcon_{register|unregister}_usb_notifier(struct extcon_dev *edev, struct notifier_block *nb);
>> - list of notifier events
>>         #define EXTCON_USB_ID_L_VBUS_L0       /* ID low  and VBUS low */
>>         #define EXTCON_USB_ID_L_VBUS_H1       /* ID low  and VBUS high */
>>         #define EXTCON_USB_ID_H_VBUS_L2       /* ID high and VBUS low */
>>         #define EXTCON_USB_ID_H_VBUS_H3       /* ID high and VBUS high */
> 
> I am still confused, why we mix ID and VBUS events into one? 
> Those are two lines and they are not necessarily handled by
> the same extcon_dev.

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
	

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Peter Chen]

On Thu, Apr 16, 2015 at 10:13:44AM +0300, Ivan T. Ivanov wrote:
> Hi,
> 
> On Thu, 2015-04-16 at 16:00 +0900, Chanwoo Choi wrote:
> > Hi Peter,
> > 
> > On 04/16/2015 10:59 AM, Peter Chen wrote:
> > > 
> 
> > > Ok, from USB point, external id/vbus value can't decide
> > > which role the controller will be, the controller driver
> > > will decide role according to many things, eg, user configurations,
> > > id/vbus value, OTG HNP, etc.
> > > 
> > > So, from USB controller/phy driver, it doesn't care which cable is
> > > inserted, it cares about id/vbus value. Eg, it can get id/vbus value
> > > and it will be notified when the id/vbus value has changed.
> > 
> > OK, I change the notifier name and add notifier events as following:
> > 
> > - extcon_{register|unregister}_usb_notifier(struct extcon_dev *edev, struct notifier_block *nb);
> > - list of notifier events
> >         #define EXTCON_USB_ID_L_VBUS_L0       /* ID low  and VBUS low */
> >         #define EXTCON_USB_ID_L_VBUS_H1       /* ID low  and VBUS high */
> >         #define EXTCON_USB_ID_H_VBUS_L2       /* ID high and VBUS low */
> >         #define EXTCON_USB_ID_H_VBUS_H3       /* ID high and VBUS high */
> 
> I am still confused, why we mix ID and VBUS events into one? 
> Those are two lines and they are not necessarily handled by
> the same extcon_dev.

Yes, from consumer side, we only need one element per time, eg vbus
or id.

-- 

Best Regards,
Peter Chen

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Peter Chen]

On Thu, Apr 16, 2015 at 04:59:31PM +0900, Chanwoo Choi wrote:
> On 04/16/2015 04:13 PM, Ivan T. Ivanov wrote:
> > Hi,
> > 
> > On Thu, 2015-04-16 at 16:00 +0900, Chanwoo Choi wrote:
> >> Hi Peter,
> >>
> >> On 04/16/2015 10:59 AM, Peter Chen wrote:
> >>>
> > 
> >>> Ok, from USB point, external id/vbus value can't decide
> >>> which role the controller will be, the controller driver
> >>> will decide role according to many things, eg, user configurations,
> >>> id/vbus value, OTG HNP, etc.
> >>>
> >>> So, from USB controller/phy driver, it doesn't care which cable is
> >>> inserted, it cares about id/vbus value. Eg, it can get id/vbus value
> >>> and it will be notified when the id/vbus value has changed.
> >>
> >> OK, I change the notifier name and add notifier events as following:
> >>
> >> - extcon_{register|unregister}_usb_notifier(struct extcon_dev *edev, struct notifier_block *nb);
> >> - list of notifier events
> >>         #define EXTCON_USB_ID_L_VBUS_L0       /* ID low  and VBUS low */
> >>         #define EXTCON_USB_ID_L_VBUS_H1       /* ID low  and VBUS high */
> >>         #define EXTCON_USB_ID_H_VBUS_L2       /* ID high and VBUS low */
> >>         #define EXTCON_USB_ID_H_VBUS_H3       /* ID high and VBUS high */
> > 
> > I am still confused, why we mix ID and VBUS events into one? 
> > Those are two lines and they are not necessarily handled by
> > the same extcon_dev.
> 
> 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
> 	

I agree with above definition.

-- 

Best Regards,
Peter Chen

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Chanwoo Choi]

On 04/16/2015 05:01 PM, Peter Chen wrote:
> On Thu, Apr 16, 2015 at 04:59:31PM +0900, Chanwoo Choi wrote:
>> On 04/16/2015 04:13 PM, Ivan T. Ivanov wrote:
>>> Hi,
>>>
>>> On Thu, 2015-04-16 at 16:00 +0900, Chanwoo Choi wrote:
>>>> Hi Peter,
>>>>
>>>> On 04/16/2015 10:59 AM, Peter Chen wrote:
>>>>>
>>>
>>>>> Ok, from USB point, external id/vbus value can't decide
>>>>> which role the controller will be, the controller driver
>>>>> will decide role according to many things, eg, user configurations,
>>>>> id/vbus value, OTG HNP, etc.
>>>>>
>>>>> So, from USB controller/phy driver, it doesn't care which cable is
>>>>> inserted, it cares about id/vbus value. Eg, it can get id/vbus value
>>>>> and it will be notified when the id/vbus value has changed.
>>>>
>>>> OK, I change the notifier name and add notifier events as following:
>>>>
>>>> - extcon_{register|unregister}_usb_notifier(struct extcon_dev *edev, struct notifier_block *nb);
>>>> - list of notifier events
>>>>         #define EXTCON_USB_ID_L_VBUS_L0       /* ID low  and VBUS low */
>>>>         #define EXTCON_USB_ID_L_VBUS_H1       /* ID low  and VBUS high */
>>>>         #define EXTCON_USB_ID_H_VBUS_L2       /* ID high and VBUS low */
>>>>         #define EXTCON_USB_ID_H_VBUS_H3       /* ID high and VBUS high */
>>>
>>> I am still confused, why we mix ID and VBUS events into one? 
>>> Those are two lines and they are not necessarily handled by
>>> the same extcon_dev.
>>
>> 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
>> 	
> 
> I agree with above definition.
> 

OK. I understand.

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Roger Quadros]

On 16/04/15 11:26, Chanwoo Choi wrote:
> On 04/16/2015 05:01 PM, Peter Chen wrote:
>> On Thu, Apr 16, 2015 at 04:59:31PM +0900, Chanwoo Choi wrote:
>>> On 04/16/2015 04:13 PM, Ivan T. Ivanov wrote:
>>>> Hi,
>>>>
>>>> On Thu, 2015-04-16 at 16:00 +0900, Chanwoo Choi wrote:
>>>>> Hi Peter,
>>>>>
>>>>> On 04/16/2015 10:59 AM, Peter Chen wrote:
>>>>>>
>>>>
>>>>>> Ok, from USB point, external id/vbus value can't decide
>>>>>> which role the controller will be, the controller driver
>>>>>> will decide role according to many things, eg, user configurations,
>>>>>> id/vbus value, OTG HNP, etc.
>>>>>>
>>>>>> So, from USB controller/phy driver, it doesn't care which cable is
>>>>>> inserted, it cares about id/vbus value. Eg, it can get id/vbus value
>>>>>> and it will be notified when the id/vbus value has changed.
>>>>>
>>>>> OK, I change the notifier name and add notifier events as following:
>>>>>
>>>>> - extcon_{register|unregister}_usb_notifier(struct extcon_dev *edev, struct notifier_block *nb);
>>>>> - list of notifier events
>>>>>         #define EXTCON_USB_ID_L_VBUS_L0       /* ID low  and VBUS low */
>>>>>         #define EXTCON_USB_ID_L_VBUS_H1       /* ID low  and VBUS high */
>>>>>         #define EXTCON_USB_ID_H_VBUS_L2       /* ID high and VBUS low */
>>>>>         #define EXTCON_USB_ID_H_VBUS_H3       /* ID high and VBUS high */
>>>>
>>>> I am still confused, why we mix ID and VBUS events into one? 
>>>> Those are two lines and they are not necessarily handled by
>>>> the same extcon_dev.
>>>
>>> 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
>>> 	
>>
>> I agree with above definition.
>>
> 
> OK. I understand.
> 
> 
Chanwoo, Robert,

Do we have an agreement on a common solution then?
IMO the above mentioned 4 notifier events should meet all our USB needs.

cheers,
-roger

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Chanwoo Choi]

On 04/30/2015 04:32 PM, Roger Quadros wrote:
> On 16/04/15 11:26, Chanwoo Choi wrote:
>> On 04/16/2015 05:01 PM, Peter Chen wrote:
>>> On Thu, Apr 16, 2015 at 04:59:31PM +0900, Chanwoo Choi wrote:
>>>> On 04/16/2015 04:13 PM, Ivan T. Ivanov wrote:
>>>>> Hi,
>>>>>
>>>>> On Thu, 2015-04-16 at 16:00 +0900, Chanwoo Choi wrote:
>>>>>> Hi Peter,
>>>>>>
>>>>>> On 04/16/2015 10:59 AM, Peter Chen wrote:
>>>>>>>
>>>>>
>>>>>>> Ok, from USB point, external id/vbus value can't decide
>>>>>>> which role the controller will be, the controller driver
>>>>>>> will decide role according to many things, eg, user configurations,
>>>>>>> id/vbus value, OTG HNP, etc.
>>>>>>>
>>>>>>> So, from USB controller/phy driver, it doesn't care which cable is
>>>>>>> inserted, it cares about id/vbus value. Eg, it can get id/vbus value
>>>>>>> and it will be notified when the id/vbus value has changed.
>>>>>>
>>>>>> OK, I change the notifier name and add notifier events as following:
>>>>>>
>>>>>> - extcon_{register|unregister}_usb_notifier(struct extcon_dev *edev, struct notifier_block *nb);
>>>>>> - list of notifier events
>>>>>>         #define EXTCON_USB_ID_L_VBUS_L0       /* ID low  and VBUS low */
>>>>>>         #define EXTCON_USB_ID_L_VBUS_H1       /* ID low  and VBUS high */
>>>>>>         #define EXTCON_USB_ID_H_VBUS_L2       /* ID high and VBUS low */
>>>>>>         #define EXTCON_USB_ID_H_VBUS_H3       /* ID high and VBUS high */
>>>>>
>>>>> I am still confused, why we mix ID and VBUS events into one? 
>>>>> Those are two lines and they are not necessarily handled by
>>>>> the same extcon_dev.
>>>>
>>>> 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
>>>> 	
>>>
>>> I agree with above definition.
>>>
>>
>> OK. I understand.
>>
>>
> Chanwoo, Robert,
> 

Hi Roger,

> Do we have an agreement on a common solution then?

Yes.

> IMO the above mentioned 4 notifier events should meet all our USB needs.

I'll support usb notifier chain which includes 4 notifier events.

Thanks,
Chanwoo Choi

Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection

[Roger Quadros]

On 30/04/15 10:55, Chanwoo Choi wrote:
> On 04/30/2015 04:32 PM, Roger Quadros wrote:
>> On 16/04/15 11:26, Chanwoo Choi wrote:
>>> On 04/16/2015 05:01 PM, Peter Chen wrote:
>>>> On Thu, Apr 16, 2015 at 04:59:31PM +0900, Chanwoo Choi wrote:
>>>>> On 04/16/2015 04:13 PM, Ivan T. Ivanov wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Thu, 2015-04-16 at 16:00 +0900, Chanwoo Choi wrote:
>>>>>>> Hi Peter,
>>>>>>>
>>>>>>> On 04/16/2015 10:59 AM, Peter Chen wrote:
>>>>>>>>
>>>>>>
>>>>>>>> Ok, from USB point, external id/vbus value can't decide
>>>>>>>> which role the controller will be, the controller driver
>>>>>>>> will decide role according to many things, eg, user configurations,
>>>>>>>> id/vbus value, OTG HNP, etc.
>>>>>>>>
>>>>>>>> So, from USB controller/phy driver, it doesn't care which cable is
>>>>>>>> inserted, it cares about id/vbus value. Eg, it can get id/vbus value
>>>>>>>> and it will be notified when the id/vbus value has changed.
>>>>>>>
>>>>>>> OK, I change the notifier name and add notifier events as following:
>>>>>>>
>>>>>>> - extcon_{register|unregister}_usb_notifier(struct extcon_dev *edev, struct notifier_block *nb);
>>>>>>> - list of notifier events
>>>>>>>         #define EXTCON_USB_ID_L_VBUS_L0       /* ID low  and VBUS low */
>>>>>>>         #define EXTCON_USB_ID_L_VBUS_H1       /* ID low  and VBUS high */
>>>>>>>         #define EXTCON_USB_ID_H_VBUS_L2       /* ID high and VBUS low */
>>>>>>>         #define EXTCON_USB_ID_H_VBUS_H3       /* ID high and VBUS high */
>>>>>>
>>>>>> I am still confused, why we mix ID and VBUS events into one? 
>>>>>> Those are two lines and they are not necessarily handled by
>>>>>> the same extcon_dev.
>>>>>
>>>>> 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
>>>>> 	
>>>>
>>>> I agree with above definition.
>>>>
>>>
>>> OK. I understand.
>>>
>>>
>> Chanwoo, Robert,
>>
> 
> Hi Roger,
> 
>> Do we have an agreement on a common solution then?
> 
> Yes.
> 
>> IMO the above mentioned 4 notifier events should meet all our USB needs.
> 
> I'll support usb notifier chain which includes 4 notifier events.

Great, then Robert can base his patches on that. Thanks.

cheers,
-roger