2013-06-03 16:14:32

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: [PATCH 0/5] dwc3: omap: adapt dwc3 to use extcon framework

The first three patches deals with cleanup of extcon inorder to get
through compilation without any issues. It also adds an API to get
extcon device from dt node which I felt was missing.

The next two patches deals with adapt dwc3 to use extcon framework.
The 4th patch (usb: dwc3: omap: improve error handling of dwc3_omap_probe)
should have ideally been sent to Felipe, however since
there will be merge conflicts with the 5th patch
(usb: dwc3: use extcon fwrk to receive connect/disconnect)
in this series, I've sent in the same series.

Once this patch series get merged, dwc3 in omap would be functional
in device mode. However there is still some discussion on modelling
SMPS10 regulator. Once that gets finalized, dwc3 should be functional
in host mode as well.

Kishon Vijay Abraham I (5):
extcon: Add an API to get extcon device from dt node
extcon: Kconfig: Make extcon config type as bool
extcon: add EXPORT_SYMBOL_GPL for exported functions
usb: dwc3: omap: improve error handling of dwc3_omap_probe
usb: dwc3: use extcon fwrk to receive connect/disconnect

Documentation/devicetree/bindings/usb/omap-usb.txt | 5 +
drivers/extcon/Kconfig | 2 +-
drivers/extcon/extcon-class.c | 42 +++++++
drivers/usb/dwc3/dwc3-omap.c | 133 ++++++++++++++++----
include/linux/extcon.h | 8 ++
include/linux/usb/dwc3-omap.h | 30 -----
6 files changed, 168 insertions(+), 52 deletions(-)
delete mode 100644 include/linux/usb/dwc3-omap.h

--
1.7.10.4


2013-06-03 16:14:36

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: [v3 PATCH 5/5] usb: dwc3: use extcon fwrk to receive connect/disconnect

Modified dwc3-omap to receive connect and disconnect notification using
extcon framework. Also did the necessary cleanups required after
adapting to extcon framework.

Signed-off-by: Kishon Vijay Abraham I <[email protected]>
Acked-by: Felipe Balbi <[email protected]>
---
Changes from v2:
* updated the Documentation with dwc3 dt binding information.
* used of_extcon_get_extcon_dev to get extcon device from device tree data.
Changes from v1:
* regulator enable/disable is now done here instead of palmas-usb as some users
of palmas-usb wont need regulator.
Documentation/devicetree/bindings/usb/omap-usb.txt | 5 +
drivers/usb/dwc3/dwc3-omap.c | 118 ++++++++++++++++----
include/linux/usb/dwc3-omap.h | 30 -----
3 files changed, 104 insertions(+), 49 deletions(-)
delete mode 100644 include/linux/usb/dwc3-omap.h

diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt
index d4769f3..f1c15f3 100644
--- a/Documentation/devicetree/bindings/usb/omap-usb.txt
+++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
@@ -53,6 +53,11 @@ OMAP DWC3 GLUE
It should be set to "1" for HW mode and "2" for SW mode.
- ranges: the child address space are mapped 1:1 onto the parent address space

+Optional Properties:
+ - extcon : phandle for the extcon device omap dwc3 uses to detect
+ connect/disconnect events.
+ - vbus-supply : phandle to the regulator device tree node if needed.
+
Sub-nodes:
The dwc3 core should be added as subnode to omap dwc3 glue.
- dwc3 :
diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index f8f76e6..e1aac90 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -43,13 +43,14 @@
#include <linux/spinlock.h>
#include <linux/platform_device.h>
#include <linux/platform_data/dwc3-omap.h>
-#include <linux/usb/dwc3-omap.h>
#include <linux/pm_runtime.h>
#include <linux/dma-mapping.h>
#include <linux/ioport.h>
#include <linux/io.h>
#include <linux/of.h>
#include <linux/of_platform.h>
+#include <linux/extcon.h>
+#include <linux/regulator/consumer.h>

#include <linux/usb/otg.h>

@@ -124,9 +125,21 @@ struct dwc3_omap {
u32 utmi_otg_status;

u32 dma_status:1;
+
+ struct extcon_specific_cable_nb extcon_vbus_dev;
+ struct extcon_specific_cable_nb extcon_id_dev;
+ struct notifier_block vbus_nb;
+ struct notifier_block id_nb;
+
+ struct regulator *vbus_reg;
};

-static struct dwc3_omap *_omap;
+enum omap_dwc3_vbus_id_status {
+ OMAP_DWC3_ID_FLOAT,
+ OMAP_DWC3_ID_GROUND,
+ OMAP_DWC3_VBUS_OFF,
+ OMAP_DWC3_VBUS_VALID,
+};

static inline u32 dwc3_omap_readl(void __iomem *base, u32 offset)
{
@@ -138,18 +151,23 @@ static inline void dwc3_omap_writel(void __iomem *base, u32 offset, u32 value)
writel(value, base + offset);
}

-int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
+static void dwc3_omap_set_mailbox(struct dwc3_omap *omap,
+ enum omap_dwc3_vbus_id_status status)
{
- u32 val;
- struct dwc3_omap *omap = _omap;
-
- if (!omap)
- return -EPROBE_DEFER;
+ int ret;
+ u32 val;

switch (status) {
case OMAP_DWC3_ID_GROUND:
dev_dbg(omap->dev, "ID GND\n");

+ if (omap->vbus_reg) {
+ ret = regulator_enable(omap->vbus_reg);
+ if (ret) {
+ dev_dbg(omap->dev, "regulator enable failed\n");
+ return;
+ }
+ }
val = dwc3_omap_readl(omap->base, USBOTGSS_UTMI_OTG_STATUS);
val &= ~(USBOTGSS_UTMI_OTG_STATUS_IDDIG
| USBOTGSS_UTMI_OTG_STATUS_VBUSVALID
@@ -172,6 +190,9 @@ int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
break;

case OMAP_DWC3_ID_FLOAT:
+ if (omap->vbus_reg)
+ regulator_disable(omap->vbus_reg);
+
case OMAP_DWC3_VBUS_OFF:
dev_dbg(omap->dev, "VBUS Disconnect\n");

@@ -185,12 +206,9 @@ int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
break;

default:
- dev_dbg(omap->dev, "ID float\n");
+ dev_dbg(omap->dev, "invalid state\n");
}
-
- return 0;
}
-EXPORT_SYMBOL_GPL(dwc3_omap_mailbox);

static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap)
{
@@ -282,6 +300,32 @@ static void dwc3_omap_disable_irqs(struct dwc3_omap *omap)

static u64 dwc3_omap_dma_mask = DMA_BIT_MASK(32);

+static int dwc3_omap_id_notifier(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct dwc3_omap *omap = container_of(nb, struct dwc3_omap, id_nb);
+
+ if (event)
+ dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
+ else
+ dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_FLOAT);
+
+ return NOTIFY_DONE;
+}
+
+static int dwc3_omap_vbus_notifier(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct dwc3_omap *omap = container_of(nb, struct dwc3_omap, vbus_nb);
+
+ if (event)
+ dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
+ else
+ dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_OFF);
+
+ return NOTIFY_DONE;
+}
+
static int dwc3_omap_probe(struct platform_device *pdev)
{
struct device_node *node = pdev->dev.of_node;
@@ -289,6 +333,8 @@ static int dwc3_omap_probe(struct platform_device *pdev)
struct dwc3_omap *omap;
struct resource *res;
struct device *dev = &pdev->dev;
+ struct extcon_dev *edev;
+ struct regulator *vbus_reg = NULL;

int ret = -ENOMEM;
int irq;
@@ -330,19 +376,22 @@ static int dwc3_omap_probe(struct platform_device *pdev)
return -ENOMEM;
}

+ if (of_property_read_bool(node, "vbus-supply")) {
+ vbus_reg = devm_regulator_get(dev, "vbus");
+ if (IS_ERR(vbus_reg)) {
+ dev_err(dev, "vbus init failed\n");
+ return PTR_ERR(vbus_reg);
+ }
+ }
+
spin_lock_init(&omap->lock);

omap->dev = dev;
omap->irq = irq;
omap->base = base;
+ omap->vbus_reg = vbus_reg;
dev->dma_mask = &dwc3_omap_dma_mask;

- /*
- * REVISIT if we ever have two instances of the wrapper, we will be
- * in big trouble
- */
- _omap = omap;
-
pm_runtime_enable(dev);
ret = pm_runtime_get_sync(dev);
if (ret < 0) {
@@ -381,14 +430,41 @@ static int dwc3_omap_probe(struct platform_device *pdev)

dwc3_omap_enable_irqs(omap);

+ if (of_property_read_bool(node, "extcon")) {
+ edev = of_extcon_get_extcon_dev(dev, 0);
+ if (IS_ERR(edev)) {
+ dev_vdbg(dev, "couldn't get extcon device\n");
+ ret = PTR_ERR(edev);
+ goto err2;
+ }
+
+ omap->vbus_nb.notifier_call = dwc3_omap_vbus_notifier;
+ extcon_register_interest(&omap->extcon_vbus_dev, edev->name,
+ "USB", &omap->vbus_nb);
+ omap->id_nb.notifier_call = dwc3_omap_id_notifier;
+ extcon_register_interest(&omap->extcon_id_dev, edev->name,
+ "USB-HOST", &omap->id_nb);
+
+ if (extcon_get_cable_state(edev, "USB") == true)
+ dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
+ if (extcon_get_cable_state(edev, "USB-HOST") == true)
+ dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
+ }
+
ret = of_platform_populate(node, NULL, NULL, dev);
if (ret) {
dev_err(&pdev->dev, "failed to create dwc3 core\n");
- goto err2;
+ goto err3;
}

return 0;

+err3:
+ if (omap->extcon_vbus_dev.edev)
+ extcon_unregister_interest(&omap->extcon_vbus_dev);
+ if (omap->extcon_id_dev.edev)
+ extcon_unregister_interest(&omap->extcon_id_dev);
+
err2:
dwc3_omap_disable_irqs(omap);

@@ -405,6 +481,10 @@ static int dwc3_omap_remove(struct platform_device *pdev)
{
struct dwc3_omap *omap = platform_get_drvdata(pdev);

+ if (omap->extcon_vbus_dev.edev)
+ extcon_unregister_interest(&omap->extcon_vbus_dev);
+ if (omap->extcon_id_dev.edev)
+ extcon_unregister_interest(&omap->extcon_id_dev);
dwc3_omap_disable_irqs(omap);
pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
diff --git a/include/linux/usb/dwc3-omap.h b/include/linux/usb/dwc3-omap.h
deleted file mode 100644
index 5615f4d..0000000
--- a/include/linux/usb/dwc3-omap.h
+++ /dev/null
@@ -1,30 +0,0 @@
-/*
- * Copyright (C) 2013 by Texas Instruments
- *
- * The Inventra Controller Driver for Linux 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.
- */
-
-#ifndef __DWC3_OMAP_H__
-#define __DWC3_OMAP_H__
-
-enum omap_dwc3_vbus_id_status {
- OMAP_DWC3_UNKNOWN = 0,
- OMAP_DWC3_ID_GROUND,
- OMAP_DWC3_ID_FLOAT,
- OMAP_DWC3_VBUS_VALID,
- OMAP_DWC3_VBUS_OFF,
-};
-
-#if (defined(CONFIG_USB_DWC3) || defined(CONFIG_USB_DWC3_MODULE))
-extern int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status);
-#else
-static inline int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
-{
- return -ENODEV;
-}
-#endif
-
-#endif /* __DWC3_OMAP_H__ */
--
1.7.10.4

2013-06-03 16:14:29

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: [PATCH 4/5] usb: dwc3: omap: improve error handling of dwc3_omap_probe

Improved the error handling of dwc3_omap_probe so that on error
conditions dwc3_omap is left in the original state.

Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/usb/dwc3/dwc3-omap.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 34638b9..f8f76e6 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -347,7 +347,7 @@ static int dwc3_omap_probe(struct platform_device *pdev)
ret = pm_runtime_get_sync(dev);
if (ret < 0) {
dev_err(dev, "get_sync failed with err %d\n", ret);
- return ret;
+ goto err0;
}

reg = dwc3_omap_readl(omap->base, USBOTGSS_UTMI_OTG_STATUS);
@@ -376,7 +376,7 @@ static int dwc3_omap_probe(struct platform_device *pdev)
if (ret) {
dev_err(dev, "failed to request IRQ #%d --> %d\n",
omap->irq, ret);
- return ret;
+ goto err1;
}

dwc3_omap_enable_irqs(omap);
@@ -384,10 +384,21 @@ static int dwc3_omap_probe(struct platform_device *pdev)
ret = of_platform_populate(node, NULL, NULL, dev);
if (ret) {
dev_err(&pdev->dev, "failed to create dwc3 core\n");
- return ret;
+ goto err2;
}

return 0;
+
+err2:
+ dwc3_omap_disable_irqs(omap);
+
+err1:
+ pm_runtime_put_sync(dev);
+
+err0:
+ pm_runtime_disable(dev);
+
+ return ret;
}

static int dwc3_omap_remove(struct platform_device *pdev)
--
1.7.10.4

2013-06-03 16:14:25

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: [PATCH 3/5] extcon: add EXPORT_SYMBOL_GPL for exported functions

Added EXPORT_SYMBOL_GPL() for extcon_register_interest and
extcon_register_notifier in order to avoid undefined reference
error when building the consumer modules of extcon as _modules_.

Signed-off-by: Kishon Vijay Abraham I <[email protected]>
Signed-off-by: Lokesh Vutla <[email protected]>
---
drivers/extcon/extcon-class.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index 265d549..51a5f5f 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -541,6 +541,7 @@ int extcon_register_interest(struct extcon_specific_cable_nb *obj,
return -ENODEV;
}
}
+EXPORT_SYMBOL_GPL(extcon_register_interest);

/**
* extcon_unregister_interest() - Unregister the notifier registered by
@@ -555,6 +556,7 @@ int extcon_unregister_interest(struct extcon_specific_cable_nb *obj)

return raw_notifier_chain_unregister(&obj->edev->nh, &obj->internal_nb);
}
+EXPORT_SYMBOL_GPL(extcon_unregister_interest);

/**
* extcon_register_notifier() - Register a notifiee to get notified by
--
1.7.10.4

2013-06-03 16:14:23

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: [PATCH 2/5] extcon: Kconfig: Make extcon config type as bool

Changed the extcon config type to bool from module. Having extcon
config type as module leads to some "undefined reference to" errors
if the modules that uses these APIs are made as built-in and extcon
as module.

Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/extcon/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index 63f454e..ee9b7d4 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -1,5 +1,5 @@
menuconfig EXTCON
- tristate "External Connector Class (extcon) support"
+ bool "External Connector Class (extcon) support"
help
Say Y here to enable external connector class (extcon) support.
This allows monitoring external connectors by userspace
--
1.7.10.4

2013-06-03 16:16:06

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: [PATCH 1/5] extcon: Add an API to get extcon device from dt node

Added an API of_extcon_get_extcon_dev() to be used by drivers to get
extcon device in the case of dt boot (this can be used instead of
extcon_get_extcon_dev()).

Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/extcon/extcon-class.c | 40 ++++++++++++++++++++++++++++++++++++++++
include/linux/extcon.h | 8 ++++++++
2 files changed, 48 insertions(+)

diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index 60adc04..265d549 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -31,6 +31,7 @@
#include <linux/extcon.h>
#include <linux/slab.h>
#include <linux/sysfs.h>
+#include <linux/of_platform.h>

/*
* extcon_cable_name suggests the standard cable names for commonly used
@@ -392,6 +393,45 @@ int extcon_set_cable_state(struct extcon_dev *edev,
}
EXPORT_SYMBOL_GPL(extcon_set_cable_state);

+struct extcon_dev *of_extcon_get_extcon_dev(struct device *dev, int index)
+{
+ struct class_dev_iter iter;
+ struct device *extcon_dev;
+ struct device_node *node;
+ struct platform_device *extcon_parent_dev;
+
+ if (!dev->of_node) {
+ dev_dbg(dev, "device does not have a device node entry\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ node = of_parse_phandle(dev->of_node, "extcon", index);
+ if (!node) {
+ dev_dbg(dev, "failed to get phandle in %s node\n",
+ dev->of_node->full_name);
+ return ERR_PTR(-ENODEV);
+ }
+
+ extcon_parent_dev = of_find_device_by_node(node);
+ if (!extcon_parent_dev) {
+ dev_dbg(dev, "unable to find device by node\n");
+ return ERR_PTR(-EPROBE_DEFER);
+ }
+
+ class_dev_iter_init(&iter, extcon_class, NULL, NULL);
+ while ((extcon_dev = class_dev_iter_next(&iter))) {
+ if (extcon_dev->parent != &extcon_parent_dev->dev)
+ continue;
+
+ class_dev_iter_exit(&iter);
+ return dev_get_drvdata(extcon_dev);
+ }
+
+ class_dev_iter_exit(&iter);
+ return ERR_PTR(-ENODEV);
+}
+EXPORT_SYMBOL_GPL(of_extcon_get_extcon_dev);
+
/**
* extcon_get_extcon_dev() - Get the extcon device instance from the name
* @extcon_name: The extcon name provided with extcon_dev_register()
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index fcb51c8..3858bb9 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -182,6 +182,8 @@ struct extcon_specific_cable_nb {
*/
extern int extcon_dev_register(struct extcon_dev *edev, struct device *dev);
extern void extcon_dev_unregister(struct extcon_dev *edev);
+extern struct extcon_dev *of_extcon_get_extcon_dev(struct device *dev,
+ int index);
extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);

/*
@@ -292,6 +294,12 @@ static inline int extcon_set_cable_state(struct extcon_dev *edev,
return 0;
}

+static inline struct extcon_dev *of_extcon_get_extcon_dev(struct device *dev,
+ int index)
+{
+ return NULL;
+}
+
static inline struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
{
return NULL;
--
1.7.10.4

2013-06-04 14:23:52

by Ruchika Kharwar

[permalink] [raw]
Subject: Re: [PATCH 0/5] dwc3: omap: adapt dwc3 to use extcon framework

Kishon,
What is the expectation when there is no palmas tied to dwc3/dwc3-omap ?
Thank you
Regards
Ruchika

On 06/03/2013 11:13 AM, Kishon Vijay Abraham I wrote:
> The first three patches deals with cleanup of extcon inorder to get
> through compilation without any issues. It also adds an API to get
> extcon device from dt node which I felt was missing.
>
> The next two patches deals with adapt dwc3 to use extcon framework.
> The 4th patch (usb: dwc3: omap: improve error handling of dwc3_omap_probe)
> should have ideally been sent to Felipe, however since
> there will be merge conflicts with the 5th patch
> (usb: dwc3: use extcon fwrk to receive connect/disconnect)
> in this series, I've sent in the same series.
>
> Once this patch series get merged, dwc3 in omap would be functional
> in device mode. However there is still some discussion on modelling
> SMPS10 regulator. Once that gets finalized, dwc3 should be functional
> in host mode as well.
>
> Kishon Vijay Abraham I (5):
> extcon: Add an API to get extcon device from dt node
> extcon: Kconfig: Make extcon config type as bool
> extcon: add EXPORT_SYMBOL_GPL for exported functions
> usb: dwc3: omap: improve error handling of dwc3_omap_probe
> usb: dwc3: use extcon fwrk to receive connect/disconnect
>
> Documentation/devicetree/bindings/usb/omap-usb.txt | 5 +
> drivers/extcon/Kconfig | 2 +-
> drivers/extcon/extcon-class.c | 42 +++++++
> drivers/usb/dwc3/dwc3-omap.c | 133 ++++++++++++++++----
> include/linux/extcon.h | 8 ++
> include/linux/usb/dwc3-omap.h | 30 -----
> 6 files changed, 168 insertions(+), 52 deletions(-)
> delete mode 100644 include/linux/usb/dwc3-omap.h
>

2013-06-05 04:50:01

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH 0/5] dwc3: omap: adapt dwc3 to use extcon framework

Hi Ruchika,

On Tuesday 04 June 2013 07:53 PM, Ruchika Kharwar wrote:
> Kishon,
> What is the expectation when there is no palmas tied to dwc3/dwc3-omap ?

In the probe of dwc3-omap I have this check
"if (of_property_read_bool(node, "extcon"))"
So If dwc3 node does not have extcon property, it wont try to get extcon
device at all.

You can refer this patch to see how its handled
usb: dwc3: use extcon fwrk to receive connect/disconnect.

Thanks
Kishon

2013-06-12 00:55:03

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH 0/5] dwc3: omap: adapt dwc3 to use extcon framework

Hi Kishon,

Sorry for late reply.

I applied patch1,2 on extcon-linus branch.
- http://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/log/?h=extcon-linus

But, I have comment of patch 3 about dt API. I send comment on patch 3 mailing thread.

Thanks,
Chanwoo Choi

On 06/04/2013 01:13 AM, Kishon Vijay Abraham I wrote:
> The first three patches deals with cleanup of extcon inorder to get
> through compilation without any issues. It also adds an API to get
> extcon device from dt node which I felt was missing.
>
> The next two patches deals with adapt dwc3 to use extcon framework.
> The 4th patch (usb: dwc3: omap: improve error handling of dwc3_omap_probe)
> should have ideally been sent to Felipe, however since
> there will be merge conflicts with the 5th patch
> (usb: dwc3: use extcon fwrk to receive connect/disconnect)
> in this series, I've sent in the same series.
>
> Once this patch series get merged, dwc3 in omap would be functional
> in device mode. However there is still some discussion on modelling
> SMPS10 regulator. Once that gets finalized, dwc3 should be functional
> in host mode as well.
>
> Kishon Vijay Abraham I (5):
> extcon: Add an API to get extcon device from dt node
> extcon: Kconfig: Make extcon config type as bool
> extcon: add EXPORT_SYMBOL_GPL for exported functions
> usb: dwc3: omap: improve error handling of dwc3_omap_probe
> usb: dwc3: use extcon fwrk to receive connect/disconnect
>
> Documentation/devicetree/bindings/usb/omap-usb.txt | 5 +
> drivers/extcon/Kconfig | 2 +-
> drivers/extcon/extcon-class.c | 42 +++++++
> drivers/usb/dwc3/dwc3-omap.c | 133 ++++++++++++++++----
> include/linux/extcon.h | 8 ++
> include/linux/usb/dwc3-omap.h | 30 -----
> 6 files changed, 168 insertions(+), 52 deletions(-)
> delete mode 100644 include/linux/usb/dwc3-omap.h
>

2013-06-12 00:59:18

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH 0/5] dwc3: omap: adapt dwc3 to use extcon framework

Hi Kishon,

I confused patch number. I applied patch2,3 on extcon-linus branch.

extcon: Kconfig: Make extcon config type as bool
extcon: add EXPORT_SYMBOL_GPL for exported functions


And I will reply comment about patch1 soon.

extcon: Add an API to get extcon device from dt node


Thanks,
Chanwoo Choi

On 06/04/2013 01:13 AM, Kishon Vijay Abraham I wrote:
> The first three patches deals with cleanup of extcon inorder to get
> through compilation without any issues. It also adds an API to get
> extcon device from dt node which I felt was missing.
>
> The next two patches deals with adapt dwc3 to use extcon framework.
> The 4th patch (usb: dwc3: omap: improve error handling of dwc3_omap_probe)
> should have ideally been sent to Felipe, however since
> there will be merge conflicts with the 5th patch
> (usb: dwc3: use extcon fwrk to receive connect/disconnect)
> in this series, I've sent in the same series.
>
> Once this patch series get merged, dwc3 in omap would be functional
> in device mode. However there is still some discussion on modelling
> SMPS10 regulator. Once that gets finalized, dwc3 should be functional
> in host mode as well.
>
> Kishon Vijay Abraham I (5):
> extcon: Add an API to get extcon device from dt node
> extcon: Kconfig: Make extcon config type as bool
> extcon: add EXPORT_SYMBOL_GPL for exported functions
> usb: dwc3: omap: improve error handling of dwc3_omap_probe
> usb: dwc3: use extcon fwrk to receive connect/disconnect
>
> Documentation/devicetree/bindings/usb/omap-usb.txt | 5 +
> drivers/extcon/Kconfig | 2 +-
> drivers/extcon/extcon-class.c | 42 +++++++
> drivers/usb/dwc3/dwc3-omap.c | 133 ++++++++++++++++----
> include/linux/extcon.h | 8 ++
> include/linux/usb/dwc3-omap.h | 30 -----
> 6 files changed, 168 insertions(+), 52 deletions(-)
> delete mode 100644 include/linux/usb/dwc3-omap.h
>

2013-06-12 01:28:06

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH 1/5] extcon: Add an API to get extcon device from dt node

Hi Kishon,

On 06/04/2013 01:13 AM, Kishon Vijay Abraham I wrote:
> Added an API of_extcon_get_extcon_dev() to be used by drivers to get
> extcon device in the case of dt boot (this can be used instead of
> extcon_get_extcon_dev()).
>
> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> ---
> drivers/extcon/extcon-class.c | 40 ++++++++++++++++++++++++++++++++++++++++
> include/linux/extcon.h | 8 ++++++++
> 2 files changed, 48 insertions(+)

I don't prefer that add of helper API in extcon core. I want to add
new of helper file as drivers/extcon/of-extcon.c.
So, I add drivers/extcon/of-extcon.c and include/linux/extcon/of-extcon.h
based on your this patch. I will send modified patch at once.

> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
> index 60adc04..265d549 100644
> --- a/drivers/extcon/extcon-class.c
> +++ b/drivers/extcon/extcon-class.c
> @@ -31,6 +31,7 @@
> #include <linux/extcon.h>
> #include <linux/slab.h>
> #include <linux/sysfs.h>
> +#include <linux/of_platform.h>
>
> /*
> * extcon_cable_name suggests the standard cable names for commonly used
> @@ -392,6 +393,45 @@ int extcon_set_cable_state(struct extcon_dev *edev,
> }
> EXPORT_SYMBOL_GPL(extcon_set_cable_state);
>
> +struct extcon_dev *of_extcon_get_extcon_dev(struct device *dev, int index)
> +{
> + struct class_dev_iter iter;
> + struct device *extcon_dev;
> + struct device_node *node;
> + struct platform_device *extcon_parent_dev;
> +
> + if (!dev->of_node) {
> + dev_dbg(dev, "device does not have a device node entry\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + node = of_parse_phandle(dev->of_node, "extcon", index);
> + if (!node) {
> + dev_dbg(dev, "failed to get phandle in %s node\n",
> + dev->of_node->full_name);
> + return ERR_PTR(-ENODEV);
> + }
> +
> + extcon_parent_dev = of_find_device_by_node(node);
> + if (!extcon_parent_dev) {
> + dev_dbg(dev, "unable to find device by node\n");
> + return ERR_PTR(-EPROBE_DEFER);
> + }
> +
> + class_dev_iter_init(&iter, extcon_class, NULL, NULL);
> + while ((extcon_dev = class_dev_iter_next(&iter))) {
> + if (extcon_dev->parent != &extcon_parent_dev->dev)
> + continue;
> +
> + class_dev_iter_exit(&iter);
> + return dev_get_drvdata(extcon_dev);
> + }
> +
> + class_dev_iter_exit(&iter);

Use extcon_get_extcon_dev() instead of using class_dev_iter_init/exit()

> + return ERR_PTR(-ENODEV);
> +}
> +EXPORT_SYMBOL_GPL(of_extcon_get_extcon_dev);
> +
> /**
> * extcon_get_extcon_dev() - Get the extcon device instance from the name
> * @extcon_name: The extcon name provided with extcon_dev_register()
> diff --git a/include/linux/extcon.h b/include/linux/extcon.h
> index fcb51c8..3858bb9 100644
> --- a/include/linux/extcon.h
> +++ b/include/linux/extcon.h
> @@ -182,6 +182,8 @@ struct extcon_specific_cable_nb {
> */
> extern int extcon_dev_register(struct extcon_dev *edev, struct device *dev);
> extern void extcon_dev_unregister(struct extcon_dev *edev);
> +extern struct extcon_dev *of_extcon_get_extcon_dev(struct device *dev,
> + int index);
> extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);
>
> /*
> @@ -292,6 +294,12 @@ static inline int extcon_set_cable_state(struct extcon_dev *edev,
> return 0;
> }
>
> +static inline struct extcon_dev *of_extcon_get_extcon_dev(struct device *dev,
> + int index)
> +{
> + return NULL;
> +}
> +
> static inline struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
> {
> return NULL;

Thanks,
Chanwoo Choi





2013-06-12 01:40:14

by Chanwoo Choi

[permalink] [raw]
Subject: [PATCH] extcon: Add an API to get extcon device from dt node

From: Kishon Vijay Abraham I <[email protected]>

Added an API of_extcon_get_extcon_dev() to be used by drivers to get
extcon device in the case of dt boot (this can be used instead of
extcon_get_extcon_dev()).

Signed-off-by: Kishon Vijay Abraham I <[email protected]>
Signed-off-by: Chanwoo Choi <[email protected]>
Signed-off-by: Myungjoo Ham <[email protected]>
---
drivers/extcon/Makefile | 3 +++
drivers/extcon/of-extcon.c | 56 ++++++++++++++++++++++++++++++++++++++++
include/linux/extcon/of-extcon.h | 30 +++++++++++++++++++++
3 files changed, 89 insertions(+)
create mode 100644 drivers/extcon/of-extcon.c
create mode 100644 include/linux/extcon/of-extcon.h

diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
index 540e2c3..39cdf95 100644
--- a/drivers/extcon/Makefile
+++ b/drivers/extcon/Makefile
@@ -2,9 +2,12 @@
# Makefile for external connector class (extcon) devices
#

+obj-$(CONFIG_OF) += of-extcon.o
+
obj-$(CONFIG_EXTCON) += extcon-class.o
obj-$(CONFIG_EXTCON_GPIO) += extcon-gpio.o
obj-$(CONFIG_EXTCON_ADC_JACK) += extcon-adc-jack.o
+
obj-$(CONFIG_EXTCON_MAX77693) += extcon-max77693.o
obj-$(CONFIG_EXTCON_MAX8997) += extcon-max8997.o
obj-$(CONFIG_EXTCON_ARIZONA) += extcon-arizona.o
diff --git a/drivers/extcon/of-extcon.c b/drivers/extcon/of-extcon.c
new file mode 100644
index 0000000..29f82b4
--- /dev/null
+++ b/drivers/extcon/of-extcon.c
@@ -0,0 +1,56 @@
+/*
+ * OF helpers for External connector (extcon) framework
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ * Kishon Vijay Abraham I <[email protected]>
+ *
+ * Copyright (C) 2013 Samsung Electronics
+ * Chanwoo Choi <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/extcon.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/extcon/of-extcon.h>
+
+struct extcon_dev *of_extcon_get_extcon_dev(struct device *dev, int index)
+{
+ struct device_node *node;
+ struct extcon_dev *edev;
+ struct platform_device *extcon_parent_dev;
+
+ if (!dev->of_node) {
+ dev_dbg(dev, "device does not have a device node entry\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ node = of_parse_phandle(dev->of_node, "extcon", index);
+ if (!node) {
+ dev_dbg(dev, "failed to get phandle in %s node\n",
+ dev->of_node->full_name);
+ return ERR_PTR(-ENODEV);
+ }
+
+ extcon_parent_dev = of_find_device_by_node(node);
+ if (!extcon_parent_dev) {
+ dev_dbg(dev, "unable to find device by node\n");
+ return ERR_PTR(-EPROBE_DEFER);
+ }
+
+ edev = extcon_get_extcon_dev(dev_name(&extcon_parent_dev->dev));
+ if (!edev) {
+ dev_dbg(dev, "unable to get extcon device : %s\n",
+ dev_name(&extcon_parent_dev->dev));
+ return ERR_PTR(-ENODEV);
+ }
+
+ return edev;
+}
+EXPORT_SYMBOL_GPL(of_extcon_get_extcon_dev);
diff --git a/include/linux/extcon/of-extcon.h b/include/linux/extcon/of-extcon.h
new file mode 100644
index 0000000..77d01ba
--- /dev/null
+++ b/include/linux/extcon/of-extcon.h
@@ -0,0 +1,30 @@
+/*
+ * OF helpers for External connector (extcon) framework
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ * Kishon Vijay Abraham I <[email protected]>
+ *
+ * Copyright (C) 2013 Samsung Electronics
+ * Chanwoo Choi <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __LINUX_OF_EXTCON_H
+#define __LINUX_OF_EXTCON_H
+
+#if defined(CONFIG_OF)
+extern struct extcon_dev
+ *of_extcon_get_extcon_dev(struct device *dev, int index);
+#else
+static inline struct extcon_dev
+ *of_extcon_get_extcon_dev(struct device *dev, int index);
+{
+ return NULL;
+}
+#endif /* CONFIG_OF */
+
+#endif /* __LINUX_OF_EXTCON_H */
--
1.8.0

2013-06-12 05:16:55

by anish singh

[permalink] [raw]
Subject: Re: [PATCH] extcon: Add an API to get extcon device from dt node

On Wed, Jun 12, 2013 at 7:09 AM, Chanwoo Choi <[email protected]> wrote:
> From: Kishon Vijay Abraham I <[email protected]>
>
> Added an API of_extcon_get_extcon_dev() to be used by drivers to get
> extcon device in the case of dt boot (this can be used instead of
> extcon_get_extcon_dev()).
>
> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> Signed-off-by: Chanwoo Choi <[email protected]>
> Signed-off-by: Myungjoo Ham <[email protected]>
> ---
> drivers/extcon/Makefile | 3 +++
> drivers/extcon/of-extcon.c | 56 ++++++++++++++++++++++++++++++++++++++++
> include/linux/extcon/of-extcon.h | 30 +++++++++++++++++++++
> 3 files changed, 89 insertions(+)
> create mode 100644 drivers/extcon/of-extcon.c
> create mode 100644 include/linux/extcon/of-extcon.h
>
> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
> index 540e2c3..39cdf95 100644
> --- a/drivers/extcon/Makefile
> +++ b/drivers/extcon/Makefile
> @@ -2,9 +2,12 @@
> # Makefile for external connector class (extcon) devices
> #
>
> +obj-$(CONFIG_OF) += of-extcon.o
> +
> obj-$(CONFIG_EXTCON) += extcon-class.o
> obj-$(CONFIG_EXTCON_GPIO) += extcon-gpio.o
> obj-$(CONFIG_EXTCON_ADC_JACK) += extcon-adc-jack.o
> +
> obj-$(CONFIG_EXTCON_MAX77693) += extcon-max77693.o
> obj-$(CONFIG_EXTCON_MAX8997) += extcon-max8997.o
> obj-$(CONFIG_EXTCON_ARIZONA) += extcon-arizona.o
> diff --git a/drivers/extcon/of-extcon.c b/drivers/extcon/of-extcon.c
> new file mode 100644
> index 0000000..29f82b4
> --- /dev/null
> +++ b/drivers/extcon/of-extcon.c
> @@ -0,0 +1,56 @@
> +/*
> + * OF helpers for External connector (extcon) framework
> + *
> + * Copyright (C) 2013 Texas Instruments, Inc.
> + * Kishon Vijay Abraham I <[email protected]>
> + *
> + * Copyright (C) 2013 Samsung Electronics
> + * Chanwoo Choi <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/extcon.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/extcon/of-extcon.h>
> +
> +struct extcon_dev *of_extcon_get_extcon_dev(struct device *dev, int index)
Can this be simpler name? of_extcon_get_dev or something like that?
> +{
> + struct device_node *node;
> + struct extcon_dev *edev;
> + struct platform_device *extcon_parent_dev;
> +
> + if (!dev->of_node) {
> + dev_dbg(dev, "device does not have a device node entry\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + node = of_parse_phandle(dev->of_node, "extcon", index);
> + if (!node) {
> + dev_dbg(dev, "failed to get phandle in %s node\n",
> + dev->of_node->full_name);
> + return ERR_PTR(-ENODEV);
> + }
> +
> + extcon_parent_dev = of_find_device_by_node(node);
> + if (!extcon_parent_dev) {
> + dev_dbg(dev, "unable to find device by node\n");
> + return ERR_PTR(-EPROBE_DEFER);
> + }
> +
> + edev = extcon_get_extcon_dev(dev_name(&extcon_parent_dev->dev));
> + if (!edev) {
> + dev_dbg(dev, "unable to get extcon device : %s\n",
> + dev_name(&extcon_parent_dev->dev));
> + return ERR_PTR(-ENODEV);
> + }
> +
> + return edev;
> +}
> +EXPORT_SYMBOL_GPL(of_extcon_get_extcon_dev);
> diff --git a/include/linux/extcon/of-extcon.h b/include/linux/extcon/of-extcon.h
> new file mode 100644
> index 0000000..77d01ba
> --- /dev/null
> +++ b/include/linux/extcon/of-extcon.h
> @@ -0,0 +1,30 @@
> +/*
> + * OF helpers for External connector (extcon) framework
> + *
> + * Copyright (C) 2013 Texas Instruments, Inc.
> + * Kishon Vijay Abraham I <[email protected]>
> + *
> + * Copyright (C) 2013 Samsung Electronics
> + * Chanwoo Choi <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#ifndef __LINUX_OF_EXTCON_H
> +#define __LINUX_OF_EXTCON_H
> +
> +#if defined(CONFIG_OF)
> +extern struct extcon_dev
> + *of_extcon_get_extcon_dev(struct device *dev, int index);
> +#else
> +static inline struct extcon_dev
> + *of_extcon_get_extcon_dev(struct device *dev, int index);
> +{
> + return NULL;
> +}
> +#endif /* CONFIG_OF */
> +
> +#endif /* __LINUX_OF_EXTCON_H */
> --
> 1.8.0
>

2013-06-12 06:55:33

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH] extcon: Add an API to get extcon device from dt node

Hi Chanwoo Choi,

On Wednesday 12 June 2013 07:09 AM, Chanwoo Choi wrote:
> From: Kishon Vijay Abraham I <[email protected]>
>
> Added an API of_extcon_get_extcon_dev() to be used by drivers to get
> extcon device in the case of dt boot (this can be used instead of
> extcon_get_extcon_dev()).
>
> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> Signed-off-by: Chanwoo Choi <[email protected]>
> Signed-off-by: Myungjoo Ham <[email protected]>
> ---
> drivers/extcon/Makefile | 3 +++
> drivers/extcon/of-extcon.c | 56 ++++++++++++++++++++++++++++++++++++++++
> include/linux/extcon/of-extcon.h | 30 +++++++++++++++++++++
> 3 files changed, 89 insertions(+)
> create mode 100644 drivers/extcon/of-extcon.c
> create mode 100644 include/linux/extcon/of-extcon.h
>
> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
> index 540e2c3..39cdf95 100644
> --- a/drivers/extcon/Makefile
> +++ b/drivers/extcon/Makefile
> @@ -2,9 +2,12 @@
> # Makefile for external connector class (extcon) devices
> #
>
> +obj-$(CONFIG_OF) += of-extcon.o
> +
> obj-$(CONFIG_EXTCON) += extcon-class.o
> obj-$(CONFIG_EXTCON_GPIO) += extcon-gpio.o
> obj-$(CONFIG_EXTCON_ADC_JACK) += extcon-adc-jack.o
> +
> obj-$(CONFIG_EXTCON_MAX77693) += extcon-max77693.o
> obj-$(CONFIG_EXTCON_MAX8997) += extcon-max8997.o
> obj-$(CONFIG_EXTCON_ARIZONA) += extcon-arizona.o
> diff --git a/drivers/extcon/of-extcon.c b/drivers/extcon/of-extcon.c
> new file mode 100644
> index 0000000..29f82b4
> --- /dev/null
> +++ b/drivers/extcon/of-extcon.c
> @@ -0,0 +1,56 @@
> +/*
> + * OF helpers for External connector (extcon) framework
> + *
> + * Copyright (C) 2013 Texas Instruments, Inc.
> + * Kishon Vijay Abraham I <[email protected]>
> + *
> + * Copyright (C) 2013 Samsung Electronics
> + * Chanwoo Choi <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/extcon.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/extcon/of-extcon.h>
> +
> +struct extcon_dev *of_extcon_get_extcon_dev(struct device *dev, int index)
> +{
> + struct device_node *node;
> + struct extcon_dev *edev;
> + struct platform_device *extcon_parent_dev;
> +
> + if (!dev->of_node) {
> + dev_dbg(dev, "device does not have a device node entry\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + node = of_parse_phandle(dev->of_node, "extcon", index);
> + if (!node) {
> + dev_dbg(dev, "failed to get phandle in %s node\n",
> + dev->of_node->full_name);
> + return ERR_PTR(-ENODEV);
> + }
> +
> + extcon_parent_dev = of_find_device_by_node(node);
> + if (!extcon_parent_dev) {
> + dev_dbg(dev, "unable to find device by node\n");
> + return ERR_PTR(-EPROBE_DEFER);
> + }
> +
> + edev = extcon_get_extcon_dev(dev_name(&extcon_parent_dev->dev));

In order to get this working, I needed a small fix in extcon_dev_register

- dev_set_name(edev->dev, edev->name ? edev->name : dev_name(dev));
+ edev->name = edev->name ? edev->name : dev_name(dev);
+ dev_set_name(edev->dev, edev->name);

Also using extcon_get_extcon_dev here requires the extcon driver not to
set edev.name since we use *dev_name* here. Hence I recommend using my
initial class iterative approach. Anyway its your call. Let me know if
have to float a new version of the patch (either the iterative approach
or having the fix I mentioned in extcon_dev_register).

Thanks
Kishon

2013-06-14 05:08:35

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH] extcon: Add an API to get extcon device from dt node

On 06/12/2013 03:55 PM, Kishon Vijay Abraham I wrote:
> Hi Chanwoo Choi,
>
> On Wednesday 12 June 2013 07:09 AM, Chanwoo Choi wrote:
>> From: Kishon Vijay Abraham I <[email protected]>
>>
>> Added an API of_extcon_get_extcon_dev() to be used by drivers to get
>> extcon device in the case of dt boot (this can be used instead of
>> extcon_get_extcon_dev()).
>>
>> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
>> Signed-off-by: Chanwoo Choi <[email protected]>
>> Signed-off-by: Myungjoo Ham <[email protected]>
>> ---
>> drivers/extcon/Makefile | 3 +++
>> drivers/extcon/of-extcon.c | 56 ++++++++++++++++++++++++++++++++++++++++
>> include/linux/extcon/of-extcon.h | 30 +++++++++++++++++++++
>> 3 files changed, 89 insertions(+)
>> create mode 100644 drivers/extcon/of-extcon.c
>> create mode 100644 include/linux/extcon/of-extcon.h
>>
>> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
>> index 540e2c3..39cdf95 100644
>> --- a/drivers/extcon/Makefile
>> +++ b/drivers/extcon/Makefile
>> @@ -2,9 +2,12 @@
>> # Makefile for external connector class (extcon) devices
>> #
>>
>> +obj-$(CONFIG_OF) += of-extcon.o
>> +
>> obj-$(CONFIG_EXTCON) += extcon-class.o
>> obj-$(CONFIG_EXTCON_GPIO) += extcon-gpio.o
>> obj-$(CONFIG_EXTCON_ADC_JACK) += extcon-adc-jack.o
>> +
>> obj-$(CONFIG_EXTCON_MAX77693) += extcon-max77693.o
>> obj-$(CONFIG_EXTCON_MAX8997) += extcon-max8997.o
>> obj-$(CONFIG_EXTCON_ARIZONA) += extcon-arizona.o
>> diff --git a/drivers/extcon/of-extcon.c b/drivers/extcon/of-extcon.c
>> new file mode 100644
>> index 0000000..29f82b4
>> --- /dev/null
>> +++ b/drivers/extcon/of-extcon.c
>> @@ -0,0 +1,56 @@
>> +/*
>> + * OF helpers for External connector (extcon) framework
>> + *
>> + * Copyright (C) 2013 Texas Instruments, Inc.
>> + * Kishon Vijay Abraham I <[email protected]>
>> + *
>> + * Copyright (C) 2013 Samsung Electronics
>> + * Chanwoo Choi <[email protected]>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/slab.h>
>> +#include <linux/extcon.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/extcon/of-extcon.h>
>> +
>> +struct extcon_dev *of_extcon_get_extcon_dev(struct device *dev, int index)
>> +{
>> + struct device_node *node;
>> + struct extcon_dev *edev;
>> + struct platform_device *extcon_parent_dev;
>> +
>> + if (!dev->of_node) {
>> + dev_dbg(dev, "device does not have a device node entry\n");
>> + return ERR_PTR(-EINVAL);
>> + }
>> +
>> + node = of_parse_phandle(dev->of_node, "extcon", index);
>> + if (!node) {
>> + dev_dbg(dev, "failed to get phandle in %s node\n",
>> + dev->of_node->full_name);
>> + return ERR_PTR(-ENODEV);
>> + }
>> +
>> + extcon_parent_dev = of_find_device_by_node(node);
>> + if (!extcon_parent_dev) {
>> + dev_dbg(dev, "unable to find device by node\n");
>> + return ERR_PTR(-EPROBE_DEFER);
>> + }
>> +
>> + edev = extcon_get_extcon_dev(dev_name(&extcon_parent_dev->dev));
>
> In order to get this working, I needed a small fix in extcon_dev_register
>
> - dev_set_name(edev->dev, edev->name ? edev->name : dev_name(dev));
> + edev->name = edev->name ? edev->name : dev_name(dev);
> + dev_set_name(edev->dev, edev->name);
>

OK, I added it on this patch.

> Also using extcon_get_extcon_dev here requires the extcon driver not to set edev.name since we use *dev_name* here. Hence I recommend using my initial class iterative approach. Anyway its your call. Let me know if have to float a new version of the patch (either the iterative approach or having the fix I mentioned in extcon_dev_register).

I prefer to imprement "of_extcon_get_extcon_dev()" separately from extcon core.

I will resend modified patch with your comment of extcon_dev_register() at once.

Thanks,
Chanwoo Choi

2013-06-14 07:15:58

by Chanwoo Choi

[permalink] [raw]
Subject: [PATCH v2] extcon: Add an API to get extcon device from dt node

From: Kishon Vijay Abraham I <[email protected]>

Added an API of_extcon_get_extcon_dev() to be used by drivers to get
extcon device in the case of dt boot (this can be used instead of
extcon_get_extcon_dev()).

Signed-off-by: Kishon Vijay Abraham I <[email protected]>
Signed-off-by: Chanwoo Choi <[email protected]>
Signed-off-by: Myungjoo Ham <[email protected]>
---
Changes since v1:
- If edev->name is NULL, dev_name(dev) is used as edev->name.
- Change filename from of-extcon.* to of_extcon.*
- Fix build error when CONFIG_OF is not set
- Add header file(linux/err.h) to of_extcon.c

drivers/extcon/Makefile | 2 ++
drivers/extcon/extcon-class.c | 3 +-
drivers/extcon/of_extcon.c | 64 ++++++++++++++++++++++++++++++++++++++++
include/linux/extcon/of_extcon.h | 30 +++++++++++++++++++
4 files changed, 98 insertions(+), 1 deletion(-)
create mode 100644 drivers/extcon/of_extcon.c
create mode 100644 include/linux/extcon/of_extcon.h

diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
index 540e2c3..83468f7 100644
--- a/drivers/extcon/Makefile
+++ b/drivers/extcon/Makefile
@@ -2,6 +2,8 @@
# Makefile for external connector class (extcon) devices
#

+obj-$(CONFIG_OF) += of_extcon.o
+
obj-$(CONFIG_EXTCON) += extcon-class.o
obj-$(CONFIG_EXTCON_GPIO) += extcon-gpio.o
obj-$(CONFIG_EXTCON_ADC_JACK) += extcon-adc-jack.o
diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index 23f11ea..08509ea 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -602,7 +602,8 @@ int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
edev->dev->class = extcon_class;
edev->dev->release = extcon_dev_release;

- dev_set_name(edev->dev, edev->name ? edev->name : dev_name(dev));
+ edev->name = edev->name ? edev->name : dev_name(dev);
+ dev_set_name(edev->dev, edev->name);

if (edev->max_supported) {
char buf[10];
diff --git a/drivers/extcon/of_extcon.c b/drivers/extcon/of_extcon.c
new file mode 100644
index 0000000..72173ec
--- /dev/null
+++ b/drivers/extcon/of_extcon.c
@@ -0,0 +1,64 @@
+/*
+ * OF helpers for External connector (extcon) framework
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ * Kishon Vijay Abraham I <[email protected]>
+ *
+ * Copyright (C) 2013 Samsung Electronics
+ * Chanwoo Choi <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/extcon.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/extcon/of_extcon.h>
+
+/*
+ * of_extcon_get_extcon_dev - Get the name of extcon device from devicetree
+ * @dev - instance to the given device
+ * @index - index into list of extcon_dev
+ *
+ * return the instance of extcon device
+ */
+struct extcon_dev *of_extcon_get_extcon_dev(struct device *dev, int index)
+{
+ struct device_node *node;
+ struct extcon_dev *edev;
+ struct platform_device *extcon_parent_dev;
+
+ if (!dev->of_node) {
+ dev_dbg(dev, "device does not have a device node entry\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ node = of_parse_phandle(dev->of_node, "extcon", index);
+ if (!node) {
+ dev_dbg(dev, "failed to get phandle in %s node\n",
+ dev->of_node->full_name);
+ return ERR_PTR(-ENODEV);
+ }
+
+ extcon_parent_dev = of_find_device_by_node(node);
+ if (!extcon_parent_dev) {
+ dev_dbg(dev, "unable to find device by node\n");
+ return ERR_PTR(-EPROBE_DEFER);
+ }
+
+ edev = extcon_get_extcon_dev(dev_name(&extcon_parent_dev->dev));
+ if (!edev) {
+ dev_dbg(dev, "unable to get extcon device : %s\n",
+ dev_name(&extcon_parent_dev->dev));
+ return ERR_PTR(-ENODEV);
+ }
+
+ return edev;
+}
+EXPORT_SYMBOL_GPL(of_extcon_get_extcon_dev);
diff --git a/include/linux/extcon/of_extcon.h b/include/linux/extcon/of_extcon.h
new file mode 100644
index 0000000..462f071
--- /dev/null
+++ b/include/linux/extcon/of_extcon.h
@@ -0,0 +1,30 @@
+/*
+ * OF helpers for External connector (extcon) framework
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ * Kishon Vijay Abraham I <[email protected]>
+ *
+ * Copyright (C) 2013 Samsung Electronics
+ * Chanwoo Choi <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __LINUX_OF_EXTCON_H
+#define __LINUX_OF_EXTCON_H
+
+#if defined(CONFIG_OF) && defined(CONFIG_EXTCON)
+extern struct extcon_dev
+ *of_extcon_get_extcon_dev(struct device *dev, int index);
+#else
+static inline struct extcon_dev
+ *of_extcon_get_extcon_dev(struct device *dev, int index)
+{
+ return NULL;
+}
+#endif /* CONFIG_OF */
+
+#endif /* __LINUX_OF_EXTCON_H */
--
1.8.0

2013-06-14 08:37:28

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH v2] extcon: Add an API to get extcon device from dt node

Hi,

On Friday 14 June 2013 12:45 PM, Chanwoo Choi wrote:
> From: Kishon Vijay Abraham I <[email protected]>
>
> Added an API of_extcon_get_extcon_dev() to be used by drivers to get
> extcon device in the case of dt boot (this can be used instead of
> extcon_get_extcon_dev()).
>
> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> Signed-off-by: Chanwoo Choi <[email protected]>
> Signed-off-by: Myungjoo Ham <[email protected]>
> ---
> Changes since v1:
> - If edev->name is NULL, dev_name(dev) is used as edev->name.
> - Change filename from of-extcon.* to of_extcon.*
> - Fix build error when CONFIG_OF is not set
> - Add header file(linux/err.h) to of_extcon.c
>
> drivers/extcon/Makefile | 2 ++
> drivers/extcon/extcon-class.c | 3 +-
> drivers/extcon/of_extcon.c | 64 ++++++++++++++++++++++++++++++++++++++++
> include/linux/extcon/of_extcon.h | 30 +++++++++++++++++++
> 4 files changed, 98 insertions(+), 1 deletion(-)
> create mode 100644 drivers/extcon/of_extcon.c
> create mode 100644 include/linux/extcon/of_extcon.h
>
> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
> index 540e2c3..83468f7 100644
> --- a/drivers/extcon/Makefile
> +++ b/drivers/extcon/Makefile
> @@ -2,6 +2,8 @@
> # Makefile for external connector class (extcon) devices
> #
>
> +obj-$(CONFIG_OF) += of_extcon.o
> +
> obj-$(CONFIG_EXTCON) += extcon-class.o
> obj-$(CONFIG_EXTCON_GPIO) += extcon-gpio.o
> obj-$(CONFIG_EXTCON_ADC_JACK) += extcon-adc-jack.o
> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
> index 23f11ea..08509ea 100644
> --- a/drivers/extcon/extcon-class.c
> +++ b/drivers/extcon/extcon-class.c
> @@ -602,7 +602,8 @@ int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
> edev->dev->class = extcon_class;
> edev->dev->release = extcon_dev_release;
>
> - dev_set_name(edev->dev, edev->name ? edev->name : dev_name(dev));
> + edev->name = edev->name ? edev->name : dev_name(dev);
> + dev_set_name(edev->dev, edev->name);
>
> if (edev->max_supported) {
> char buf[10];
> diff --git a/drivers/extcon/of_extcon.c b/drivers/extcon/of_extcon.c
> new file mode 100644
> index 0000000..72173ec
> --- /dev/null
> +++ b/drivers/extcon/of_extcon.c
> @@ -0,0 +1,64 @@
> +/*
> + * OF helpers for External connector (extcon) framework
> + *
> + * Copyright (C) 2013 Texas Instruments, Inc.
> + * Kishon Vijay Abraham I <[email protected]>
> + *
> + * Copyright (C) 2013 Samsung Electronics
> + * Chanwoo Choi <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/err.h>
> +#include <linux/extcon.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/extcon/of_extcon.h>
> +
> +/*
> + * of_extcon_get_extcon_dev - Get the name of extcon device from devicetree
> + * @dev - instance to the given device
> + * @index - index into list of extcon_dev
> + *
> + * return the instance of extcon device
> + */
> +struct extcon_dev *of_extcon_get_extcon_dev(struct device *dev, int index)
> +{
> + struct device_node *node;
> + struct extcon_dev *edev;
> + struct platform_device *extcon_parent_dev;
> +
> + if (!dev->of_node) {
> + dev_dbg(dev, "device does not have a device node entry\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + node = of_parse_phandle(dev->of_node, "extcon", index);
> + if (!node) {
> + dev_dbg(dev, "failed to get phandle in %s node\n",
> + dev->of_node->full_name);
> + return ERR_PTR(-ENODEV);
> + }
> +
> + extcon_parent_dev = of_find_device_by_node(node);
> + if (!extcon_parent_dev) {
> + dev_dbg(dev, "unable to find device by node\n");
> + return ERR_PTR(-EPROBE_DEFER);
> + }
> +
> + edev = extcon_get_extcon_dev(dev_name(&extcon_parent_dev->dev));
> + if (!edev) {
> + dev_dbg(dev, "unable to get extcon device : %s\n",
> + dev_name(&extcon_parent_dev->dev));
> + return ERR_PTR(-ENODEV);
> + }
> +
> + return edev;
> +}
> +EXPORT_SYMBOL_GPL(of_extcon_get_extcon_dev);
> diff --git a/include/linux/extcon/of_extcon.h b/include/linux/extcon/of_extcon.h
> new file mode 100644
> index 0000000..462f071
> --- /dev/null
> +++ b/include/linux/extcon/of_extcon.h
> @@ -0,0 +1,30 @@
> +/*
> + * OF helpers for External connector (extcon) framework
> + *
> + * Copyright (C) 2013 Texas Instruments, Inc.
> + * Kishon Vijay Abraham I <[email protected]>
> + *
> + * Copyright (C) 2013 Samsung Electronics
> + * Chanwoo Choi <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#ifndef __LINUX_OF_EXTCON_H
> +#define __LINUX_OF_EXTCON_H
> +
> +#if defined(CONFIG_OF) && defined(CONFIG_EXTCON)
> +extern struct extcon_dev
> + *of_extcon_get_extcon_dev(struct device *dev, int index);
> +#else
> +static inline struct extcon_dev
> + *of_extcon_get_extcon_dev(struct device *dev, int index)
> +{
> + return NULL;

Can we have some error value returned here instead of NULL?
ERR_PTR(-ENOSYS)?

Thanks
Kishon

2013-06-14 08:51:25

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH v2] extcon: Add an API to get extcon device from dt node

On 06/14/2013 05:36 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Friday 14 June 2013 12:45 PM, Chanwoo Choi wrote:
>> From: Kishon Vijay Abraham I <[email protected]>
>>
>> Added an API of_extcon_get_extcon_dev() to be used by drivers to get
>> extcon device in the case of dt boot (this can be used instead of
>> extcon_get_extcon_dev()).
>>
>> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
>> Signed-off-by: Chanwoo Choi <[email protected]>
>> Signed-off-by: Myungjoo Ham <[email protected]>
>> ---
>> Changes since v1:
>> - If edev->name is NULL, dev_name(dev) is used as edev->name.
>> - Change filename from of-extcon.* to of_extcon.*
>> - Fix build error when CONFIG_OF is not set
>> - Add header file(linux/err.h) to of_extcon.c
>>
>> drivers/extcon/Makefile | 2 ++
>> drivers/extcon/extcon-class.c | 3 +-
>> drivers/extcon/of_extcon.c | 64 ++++++++++++++++++++++++++++++++++++++++
>> include/linux/extcon/of_extcon.h | 30 +++++++++++++++++++
>> 4 files changed, 98 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/extcon/of_extcon.c
>> create mode 100644 include/linux/extcon/of_extcon.h
>>
>> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
>> index 540e2c3..83468f7 100644
>> --- a/drivers/extcon/Makefile
>> +++ b/drivers/extcon/Makefile
>> @@ -2,6 +2,8 @@
>> # Makefile for external connector class (extcon) devices
>> #
>>
>> +obj-$(CONFIG_OF) += of_extcon.o
>> +
>> obj-$(CONFIG_EXTCON) += extcon-class.o
>> obj-$(CONFIG_EXTCON_GPIO) += extcon-gpio.o
>> obj-$(CONFIG_EXTCON_ADC_JACK) += extcon-adc-jack.o
>> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
>> index 23f11ea..08509ea 100644
>> --- a/drivers/extcon/extcon-class.c
>> +++ b/drivers/extcon/extcon-class.c
>> @@ -602,7 +602,8 @@ int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
>> edev->dev->class = extcon_class;
>> edev->dev->release = extcon_dev_release;
>>
>> - dev_set_name(edev->dev, edev->name ? edev->name : dev_name(dev));
>> + edev->name = edev->name ? edev->name : dev_name(dev);
>> + dev_set_name(edev->dev, edev->name);
>>
>> if (edev->max_supported) {
>> char buf[10];
>> diff --git a/drivers/extcon/of_extcon.c b/drivers/extcon/of_extcon.c
>> new file mode 100644
>> index 0000000..72173ec
>> --- /dev/null
>> +++ b/drivers/extcon/of_extcon.c
>> @@ -0,0 +1,64 @@
>> +/*
>> + * OF helpers for External connector (extcon) framework
>> + *
>> + * Copyright (C) 2013 Texas Instruments, Inc.
>> + * Kishon Vijay Abraham I <[email protected]>
>> + *
>> + * Copyright (C) 2013 Samsung Electronics
>> + * Chanwoo Choi <[email protected]>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/slab.h>
>> +#include <linux/err.h>
>> +#include <linux/extcon.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/extcon/of_extcon.h>
>> +
>> +/*
>> + * of_extcon_get_extcon_dev - Get the name of extcon device from devicetree
>> + * @dev - instance to the given device
>> + * @index - index into list of extcon_dev
>> + *
>> + * return the instance of extcon device
>> + */
>> +struct extcon_dev *of_extcon_get_extcon_dev(struct device *dev, int index)
>> +{
>> + struct device_node *node;
>> + struct extcon_dev *edev;
>> + struct platform_device *extcon_parent_dev;
>> +
>> + if (!dev->of_node) {
>> + dev_dbg(dev, "device does not have a device node entry\n");
>> + return ERR_PTR(-EINVAL);
>> + }
>> +
>> + node = of_parse_phandle(dev->of_node, "extcon", index);
>> + if (!node) {
>> + dev_dbg(dev, "failed to get phandle in %s node\n",
>> + dev->of_node->full_name);
>> + return ERR_PTR(-ENODEV);
>> + }
>> +
>> + extcon_parent_dev = of_find_device_by_node(node);
>> + if (!extcon_parent_dev) {
>> + dev_dbg(dev, "unable to find device by node\n");
>> + return ERR_PTR(-EPROBE_DEFER);
>> + }
>> +
>> + edev = extcon_get_extcon_dev(dev_name(&extcon_parent_dev->dev));
>> + if (!edev) {
>> + dev_dbg(dev, "unable to get extcon device : %s\n",
>> + dev_name(&extcon_parent_dev->dev));
>> + return ERR_PTR(-ENODEV);
>> + }
>> +
>> + return edev;
>> +}
>> +EXPORT_SYMBOL_GPL(of_extcon_get_extcon_dev);
>> diff --git a/include/linux/extcon/of_extcon.h b/include/linux/extcon/of_extcon.h
>> new file mode 100644
>> index 0000000..462f071
>> --- /dev/null
>> +++ b/include/linux/extcon/of_extcon.h
>> @@ -0,0 +1,30 @@
>> +/*
>> + * OF helpers for External connector (extcon) framework
>> + *
>> + * Copyright (C) 2013 Texas Instruments, Inc.
>> + * Kishon Vijay Abraham I <[email protected]>
>> + *
>> + * Copyright (C) 2013 Samsung Electronics
>> + * Chanwoo Choi <[email protected]>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#ifndef __LINUX_OF_EXTCON_H
>> +#define __LINUX_OF_EXTCON_H
>> +
>> +#if defined(CONFIG_OF) && defined(CONFIG_EXTCON)
>> +extern struct extcon_dev
>> + *of_extcon_get_extcon_dev(struct device *dev, int index);
>> +#else
>> +static inline struct extcon_dev
>> + *of_extcon_get_extcon_dev(struct device *dev, int index)
>> +{
>> + return NULL;
>
> Can we have some error value returned here instead of NULL?
> ERR_PTR(-ENOSYS)?
>

OK, I modified it.

You can check modified patch on below git repo:
http://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/commit/?h=extcon-next&id=2ae384a41c25d6ff8f12091c426fc7324141096f

Thanks,
Chanwoo Choi

2013-06-14 13:10:53

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: [PATCH v4] usb: dwc3: use extcon fwrk to receive connect/disconnect

Modified dwc3-omap to receive connect and disconnect notification using
extcon framework. Also did the necessary cleanups required after
adapting to extcon framework.

Signed-off-by: Kishon Vijay Abraham I <[email protected]>
Acked-by: Felipe Balbi <[email protected]>
---
This patch depends on
git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git commit:f7ae906

It should also be applied on top of
usb: dwc3: omap: improve error handling of dwc3_omap_probe patch which is
merged in Felipe's tree.

So I'm not sure on whose tree this patch should go in.

Changes from v3:
* did #include of of_extcon.h after Chanwoo resent the patch separating
extcon-class.c from of_extcon.c
Changes from v2:
* updated the Documentation with dwc3 dt binding information.
* used of_extcon_get_extcon_dev to get extcon device from device tree data.
Changes from v1:
* regulator enable/disable is now done here instead of palmas-usb as some users
of palmas-usb wont need regulator.

Documentation/devicetree/bindings/usb/omap-usb.txt | 5 +
drivers/usb/dwc3/dwc3-omap.c | 119 ++++++++++++++++----
include/linux/usb/dwc3-omap.h | 30 -----
3 files changed, 105 insertions(+), 49 deletions(-)
delete mode 100644 include/linux/usb/dwc3-omap.h

diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt
index d4769f3..f1c15f3 100644
--- a/Documentation/devicetree/bindings/usb/omap-usb.txt
+++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
@@ -53,6 +53,11 @@ OMAP DWC3 GLUE
It should be set to "1" for HW mode and "2" for SW mode.
- ranges: the child address space are mapped 1:1 onto the parent address space

+Optional Properties:
+ - extcon : phandle for the extcon device omap dwc3 uses to detect
+ connect/disconnect events.
+ - vbus-supply : phandle to the regulator device tree node if needed.
+
Sub-nodes:
The dwc3 core should be added as subnode to omap dwc3 glue.
- dwc3 :
diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index f8f76e6..14c1f1b 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -43,13 +43,15 @@
#include <linux/spinlock.h>
#include <linux/platform_device.h>
#include <linux/platform_data/dwc3-omap.h>
-#include <linux/usb/dwc3-omap.h>
#include <linux/pm_runtime.h>
#include <linux/dma-mapping.h>
#include <linux/ioport.h>
#include <linux/io.h>
#include <linux/of.h>
#include <linux/of_platform.h>
+#include <linux/extcon.h>
+#include <linux/extcon/of_extcon.h>
+#include <linux/regulator/consumer.h>

#include <linux/usb/otg.h>

@@ -124,9 +126,21 @@ struct dwc3_omap {
u32 utmi_otg_status;

u32 dma_status:1;
+
+ struct extcon_specific_cable_nb extcon_vbus_dev;
+ struct extcon_specific_cable_nb extcon_id_dev;
+ struct notifier_block vbus_nb;
+ struct notifier_block id_nb;
+
+ struct regulator *vbus_reg;
};

-static struct dwc3_omap *_omap;
+enum omap_dwc3_vbus_id_status {
+ OMAP_DWC3_ID_FLOAT,
+ OMAP_DWC3_ID_GROUND,
+ OMAP_DWC3_VBUS_OFF,
+ OMAP_DWC3_VBUS_VALID,
+};

static inline u32 dwc3_omap_readl(void __iomem *base, u32 offset)
{
@@ -138,18 +152,23 @@ static inline void dwc3_omap_writel(void __iomem *base, u32 offset, u32 value)
writel(value, base + offset);
}

-int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
+static void dwc3_omap_set_mailbox(struct dwc3_omap *omap,
+ enum omap_dwc3_vbus_id_status status)
{
- u32 val;
- struct dwc3_omap *omap = _omap;
-
- if (!omap)
- return -EPROBE_DEFER;
+ int ret;
+ u32 val;

switch (status) {
case OMAP_DWC3_ID_GROUND:
dev_dbg(omap->dev, "ID GND\n");

+ if (omap->vbus_reg) {
+ ret = regulator_enable(omap->vbus_reg);
+ if (ret) {
+ dev_dbg(omap->dev, "regulator enable failed\n");
+ return;
+ }
+ }
val = dwc3_omap_readl(omap->base, USBOTGSS_UTMI_OTG_STATUS);
val &= ~(USBOTGSS_UTMI_OTG_STATUS_IDDIG
| USBOTGSS_UTMI_OTG_STATUS_VBUSVALID
@@ -172,6 +191,9 @@ int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
break;

case OMAP_DWC3_ID_FLOAT:
+ if (omap->vbus_reg)
+ regulator_disable(omap->vbus_reg);
+
case OMAP_DWC3_VBUS_OFF:
dev_dbg(omap->dev, "VBUS Disconnect\n");

@@ -185,12 +207,9 @@ int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
break;

default:
- dev_dbg(omap->dev, "ID float\n");
+ dev_dbg(omap->dev, "invalid state\n");
}
-
- return 0;
}
-EXPORT_SYMBOL_GPL(dwc3_omap_mailbox);

static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap)
{
@@ -282,6 +301,32 @@ static void dwc3_omap_disable_irqs(struct dwc3_omap *omap)

static u64 dwc3_omap_dma_mask = DMA_BIT_MASK(32);

+static int dwc3_omap_id_notifier(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct dwc3_omap *omap = container_of(nb, struct dwc3_omap, id_nb);
+
+ if (event)
+ dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
+ else
+ dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_FLOAT);
+
+ return NOTIFY_DONE;
+}
+
+static int dwc3_omap_vbus_notifier(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct dwc3_omap *omap = container_of(nb, struct dwc3_omap, vbus_nb);
+
+ if (event)
+ dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
+ else
+ dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_OFF);
+
+ return NOTIFY_DONE;
+}
+
static int dwc3_omap_probe(struct platform_device *pdev)
{
struct device_node *node = pdev->dev.of_node;
@@ -289,6 +334,8 @@ static int dwc3_omap_probe(struct platform_device *pdev)
struct dwc3_omap *omap;
struct resource *res;
struct device *dev = &pdev->dev;
+ struct extcon_dev *edev;
+ struct regulator *vbus_reg = NULL;

int ret = -ENOMEM;
int irq;
@@ -330,19 +377,22 @@ static int dwc3_omap_probe(struct platform_device *pdev)
return -ENOMEM;
}

+ if (of_property_read_bool(node, "vbus-supply")) {
+ vbus_reg = devm_regulator_get(dev, "vbus");
+ if (IS_ERR(vbus_reg)) {
+ dev_err(dev, "vbus init failed\n");
+ return PTR_ERR(vbus_reg);
+ }
+ }
+
spin_lock_init(&omap->lock);

omap->dev = dev;
omap->irq = irq;
omap->base = base;
+ omap->vbus_reg = vbus_reg;
dev->dma_mask = &dwc3_omap_dma_mask;

- /*
- * REVISIT if we ever have two instances of the wrapper, we will be
- * in big trouble
- */
- _omap = omap;
-
pm_runtime_enable(dev);
ret = pm_runtime_get_sync(dev);
if (ret < 0) {
@@ -381,14 +431,41 @@ static int dwc3_omap_probe(struct platform_device *pdev)

dwc3_omap_enable_irqs(omap);

+ if (of_property_read_bool(node, "extcon")) {
+ edev = of_extcon_get_extcon_dev(dev, 0);
+ if (IS_ERR(edev)) {
+ dev_vdbg(dev, "couldn't get extcon device\n");
+ ret = PTR_ERR(edev);
+ goto err2;
+ }
+
+ omap->vbus_nb.notifier_call = dwc3_omap_vbus_notifier;
+ extcon_register_interest(&omap->extcon_vbus_dev, edev->name,
+ "USB", &omap->vbus_nb);
+ omap->id_nb.notifier_call = dwc3_omap_id_notifier;
+ extcon_register_interest(&omap->extcon_id_dev, edev->name,
+ "USB-HOST", &omap->id_nb);
+
+ if (extcon_get_cable_state(edev, "USB") == true)
+ dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
+ if (extcon_get_cable_state(edev, "USB-HOST") == true)
+ dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
+ }
+
ret = of_platform_populate(node, NULL, NULL, dev);
if (ret) {
dev_err(&pdev->dev, "failed to create dwc3 core\n");
- goto err2;
+ goto err3;
}

return 0;

+err3:
+ if (omap->extcon_vbus_dev.edev)
+ extcon_unregister_interest(&omap->extcon_vbus_dev);
+ if (omap->extcon_id_dev.edev)
+ extcon_unregister_interest(&omap->extcon_id_dev);
+
err2:
dwc3_omap_disable_irqs(omap);

@@ -405,6 +482,10 @@ static int dwc3_omap_remove(struct platform_device *pdev)
{
struct dwc3_omap *omap = platform_get_drvdata(pdev);

+ if (omap->extcon_vbus_dev.edev)
+ extcon_unregister_interest(&omap->extcon_vbus_dev);
+ if (omap->extcon_id_dev.edev)
+ extcon_unregister_interest(&omap->extcon_id_dev);
dwc3_omap_disable_irqs(omap);
pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
diff --git a/include/linux/usb/dwc3-omap.h b/include/linux/usb/dwc3-omap.h
deleted file mode 100644
index 5615f4d..0000000
--
1.7.10.4

2013-06-17 04:09:55

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH v4] usb: dwc3: use extcon fwrk to receive connect/disconnect

On 06/14/2013 10:10 PM, Kishon Vijay Abraham I wrote:
> Modified dwc3-omap to receive connect and disconnect notification using
> extcon framework. Also did the necessary cleanups required after
> adapting to extcon framework.
>
> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> Acked-by: Felipe Balbi <[email protected]>
> ---
> This patch depends on
> git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git commit:f7ae906
>
> It should also be applied on top of
> usb: dwc3: omap: improve error handling of dwc3_omap_probe patch which is
> merged in Felipe's tree.
>
> So I'm not sure on whose tree this patch should go in.
>
> Changes from v3:
> * did #include of of_extcon.h after Chanwoo resent the patch separating
> extcon-class.c from of_extcon.c
> Changes from v2:
> * updated the Documentation with dwc3 dt binding information.
> * used of_extcon_get_extcon_dev to get extcon device from device tree data.
> Changes from v1:
> * regulator enable/disable is now done here instead of palmas-usb as some users
> of palmas-usb wont need regulator.
>
> Documentation/devicetree/bindings/usb/omap-usb.txt | 5 +
> drivers/usb/dwc3/dwc3-omap.c | 119 ++++++++++++++++----
> include/linux/usb/dwc3-omap.h | 30 -----
> 3 files changed, 105 insertions(+), 49 deletions(-)
> delete mode 100644 include/linux/usb/dwc3-omap.h
>
> diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt
> index d4769f3..f1c15f3 100644
> --- a/Documentation/devicetree/bindings/usb/omap-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
> @@ -53,6 +53,11 @@ OMAP DWC3 GLUE
> It should be set to "1" for HW mode and "2" for SW mode.
> - ranges: the child address space are mapped 1:1 onto the parent address space
>
> +Optional Properties:
> + - extcon : phandle for the extcon device omap dwc3 uses to detect
> + connect/disconnect events.
> + - vbus-supply : phandle to the regulator device tree node if needed.
> +
> Sub-nodes:
> The dwc3 core should be added as subnode to omap dwc3 glue.
> - dwc3 :
> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
> index f8f76e6..14c1f1b 100644
> --- a/drivers/usb/dwc3/dwc3-omap.c
> +++ b/drivers/usb/dwc3/dwc3-omap.c
> @@ -43,13 +43,15 @@
> #include <linux/spinlock.h>
> #include <linux/platform_device.h>
> #include <linux/platform_data/dwc3-omap.h>
> -#include <linux/usb/dwc3-omap.h>
> #include <linux/pm_runtime.h>
> #include <linux/dma-mapping.h>
> #include <linux/ioport.h>
> #include <linux/io.h>
> #include <linux/of.h>
> #include <linux/of_platform.h>
> +#include <linux/extcon.h>
> +#include <linux/extcon/of_extcon.h>
> +#include <linux/regulator/consumer.h>
>
> #include <linux/usb/otg.h>
>
> @@ -124,9 +126,21 @@ struct dwc3_omap {
> u32 utmi_otg_status;
>
> u32 dma_status:1;
> +
> + struct extcon_specific_cable_nb extcon_vbus_dev;
> + struct extcon_specific_cable_nb extcon_id_dev;
> + struct notifier_block vbus_nb;
> + struct notifier_block id_nb;
> +
> + struct regulator *vbus_reg;
> };
>
> -static struct dwc3_omap *_omap;
> +enum omap_dwc3_vbus_id_status {
> + OMAP_DWC3_ID_FLOAT,
> + OMAP_DWC3_ID_GROUND,
> + OMAP_DWC3_VBUS_OFF,
> + OMAP_DWC3_VBUS_VALID,
> +};
>
> static inline u32 dwc3_omap_readl(void __iomem *base, u32 offset)
> {
> @@ -138,18 +152,23 @@ static inline void dwc3_omap_writel(void __iomem *base, u32 offset, u32 value)
> writel(value, base + offset);
> }
>
> -int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
> +static void dwc3_omap_set_mailbox(struct dwc3_omap *omap,
> + enum omap_dwc3_vbus_id_status status)
> {
> - u32 val;
> - struct dwc3_omap *omap = _omap;
> -
> - if (!omap)
> - return -EPROBE_DEFER;
> + int ret;
> + u32 val;
>
> switch (status) {
> case OMAP_DWC3_ID_GROUND:
> dev_dbg(omap->dev, "ID GND\n");
>
> + if (omap->vbus_reg) {
> + ret = regulator_enable(omap->vbus_reg);
> + if (ret) {
> + dev_dbg(omap->dev, "regulator enable failed\n");
> + return;
> + }
> + }
> val = dwc3_omap_readl(omap->base, USBOTGSS_UTMI_OTG_STATUS);
> val &= ~(USBOTGSS_UTMI_OTG_STATUS_IDDIG
> | USBOTGSS_UTMI_OTG_STATUS_VBUSVALID
> @@ -172,6 +191,9 @@ int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
> break;
>
> case OMAP_DWC3_ID_FLOAT:
> + if (omap->vbus_reg)
> + regulator_disable(omap->vbus_reg);
> +
> case OMAP_DWC3_VBUS_OFF:
> dev_dbg(omap->dev, "VBUS Disconnect\n");
>
> @@ -185,12 +207,9 @@ int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
> break;
>
> default:
> - dev_dbg(omap->dev, "ID float\n");
> + dev_dbg(omap->dev, "invalid state\n");
> }
> -
> - return 0;
> }
> -EXPORT_SYMBOL_GPL(dwc3_omap_mailbox);
>
> static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap)
> {
> @@ -282,6 +301,32 @@ static void dwc3_omap_disable_irqs(struct dwc3_omap *omap)
>
> static u64 dwc3_omap_dma_mask = DMA_BIT_MASK(32);
>
> +static int dwc3_omap_id_notifier(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
> + struct dwc3_omap *omap = container_of(nb, struct dwc3_omap, id_nb);
> +
> + if (event)
> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
> + else
> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_FLOAT);
> +
> + return NOTIFY_DONE;
> +}
> +
> +static int dwc3_omap_vbus_notifier(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
> + struct dwc3_omap *omap = container_of(nb, struct dwc3_omap, vbus_nb);
> +
> + if (event)
> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
> + else
> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_OFF);
> +
> + return NOTIFY_DONE;
> +}
> +
> static int dwc3_omap_probe(struct platform_device *pdev)
> {
> struct device_node *node = pdev->dev.of_node;
> @@ -289,6 +334,8 @@ static int dwc3_omap_probe(struct platform_device *pdev)
> struct dwc3_omap *omap;
> struct resource *res;
> struct device *dev = &pdev->dev;
> + struct extcon_dev *edev;
> + struct regulator *vbus_reg = NULL;
>
> int ret = -ENOMEM;
> int irq;
> @@ -330,19 +377,22 @@ static int dwc3_omap_probe(struct platform_device *pdev)
> return -ENOMEM;
> }
>
> + if (of_property_read_bool(node, "vbus-supply")) {
> + vbus_reg = devm_regulator_get(dev, "vbus");
> + if (IS_ERR(vbus_reg)) {
> + dev_err(dev, "vbus init failed\n");
> + return PTR_ERR(vbus_reg);
> + }
> + }
> +
> spin_lock_init(&omap->lock);
>
> omap->dev = dev;
> omap->irq = irq;
> omap->base = base;
> + omap->vbus_reg = vbus_reg;
> dev->dma_mask = &dwc3_omap_dma_mask;
>
> - /*
> - * REVISIT if we ever have two instances of the wrapper, we will be
> - * in big trouble
> - */
> - _omap = omap;
> -
> pm_runtime_enable(dev);
> ret = pm_runtime_get_sync(dev);
> if (ret < 0) {
> @@ -381,14 +431,41 @@ static int dwc3_omap_probe(struct platform_device *pdev)
>
> dwc3_omap_enable_irqs(omap);
>
> + if (of_property_read_bool(node, "extcon")) {
> + edev = of_extcon_get_extcon_dev(dev, 0);
> + if (IS_ERR(edev)) {
> + dev_vdbg(dev, "couldn't get extcon device\n");
> + ret = PTR_ERR(edev);
> + goto err2;
> + }
> +
> + omap->vbus_nb.notifier_call = dwc3_omap_vbus_notifier;
> + extcon_register_interest(&omap->extcon_vbus_dev, edev->name,
> + "USB", &omap->vbus_nb);
> + omap->id_nb.notifier_call = dwc3_omap_id_notifier;
> + extcon_register_interest(&omap->extcon_id_dev, edev->name,
> + "USB-HOST", &omap->id_nb);

I prefer adding exception handling code about return value of extcon_register_interest().

> +
> + if (extcon_get_cable_state(edev, "USB") == true)
> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
> + if (extcon_get_cable_state(edev, "USB-HOST") == true)
> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
> + }
> +
> ret = of_platform_populate(node, NULL, NULL, dev);
> if (ret) {
> dev_err(&pdev->dev, "failed to create dwc3 core\n");
> - goto err2;
> + goto err3;
> }
>
> return 0;
>
> +err3:
> + if (omap->extcon_vbus_dev.edev)
> + extcon_unregister_interest(&omap->extcon_vbus_dev);
> + if (omap->extcon_id_dev.edev)
> + extcon_unregister_interest(&omap->extcon_id_dev);
> +
> err2:
> dwc3_omap_disable_irqs(omap);
>
> @@ -405,6 +482,10 @@ static int dwc3_omap_remove(struct platform_device *pdev)
> {
> struct dwc3_omap *omap = platform_get_drvdata(pdev);
>
> + if (omap->extcon_vbus_dev.edev)
> + extcon_unregister_interest(&omap->extcon_vbus_dev);
> + if (omap->extcon_id_dev.edev)
> + extcon_unregister_interest(&omap->extcon_id_dev);
> dwc3_omap_disable_irqs(omap);
> pm_runtime_put_sync(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
> diff --git a/include/linux/usb/dwc3-omap.h b/include/linux/usb/dwc3-omap.h
> deleted file mode 100644
> index 5615f4d..0000000
>

It looks good if you add exception handler about return value of extcon_register_interest().

Acked-by: Chanwoo Choi <[email protected]>

But, we have to apply this patch after all of the extcon patchset(for 3.11) will be applied
because this patch has dependency of extcon patch related to DT.
- http://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/commit/?h=extcon-next&id=f7ae906806279e5b57bfd302b945e1bcdddce95b

Thanks,
Chanwoo Choi

2013-06-19 05:26:36

by Chanwoo Choi

[permalink] [raw]
Subject: [PATCH v3] extcon: Add an API to get extcon device from dt node

From: Kishon Vijay Abraham I <[email protected]>

Added an API of_extcon_get_extcon_dev() to be used by drivers to get
extcon device in the case of dt boot (this can be used instead of
extcon_get_extcon_dev()).

Signed-off-by: Kishon Vijay Abraham I <[email protected]>
Signed-off-by: Chanwoo Choi <[email protected]>
Signed-off-by: Myungjoo Ham <[email protected]>
---
Changes since v2:
- Add CONFIG_OF_EXTCON to Kconfig
- Use IS_ENABLED() macro to check both CONFIG_EXTCON and CONFIG_EXTCON_MODULE

Changes since v1:
- If edev->name is NULL, dev_name(dev) is used as edev->name.
- Change filename from of-extcon.* to of_extcon.*
- Fix build error when CONFIG_OF is not set
- Add header file(linux/err.h) to of_extcon.c

drivers/extcon/Kconfig | 4 +++
drivers/extcon/Makefile | 2 ++
drivers/extcon/extcon-class.c | 3 +-
drivers/extcon/of_extcon.c | 64 ++++++++++++++++++++++++++++++++++++++++
include/linux/extcon/of_extcon.h | 31 +++++++++++++++++++
5 files changed, 103 insertions(+), 1 deletion(-)
create mode 100644 drivers/extcon/of_extcon.c
create mode 100644 include/linux/extcon/of_extcon.h

diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index 63f454e..f1d54a3 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -14,6 +14,10 @@ if EXTCON

comment "Extcon Device Drivers"

+config OF_EXTCON
+ def_tristate y
+ depends on OF
+
config EXTCON_GPIO
tristate "GPIO extcon support"
depends on GPIOLIB
diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
index 540e2c3..759fdae 100644
--- a/drivers/extcon/Makefile
+++ b/drivers/extcon/Makefile
@@ -2,6 +2,8 @@
# Makefile for external connector class (extcon) devices
#

+obj-$(CONFIG_OF_EXTCON) += of_extcon.o
+
obj-$(CONFIG_EXTCON) += extcon-class.o
obj-$(CONFIG_EXTCON_GPIO) += extcon-gpio.o
obj-$(CONFIG_EXTCON_ADC_JACK) += extcon-adc-jack.o
diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index 23f11ea..08509ea 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -602,7 +602,8 @@ int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
edev->dev->class = extcon_class;
edev->dev->release = extcon_dev_release;

- dev_set_name(edev->dev, edev->name ? edev->name : dev_name(dev));
+ edev->name = edev->name ? edev->name : dev_name(dev);
+ dev_set_name(edev->dev, edev->name);

if (edev->max_supported) {
char buf[10];
diff --git a/drivers/extcon/of_extcon.c b/drivers/extcon/of_extcon.c
new file mode 100644
index 0000000..72173ec
--- /dev/null
+++ b/drivers/extcon/of_extcon.c
@@ -0,0 +1,64 @@
+/*
+ * OF helpers for External connector (extcon) framework
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ * Kishon Vijay Abraham I <[email protected]>
+ *
+ * Copyright (C) 2013 Samsung Electronics
+ * Chanwoo Choi <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/extcon.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/extcon/of_extcon.h>
+
+/*
+ * of_extcon_get_extcon_dev - Get the name of extcon device from devicetree
+ * @dev - instance to the given device
+ * @index - index into list of extcon_dev
+ *
+ * return the instance of extcon device
+ */
+struct extcon_dev *of_extcon_get_extcon_dev(struct device *dev, int index)
+{
+ struct device_node *node;
+ struct extcon_dev *edev;
+ struct platform_device *extcon_parent_dev;
+
+ if (!dev->of_node) {
+ dev_dbg(dev, "device does not have a device node entry\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ node = of_parse_phandle(dev->of_node, "extcon", index);
+ if (!node) {
+ dev_dbg(dev, "failed to get phandle in %s node\n",
+ dev->of_node->full_name);
+ return ERR_PTR(-ENODEV);
+ }
+
+ extcon_parent_dev = of_find_device_by_node(node);
+ if (!extcon_parent_dev) {
+ dev_dbg(dev, "unable to find device by node\n");
+ return ERR_PTR(-EPROBE_DEFER);
+ }
+
+ edev = extcon_get_extcon_dev(dev_name(&extcon_parent_dev->dev));
+ if (!edev) {
+ dev_dbg(dev, "unable to get extcon device : %s\n",
+ dev_name(&extcon_parent_dev->dev));
+ return ERR_PTR(-ENODEV);
+ }
+
+ return edev;
+}
+EXPORT_SYMBOL_GPL(of_extcon_get_extcon_dev);
diff --git a/include/linux/extcon/of_extcon.h b/include/linux/extcon/of_extcon.h
new file mode 100644
index 0000000..0ebfeff
--- /dev/null
+++ b/include/linux/extcon/of_extcon.h
@@ -0,0 +1,31 @@
+/*
+ * OF helpers for External connector (extcon) framework
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ * Kishon Vijay Abraham I <[email protected]>
+ *
+ * Copyright (C) 2013 Samsung Electronics
+ * Chanwoo Choi <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __LINUX_OF_EXTCON_H
+#define __LINUX_OF_EXTCON_H
+
+#include <linux/err.h>
+
+#if IS_ENABLED(CONFIG_OF_EXTCON)
+extern struct extcon_dev
+ *of_extcon_get_extcon_dev(struct device *dev, int index);
+#else
+static inline struct extcon_dev
+ *of_extcon_get_extcon_dev(struct device *dev, int index)
+{
+ return ERR_PTR(-ENOSYS);
+}
+#endif /* CONFIG_OF_EXTCON */
+#endif /* __LINUX_OF_EXTCON_H */
--
1.8.0

2013-06-20 08:55:13

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH v4] usb: dwc3: use extcon fwrk to receive connect/disconnect

Hi,

On Monday 17 June 2013 09:39 AM, Chanwoo Choi wrote:
> On 06/14/2013 10:10 PM, Kishon Vijay Abraham I wrote:
>> Modified dwc3-omap to receive connect and disconnect notification using
>> extcon framework. Also did the necessary cleanups required after
>> adapting to extcon framework.
>>
>> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
>> Acked-by: Felipe Balbi <[email protected]>
>> ---
>> This patch depends on
>> git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git commit:f7ae906
>>
>> It should also be applied on top of
>> usb: dwc3: omap: improve error handling of dwc3_omap_probe patch which is
>> merged in Felipe's tree.
>>
>> So I'm not sure on whose tree this patch should go in.
>>
>> Changes from v3:
>> * did #include of of_extcon.h after Chanwoo resent the patch separating
>> extcon-class.c from of_extcon.c
>> Changes from v2:
>> * updated the Documentation with dwc3 dt binding information.
>> * used of_extcon_get_extcon_dev to get extcon device from device tree data.
>> Changes from v1:
>> * regulator enable/disable is now done here instead of palmas-usb as some users
>> of palmas-usb wont need regulator.
>>
>> Documentation/devicetree/bindings/usb/omap-usb.txt | 5 +
>> drivers/usb/dwc3/dwc3-omap.c | 119 ++++++++++++++++----
>> include/linux/usb/dwc3-omap.h | 30 -----
>> 3 files changed, 105 insertions(+), 49 deletions(-)
>> delete mode 100644 include/linux/usb/dwc3-omap.h
>>
>> diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt
>> index d4769f3..f1c15f3 100644
>> --- a/Documentation/devicetree/bindings/usb/omap-usb.txt
>> +++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
>> @@ -53,6 +53,11 @@ OMAP DWC3 GLUE
>> It should be set to "1" for HW mode and "2" for SW mode.
>> - ranges: the child address space are mapped 1:1 onto the parent address space
>>
>> +Optional Properties:
>> + - extcon : phandle for the extcon device omap dwc3 uses to detect
>> + connect/disconnect events.
>> + - vbus-supply : phandle to the regulator device tree node if needed.
>> +
>> Sub-nodes:
>> The dwc3 core should be added as subnode to omap dwc3 glue.
>> - dwc3 :
>> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
>> index f8f76e6..14c1f1b 100644
>> --- a/drivers/usb/dwc3/dwc3-omap.c
>> +++ b/drivers/usb/dwc3/dwc3-omap.c
>> @@ -43,13 +43,15 @@
>> #include <linux/spinlock.h>
>> #include <linux/platform_device.h>
>> #include <linux/platform_data/dwc3-omap.h>
>> -#include <linux/usb/dwc3-omap.h>
>> #include <linux/pm_runtime.h>
>> #include <linux/dma-mapping.h>
>> #include <linux/ioport.h>
>> #include <linux/io.h>
>> #include <linux/of.h>
>> #include <linux/of_platform.h>
>> +#include <linux/extcon.h>
>> +#include <linux/extcon/of_extcon.h>
>> +#include <linux/regulator/consumer.h>
>>
>> #include <linux/usb/otg.h>
>>
>> @@ -124,9 +126,21 @@ struct dwc3_omap {
>> u32 utmi_otg_status;
>>
>> u32 dma_status:1;
>> +
>> + struct extcon_specific_cable_nb extcon_vbus_dev;
>> + struct extcon_specific_cable_nb extcon_id_dev;
>> + struct notifier_block vbus_nb;
>> + struct notifier_block id_nb;
>> +
>> + struct regulator *vbus_reg;
>> };
>>
>> -static struct dwc3_omap *_omap;
>> +enum omap_dwc3_vbus_id_status {
>> + OMAP_DWC3_ID_FLOAT,
>> + OMAP_DWC3_ID_GROUND,
>> + OMAP_DWC3_VBUS_OFF,
>> + OMAP_DWC3_VBUS_VALID,
>> +};
>>
>> static inline u32 dwc3_omap_readl(void __iomem *base, u32 offset)
>> {
>> @@ -138,18 +152,23 @@ static inline void dwc3_omap_writel(void __iomem *base, u32 offset, u32 value)
>> writel(value, base + offset);
>> }
>>
>> -int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
>> +static void dwc3_omap_set_mailbox(struct dwc3_omap *omap,
>> + enum omap_dwc3_vbus_id_status status)
>> {
>> - u32 val;
>> - struct dwc3_omap *omap = _omap;
>> -
>> - if (!omap)
>> - return -EPROBE_DEFER;
>> + int ret;
>> + u32 val;
>>
>> switch (status) {
>> case OMAP_DWC3_ID_GROUND:
>> dev_dbg(omap->dev, "ID GND\n");
>>
>> + if (omap->vbus_reg) {
>> + ret = regulator_enable(omap->vbus_reg);
>> + if (ret) {
>> + dev_dbg(omap->dev, "regulator enable failed\n");
>> + return;
>> + }
>> + }
>> val = dwc3_omap_readl(omap->base, USBOTGSS_UTMI_OTG_STATUS);
>> val &= ~(USBOTGSS_UTMI_OTG_STATUS_IDDIG
>> | USBOTGSS_UTMI_OTG_STATUS_VBUSVALID
>> @@ -172,6 +191,9 @@ int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
>> break;
>>
>> case OMAP_DWC3_ID_FLOAT:
>> + if (omap->vbus_reg)
>> + regulator_disable(omap->vbus_reg);
>> +
>> case OMAP_DWC3_VBUS_OFF:
>> dev_dbg(omap->dev, "VBUS Disconnect\n");
>>
>> @@ -185,12 +207,9 @@ int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
>> break;
>>
>> default:
>> - dev_dbg(omap->dev, "ID float\n");
>> + dev_dbg(omap->dev, "invalid state\n");
>> }
>> -
>> - return 0;
>> }
>> -EXPORT_SYMBOL_GPL(dwc3_omap_mailbox);
>>
>> static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap)
>> {
>> @@ -282,6 +301,32 @@ static void dwc3_omap_disable_irqs(struct dwc3_omap *omap)
>>
>> static u64 dwc3_omap_dma_mask = DMA_BIT_MASK(32);
>>
>> +static int dwc3_omap_id_notifier(struct notifier_block *nb,
>> + unsigned long event, void *ptr)
>> +{
>> + struct dwc3_omap *omap = container_of(nb, struct dwc3_omap, id_nb);
>> +
>> + if (event)
>> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
>> + else
>> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_FLOAT);
>> +
>> + return NOTIFY_DONE;
>> +}
>> +
>> +static int dwc3_omap_vbus_notifier(struct notifier_block *nb,
>> + unsigned long event, void *ptr)
>> +{
>> + struct dwc3_omap *omap = container_of(nb, struct dwc3_omap, vbus_nb);
>> +
>> + if (event)
>> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
>> + else
>> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_OFF);
>> +
>> + return NOTIFY_DONE;
>> +}
>> +
>> static int dwc3_omap_probe(struct platform_device *pdev)
>> {
>> struct device_node *node = pdev->dev.of_node;
>> @@ -289,6 +334,8 @@ static int dwc3_omap_probe(struct platform_device *pdev)
>> struct dwc3_omap *omap;
>> struct resource *res;
>> struct device *dev = &pdev->dev;
>> + struct extcon_dev *edev;
>> + struct regulator *vbus_reg = NULL;
>>
>> int ret = -ENOMEM;
>> int irq;
>> @@ -330,19 +377,22 @@ static int dwc3_omap_probe(struct platform_device *pdev)
>> return -ENOMEM;
>> }
>>
>> + if (of_property_read_bool(node, "vbus-supply")) {
>> + vbus_reg = devm_regulator_get(dev, "vbus");
>> + if (IS_ERR(vbus_reg)) {
>> + dev_err(dev, "vbus init failed\n");
>> + return PTR_ERR(vbus_reg);
>> + }
>> + }
>> +
>> spin_lock_init(&omap->lock);
>>
>> omap->dev = dev;
>> omap->irq = irq;
>> omap->base = base;
>> + omap->vbus_reg = vbus_reg;
>> dev->dma_mask = &dwc3_omap_dma_mask;
>>
>> - /*
>> - * REVISIT if we ever have two instances of the wrapper, we will be
>> - * in big trouble
>> - */
>> - _omap = omap;
>> -
>> pm_runtime_enable(dev);
>> ret = pm_runtime_get_sync(dev);
>> if (ret < 0) {
>> @@ -381,14 +431,41 @@ static int dwc3_omap_probe(struct platform_device *pdev)
>>
>> dwc3_omap_enable_irqs(omap);
>>
>> + if (of_property_read_bool(node, "extcon")) {
>> + edev = of_extcon_get_extcon_dev(dev, 0);
>> + if (IS_ERR(edev)) {
>> + dev_vdbg(dev, "couldn't get extcon device\n");
>> + ret = PTR_ERR(edev);
>> + goto err2;
>> + }
>> +
>> + omap->vbus_nb.notifier_call = dwc3_omap_vbus_notifier;
>> + extcon_register_interest(&omap->extcon_vbus_dev, edev->name,
>> + "USB", &omap->vbus_nb);
>> + omap->id_nb.notifier_call = dwc3_omap_id_notifier;
>> + extcon_register_interest(&omap->extcon_id_dev, edev->name,
>> + "USB-HOST", &omap->id_nb);
>
> I prefer adding exception handling code about return value of extcon_register_interest().
>
>> +
>> + if (extcon_get_cable_state(edev, "USB") == true)
>> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
>> + if (extcon_get_cable_state(edev, "USB-HOST") == true)
>> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
>> + }
>> +
>> ret = of_platform_populate(node, NULL, NULL, dev);
>> if (ret) {
>> dev_err(&pdev->dev, "failed to create dwc3 core\n");
>> - goto err2;
>> + goto err3;
>> }
>>
>> return 0;
>>
>> +err3:
>> + if (omap->extcon_vbus_dev.edev)
>> + extcon_unregister_interest(&omap->extcon_vbus_dev);
>> + if (omap->extcon_id_dev.edev)
>> + extcon_unregister_interest(&omap->extcon_id_dev);
>> +
>> err2:
>> dwc3_omap_disable_irqs(omap);
>>
>> @@ -405,6 +482,10 @@ static int dwc3_omap_remove(struct platform_device *pdev)
>> {
>> struct dwc3_omap *omap = platform_get_drvdata(pdev);
>>
>> + if (omap->extcon_vbus_dev.edev)
>> + extcon_unregister_interest(&omap->extcon_vbus_dev);
>> + if (omap->extcon_id_dev.edev)
>> + extcon_unregister_interest(&omap->extcon_id_dev);
>> dwc3_omap_disable_irqs(omap);
>> pm_runtime_put_sync(&pdev->dev);
>> pm_runtime_disable(&pdev->dev);
>> diff --git a/include/linux/usb/dwc3-omap.h b/include/linux/usb/dwc3-omap.h
>> deleted file mode 100644
>> index 5615f4d..0000000
>>
>
> It looks good if you add exception handler about return value of extcon_register_interest().

Ok. I'll fix exception handler and also add depend on of EXTCON here.

Thanks
Kishon

2013-06-21 11:59:08

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: [PATCH v5] usb: dwc3: use extcon fwrk to receive connect/disconnect

Modified dwc3-omap to receive connect and disconnect notification using
extcon framework. Also did the necessary cleanups required after
adapting to extcon framework.

Signed-off-by: Kishon Vijay Abraham I <[email protected]>
Acked-by: Felipe Balbi <[email protected]>
Acked-by: Chanwoo Choi <[email protected]>
---
This patch should be applied after all of the extcon patchset will be applied
because this patch has dependency of extcon patch related to DT.
http://goo.gl/Tu3qW

Changes from v4:
* checked the return values of extcon_register_interest and print an error
message. Note that I dint do return since there might be cases where
one of USB (device mode) or USB-HOST (host mode) might succeed.
* Added depends on of EXTCON in usb_dwc3. Only some platforms might
be using EXTCON, but inorder to avoid compilation errors, added
depends on
Changes from v3:
* did #include of of_extcon.h after Chanwoo resent the patch separating
extcon-class.c from of_extcon.c
Changes from v2:
* updated the Documentation with dwc3 dt binding information.
* used of_extcon_get_extcon_dev to get extcon device from device tree data.
Changes from v1:
* regulator enable/disable is now done here instead of palmas-usb as some users
of palmas-usb wont need regulator.

Documentation/devicetree/bindings/usb/omap-usb.txt | 5 +
drivers/usb/dwc3/Kconfig | 1 +
drivers/usb/dwc3/dwc3-omap.c | 125 +++++++++++++++++---
include/linux/usb/dwc3-omap.h | 30 -----
4 files changed, 112 insertions(+), 49 deletions(-)
delete mode 100644 include/linux/usb/dwc3-omap.h

diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt
index d4769f3..f1c15f3 100644
--- a/Documentation/devicetree/bindings/usb/omap-usb.txt
+++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
@@ -53,6 +53,11 @@ OMAP DWC3 GLUE
It should be set to "1" for HW mode and "2" for SW mode.
- ranges: the child address space are mapped 1:1 onto the parent address space

+Optional Properties:
+ - extcon : phandle for the extcon device omap dwc3 uses to detect
+ connect/disconnect events.
+ - vbus-supply : phandle to the regulator device tree node if needed.
+
Sub-nodes:
The dwc3 core should be added as subnode to omap dwc3 glue.
- dwc3 :
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 757aa18..08a9fab 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -1,6 +1,7 @@
config USB_DWC3
tristate "DesignWare USB3 DRD Core Support"
depends on (USB || USB_GADGET) && GENERIC_HARDIRQS
+ depends on EXTCON
select USB_XHCI_PLATFORM if USB_SUPPORT && USB_XHCI_HCD
help
Say Y or M here if your system has a Dual Role SuperSpeed
diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index f8f76e6..80b5780 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -43,13 +43,15 @@
#include <linux/spinlock.h>
#include <linux/platform_device.h>
#include <linux/platform_data/dwc3-omap.h>
-#include <linux/usb/dwc3-omap.h>
#include <linux/pm_runtime.h>
#include <linux/dma-mapping.h>
#include <linux/ioport.h>
#include <linux/io.h>
#include <linux/of.h>
#include <linux/of_platform.h>
+#include <linux/extcon.h>
+#include <linux/extcon/of_extcon.h>
+#include <linux/regulator/consumer.h>

#include <linux/usb/otg.h>

@@ -124,9 +126,21 @@ struct dwc3_omap {
u32 utmi_otg_status;

u32 dma_status:1;
+
+ struct extcon_specific_cable_nb extcon_vbus_dev;
+ struct extcon_specific_cable_nb extcon_id_dev;
+ struct notifier_block vbus_nb;
+ struct notifier_block id_nb;
+
+ struct regulator *vbus_reg;
};

-static struct dwc3_omap *_omap;
+enum omap_dwc3_vbus_id_status {
+ OMAP_DWC3_ID_FLOAT,
+ OMAP_DWC3_ID_GROUND,
+ OMAP_DWC3_VBUS_OFF,
+ OMAP_DWC3_VBUS_VALID,
+};

static inline u32 dwc3_omap_readl(void __iomem *base, u32 offset)
{
@@ -138,18 +152,23 @@ static inline void dwc3_omap_writel(void __iomem *base, u32 offset, u32 value)
writel(value, base + offset);
}

-int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
+static void dwc3_omap_set_mailbox(struct dwc3_omap *omap,
+ enum omap_dwc3_vbus_id_status status)
{
- u32 val;
- struct dwc3_omap *omap = _omap;
-
- if (!omap)
- return -EPROBE_DEFER;
+ int ret;
+ u32 val;

switch (status) {
case OMAP_DWC3_ID_GROUND:
dev_dbg(omap->dev, "ID GND\n");

+ if (omap->vbus_reg) {
+ ret = regulator_enable(omap->vbus_reg);
+ if (ret) {
+ dev_dbg(omap->dev, "regulator enable failed\n");
+ return;
+ }
+ }
val = dwc3_omap_readl(omap->base, USBOTGSS_UTMI_OTG_STATUS);
val &= ~(USBOTGSS_UTMI_OTG_STATUS_IDDIG
| USBOTGSS_UTMI_OTG_STATUS_VBUSVALID
@@ -172,6 +191,9 @@ int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
break;

case OMAP_DWC3_ID_FLOAT:
+ if (omap->vbus_reg)
+ regulator_disable(omap->vbus_reg);
+
case OMAP_DWC3_VBUS_OFF:
dev_dbg(omap->dev, "VBUS Disconnect\n");

@@ -185,12 +207,9 @@ int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
break;

default:
- dev_dbg(omap->dev, "ID float\n");
+ dev_dbg(omap->dev, "invalid state\n");
}
-
- return 0;
}
-EXPORT_SYMBOL_GPL(dwc3_omap_mailbox);

static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap)
{
@@ -282,6 +301,32 @@ static void dwc3_omap_disable_irqs(struct dwc3_omap *omap)

static u64 dwc3_omap_dma_mask = DMA_BIT_MASK(32);

+static int dwc3_omap_id_notifier(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct dwc3_omap *omap = container_of(nb, struct dwc3_omap, id_nb);
+
+ if (event)
+ dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
+ else
+ dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_FLOAT);
+
+ return NOTIFY_DONE;
+}
+
+static int dwc3_omap_vbus_notifier(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct dwc3_omap *omap = container_of(nb, struct dwc3_omap, vbus_nb);
+
+ if (event)
+ dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
+ else
+ dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_OFF);
+
+ return NOTIFY_DONE;
+}
+
static int dwc3_omap_probe(struct platform_device *pdev)
{
struct device_node *node = pdev->dev.of_node;
@@ -289,6 +334,8 @@ static int dwc3_omap_probe(struct platform_device *pdev)
struct dwc3_omap *omap;
struct resource *res;
struct device *dev = &pdev->dev;
+ struct extcon_dev *edev;
+ struct regulator *vbus_reg = NULL;

int ret = -ENOMEM;
int irq;
@@ -330,19 +377,22 @@ static int dwc3_omap_probe(struct platform_device *pdev)
return -ENOMEM;
}

+ if (of_property_read_bool(node, "vbus-supply")) {
+ vbus_reg = devm_regulator_get(dev, "vbus");
+ if (IS_ERR(vbus_reg)) {
+ dev_err(dev, "vbus init failed\n");
+ return PTR_ERR(vbus_reg);
+ }
+ }
+
spin_lock_init(&omap->lock);

omap->dev = dev;
omap->irq = irq;
omap->base = base;
+ omap->vbus_reg = vbus_reg;
dev->dma_mask = &dwc3_omap_dma_mask;

- /*
- * REVISIT if we ever have two instances of the wrapper, we will be
- * in big trouble
- */
- _omap = omap;
-
pm_runtime_enable(dev);
ret = pm_runtime_get_sync(dev);
if (ret < 0) {
@@ -381,14 +431,47 @@ static int dwc3_omap_probe(struct platform_device *pdev)

dwc3_omap_enable_irqs(omap);

+ if (of_property_read_bool(node, "extcon")) {
+ edev = of_extcon_get_extcon_dev(dev, 0);
+ if (IS_ERR(edev)) {
+ dev_vdbg(dev, "couldn't get extcon device\n");
+ ret = PTR_ERR(edev);
+ goto err2;
+ }
+
+ omap->vbus_nb.notifier_call = dwc3_omap_vbus_notifier;
+ ret = extcon_register_interest(&omap->extcon_vbus_dev,
+ edev->name, "USB", &omap->vbus_nb);
+ if (ret < 0)
+ dev_vdbg(dev,
+ "extcon register interest of *USB* failed\n");
+ omap->id_nb.notifier_call = dwc3_omap_id_notifier;
+ ret = extcon_register_interest(&omap->extcon_id_dev, edev->name,
+ "USB-HOST", &omap->id_nb);
+ if (ret < 0)
+ dev_vdbg(dev,
+ "extcon register interest of *USB-HOST* failed\n");
+
+ if (extcon_get_cable_state(edev, "USB") == true)
+ dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
+ if (extcon_get_cable_state(edev, "USB-HOST") == true)
+ dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
+ }
+
ret = of_platform_populate(node, NULL, NULL, dev);
if (ret) {
dev_err(&pdev->dev, "failed to create dwc3 core\n");
- goto err2;
+ goto err3;
}

return 0;

+err3:
+ if (omap->extcon_vbus_dev.edev)
+ extcon_unregister_interest(&omap->extcon_vbus_dev);
+ if (omap->extcon_id_dev.edev)
+ extcon_unregister_interest(&omap->extcon_id_dev);
+
err2:
dwc3_omap_disable_irqs(omap);

@@ -405,6 +488,10 @@ static int dwc3_omap_remove(struct platform_device *pdev)
{
struct dwc3_omap *omap = platform_get_drvdata(pdev);

+ if (omap->extcon_vbus_dev.edev)
+ extcon_unregister_interest(&omap->extcon_vbus_dev);
+ if (omap->extcon_id_dev.edev)
+ extcon_unregister_interest(&omap->extcon_id_dev);
dwc3_omap_disable_irqs(omap);
pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
diff --git a/include/linux/usb/dwc3-omap.h b/include/linux/usb/dwc3-omap.h
deleted file mode 100644
index 5615f4d..0000000
--
1.7.10.4

2013-06-24 11:12:51

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH v5] usb: dwc3: use extcon fwrk to receive connect/disconnect

On 06/21/2013 08:58 PM, Kishon Vijay Abraham I wrote:
> Modified dwc3-omap to receive connect and disconnect notification using
> extcon framework. Also did the necessary cleanups required after
> adapting to extcon framework.
>
> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> Acked-by: Felipe Balbi <[email protected]>
> Acked-by: Chanwoo Choi <[email protected]>
> ---
> This patch should be applied after all of the extcon patchset will be applied
> because this patch has dependency of extcon patch related to DT.
> http://goo.gl/Tu3qW
>
> Changes from v4:
> * checked the return values of extcon_register_interest and print an error
> message. Note that I dint do return since there might be cases where
> one of USB (device mode) or USB-HOST (host mode) might succeed.
> * Added depends on of EXTCON in usb_dwc3. Only some platforms might
> be using EXTCON, but inorder to avoid compilation errors, added
> depends on
> Changes from v3:
> * did #include of of_extcon.h after Chanwoo resent the patch separating
> extcon-class.c from of_extcon.c
> Changes from v2:
> * updated the Documentation with dwc3 dt binding information.
> * used of_extcon_get_extcon_dev to get extcon device from device tree data.
> Changes from v1:
> * regulator enable/disable is now done here instead of palmas-usb as some users
> of palmas-usb wont need regulator.
>
> Documentation/devicetree/bindings/usb/omap-usb.txt | 5 +
> drivers/usb/dwc3/Kconfig | 1 +
> drivers/usb/dwc3/dwc3-omap.c | 125 +++++++++++++++++---
> include/linux/usb/dwc3-omap.h | 30 -----
> 4 files changed, 112 insertions(+), 49 deletions(-)
> delete mode 100644 include/linux/usb/dwc3-omap.h
>
> diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt
> index d4769f3..f1c15f3 100644
> --- a/Documentation/devicetree/bindings/usb/omap-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
> @@ -53,6 +53,11 @@ OMAP DWC3 GLUE
> It should be set to "1" for HW mode and "2" for SW mode.
> - ranges: the child address space are mapped 1:1 onto the parent address space
>
> +Optional Properties:
> + - extcon : phandle for the extcon device omap dwc3 uses to detect
> + connect/disconnect events.
> + - vbus-supply : phandle to the regulator device tree node if needed.
> +
> Sub-nodes:
> The dwc3 core should be added as subnode to omap dwc3 glue.
> - dwc3 :
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 757aa18..08a9fab 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -1,6 +1,7 @@
> config USB_DWC3
> tristate "DesignWare USB3 DRD Core Support"
> depends on (USB || USB_GADGET) && GENERIC_HARDIRQS
> + depends on EXTCON
> select USB_XHCI_PLATFORM if USB_SUPPORT && USB_XHCI_HCD
> help
> Say Y or M here if your system has a Dual Role SuperSpeed
> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
> index f8f76e6..80b5780 100644
> --- a/drivers/usb/dwc3/dwc3-omap.c
> +++ b/drivers/usb/dwc3/dwc3-omap.c
> @@ -43,13 +43,15 @@
> #include <linux/spinlock.h>
> #include <linux/platform_device.h>
> #include <linux/platform_data/dwc3-omap.h>
> -#include <linux/usb/dwc3-omap.h>
> #include <linux/pm_runtime.h>
> #include <linux/dma-mapping.h>
> #include <linux/ioport.h>
> #include <linux/io.h>
> #include <linux/of.h>
> #include <linux/of_platform.h>
> +#include <linux/extcon.h>
> +#include <linux/extcon/of_extcon.h>
> +#include <linux/regulator/consumer.h>
>
> #include <linux/usb/otg.h>
>
> @@ -124,9 +126,21 @@ struct dwc3_omap {
> u32 utmi_otg_status;
>
> u32 dma_status:1;
> +
> + struct extcon_specific_cable_nb extcon_vbus_dev;
> + struct extcon_specific_cable_nb extcon_id_dev;
> + struct notifier_block vbus_nb;
> + struct notifier_block id_nb;
> +
> + struct regulator *vbus_reg;
> };
>
> -static struct dwc3_omap *_omap;
> +enum omap_dwc3_vbus_id_status {
> + OMAP_DWC3_ID_FLOAT,
> + OMAP_DWC3_ID_GROUND,
> + OMAP_DWC3_VBUS_OFF,
> + OMAP_DWC3_VBUS_VALID,
> +};
>
> static inline u32 dwc3_omap_readl(void __iomem *base, u32 offset)
> {
> @@ -138,18 +152,23 @@ static inline void dwc3_omap_writel(void __iomem *base, u32 offset, u32 value)
> writel(value, base + offset);
> }
>
> -int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
> +static void dwc3_omap_set_mailbox(struct dwc3_omap *omap,
> + enum omap_dwc3_vbus_id_status status)
> {
> - u32 val;
> - struct dwc3_omap *omap = _omap;
> -
> - if (!omap)
> - return -EPROBE_DEFER;
> + int ret;
> + u32 val;
>
> switch (status) {
> case OMAP_DWC3_ID_GROUND:
> dev_dbg(omap->dev, "ID GND\n");
>
> + if (omap->vbus_reg) {
> + ret = regulator_enable(omap->vbus_reg);
> + if (ret) {
> + dev_dbg(omap->dev, "regulator enable failed\n");
> + return;
> + }
> + }
> val = dwc3_omap_readl(omap->base, USBOTGSS_UTMI_OTG_STATUS);
> val &= ~(USBOTGSS_UTMI_OTG_STATUS_IDDIG
> | USBOTGSS_UTMI_OTG_STATUS_VBUSVALID
> @@ -172,6 +191,9 @@ int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
> break;
>
> case OMAP_DWC3_ID_FLOAT:
> + if (omap->vbus_reg)
> + regulator_disable(omap->vbus_reg);
> +
> case OMAP_DWC3_VBUS_OFF:
> dev_dbg(omap->dev, "VBUS Disconnect\n");
>
> @@ -185,12 +207,9 @@ int dwc3_omap_mailbox(enum omap_dwc3_vbus_id_status status)
> break;
>
> default:
> - dev_dbg(omap->dev, "ID float\n");
> + dev_dbg(omap->dev, "invalid state\n");
> }
> -
> - return 0;
> }
> -EXPORT_SYMBOL_GPL(dwc3_omap_mailbox);
>
> static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap)
> {
> @@ -282,6 +301,32 @@ static void dwc3_omap_disable_irqs(struct dwc3_omap *omap)
>
> static u64 dwc3_omap_dma_mask = DMA_BIT_MASK(32);
>
> +static int dwc3_omap_id_notifier(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
> + struct dwc3_omap *omap = container_of(nb, struct dwc3_omap, id_nb);
> +
> + if (event)
> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
> + else
> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_FLOAT);
> +
> + return NOTIFY_DONE;
> +}
> +
> +static int dwc3_omap_vbus_notifier(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
> + struct dwc3_omap *omap = container_of(nb, struct dwc3_omap, vbus_nb);
> +
> + if (event)
> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
> + else
> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_OFF);
> +
> + return NOTIFY_DONE;
> +}
> +
> static int dwc3_omap_probe(struct platform_device *pdev)
> {
> struct device_node *node = pdev->dev.of_node;
> @@ -289,6 +334,8 @@ static int dwc3_omap_probe(struct platform_device *pdev)
> struct dwc3_omap *omap;
> struct resource *res;
> struct device *dev = &pdev->dev;
> + struct extcon_dev *edev;
> + struct regulator *vbus_reg = NULL;
>
> int ret = -ENOMEM;
> int irq;
> @@ -330,19 +377,22 @@ static int dwc3_omap_probe(struct platform_device *pdev)
> return -ENOMEM;
> }
>
> + if (of_property_read_bool(node, "vbus-supply")) {
> + vbus_reg = devm_regulator_get(dev, "vbus");
> + if (IS_ERR(vbus_reg)) {
> + dev_err(dev, "vbus init failed\n");
> + return PTR_ERR(vbus_reg);
> + }
> + }
> +
> spin_lock_init(&omap->lock);
>
> omap->dev = dev;
> omap->irq = irq;
> omap->base = base;
> + omap->vbus_reg = vbus_reg;
> dev->dma_mask = &dwc3_omap_dma_mask;
>
> - /*
> - * REVISIT if we ever have two instances of the wrapper, we will be
> - * in big trouble
> - */
> - _omap = omap;
> -
> pm_runtime_enable(dev);
> ret = pm_runtime_get_sync(dev);
> if (ret < 0) {
> @@ -381,14 +431,47 @@ static int dwc3_omap_probe(struct platform_device *pdev)
>
> dwc3_omap_enable_irqs(omap);
>
> + if (of_property_read_bool(node, "extcon")) {
> + edev = of_extcon_get_extcon_dev(dev, 0);
> + if (IS_ERR(edev)) {
> + dev_vdbg(dev, "couldn't get extcon device\n");
> + ret = PTR_ERR(edev);
> + goto err2;
> + }
> +
> + omap->vbus_nb.notifier_call = dwc3_omap_vbus_notifier;
> + ret = extcon_register_interest(&omap->extcon_vbus_dev,
> + edev->name, "USB", &omap->vbus_nb);
> + if (ret < 0)
> + dev_vdbg(dev,
> + "extcon register interest of *USB* failed\n");
> + omap->id_nb.notifier_call = dwc3_omap_id_notifier;
> + ret = extcon_register_interest(&omap->extcon_id_dev, edev->name,
> + "USB-HOST", &omap->id_nb);
> + if (ret < 0)
> + dev_vdbg(dev,
> + "extcon register interest of *USB-HOST* failed\n");
> +
> + if (extcon_get_cable_state(edev, "USB") == true)
> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
> + if (extcon_get_cable_state(edev, "USB-HOST") == true)
> + dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
> + }
> +
> ret = of_platform_populate(node, NULL, NULL, dev);
> if (ret) {
> dev_err(&pdev->dev, "failed to create dwc3 core\n");
> - goto err2;
> + goto err3;
> }
>
> return 0;
>
> +err3:
> + if (omap->extcon_vbus_dev.edev)
> + extcon_unregister_interest(&omap->extcon_vbus_dev);
> + if (omap->extcon_id_dev.edev)
> + extcon_unregister_interest(&omap->extcon_id_dev);
> +
> err2:
> dwc3_omap_disable_irqs(omap);
>
> @@ -405,6 +488,10 @@ static int dwc3_omap_remove(struct platform_device *pdev)
> {
> struct dwc3_omap *omap = platform_get_drvdata(pdev);
>
> + if (omap->extcon_vbus_dev.edev)
> + extcon_unregister_interest(&omap->extcon_vbus_dev);
> + if (omap->extcon_id_dev.edev)
> + extcon_unregister_interest(&omap->extcon_id_dev);
> dwc3_omap_disable_irqs(omap);
> pm_runtime_put_sync(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
> diff --git a/include/linux/usb/dwc3-omap.h b/include/linux/usb/dwc3-omap.h
> deleted file mode 100644
> index 5615f4d..0000000
>

I have below some fixes for indentation and applied it (extcon-for-palmas branch).
- dev_vdbg(dev,
- "extcon register interest of *USB* failed\n");
+ dev_vdbg(dev, "failed to register notifier for USB\n");
omap->id_nb.notifier_call = dwc3_omap_id_notifier;
ret = extcon_register_interest(&omap->extcon_id_dev, edev->name,
"USB-HOST", &omap->id_nb);
if (ret < 0)
dev_vdbg(dev,
- "extcon register interest of *USB-HOST* failed\n");
+ "failed to register notifier for USB-HOST\n");

if (extcon_get_cable_state(edev, "USB") == true)
dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);

You can check it on following git repository.
- http://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/commit/?h=extcon-for-palmas&id=bd84f70d42b867dfce61f961fb1c50d22e3ab4a8

And, I have plan to move your patch from extcon-for-palmas to extcon-next branch
after below patch on usb.git is applied on v3.11

usb: dwc3: omap: improve error handling of dwc3_omap_probe
- https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=next&id=594daba1bcb0510cdc9dccfbab9e6fd5d9cc94e6
- git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git (next branch)


Thanks,
Chanwoo Choi