Re: [PATCH 1/5] extcon: gpio-usb: Introduce gpio usb extcon driver

2015-01-22 Thread Roger Quadros
Hi Chanwoo,

On 21/01/15 07:28, Chanwoo Choi wrote:
 Hi Roger,
 
 On 01/20/2015 02:52 AM, Roger Quadros wrote:
 This driver observes the USB ID pin connected over a GPIO and
 updates the USB cable extcon states accordingly.

 The existing GPIO extcon driver is not suitable for this purpose
 as it needs to be taught to understand USB cable states and it
 can't handle more than one cable per instance.

 For the USB case we need to handle 2 cable states.
 1) USB (attach/detach)
 2) USB-Host (attach/detach)

 This driver can be easily updated in the future to handle VBUS
 events in case it happens to be available on GPIO for any platform.

 Signed-off-by: Roger Quadros rog...@ti.com
 ---
  .../devicetree/bindings/extcon/extcon-usb.txt  |  20 ++
  drivers/extcon/Kconfig |   7 +
  drivers/extcon/Makefile|   1 +
  drivers/extcon/extcon-gpio-usb.c   | 225 
 +
  4 files changed, 253 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/extcon/extcon-usb.txt
  create mode 100644 drivers/extcon/extcon-gpio-usb.c

 diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb.txt 
 b/Documentation/devicetree/bindings/extcon/extcon-usb.txt
 new file mode 100644
 index 000..171c5a4
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/extcon/extcon-usb.txt
 
 Need to rename from extcon-usb.txt to extcon-gpio-usb.txt.
 
 @@ -0,0 +1,20 @@
 +USB Extcon device
 +
 +This is a virtual device used to generate USB cable states from the USB ID 
 pin
 +connected to a GPIO pin.
 +
 +Required properties:
 +- compatible: Should be linux,extcon-usb
 
 I think you better use linux,extcon-gpio-usb
 because the point of this driver use the gpio for usb cable.

I will change all instances of extcon-usb to extcon-usb-gpio.

 
 +- id-gpio: gpio for USB ID pin. See gpio binding.
 +
 +Example:
 +extcon_usb1 {
 +compatible = linux,extcon-usb;
 
 ditto.
 
 +id-gpio = gpio6 1 GPIO_ACTIVE_HIGH;
 +}
 +
 +usb@1 {
 +...
 +extcon = extcon_usb1;
 +...
 +};
 diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
 index 6a1f7de..8106a83 100644
 --- a/drivers/extcon/Kconfig
 +++ b/drivers/extcon/Kconfig
 @@ -35,6 +35,13 @@ config EXTCON_GPIO
Say Y here to enable GPIO based extcon support. Note that GPIO
extcon supports single state per extcon instance.
  
 +config EXTCON_GPIO_USB
 +tristate USB GPIO extcon support
 +depends on GPIOLIB
 +help
 +  Say Y here to enable GPIO based USB cable detection extcon support.
 +  Used typically if GPIO is used for USB ID pin detection.
 +
  config EXTCON_MAX14577
  tristate MAX14577/77836 EXTCON Support
  depends on MFD_MAX14577
 diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
 index 0370b42..bae594b 100644
 --- a/drivers/extcon/Makefile
 +++ b/drivers/extcon/Makefile
 @@ -12,3 +12,4 @@ obj-$(CONFIG_EXTCON_MAX8997)   += extcon-max8997.o
  obj-$(CONFIG_EXTCON_PALMAS) += extcon-palmas.o
  obj-$(CONFIG_EXTCON_RT8973A)+= extcon-rt8973a.o
  obj-$(CONFIG_EXTCON_SM5502) += extcon-sm5502.o
 +obj-$(CONFIG_EXTCON_GPIO_USB)   += extcon-gpio-usb.o
 
 Need to re-order it alphabetically.

OK. I will change this to extcon-usb-gpio so it will remain last.
 
 diff --git a/drivers/extcon/extcon-gpio-usb.c 
 b/drivers/extcon/extcon-gpio-usb.c
 new file mode 100644
 index 000..aeb2298
 --- /dev/null
 +++ b/drivers/extcon/extcon-gpio-usb.c
 @@ -0,0 +1,225 @@
 +/**
 + * drivers/extcon/extcon_gpio_usb.c - USB GPIO extcon driver
 + *
 + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
 + *
 
 Remove un-necessary blank line.

OK.
 
 + * Author: Roger Quadros rog...@ti.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + */
 +
 +#include linux/extcon.h
 +#include linux/extcon/extcon-gpio.h
 
 Is it necessary? I think it is your mistake?

right. my bad.
 
 +#include linux/gpio.h
 
 Don't need it because 'of_gpio.h' includes already 'gpio.h'.

agreed.

 
 +#include linux/init.h
 +#include linux/interrupt.h
 +#include linux/irq.h
 +#include linux/kernel.h
 +#include linux/module.h
 +#include linux/of_gpio.h
 +#include linux/platform_device.h
 +#include linux/slab.h
 +#include linux/workqueue.h
 +
 +#define USB_GPIO_DEBOUNCE_MS20  /* ms */
 +
 +struct usb_extcon_info {
 +struct device *dev;
 +struct extcon_dev *edev;
 +
 +struct gpio_desc *id_gpiod;
 +int id_irq;
 +
 +unsigned long debounce_jiffies;
 +struct 

Re: [PATCH 1/5] extcon: gpio-usb: Introduce gpio usb extcon driver

2015-01-20 Thread Felipe Balbi
On Mon, Jan 19, 2015 at 07:52:18PM +0200, Roger Quadros wrote:
 This driver observes the USB ID pin connected over a GPIO and
 updates the USB cable extcon states accordingly.
 
 The existing GPIO extcon driver is not suitable for this purpose
 as it needs to be taught to understand USB cable states and it
 can't handle more than one cable per instance.
 
 For the USB case we need to handle 2 cable states.
 1) USB (attach/detach)
 2) USB-Host (attach/detach)
 
 This driver can be easily updated in the future to handle VBUS
 events in case it happens to be available on GPIO for any platform.
 
 Signed-off-by: Roger Quadros rog...@ti.com

Reviewed-by: Felipe Balbi ba...@ti.com
Acked-by: Felipe Balbi ba...@ti.com

 ---
  .../devicetree/bindings/extcon/extcon-usb.txt  |  20 ++
  drivers/extcon/Kconfig |   7 +
  drivers/extcon/Makefile|   1 +
  drivers/extcon/extcon-gpio-usb.c   | 225 
 +
  4 files changed, 253 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/extcon/extcon-usb.txt
  create mode 100644 drivers/extcon/extcon-gpio-usb.c
 
 diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb.txt 
 b/Documentation/devicetree/bindings/extcon/extcon-usb.txt
 new file mode 100644
 index 000..171c5a4
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/extcon/extcon-usb.txt
 @@ -0,0 +1,20 @@
 +USB Extcon device
 +
 +This is a virtual device used to generate USB cable states from the USB ID 
 pin
 +connected to a GPIO pin.
 +
 +Required properties:
 +- compatible: Should be linux,extcon-usb
 +- id-gpio: gpio for USB ID pin. See gpio binding.
 +
 +Example:
 + extcon_usb1 {
 + compatible = linux,extcon-usb;
 + id-gpio = gpio6 1 GPIO_ACTIVE_HIGH;
 + }
 +
 + usb@1 {
 + ...
 + extcon = extcon_usb1;
 + ...
 + };
 diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
 index 6a1f7de..8106a83 100644
 --- a/drivers/extcon/Kconfig
 +++ b/drivers/extcon/Kconfig
 @@ -35,6 +35,13 @@ config EXTCON_GPIO
 Say Y here to enable GPIO based extcon support. Note that GPIO
 extcon supports single state per extcon instance.
  
 +config EXTCON_GPIO_USB
 + tristate USB GPIO extcon support
 + depends on GPIOLIB
 + help
 +   Say Y here to enable GPIO based USB cable detection extcon support.
 +   Used typically if GPIO is used for USB ID pin detection.
 +
  config EXTCON_MAX14577
   tristate MAX14577/77836 EXTCON Support
   depends on MFD_MAX14577
 diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
 index 0370b42..bae594b 100644
 --- a/drivers/extcon/Makefile
 +++ b/drivers/extcon/Makefile
 @@ -12,3 +12,4 @@ obj-$(CONFIG_EXTCON_MAX8997)+= extcon-max8997.o
  obj-$(CONFIG_EXTCON_PALMAS)  += extcon-palmas.o
  obj-$(CONFIG_EXTCON_RT8973A) += extcon-rt8973a.o
  obj-$(CONFIG_EXTCON_SM5502)  += extcon-sm5502.o
 +obj-$(CONFIG_EXTCON_GPIO_USB)+= extcon-gpio-usb.o
 diff --git a/drivers/extcon/extcon-gpio-usb.c 
 b/drivers/extcon/extcon-gpio-usb.c
 new file mode 100644
 index 000..aeb2298
 --- /dev/null
 +++ b/drivers/extcon/extcon-gpio-usb.c
 @@ -0,0 +1,225 @@
 +/**
 + * drivers/extcon/extcon_gpio_usb.c - USB GPIO extcon driver
 + *
 + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
 + *
 + * Author: Roger Quadros rog...@ti.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + */
 +
 +#include linux/extcon.h
 +#include linux/extcon/extcon-gpio.h
 +#include linux/gpio.h
 +#include linux/init.h
 +#include linux/interrupt.h
 +#include linux/irq.h
 +#include linux/kernel.h
 +#include linux/module.h
 +#include linux/of_gpio.h
 +#include linux/platform_device.h
 +#include linux/slab.h
 +#include linux/workqueue.h
 +
 +#define USB_GPIO_DEBOUNCE_MS 20  /* ms */
 +
 +struct usb_extcon_info {
 + struct device *dev;
 + struct extcon_dev *edev;
 +
 + struct gpio_desc *id_gpiod;
 + int id_irq;
 +
 + unsigned long debounce_jiffies;
 + struct delayed_work wq_detcable;
 +};
 +
 +/* List of detectable cables */
 +enum {
 + EXTCON_CABLE_USB = 0,
 + EXTCON_CABLE_USB_HOST,
 +
 + EXTCON_CABLE_END,
 +};
 +
 +static const char *usb_extcon_cable[] = {
 + [EXTCON_CABLE_USB] = USB,
 + [EXTCON_CABLE_USB_HOST] = USB-Host,
 + NULL,
 +};
 +
 +static void usb_extcon_detect_cable(struct work_struct *work)
 +{
 + int id;
 + struct usb_extcon_info *info;
 + const char **cable_names;
 +
 + info  = 

Re: [PATCH 1/5] extcon: gpio-usb: Introduce gpio usb extcon driver

2015-01-20 Thread Chanwoo Choi
Hi Roger,

On 01/20/2015 02:52 AM, Roger Quadros wrote:
 This driver observes the USB ID pin connected over a GPIO and
 updates the USB cable extcon states accordingly.
 
 The existing GPIO extcon driver is not suitable for this purpose
 as it needs to be taught to understand USB cable states and it
 can't handle more than one cable per instance.
 
 For the USB case we need to handle 2 cable states.
 1) USB (attach/detach)
 2) USB-Host (attach/detach)
 
 This driver can be easily updated in the future to handle VBUS
 events in case it happens to be available on GPIO for any platform.
 
 Signed-off-by: Roger Quadros rog...@ti.com
 ---
  .../devicetree/bindings/extcon/extcon-usb.txt  |  20 ++
  drivers/extcon/Kconfig |   7 +
  drivers/extcon/Makefile|   1 +
  drivers/extcon/extcon-gpio-usb.c   | 225 
 +
  4 files changed, 253 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/extcon/extcon-usb.txt
  create mode 100644 drivers/extcon/extcon-gpio-usb.c
 
 diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb.txt 
 b/Documentation/devicetree/bindings/extcon/extcon-usb.txt
 new file mode 100644
 index 000..171c5a4
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/extcon/extcon-usb.txt

Need to rename from extcon-usb.txt to extcon-gpio-usb.txt.

 @@ -0,0 +1,20 @@
 +USB Extcon device
 +
 +This is a virtual device used to generate USB cable states from the USB ID 
 pin
 +connected to a GPIO pin.
 +
 +Required properties:
 +- compatible: Should be linux,extcon-usb

I think you better use linux,extcon-gpio-usb
because the point of this driver use the gpio for usb cable.

 +- id-gpio: gpio for USB ID pin. See gpio binding.
 +
 +Example:
 + extcon_usb1 {
 + compatible = linux,extcon-usb;

ditto.

 + id-gpio = gpio6 1 GPIO_ACTIVE_HIGH;
 + }
 +
 + usb@1 {
 + ...
 + extcon = extcon_usb1;
 + ...
 + };
 diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
 index 6a1f7de..8106a83 100644
 --- a/drivers/extcon/Kconfig
 +++ b/drivers/extcon/Kconfig
 @@ -35,6 +35,13 @@ config EXTCON_GPIO
 Say Y here to enable GPIO based extcon support. Note that GPIO
 extcon supports single state per extcon instance.
  
 +config EXTCON_GPIO_USB
 + tristate USB GPIO extcon support
 + depends on GPIOLIB
 + help
 +   Say Y here to enable GPIO based USB cable detection extcon support.
 +   Used typically if GPIO is used for USB ID pin detection.
 +
  config EXTCON_MAX14577
   tristate MAX14577/77836 EXTCON Support
   depends on MFD_MAX14577
 diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
 index 0370b42..bae594b 100644
 --- a/drivers/extcon/Makefile
 +++ b/drivers/extcon/Makefile
 @@ -12,3 +12,4 @@ obj-$(CONFIG_EXTCON_MAX8997)+= extcon-max8997.o
  obj-$(CONFIG_EXTCON_PALMAS)  += extcon-palmas.o
  obj-$(CONFIG_EXTCON_RT8973A) += extcon-rt8973a.o
  obj-$(CONFIG_EXTCON_SM5502)  += extcon-sm5502.o
 +obj-$(CONFIG_EXTCON_GPIO_USB)+= extcon-gpio-usb.o

Need to re-order it alphabetically.

 diff --git a/drivers/extcon/extcon-gpio-usb.c 
 b/drivers/extcon/extcon-gpio-usb.c
 new file mode 100644
 index 000..aeb2298
 --- /dev/null
 +++ b/drivers/extcon/extcon-gpio-usb.c
 @@ -0,0 +1,225 @@
 +/**
 + * drivers/extcon/extcon_gpio_usb.c - USB GPIO extcon driver
 + *
 + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
 + *

Remove un-necessary blank line.

 + * Author: Roger Quadros rog...@ti.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + */
 +
 +#include linux/extcon.h
 +#include linux/extcon/extcon-gpio.h

Is it necessary? I think it is your mistake?

 +#include linux/gpio.h

Don't need it because 'of_gpio.h' includes already 'gpio.h'.

 +#include linux/init.h
 +#include linux/interrupt.h
 +#include linux/irq.h
 +#include linux/kernel.h
 +#include linux/module.h
 +#include linux/of_gpio.h
 +#include linux/platform_device.h
 +#include linux/slab.h
 +#include linux/workqueue.h
 +
 +#define USB_GPIO_DEBOUNCE_MS 20  /* ms */
 +
 +struct usb_extcon_info {
 + struct device *dev;
 + struct extcon_dev *edev;
 +
 + struct gpio_desc *id_gpiod;
 + int id_irq;
 +
 + unsigned long debounce_jiffies;
 + struct delayed_work wq_detcable;
 +};
 +
 +/* List of detectable cables */
 +enum {
 + EXTCON_CABLE_USB = 0,
 + EXTCON_CABLE_USB_HOST,
 +
 + EXTCON_CABLE_END,
 +};
 +
 +static const char *usb_extcon_cable[] = {
 +   

[PATCH 1/5] extcon: gpio-usb: Introduce gpio usb extcon driver

2015-01-19 Thread Roger Quadros
This driver observes the USB ID pin connected over a GPIO and
updates the USB cable extcon states accordingly.

The existing GPIO extcon driver is not suitable for this purpose
as it needs to be taught to understand USB cable states and it
can't handle more than one cable per instance.

For the USB case we need to handle 2 cable states.
1) USB (attach/detach)
2) USB-Host (attach/detach)

This driver can be easily updated in the future to handle VBUS
events in case it happens to be available on GPIO for any platform.

Signed-off-by: Roger Quadros rog...@ti.com
---
 .../devicetree/bindings/extcon/extcon-usb.txt  |  20 ++
 drivers/extcon/Kconfig |   7 +
 drivers/extcon/Makefile|   1 +
 drivers/extcon/extcon-gpio-usb.c   | 225 +
 4 files changed, 253 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/extcon/extcon-usb.txt
 create mode 100644 drivers/extcon/extcon-gpio-usb.c

diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb.txt 
b/Documentation/devicetree/bindings/extcon/extcon-usb.txt
new file mode 100644
index 000..171c5a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/extcon/extcon-usb.txt
@@ -0,0 +1,20 @@
+USB Extcon device
+
+This is a virtual device used to generate USB cable states from the USB ID pin
+connected to a GPIO pin.
+
+Required properties:
+- compatible: Should be linux,extcon-usb
+- id-gpio: gpio for USB ID pin. See gpio binding.
+
+Example:
+   extcon_usb1 {
+   compatible = linux,extcon-usb;
+   id-gpio = gpio6 1 GPIO_ACTIVE_HIGH;
+   }
+
+   usb@1 {
+   ...
+   extcon = extcon_usb1;
+   ...
+   };
diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index 6a1f7de..8106a83 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -35,6 +35,13 @@ config EXTCON_GPIO
  Say Y here to enable GPIO based extcon support. Note that GPIO
  extcon supports single state per extcon instance.
 
+config EXTCON_GPIO_USB
+   tristate USB GPIO extcon support
+   depends on GPIOLIB
+   help
+ Say Y here to enable GPIO based USB cable detection extcon support.
+ Used typically if GPIO is used for USB ID pin detection.
+
 config EXTCON_MAX14577
tristate MAX14577/77836 EXTCON Support
depends on MFD_MAX14577
diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
index 0370b42..bae594b 100644
--- a/drivers/extcon/Makefile
+++ b/drivers/extcon/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_EXTCON_MAX8997)  += extcon-max8997.o
 obj-$(CONFIG_EXTCON_PALMAS)+= extcon-palmas.o
 obj-$(CONFIG_EXTCON_RT8973A)   += extcon-rt8973a.o
 obj-$(CONFIG_EXTCON_SM5502)+= extcon-sm5502.o
+obj-$(CONFIG_EXTCON_GPIO_USB)  += extcon-gpio-usb.o
diff --git a/drivers/extcon/extcon-gpio-usb.c b/drivers/extcon/extcon-gpio-usb.c
new file mode 100644
index 000..aeb2298
--- /dev/null
+++ b/drivers/extcon/extcon-gpio-usb.c
@@ -0,0 +1,225 @@
+/**
+ * drivers/extcon/extcon_gpio_usb.c - USB GPIO extcon driver
+ *
+ * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
+ *
+ * Author: Roger Quadros rog...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/extcon.h
+#include linux/extcon/extcon-gpio.h
+#include linux/gpio.h
+#include linux/init.h
+#include linux/interrupt.h
+#include linux/irq.h
+#include linux/kernel.h
+#include linux/module.h
+#include linux/of_gpio.h
+#include linux/platform_device.h
+#include linux/slab.h
+#include linux/workqueue.h
+
+#define USB_GPIO_DEBOUNCE_MS   20  /* ms */
+
+struct usb_extcon_info {
+   struct device *dev;
+   struct extcon_dev *edev;
+
+   struct gpio_desc *id_gpiod;
+   int id_irq;
+
+   unsigned long debounce_jiffies;
+   struct delayed_work wq_detcable;
+};
+
+/* List of detectable cables */
+enum {
+   EXTCON_CABLE_USB = 0,
+   EXTCON_CABLE_USB_HOST,
+
+   EXTCON_CABLE_END,
+};
+
+static const char *usb_extcon_cable[] = {
+   [EXTCON_CABLE_USB] = USB,
+   [EXTCON_CABLE_USB_HOST] = USB-Host,
+   NULL,
+};
+
+static void usb_extcon_detect_cable(struct work_struct *work)
+{
+   int id;
+   struct usb_extcon_info *info;
+   const char **cable_names;
+
+   info  = container_of(to_delayed_work(work), struct usb_extcon_info,
+wq_detcable);
+   cable_names = info-edev-supported_cable;
+
+   /* check ID and update cable state */
+   id =