[PATCH v4] USB: add devicetree helpers for determining dr_mode and phy_type

[PATCH v4] USB: add devicetree helpers for determining dr_mode and phy_type

[Sascha Hauer]

4th round of patches.

Peter, I would be glad if you could test them before your holiday. I rebased
your last round of Chipidea OTG patches onto this series which you can pull
here:

git://git.pengutronix.de/git/imx/linux-2.6.git tags/usb-chipidea-otg-for-next

I couldn't really test the otg patches since my current hardware does not have
the ID pin connected, but I can verify that my usecase still works with your
patches applied.

Alex, should the patches work for you and are fine otherwise, could you apply
them for v3.9?

Sascha



changes since v3:

- add phy patches (which were accidently already part of v2)
- Use OP_DEVLC instead of OP_PORTSC for lpm case
- Use enum usb_dr_mode ub ci_hdrc_probe()

changes since v2:

- fix adding of GPL Header was in wrong patch
- add missing hunk for new file of.c

changes since v1:
- move phy specific of helper to drivers/usb/phy/of.c
- use strcmp instead of strcasecmp for matching property values
- change usb_phy_dr_mode to usb_dr_mode
- change USBPHY_INTERFACE_MODE_NA to USBPHY_INTERFACE_MODE_UNKNOWN
- add copyright header to new files
- chipidea: drop mdelay at end of PTS/PTW setup
- chipidea: implement lpm core type handling for PTS/PTW


The following changes since commit 2f0760774711c957c395b31131b848043af98edf:

  USB: GADGET: optionally force full-speed for net2280 UDC (2013-01-31 10:09:19 +0100)

are available in the git repository at:

  git://git.pengutronix.de/git/imx/linux-2.6.git tags/usb-chipidea-for-next

for you to fetch changes up to 25682afd7be85f1462647d8530dca1bf848074fc:

  USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy (2013-02-04 12:28:53 +0100)

----------------------------------------------------------------
USB: chipidea patches for v3.9

These add OF helpers for handling the dr_mode and phy_type property
and makes use of them in the chipidea driver.

----------------------------------------------------------------
Marc Kleine-Budde (1):
      usb: otg: use try_module_get in all usb_get_phy functions and add missing module_put

Michael Grzeschik (3):
      USB: add devicetree helpers for determining dr_mode and phy_type
      USB: chipidea: ci13xxx-imx: create dynamic platformdata
      USB: chipidea: add PTW and PTS handling

Sascha Hauer (5):
      USB: move bulk of otg/otg.c to phy/phy.c
      USB chipidea: introduce dual role mode pdata flags
      USB chipidea i.MX: introduce dr_mode property
      USB mxs-phy: Register phy with framework
      USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy

 .../devicetree/bindings/usb/ci13xxx-imx.txt        |    6 +
 drivers/usb/chipidea/bits.h                        |   14 +-
 drivers/usb/chipidea/ci13xxx_imx.c                 |   68 +--
 drivers/usb/chipidea/core.c                        |   60 ++-
 drivers/usb/otg/mxs-phy.c                          |    9 +
 drivers/usb/otg/otg.c                              |  423 -------------------
 drivers/usb/phy/Makefile                           |    2 +
 drivers/usb/phy/of.c                               |   47 +++
 drivers/usb/phy/phy.c                              |  438 ++++++++++++++++++++
 drivers/usb/usb-common.c                           |   36 ++
 include/linux/usb/chipidea.h                       |    3 +-
 include/linux/usb/of.h                             |   27 ++
 include/linux/usb/otg.h                            |    7 +
 include/linux/usb/phy.h                            |    9 +
 14 files changed, 687 insertions(+), 462 deletions(-)
 create mode 100644 drivers/usb/phy/of.c
 create mode 100644 drivers/usb/phy/phy.c
 create mode 100644 include/linux/usb/of.h

[PATCH 1/9] usb: otg: use try_module_get in all usb_get_phy functions and add missing module_put

[Sascha Hauer]

From: Marc Kleine-Budde <mkl@pengutronix.de>

In patch "5d3c28b usb: otg: add device tree support to otg library"
devm_usb_get_phy_by_phandle() was added. It uses try_module_get() to lock the
phy driver in memory. The corresponding module_put() is missing in that patch.

This patch adds try_module_get() to usb_get_phy() and usb_get_phy_dev().
Further the missing module_put() is added to usb_put_phy().

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/usb/otg/otg.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
index e181439..2bd03d2 100644
--- a/drivers/usb/otg/otg.c
+++ b/drivers/usb/otg/otg.c
@@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
 	spin_lock_irqsave(&phy_lock, flags);
 
 	phy = __usb_find_phy(&phy_list, type);
-	if (IS_ERR(phy)) {
+	if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
 		pr_err("unable to find transceiver of type %s\n",
 			usb_phy_type_string(type));
 		goto err0;
@@ -228,7 +228,7 @@ struct usb_phy *usb_get_phy_dev(struct device *dev, u8 index)
 	spin_lock_irqsave(&phy_lock, flags);
 
 	phy = __usb_find_phy_dev(dev, &phy_bind_list, index);
-	if (IS_ERR(phy)) {
+	if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
 		pr_err("unable to find transceiver\n");
 		goto err0;
 	}
@@ -301,8 +301,12 @@ EXPORT_SYMBOL(devm_usb_put_phy);
  */
 void usb_put_phy(struct usb_phy *x)
 {
-	if (x)
+	if (x) {
+		struct module *owner = x->dev->driver->owner;
+
 		put_device(x->dev);
+		module_put(owner);
+	}
 }
 EXPORT_SYMBOL(usb_put_phy);
 
-- 
1.7.10.4

[PATCH 2/9] USB: move bulk of otg/otg.c to phy/phy.c

[Sascha Hauer]

Most of otg/otg.c is not otg specific, but phy specific, so move it
to the phy directory.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reported-by: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/otg/otg.c    |  427 --------------------------------------------
 drivers/usb/phy/Makefile |    1 +
 drivers/usb/phy/phy.c    |  438 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 439 insertions(+), 427 deletions(-)
 create mode 100644 drivers/usb/phy/phy.c

diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
index 2bd03d2..358cfd9 100644
--- a/drivers/usb/otg/otg.c
+++ b/drivers/usb/otg/otg.c
@@ -8,436 +8,9 @@
  * the Free Software Foundation; either version 2 of the License, or
  * (at your option) any later version.
  */
-
-#include <linux/kernel.h>
 #include <linux/export.h>
-#include <linux/err.h>
-#include <linux/device.h>
-#include <linux/module.h>
-#include <linux/slab.h>
-#include <linux/of.h>
-
 #include <linux/usb/otg.h>
 
-static LIST_HEAD(phy_list);
-static LIST_HEAD(phy_bind_list);
-static DEFINE_SPINLOCK(phy_lock);
-
-static struct usb_phy *__usb_find_phy(struct list_head *list,
-	enum usb_phy_type type)
-{
-	struct usb_phy  *phy = NULL;
-
-	list_for_each_entry(phy, list, head) {
-		if (phy->type != type)
-			continue;
-
-		return phy;
-	}
-
-	return ERR_PTR(-ENODEV);
-}
-
-static struct usb_phy *__usb_find_phy_dev(struct device *dev,
-	struct list_head *list, u8 index)
-{
-	struct usb_phy_bind *phy_bind = NULL;
-
-	list_for_each_entry(phy_bind, list, list) {
-		if (!(strcmp(phy_bind->dev_name, dev_name(dev))) &&
-				phy_bind->index == index) {
-			if (phy_bind->phy)
-				return phy_bind->phy;
-			else
-				return ERR_PTR(-EPROBE_DEFER);
-		}
-	}
-
-	return ERR_PTR(-ENODEV);
-}
-
-static struct usb_phy *__of_usb_find_phy(struct device_node *node)
-{
-	struct usb_phy  *phy;
-
-	list_for_each_entry(phy, &phy_list, head) {
-		if (node != phy->dev->of_node)
-			continue;
-
-		return phy;
-	}
-
-	return ERR_PTR(-ENODEV);
-}
-
-static void devm_usb_phy_release(struct device *dev, void *res)
-{
-	struct usb_phy *phy = *(struct usb_phy **)res;
-
-	usb_put_phy(phy);
-}
-
-static int devm_usb_phy_match(struct device *dev, void *res, void *match_data)
-{
-	return res == match_data;
-}
-
-/**
- * devm_usb_get_phy - find the USB PHY
- * @dev - device that requests this phy
- * @type - the type of the phy the controller requires
- *
- * Gets the phy using usb_get_phy(), and associates a device with it using
- * devres. On driver detach, release function is invoked on the devres data,
- * then, devres data is freed.
- *
- * For use by USB host and peripheral drivers.
- */
-struct usb_phy *devm_usb_get_phy(struct device *dev, enum usb_phy_type type)
-{
-	struct usb_phy **ptr, *phy;
-
-	ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL);
-	if (!ptr)
-		return NULL;
-
-	phy = usb_get_phy(type);
-	if (!IS_ERR(phy)) {
-		*ptr = phy;
-		devres_add(dev, ptr);
-	} else
-		devres_free(ptr);
-
-	return phy;
-}
-EXPORT_SYMBOL(devm_usb_get_phy);
-
-/**
- * usb_get_phy - find the USB PHY
- * @type - the type of the phy the controller requires
- *
- * Returns the phy driver, after getting a refcount to it; or
- * -ENODEV if there is no such phy.  The caller is responsible for
- * calling usb_put_phy() to release that count.
- *
- * For use by USB host and peripheral drivers.
- */
-struct usb_phy *usb_get_phy(enum usb_phy_type type)
-{
-	struct usb_phy	*phy = NULL;
-	unsigned long	flags;
-
-	spin_lock_irqsave(&phy_lock, flags);
-
-	phy = __usb_find_phy(&phy_list, type);
-	if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
-		pr_err("unable to find transceiver of type %s\n",
-			usb_phy_type_string(type));
-		goto err0;
-	}
-
-	get_device(phy->dev);
-
-err0:
-	spin_unlock_irqrestore(&phy_lock, flags);
-
-	return phy;
-}
-EXPORT_SYMBOL(usb_get_phy);
-
- /**
- * devm_usb_get_phy_by_phandle - find the USB PHY by phandle
- * @dev - device that requests this phy
- * @phandle - name of the property holding the phy phandle value
- * @index - the index of the phy
- *
- * Returns the phy driver associated with the given phandle value,
- * after getting a refcount to it, -ENODEV if there is no such phy or
- * -EPROBE_DEFER if there is a phandle to the phy, but the device is
- * not yet loaded. While at that, it also associates the device with
- * the phy using devres. On driver detach, release function is invoked
- * on the devres data, then, devres data is freed.
- *
- * For use by USB host and peripheral drivers.
- */
-struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
-	const char *phandle, u8 index)
-{
-	struct usb_phy	*phy = ERR_PTR(-ENOMEM), **ptr;
-	unsigned long	flags;
-	struct device_node *node;
-
-	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, phandle, index);
-	if (!node) {
-		dev_dbg(dev, "failed to get %s phandle in %s node\n", phandle,
-			dev->of_node->full_name);
-		return ERR_PTR(-ENODEV);
-	}
-
-	ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL);
-	if (!ptr) {
-		dev_dbg(dev, "failed to allocate memory for devres\n");
-		goto err0;
-	}
-
-	spin_lock_irqsave(&phy_lock, flags);
-
-	phy = __of_usb_find_phy(node);
-	if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
-		phy = ERR_PTR(-EPROBE_DEFER);
-		devres_free(ptr);
-		goto err1;
-	}
-
-	*ptr = phy;
-	devres_add(dev, ptr);
-
-	get_device(phy->dev);
-
-err1:
-	spin_unlock_irqrestore(&phy_lock, flags);
-
-err0:
-	of_node_put(node);
-
-	return phy;
-}
-EXPORT_SYMBOL(devm_usb_get_phy_by_phandle);
-
-/**
- * usb_get_phy_dev - find the USB PHY
- * @dev - device that requests this phy
- * @index - the index of the phy
- *
- * Returns the phy driver, after getting a refcount to it; or
- * -ENODEV if there is no such phy.  The caller is responsible for
- * calling usb_put_phy() to release that count.
- *
- * For use by USB host and peripheral drivers.
- */
-struct usb_phy *usb_get_phy_dev(struct device *dev, u8 index)
-{
-	struct usb_phy	*phy = NULL;
-	unsigned long	flags;
-
-	spin_lock_irqsave(&phy_lock, flags);
-
-	phy = __usb_find_phy_dev(dev, &phy_bind_list, index);
-	if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
-		pr_err("unable to find transceiver\n");
-		goto err0;
-	}
-
-	get_device(phy->dev);
-
-err0:
-	spin_unlock_irqrestore(&phy_lock, flags);
-
-	return phy;
-}
-EXPORT_SYMBOL(usb_get_phy_dev);
-
-/**
- * devm_usb_get_phy_dev - find the USB PHY using device ptr and index
- * @dev - device that requests this phy
- * @index - the index of the phy
- *
- * Gets the phy using usb_get_phy_dev(), and associates a device with it using
- * devres. On driver detach, release function is invoked on the devres data,
- * then, devres data is freed.
- *
- * For use by USB host and peripheral drivers.
- */
-struct usb_phy *devm_usb_get_phy_dev(struct device *dev, u8 index)
-{
-	struct usb_phy **ptr, *phy;
-
-	ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL);
-	if (!ptr)
-		return NULL;
-
-	phy = usb_get_phy_dev(dev, index);
-	if (!IS_ERR(phy)) {
-		*ptr = phy;
-		devres_add(dev, ptr);
-	} else
-		devres_free(ptr);
-
-	return phy;
-}
-EXPORT_SYMBOL(devm_usb_get_phy_dev);
-
-/**
- * devm_usb_put_phy - release the USB PHY
- * @dev - device that wants to release this phy
- * @phy - the phy returned by devm_usb_get_phy()
- *
- * destroys the devres associated with this phy and invokes usb_put_phy
- * to release the phy.
- *
- * For use by USB host and peripheral drivers.
- */
-void devm_usb_put_phy(struct device *dev, struct usb_phy *phy)
-{
-	int r;
-
-	r = devres_destroy(dev, devm_usb_phy_release, devm_usb_phy_match, phy);
-	dev_WARN_ONCE(dev, r, "couldn't find PHY resource\n");
-}
-EXPORT_SYMBOL(devm_usb_put_phy);
-
-/**
- * usb_put_phy - release the USB PHY
- * @x: the phy returned by usb_get_phy()
- *
- * Releases a refcount the caller received from usb_get_phy().
- *
- * For use by USB host and peripheral drivers.
- */
-void usb_put_phy(struct usb_phy *x)
-{
-	if (x) {
-		struct module *owner = x->dev->driver->owner;
-
-		put_device(x->dev);
-		module_put(owner);
-	}
-}
-EXPORT_SYMBOL(usb_put_phy);
-
-/**
- * usb_add_phy - declare the USB PHY
- * @x: the USB phy to be used; or NULL
- * @type - the type of this PHY
- *
- * This call is exclusively for use by phy drivers, which
- * coordinate the activities of drivers for host and peripheral
- * controllers, and in some cases for VBUS current regulation.
- */
-int usb_add_phy(struct usb_phy *x, enum usb_phy_type type)
-{
-	int		ret = 0;
-	unsigned long	flags;
-	struct usb_phy	*phy;
-
-	if (x->type != USB_PHY_TYPE_UNDEFINED) {
-		dev_err(x->dev, "not accepting initialized PHY %s\n", x->label);
-		return -EINVAL;
-	}
-
-	spin_lock_irqsave(&phy_lock, flags);
-
-	list_for_each_entry(phy, &phy_list, head) {
-		if (phy->type == type) {
-			ret = -EBUSY;
-			dev_err(x->dev, "transceiver type %s already exists\n",
-						usb_phy_type_string(type));
-			goto out;
-		}
-	}
-
-	x->type = type;
-	list_add_tail(&x->head, &phy_list);
-
-out:
-	spin_unlock_irqrestore(&phy_lock, flags);
-	return ret;
-}
-EXPORT_SYMBOL(usb_add_phy);
-
-/**
- * usb_add_phy_dev - declare the USB PHY
- * @x: the USB phy to be used; or NULL
- *
- * This call is exclusively for use by phy drivers, which
- * coordinate the activities of drivers for host and peripheral
- * controllers, and in some cases for VBUS current regulation.
- */
-int usb_add_phy_dev(struct usb_phy *x)
-{
-	struct usb_phy_bind *phy_bind;
-	unsigned long flags;
-
-	if (!x->dev) {
-		dev_err(x->dev, "no device provided for PHY\n");
-		return -EINVAL;
-	}
-
-	spin_lock_irqsave(&phy_lock, flags);
-	list_for_each_entry(phy_bind, &phy_bind_list, list)
-		if (!(strcmp(phy_bind->phy_dev_name, dev_name(x->dev))))
-			phy_bind->phy = x;
-
-	list_add_tail(&x->head, &phy_list);
-
-	spin_unlock_irqrestore(&phy_lock, flags);
-	return 0;
-}
-EXPORT_SYMBOL(usb_add_phy_dev);
-
-/**
- * usb_remove_phy - remove the OTG PHY
- * @x: the USB OTG PHY to be removed;
- *
- * This reverts the effects of usb_add_phy
- */
-void usb_remove_phy(struct usb_phy *x)
-{
-	unsigned long	flags;
-	struct usb_phy_bind *phy_bind;
-
-	spin_lock_irqsave(&phy_lock, flags);
-	if (x) {
-		list_for_each_entry(phy_bind, &phy_bind_list, list)
-			if (phy_bind->phy == x)
-				phy_bind->phy = NULL;
-		list_del(&x->head);
-	}
-	spin_unlock_irqrestore(&phy_lock, flags);
-}
-EXPORT_SYMBOL(usb_remove_phy);
-
-/**
- * usb_bind_phy - bind the phy and the controller that uses the phy
- * @dev_name: the device name of the device that will bind to the phy
- * @index: index to specify the port number
- * @phy_dev_name: the device name of the phy
- *
- * Fills the phy_bind structure with the dev_name and phy_dev_name. This will
- * be used when the phy driver registers the phy and when the controller
- * requests this phy.
- *
- * To be used by platform specific initialization code.
- */
-int __init usb_bind_phy(const char *dev_name, u8 index,
-				const char *phy_dev_name)
-{
-	struct usb_phy_bind *phy_bind;
-	unsigned long flags;
-
-	phy_bind = kzalloc(sizeof(*phy_bind), GFP_KERNEL);
-	if (!phy_bind) {
-		pr_err("phy_bind(): No memory for phy_bind");
-		return -ENOMEM;
-	}
-
-	phy_bind->dev_name = dev_name;
-	phy_bind->phy_dev_name = phy_dev_name;
-	phy_bind->index = index;
-
-	spin_lock_irqsave(&phy_lock, flags);
-	list_add_tail(&phy_bind->list, &phy_bind_list);
-	spin_unlock_irqrestore(&phy_lock, flags);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(usb_bind_phy);
-
 const char *otg_state_string(enum usb_otg_state state)
 {
 	switch (state) {
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index b13faa1..9fa6327 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -4,6 +4,7 @@
 
 ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
 
+obj-$(CONFIG_USB_OTG_UTILS)		+= phy.o
 obj-$(CONFIG_OMAP_USB2)			+= omap-usb2.o
 obj-$(CONFIG_OMAP_USB3)			+= omap-usb3.o
 obj-$(CONFIG_OMAP_CONTROL_USB)		+= omap-control-usb.o
diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
new file mode 100644
index 0000000..bc1970c
--- /dev/null
+++ b/drivers/usb/phy/phy.c
@@ -0,0 +1,438 @@
+/*
+ * phy.c -- USB phy handling
+ *
+ * Copyright (C) 2004-2013 Texas Instruments
+ *
+ * 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/kernel.h>
+#include <linux/export.h>
+#include <linux/err.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+
+#include <linux/usb/phy.h>
+
+static LIST_HEAD(phy_list);
+static LIST_HEAD(phy_bind_list);
+static DEFINE_SPINLOCK(phy_lock);
+
+static struct usb_phy *__usb_find_phy(struct list_head *list,
+	enum usb_phy_type type)
+{
+	struct usb_phy  *phy = NULL;
+
+	list_for_each_entry(phy, list, head) {
+		if (phy->type != type)
+			continue;
+
+		return phy;
+	}
+
+	return ERR_PTR(-ENODEV);
+}
+
+static struct usb_phy *__usb_find_phy_dev(struct device *dev,
+	struct list_head *list, u8 index)
+{
+	struct usb_phy_bind *phy_bind = NULL;
+
+	list_for_each_entry(phy_bind, list, list) {
+		if (!(strcmp(phy_bind->dev_name, dev_name(dev))) &&
+				phy_bind->index == index) {
+			if (phy_bind->phy)
+				return phy_bind->phy;
+			else
+				return ERR_PTR(-EPROBE_DEFER);
+		}
+	}
+
+	return ERR_PTR(-ENODEV);
+}
+
+static struct usb_phy *__of_usb_find_phy(struct device_node *node)
+{
+	struct usb_phy  *phy;
+
+	list_for_each_entry(phy, &phy_list, head) {
+		if (node != phy->dev->of_node)
+			continue;
+
+		return phy;
+	}
+
+	return ERR_PTR(-ENODEV);
+}
+
+static void devm_usb_phy_release(struct device *dev, void *res)
+{
+	struct usb_phy *phy = *(struct usb_phy **)res;
+
+	usb_put_phy(phy);
+}
+
+static int devm_usb_phy_match(struct device *dev, void *res, void *match_data)
+{
+	return res == match_data;
+}
+
+/**
+ * devm_usb_get_phy - find the USB PHY
+ * @dev - device that requests this phy
+ * @type - the type of the phy the controller requires
+ *
+ * Gets the phy using usb_get_phy(), and associates a device with it using
+ * devres. On driver detach, release function is invoked on the devres data,
+ * then, devres data is freed.
+ *
+ * For use by USB host and peripheral drivers.
+ */
+struct usb_phy *devm_usb_get_phy(struct device *dev, enum usb_phy_type type)
+{
+	struct usb_phy **ptr, *phy;
+
+	ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return NULL;
+
+	phy = usb_get_phy(type);
+	if (!IS_ERR(phy)) {
+		*ptr = phy;
+		devres_add(dev, ptr);
+	} else
+		devres_free(ptr);
+
+	return phy;
+}
+EXPORT_SYMBOL(devm_usb_get_phy);
+
+/**
+ * usb_get_phy - find the USB PHY
+ * @type - the type of the phy the controller requires
+ *
+ * Returns the phy driver, after getting a refcount to it; or
+ * -ENODEV if there is no such phy.  The caller is responsible for
+ * calling usb_put_phy() to release that count.
+ *
+ * For use by USB host and peripheral drivers.
+ */
+struct usb_phy *usb_get_phy(enum usb_phy_type type)
+{
+	struct usb_phy	*phy = NULL;
+	unsigned long	flags;
+
+	spin_lock_irqsave(&phy_lock, flags);
+
+	phy = __usb_find_phy(&phy_list, type);
+	if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
+		pr_err("unable to find transceiver of type %s\n",
+			usb_phy_type_string(type));
+		goto err0;
+	}
+
+	get_device(phy->dev);
+
+err0:
+	spin_unlock_irqrestore(&phy_lock, flags);
+
+	return phy;
+}
+EXPORT_SYMBOL(usb_get_phy);
+
+ /**
+ * devm_usb_get_phy_by_phandle - find the USB PHY by phandle
+ * @dev - device that requests this phy
+ * @phandle - name of the property holding the phy phandle value
+ * @index - the index of the phy
+ *
+ * Returns the phy driver associated with the given phandle value,
+ * after getting a refcount to it, -ENODEV if there is no such phy or
+ * -EPROBE_DEFER if there is a phandle to the phy, but the device is
+ * not yet loaded. While at that, it also associates the device with
+ * the phy using devres. On driver detach, release function is invoked
+ * on the devres data, then, devres data is freed.
+ *
+ * For use by USB host and peripheral drivers.
+ */
+struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
+	const char *phandle, u8 index)
+{
+	struct usb_phy	*phy = ERR_PTR(-ENOMEM), **ptr;
+	unsigned long	flags;
+	struct device_node *node;
+
+	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, phandle, index);
+	if (!node) {
+		dev_dbg(dev, "failed to get %s phandle in %s node\n", phandle,
+			dev->of_node->full_name);
+		return ERR_PTR(-ENODEV);
+	}
+
+	ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr) {
+		dev_dbg(dev, "failed to allocate memory for devres\n");
+		goto err0;
+	}
+
+	spin_lock_irqsave(&phy_lock, flags);
+
+	phy = __of_usb_find_phy(node);
+	if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
+		phy = ERR_PTR(-EPROBE_DEFER);
+		devres_free(ptr);
+		goto err1;
+	}
+
+	*ptr = phy;
+	devres_add(dev, ptr);
+
+	get_device(phy->dev);
+
+err1:
+	spin_unlock_irqrestore(&phy_lock, flags);
+
+err0:
+	of_node_put(node);
+
+	return phy;
+}
+EXPORT_SYMBOL(devm_usb_get_phy_by_phandle);
+
+/**
+ * usb_get_phy_dev - find the USB PHY
+ * @dev - device that requests this phy
+ * @index - the index of the phy
+ *
+ * Returns the phy driver, after getting a refcount to it; or
+ * -ENODEV if there is no such phy.  The caller is responsible for
+ * calling usb_put_phy() to release that count.
+ *
+ * For use by USB host and peripheral drivers.
+ */
+struct usb_phy *usb_get_phy_dev(struct device *dev, u8 index)
+{
+	struct usb_phy	*phy = NULL;
+	unsigned long	flags;
+
+	spin_lock_irqsave(&phy_lock, flags);
+
+	phy = __usb_find_phy_dev(dev, &phy_bind_list, index);
+	if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
+		pr_err("unable to find transceiver\n");
+		goto err0;
+	}
+
+	get_device(phy->dev);
+
+err0:
+	spin_unlock_irqrestore(&phy_lock, flags);
+
+	return phy;
+}
+EXPORT_SYMBOL(usb_get_phy_dev);
+
+/**
+ * devm_usb_get_phy_dev - find the USB PHY using device ptr and index
+ * @dev - device that requests this phy
+ * @index - the index of the phy
+ *
+ * Gets the phy using usb_get_phy_dev(), and associates a device with it using
+ * devres. On driver detach, release function is invoked on the devres data,
+ * then, devres data is freed.
+ *
+ * For use by USB host and peripheral drivers.
+ */
+struct usb_phy *devm_usb_get_phy_dev(struct device *dev, u8 index)
+{
+	struct usb_phy **ptr, *phy;
+
+	ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return NULL;
+
+	phy = usb_get_phy_dev(dev, index);
+	if (!IS_ERR(phy)) {
+		*ptr = phy;
+		devres_add(dev, ptr);
+	} else
+		devres_free(ptr);
+
+	return phy;
+}
+EXPORT_SYMBOL(devm_usb_get_phy_dev);
+
+/**
+ * devm_usb_put_phy - release the USB PHY
+ * @dev - device that wants to release this phy
+ * @phy - the phy returned by devm_usb_get_phy()
+ *
+ * destroys the devres associated with this phy and invokes usb_put_phy
+ * to release the phy.
+ *
+ * For use by USB host and peripheral drivers.
+ */
+void devm_usb_put_phy(struct device *dev, struct usb_phy *phy)
+{
+	int r;
+
+	r = devres_destroy(dev, devm_usb_phy_release, devm_usb_phy_match, phy);
+	dev_WARN_ONCE(dev, r, "couldn't find PHY resource\n");
+}
+EXPORT_SYMBOL(devm_usb_put_phy);
+
+/**
+ * usb_put_phy - release the USB PHY
+ * @x: the phy returned by usb_get_phy()
+ *
+ * Releases a refcount the caller received from usb_get_phy().
+ *
+ * For use by USB host and peripheral drivers.
+ */
+void usb_put_phy(struct usb_phy *x)
+{
+	if (x) {
+		struct module *owner = x->dev->driver->owner;
+
+		put_device(x->dev);
+		module_put(owner);
+	}
+}
+EXPORT_SYMBOL(usb_put_phy);
+
+/**
+ * usb_add_phy - declare the USB PHY
+ * @x: the USB phy to be used; or NULL
+ * @type - the type of this PHY
+ *
+ * This call is exclusively for use by phy drivers, which
+ * coordinate the activities of drivers for host and peripheral
+ * controllers, and in some cases for VBUS current regulation.
+ */
+int usb_add_phy(struct usb_phy *x, enum usb_phy_type type)
+{
+	int		ret = 0;
+	unsigned long	flags;
+	struct usb_phy	*phy;
+
+	if (x->type != USB_PHY_TYPE_UNDEFINED) {
+		dev_err(x->dev, "not accepting initialized PHY %s\n", x->label);
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(&phy_lock, flags);
+
+	list_for_each_entry(phy, &phy_list, head) {
+		if (phy->type == type) {
+			ret = -EBUSY;
+			dev_err(x->dev, "transceiver type %s already exists\n",
+						usb_phy_type_string(type));
+			goto out;
+		}
+	}
+
+	x->type = type;
+	list_add_tail(&x->head, &phy_list);
+
+out:
+	spin_unlock_irqrestore(&phy_lock, flags);
+	return ret;
+}
+EXPORT_SYMBOL(usb_add_phy);
+
+/**
+ * usb_add_phy_dev - declare the USB PHY
+ * @x: the USB phy to be used; or NULL
+ *
+ * This call is exclusively for use by phy drivers, which
+ * coordinate the activities of drivers for host and peripheral
+ * controllers, and in some cases for VBUS current regulation.
+ */
+int usb_add_phy_dev(struct usb_phy *x)
+{
+	struct usb_phy_bind *phy_bind;
+	unsigned long flags;
+
+	if (!x->dev) {
+		dev_err(x->dev, "no device provided for PHY\n");
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(&phy_lock, flags);
+	list_for_each_entry(phy_bind, &phy_bind_list, list)
+		if (!(strcmp(phy_bind->phy_dev_name, dev_name(x->dev))))
+			phy_bind->phy = x;
+
+	list_add_tail(&x->head, &phy_list);
+
+	spin_unlock_irqrestore(&phy_lock, flags);
+	return 0;
+}
+EXPORT_SYMBOL(usb_add_phy_dev);
+
+/**
+ * usb_remove_phy - remove the OTG PHY
+ * @x: the USB OTG PHY to be removed;
+ *
+ * This reverts the effects of usb_add_phy
+ */
+void usb_remove_phy(struct usb_phy *x)
+{
+	unsigned long	flags;
+	struct usb_phy_bind *phy_bind;
+
+	spin_lock_irqsave(&phy_lock, flags);
+	if (x) {
+		list_for_each_entry(phy_bind, &phy_bind_list, list)
+			if (phy_bind->phy == x)
+				phy_bind->phy = NULL;
+		list_del(&x->head);
+	}
+	spin_unlock_irqrestore(&phy_lock, flags);
+}
+EXPORT_SYMBOL(usb_remove_phy);
+
+/**
+ * usb_bind_phy - bind the phy and the controller that uses the phy
+ * @dev_name: the device name of the device that will bind to the phy
+ * @index: index to specify the port number
+ * @phy_dev_name: the device name of the phy
+ *
+ * Fills the phy_bind structure with the dev_name and phy_dev_name. This will
+ * be used when the phy driver registers the phy and when the controller
+ * requests this phy.
+ *
+ * To be used by platform specific initialization code.
+ */
+int __init usb_bind_phy(const char *dev_name, u8 index,
+				const char *phy_dev_name)
+{
+	struct usb_phy_bind *phy_bind;
+	unsigned long flags;
+
+	phy_bind = kzalloc(sizeof(*phy_bind), GFP_KERNEL);
+	if (!phy_bind) {
+		pr_err("phy_bind(): No memory for phy_bind");
+		return -ENOMEM;
+	}
+
+	phy_bind->dev_name = dev_name;
+	phy_bind->phy_dev_name = phy_dev_name;
+	phy_bind->index = index;
+
+	spin_lock_irqsave(&phy_lock, flags);
+	list_add_tail(&phy_bind->list, &phy_bind_list);
+	spin_unlock_irqrestore(&phy_lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_bind_phy);
-- 
1.7.10.4

[PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type

[Sascha Hauer]

From: Michael Grzeschik <m.grzeschik@pengutronix.de>

This adds two little devicetree helper functions for determining the
dr_mode (host, peripheral, otg) and phy_type (utmi, ulpi,...) from
the devicetree.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/usb/phy/Makefile |    1 +
 drivers/usb/phy/of.c     |   47 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/usb-common.c |   36 +++++++++++++++++++++++++++++++++++
 include/linux/usb/of.h   |   27 ++++++++++++++++++++++++++
 include/linux/usb/otg.h  |    7 +++++++
 include/linux/usb/phy.h  |    9 +++++++++
 6 files changed, 127 insertions(+)
 create mode 100644 drivers/usb/phy/of.c
 create mode 100644 include/linux/usb/of.h

diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 9fa6327..e1be1e8 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -5,6 +5,7 @@
 ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
 
 obj-$(CONFIG_USB_OTG_UTILS)		+= phy.o
+obj-$(CONFIG_OF)			+= of.o
 obj-$(CONFIG_OMAP_USB2)			+= omap-usb2.o
 obj-$(CONFIG_OMAP_USB3)			+= omap-usb3.o
 obj-$(CONFIG_OMAP_CONTROL_USB)		+= omap-control-usb.o
diff --git a/drivers/usb/phy/of.c b/drivers/usb/phy/of.c
new file mode 100644
index 0000000..e6f3b74
--- /dev/null
+++ b/drivers/usb/phy/of.c
@@ -0,0 +1,47 @@
+/*
+ * USB of helper code
+ *
+ * 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/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/usb/of.h>
+#include <linux/usb/otg.h>
+
+static const char *usbphy_modes[] = {
+	[USBPHY_INTERFACE_MODE_UNKNOWN]	= "",
+	[USBPHY_INTERFACE_MODE_UTMI]	= "utmi",
+	[USBPHY_INTERFACE_MODE_UTMIW]	= "utmi_wide",
+	[USBPHY_INTERFACE_MODE_ULPI]	= "ulpi",
+	[USBPHY_INTERFACE_MODE_SERIAL]	= "serial",
+	[USBPHY_INTERFACE_MODE_HSIC]	= "hsic",
+};
+
+/**
+ * of_usb_get_phy_mode - Get phy mode for given device_node
+ * @np:	Pointer to the given device_node
+ *
+ * The function gets phy interface string from property 'phy_type',
+ * and returns the correspondig enum usb_phy_interface
+ */
+enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np)
+{
+	const char *phy_type;
+	int err, i;
+
+	err = of_property_read_string(np, "phy_type", &phy_type);
+	if (err < 0)
+		return USBPHY_INTERFACE_MODE_UNKNOWN;
+
+	for (i = 0; i < ARRAY_SIZE(usbphy_modes); i++)
+		if (!strcmp(phy_type, usbphy_modes[i]))
+			return i;
+
+	return USBPHY_INTERFACE_MODE_UNKNOWN;
+}
+EXPORT_SYMBOL_GPL(of_usb_get_phy_mode);
diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
index d29503e..ad4d87d 100644
--- a/drivers/usb/usb-common.c
+++ b/drivers/usb/usb-common.c
@@ -14,6 +14,9 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/usb/ch9.h>
+#include <linux/of.h>
+#include <linux/usb/of.h>
+#include <linux/usb/otg.h>
 
 const char *usb_speed_string(enum usb_device_speed speed)
 {
@@ -32,4 +35,37 @@ const char *usb_speed_string(enum usb_device_speed speed)
 }
 EXPORT_SYMBOL_GPL(usb_speed_string);
 
+#ifdef CONFIG_OF
+static const char *usb_dr_modes[] = {
+	[USB_DR_MODE_UNKNOWN]		= "",
+	[USB_DR_MODE_HOST]		= "host",
+	[USB_DR_MODE_PERIPHERAL]	= "peripheral",
+	[USB_DR_MODE_OTG]		= "otg",
+};
+
+/**
+ * of_usb_get_dr_mode - Get dual role mode for given device_node
+ * @np:	Pointer to the given device_node
+ *
+ * The function gets phy interface string from property 'dr_mode',
+ * and returns the correspondig enum usb_dr_mode
+ */
+enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
+{
+	const char *dr_mode;
+	int err, i;
+
+	err = of_property_read_string(np, "dr_mode", &dr_mode);
+	if (err < 0)
+		return USB_DR_MODE_UNKNOWN;
+
+	for (i = 0; i < ARRAY_SIZE(usb_dr_modes); i++)
+		if (!strcmp(dr_mode, usb_dr_modes[i]))
+			return i;
+
+	return USB_DR_MODE_UNKNOWN;
+}
+EXPORT_SYMBOL_GPL(of_usb_get_dr_mode);
+#endif
+
 MODULE_LICENSE("GPL");
diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h
new file mode 100644
index 0000000..4681a20
--- /dev/null
+++ b/include/linux/usb/of.h
@@ -0,0 +1,27 @@
+/*
+ * OF helpers for usb devices.
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_USB_OF_H
+#define __LINUX_USB_OF_H
+
+#include <linux/usb/phy.h>
+
+#ifdef CONFIG_OF
+enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np);
+enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np);
+#else
+static inline enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np)
+{
+	return USBPHY_INTERFACE_MODE_UNKNOWN;
+}
+
+static inline enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
+{
+	return USB_DR_MODE_UNKNOWN;
+}
+#endif
+
+#endif /* __LINUX_USB_OF_H */
diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
index e8a5fe8..4e8bfbb 100644
--- a/include/linux/usb/otg.h
+++ b/include/linux/usb/otg.h
@@ -99,4 +99,11 @@ otg_start_srp(struct usb_otg *otg)
 /* for OTG controller drivers (and maybe other stuff) */
 extern int usb_bus_start_enum(struct usb_bus *bus, unsigned port_num);
 
+enum usb_dr_mode {
+	USB_DR_MODE_UNKNOWN,
+	USB_DR_MODE_HOST,
+	USB_DR_MODE_PERIPHERAL,
+	USB_DR_MODE_OTG,
+};
+
 #endif /* __LINUX_USB_OTG_H */
diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
index 15847cb..5edddb1 100644
--- a/include/linux/usb/phy.h
+++ b/include/linux/usb/phy.h
@@ -12,6 +12,15 @@
 #include <linux/notifier.h>
 #include <linux/usb.h>
 
+enum usb_phy_interface {
+	USBPHY_INTERFACE_MODE_UNKNOWN,
+	USBPHY_INTERFACE_MODE_UTMI,
+	USBPHY_INTERFACE_MODE_UTMIW,
+	USBPHY_INTERFACE_MODE_ULPI,
+	USBPHY_INTERFACE_MODE_SERIAL,
+	USBPHY_INTERFACE_MODE_HSIC,
+};
+
 enum usb_phy_events {
 	USB_EVENT_NONE,         /* no events or cable disconnected */
 	USB_EVENT_VBUS,         /* vbus valid event */
-- 
1.7.10.4

[PATCH 4/9] USB: chipidea: ci13xxx-imx: create dynamic platformdata

[Sascha Hauer]

From: Michael Grzeschik <m.grzeschik@pengutronix.de>

This patch removes the limitation of having only one instance of the
ci13xxx-imx platformdata and makes different configurations possible.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/chipidea/ci13xxx_imx.c |   25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/chipidea/ci13xxx_imx.c b/drivers/usb/chipidea/ci13xxx_imx.c
index 8c29122..69024e0 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -85,17 +85,10 @@ EXPORT_SYMBOL_GPL(usbmisc_get_init_data);
 
 /* End of common functions shared by usbmisc drivers*/
 
-static struct ci13xxx_platform_data ci13xxx_imx_platdata  = {
-	.name			= "ci13xxx_imx",
-	.flags			= CI13XXX_REQUIRE_TRANSCEIVER |
-				  CI13XXX_PULLUP_ON_VBUS |
-				  CI13XXX_DISABLE_STREAMING,
-	.capoffset		= DEF_CAPOFFSET,
-};
-
 static int ci13xxx_imx_probe(struct platform_device *pdev)
 {
 	struct ci13xxx_imx_data *data;
+	struct ci13xxx_platform_data *pdata;
 	struct platform_device *plat_ci, *phy_pdev;
 	struct device_node *phy_np;
 	struct resource *res;
@@ -107,6 +100,18 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
 		&& !usbmisc_ops)
 		return -EPROBE_DEFER;
 
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		dev_err(&pdev->dev, "Failed to allocate CI13xxx-IMX pdata!\n");
+		return -ENOMEM;
+	}
+
+	pdata->name = "ci13xxx_imx";
+	pdata->capoffset = DEF_CAPOFFSET;
+	pdata->flags = CI13XXX_REQUIRE_TRANSCEIVER |
+		       CI13XXX_PULLUP_ON_VBUS |
+		       CI13XXX_DISABLE_STREAMING;
+
 	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
 	if (!data) {
 		dev_err(&pdev->dev, "Failed to allocate CI13xxx-IMX data!\n");
@@ -168,7 +173,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
 		reg_vbus = NULL;
 	}
 
-	ci13xxx_imx_platdata.phy = data->phy;
+	pdata->phy = data->phy;
 
 	if (!pdev->dev.dma_mask) {
 		pdev->dev.dma_mask = devm_kzalloc(&pdev->dev,
@@ -193,7 +198,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
 
 	plat_ci = ci13xxx_add_device(&pdev->dev,
 				pdev->resource, pdev->num_resources,
-				&ci13xxx_imx_platdata);
+				pdata);
 	if (IS_ERR(plat_ci)) {
 		ret = PTR_ERR(plat_ci);
 		dev_err(&pdev->dev,
-- 
1.7.10.4

[PATCH 5/9] USB: chipidea: add PTW and PTS handling

[Sascha Hauer]

From: Michael Grzeschik <m.grzeschik@pengutronix.de>

This patch makes it possible to configure the PTW and PTS bits inside
the portsc register for host and device mode before the driver starts
and the phy can be addressed as hardware implementation is designed.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 .../devicetree/bindings/usb/ci13xxx-imx.txt        |    5 +++
 drivers/usb/chipidea/bits.h                        |   14 ++++++-
 drivers/usb/chipidea/ci13xxx_imx.c                 |    3 ++
 drivers/usb/chipidea/core.c                        |   39 ++++++++++++++++++++
 include/linux/usb/chipidea.h                       |    1 +
 5 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
index 5778b9c..dd42ccd 100644
--- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
+++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
@@ -5,6 +5,11 @@ Required properties:
 - reg: Should contain registers location and length
 - interrupts: Should contain controller interrupt
 
+Recommended properies:
+- phy_type: the type of the phy connected to the core. Should be one
+  of "utmi", "utmi_wide", "ulpi", "serial" or "hsic". Without this
+  property the PORTSC register won't be touched
+
 Optional properties:
 - fsl,usbphy: phandler of usb phy that connects to the only one port
 - fsl,usbmisc: phandler of non-core register device, with one argument
diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
index 050de85..d8ffc2f 100644
--- a/drivers/usb/chipidea/bits.h
+++ b/drivers/usb/chipidea/bits.h
@@ -48,10 +48,22 @@
 #define PORTSC_SUSP           BIT(7)
 #define PORTSC_HSP            BIT(9)
 #define PORTSC_PTC            (0x0FUL << 16)
+/* PTS and PTW for non lpm version only */
+#define PORTSC_PTS(d)         ((((d) & 0x3) << 30) | (((d) & 0x4) ? BIT(25) : 0))
+#define PORTSC_PTW            BIT(28)
 
 /* DEVLC */
 #define DEVLC_PSPD            (0x03UL << 25)
-#define    DEVLC_PSPD_HS      (0x02UL << 25)
+#define DEVLC_PSPD_HS         (0x02UL << 25)
+#define DEVLC_PTW             BIT(27)
+#define DEVLC_STS             BIT(28)
+#define DEVLC_PTS(d)          (((d) & 0x7) << 29)
+
+/* Encoding for DEVLC_PTS and PORTSC_PTS */
+#define PTS_UTMI              0
+#define PTS_ULPI              2
+#define PTS_SERIAL            3
+#define PTS_HSIC              4
 
 /* OTGSC */
 #define OTGSC_IDPU	      BIT(5)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c b/drivers/usb/chipidea/ci13xxx_imx.c
index 69024e0..ebc1148 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -21,6 +21,7 @@
 #include <linux/clk.h>
 #include <linux/regulator/consumer.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/usb/of.h>
 
 #include "ci.h"
 #include "ci13xxx_imx.h"
@@ -112,6 +113,8 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
 		       CI13XXX_PULLUP_ON_VBUS |
 		       CI13XXX_DISABLE_STREAMING;
 
+	pdata->phy_mode = of_usb_get_phy_mode(pdev->dev.of_node);
+
 	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
 	if (!data) {
 		dev_err(&pdev->dev, "Failed to allocate CI13xxx-IMX data!\n");
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 57cae1f..04d68cb 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -67,6 +67,8 @@
 #include <linux/usb/gadget.h>
 #include <linux/usb/otg.h>
 #include <linux/usb/chipidea.h>
+#include <linux/usb/of.h>
+#include <linux/phy.h>
 
 #include "ci.h"
 #include "udc.h"
@@ -211,6 +213,41 @@ static int hw_device_init(struct ci13xxx *ci, void __iomem *base)
 	return 0;
 }
 
+static void hw_phymode_configure(struct ci13xxx *ci)
+{
+	u32 portsc, lpm;
+
+	switch (ci->platdata->phy_mode) {
+	case USBPHY_INTERFACE_MODE_UTMI:
+		portsc = PORTSC_PTS(PTS_UTMI);
+		lpm = DEVLC_PTS(PTS_UTMI);
+		break;
+	case USBPHY_INTERFACE_MODE_UTMIW:
+		portsc = PORTSC_PTS(PTS_UTMI) | PORTSC_PTW;
+		lpm = DEVLC_PTS(PTS_UTMI) | DEVLC_PTW;
+		break;
+	case USBPHY_INTERFACE_MODE_ULPI:
+		portsc = PORTSC_PTS(PTS_ULPI);
+		lpm = DEVLC_PTS(PTS_ULPI);
+		break;
+	case USBPHY_INTERFACE_MODE_SERIAL:
+		portsc = PORTSC_PTS(PTS_SERIAL);
+		lpm = DEVLC_PTS(PTS_SERIAL);
+		break;
+	case USBPHY_INTERFACE_MODE_HSIC:
+		portsc = PORTSC_PTS(PTS_HSIC);
+		lpm = DEVLC_PTS(PTS_HSIC);
+		break;
+	default:
+		return;
+	}
+
+	if (ci->hw_bank.lpm)
+		hw_write(ci, OP_DEVLC, DEVLC_PTS(7) | DEVLC_PTW, lpm);
+	else
+		hw_write(ci, OP_PORTSC, PORTSC_PTS(7) | PORTSC_PTW, portsc);
+}
+
 /**
  * hw_device_reset: resets chip (execute without interruption)
  * @ci: the controller
@@ -476,6 +513,8 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 			: CI_ROLE_GADGET;
 	}
 
+	hw_phymode_configure(ci);
+
 	ret = ci_role_start(ci, ci->role);
 	if (ret) {
 		dev_err(dev, "can't start %s role\n", ci_role(ci)->name);
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 544825d..1a2aa18 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -14,6 +14,7 @@ struct ci13xxx_platform_data {
 	uintptr_t	 capoffset;
 	unsigned	 power_budget;
 	struct usb_phy	*phy;
+	enum usb_phy_interface phy_mode;
 	unsigned long	 flags;
 #define CI13XXX_REGS_SHARED		BIT(0)
 #define CI13XXX_REQUIRE_TRANSCEIVER	BIT(1)
-- 
1.7.10.4

[PATCH 6/9] USB chipidea: introduce dual role mode pdata flags

[Sascha Hauer]

Even if a chipidea core is otg capable the board may not. This allows
to explicitly set the core to host/peripheral mode. Without these
flags the driver falls back to the old behaviour.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/usb/chipidea/core.c  |   21 +++++++++++++++------
 include/linux/usb/chipidea.h |    2 +-
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 04d68cb..c89f2aa 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -435,6 +435,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	struct resource	*res;
 	void __iomem	*base;
 	int		ret;
+	enum usb_dr_mode dr_mode;
 
 	if (!dev->platform_data) {
 		dev_err(dev, "platform data missing\n");
@@ -487,14 +488,22 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	dr_mode = ci->platdata->dr_mode;
+	if (dr_mode == USB_DR_MODE_UNKNOWN)
+		dr_mode = USB_DR_MODE_OTG;
+
 	/* initialize role(s) before the interrupt is requested */
-	ret = ci_hdrc_host_init(ci);
-	if (ret)
-		dev_info(dev, "doesn't support host\n");
+	if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
+		ret = ci_hdrc_host_init(ci);
+		if (ret)
+			dev_info(dev, "doesn't support host\n");
+	}
 
-	ret = ci_hdrc_gadget_init(ci);
-	if (ret)
-		dev_info(dev, "doesn't support gadget\n");
+	if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) {
+		ret = ci_hdrc_gadget_init(ci);
+		if (ret)
+			dev_info(dev, "doesn't support gadget\n");
+	}
 
 	if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) {
 		dev_err(dev, "no supported roles\n");
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 1a2aa18..b314647 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -20,7 +20,7 @@ struct ci13xxx_platform_data {
 #define CI13XXX_REQUIRE_TRANSCEIVER	BIT(1)
 #define CI13XXX_PULLUP_ON_VBUS		BIT(2)
 #define CI13XXX_DISABLE_STREAMING	BIT(3)
-
+	enum usb_dr_mode	dr_mode;
 #define CI13XXX_CONTROLLER_RESET_EVENT		0
 #define CI13XXX_CONTROLLER_STOPPED_EVENT	1
 	void	(*notify_event) (struct ci13xxx *ci, unsigned event);
-- 
1.7.10.4

[PATCH 7/9] USB chipidea i.MX: introduce dr_mode property

[Sascha Hauer]

The dr_mode devicetree property allows to explicitly specify the
host/peripheral/otg mode. This is necessary for boards without proper
ID pin handling.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Peter Chen <peter.chen@freescale.com>
---
 Documentation/devicetree/bindings/usb/ci13xxx-imx.txt |    1 +
 drivers/usb/chipidea/ci13xxx_imx.c                    |    1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
index dd42ccd..493a414 100644
--- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
+++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
@@ -9,6 +9,7 @@ Recommended properies:
 - phy_type: the type of the phy connected to the core. Should be one
   of "utmi", "utmi_wide", "ulpi", "serial" or "hsic". Without this
   property the PORTSC register won't be touched
+- dr_mode: One of "host", "peripheral" or "otg". Defaults to "otg"
 
 Optional properties:
 - fsl,usbphy: phandler of usb phy that connects to the only one port
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c b/drivers/usb/chipidea/ci13xxx_imx.c
index ebc1148..b598bb8f 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -114,6 +114,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
 		       CI13XXX_DISABLE_STREAMING;
 
 	pdata->phy_mode = of_usb_get_phy_mode(pdev->dev.of_node);
+	pdata->dr_mode = of_usb_get_dr_mode(pdev->dev.of_node);
 
 	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
 	if (!data) {
-- 
1.7.10.4

[PATCH 8/9] USB mxs-phy: Register phy with framework

[Sascha Hauer]

We now have usb_add_phy_dev(), so use it to register with the framework
to be able to find the phy from the USB driver.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/usb/otg/mxs-phy.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c
index 5158332..5b39885 100644
--- a/drivers/usb/otg/mxs-phy.c
+++ b/drivers/usb/otg/mxs-phy.c
@@ -127,6 +127,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
 	void __iomem *base;
 	struct clk *clk;
 	struct mxs_phy *mxs_phy;
+	int ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
@@ -166,11 +167,19 @@ static int mxs_phy_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, &mxs_phy->phy);
 
+	ret = usb_add_phy_dev(&mxs_phy->phy);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
 static int mxs_phy_remove(struct platform_device *pdev)
 {
+	struct mxs_phy *mxs_phy = platform_get_drvdata(pdev);
+
+	usb_remove_phy(&mxs_phy->phy);
+
 	platform_set_drvdata(pdev, NULL);
 
 	return 0;
-- 
1.7.10.4

[PATCH 9/9] USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/usb/chipidea/ci13xxx_imx.c |   39 +++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/chipidea/ci13xxx_imx.c b/drivers/usb/chipidea/ci13xxx_imx.c
index b598bb8f..136869b 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -30,7 +30,6 @@
 	((struct usb_phy *)platform_get_drvdata(pdev))
 
 struct ci13xxx_imx_data {
-	struct device_node *phy_np;
 	struct usb_phy *phy;
 	struct platform_device *ci_pdev;
 	struct clk *clk;
@@ -90,12 +89,12 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
 {
 	struct ci13xxx_imx_data *data;
 	struct ci13xxx_platform_data *pdata;
-	struct platform_device *plat_ci, *phy_pdev;
-	struct device_node *phy_np;
+	struct platform_device *plat_ci;
 	struct resource *res;
 	struct regulator *reg_vbus;
 	struct pinctrl *pinctrl;
 	int ret;
+	struct usb_phy *phy;
 
 	if (of_find_property(pdev->dev.of_node, "fsl,usbmisc", NULL)
 		&& !usbmisc_ops)
@@ -147,19 +146,21 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	phy_np = of_parse_phandle(pdev->dev.of_node, "fsl,usbphy", 0);
-	if (phy_np) {
-		data->phy_np = phy_np;
-		phy_pdev = of_find_device_by_node(phy_np);
-		if (phy_pdev) {
-			struct usb_phy *phy;
-			phy = pdev_to_phy(phy_pdev);
-			if (phy &&
-			    try_module_get(phy_pdev->dev.driver->owner)) {
-				usb_phy_init(phy);
-				data->phy = phy;
-			}
+	phy = devm_usb_get_phy_by_phandle(&pdev->dev, "fsl,usbphy", 0);
+
+	if (PTR_ERR(phy) == -EPROBE_DEFER) {
+		ret = -EPROBE_DEFER;
+		goto err_clk;
+	}
+
+	if (!IS_ERR(phy)) {
+		ret = usb_phy_init(phy);
+		if (ret) {
+			dev_err(&pdev->dev, "unable to init phy: %d\n", ret);
+			goto err_clk;
 		}
+
+		data->phy = phy;
 	}
 
 	/* we only support host now, so enable vbus here */
@@ -170,7 +171,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev,
 				"Failed to enable vbus regulator, err=%d\n",
 				ret);
-			goto put_np;
+			goto err_clk;
 		}
 		data->reg_vbus = reg_vbus;
 	} else {
@@ -222,9 +223,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
 err:
 	if (reg_vbus)
 		regulator_disable(reg_vbus);
-put_np:
-	if (phy_np)
-		of_node_put(phy_np);
+err_clk:
 	clk_disable_unprepare(data->clk);
 	return ret;
 }
@@ -244,8 +243,6 @@ static int ci13xxx_imx_remove(struct platform_device *pdev)
 		module_put(data->phy->dev->driver->owner);
 	}
 
-	of_node_put(data->phy_np);
-
 	clk_disable_unprepare(data->clk);
 
 	platform_set_drvdata(pdev, NULL);
-- 
1.7.10.4

[PATCH 1/9] usb: otg: use try_module_get in all usb_get_phy functions and add missing module_put

[Roger Quadros]

On 02/04/2013 03:24 PM, Sascha Hauer wrote:
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> 
> In patch "5d3c28b usb: otg: add device tree support to otg library"
> devm_usb_get_phy_by_phandle() was added. It uses try_module_get() to lock the
> phy driver in memory. The corresponding module_put() is missing in that patch.
> 
> This patch adds try_module_get() to usb_get_phy() and usb_get_phy_dev().
> Further the missing module_put() is added to usb_put_phy().
> 
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/usb/otg/otg.c |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
> index e181439..2bd03d2 100644
> --- a/drivers/usb/otg/otg.c
> +++ b/drivers/usb/otg/otg.c
> @@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
>  	spin_lock_irqsave(&phy_lock, flags);
>  
>  	phy = __usb_find_phy(&phy_list, type);
> -	if (IS_ERR(phy)) {
> +	if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
>  		pr_err("unable to find transceiver of type %s\n",
>  			usb_phy_type_string(type));
>  		goto err0;

There are 2 problems with this.

- If phy was found but try_module_get failed you are not returning error code.

- If phy was found and try_module_get fails we would want to use deferred probing,
since this case is possible if the phy module is not yet loaded and alive but can be in
the future.

I would change the code to

	if (IS_ERR(phy)) {
		pr_err("unable to find transceiver of type %s\n",
			usb_phy_type_string(type));
		goto err0;
	}

	if (!try_module_get(phy->dev->driver->owner)) {
		phy = ERR_PTR(-EPROBE_DEFER);
		goto err0;
	}

> @@ -228,7 +228,7 @@ struct usb_phy *usb_get_phy_dev(struct device *dev, u8 index)
>  	spin_lock_irqsave(&phy_lock, flags);
>  
>  	phy = __usb_find_phy_dev(dev, &phy_bind_list, index);
> -	if (IS_ERR(phy)) {
> +	if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
>  		pr_err("unable to find transceiver\n");
>  		goto err0;
>  	}
> @@ -301,8 +301,12 @@ EXPORT_SYMBOL(devm_usb_put_phy);
>   */
>  void usb_put_phy(struct usb_phy *x)
>  {
> -	if (x)
> +	if (x) {
> +		struct module *owner = x->dev->driver->owner;
> +
>  		put_device(x->dev);
> +		module_put(owner);
> +	}
>  }
>  EXPORT_SYMBOL(usb_put_phy);
>  
> 

cheers,
-roger

[PATCH 1/9] usb: otg: use try_module_get in all usb_get_phy functions and add missing module_put

[Marc Kleine-Budde]

On 02/04/2013 02:59 PM, Roger Quadros wrote:
> On 02/04/2013 03:24 PM, Sascha Hauer wrote:
>> From: Marc Kleine-Budde <mkl@pengutronix.de>
>>
>> In patch "5d3c28b usb: otg: add device tree support to otg library"
>> devm_usb_get_phy_by_phandle() was added. It uses try_module_get() to lock the
>> phy driver in memory. The corresponding module_put() is missing in that patch.
>>
>> This patch adds try_module_get() to usb_get_phy() and usb_get_phy_dev().
>> Further the missing module_put() is added to usb_put_phy().
>>
>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> ---
>>  drivers/usb/otg/otg.c |   10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
>> index e181439..2bd03d2 100644
>> --- a/drivers/usb/otg/otg.c
>> +++ b/drivers/usb/otg/otg.c
>> @@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
>>  	spin_lock_irqsave(&phy_lock, flags);
>>  
>>  	phy = __usb_find_phy(&phy_list, type);
>> -	if (IS_ERR(phy)) {
>> +	if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
>>  		pr_err("unable to find transceiver of type %s\n",
>>  			usb_phy_type_string(type));
>>  		goto err0;
> 
> There are 2 problems with this.
> 
> - If phy was found but try_module_get failed you are not returning error code.

Correct - but....

> - If phy was found and try_module_get fails we would want to use deferred probing,
> since this case is possible if the phy module is not yet loaded and alive but can be in
> the future.

...this should not happen, as the phy list is protected by the
spin_lock. If a phy driver module has added a the phy to the
infrastructure, but has been rmmod'ed without removing the phy. The phy
driver is broken anyway.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130204/5aac1207/attachment-0001.sig>

[PATCH 1/9] usb: otg: use try_module_get in all usb_get_phy functions and add missing module_put

[Roger Quadros]

On 02/04/2013 04:10 PM, Marc Kleine-Budde wrote:
> On 02/04/2013 02:59 PM, Roger Quadros wrote:
>> On 02/04/2013 03:24 PM, Sascha Hauer wrote:
>>> From: Marc Kleine-Budde <mkl@pengutronix.de>
>>>
>>> In patch "5d3c28b usb: otg: add device tree support to otg library"
>>> devm_usb_get_phy_by_phandle() was added. It uses try_module_get() to lock the
>>> phy driver in memory. The corresponding module_put() is missing in that patch.
>>>
>>> This patch adds try_module_get() to usb_get_phy() and usb_get_phy_dev().
>>> Further the missing module_put() is added to usb_put_phy().
>>>
>>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>> ---
>>>  drivers/usb/otg/otg.c |   10 +++++++---
>>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
>>> index e181439..2bd03d2 100644
>>> --- a/drivers/usb/otg/otg.c
>>> +++ b/drivers/usb/otg/otg.c
>>> @@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
>>>  	spin_lock_irqsave(&phy_lock, flags);
>>>  
>>>  	phy = __usb_find_phy(&phy_list, type);
>>> -	if (IS_ERR(phy)) {
>>> +	if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
>>>  		pr_err("unable to find transceiver of type %s\n",
>>>  			usb_phy_type_string(type));
>>>  		goto err0;
>>
>> There are 2 problems with this.
>>
>> - If phy was found but try_module_get failed you are not returning error code.
> 
> Correct - but....
> 
>> - If phy was found and try_module_get fails we would want to use deferred probing,
>> since this case is possible if the phy module is not yet loaded and alive but can be in
>> the future.
> 
> ...this should not happen, as the phy list is protected by the
> spin_lock. If a phy driver module has added a the phy to the
> infrastructure, but has been rmmod'ed without removing the phy. The phy
> driver is broken anyway.
> 

Yes, you are right.

cheers,
-roger

[PATCH v4] USB: add devicetree helpers for determining dr_mode and phy_type

[Peter Chen]

On Mon, Feb 04, 2013 at 02:24:26PM +0100, Sascha Hauer wrote:
> 4th round of patches.
> 
> Peter, I would be glad if you could test them before your holiday. I rebased
> your last round of Chipidea OTG patches onto this series which you can pull
> here:
> 
> git://git.pengutronix.de/git/imx/linux-2.6.git tags/usb-chipidea-otg-for-next
> 
> I couldn't really test the otg patches since my current hardware does not have
> the ID pin connected, but I can verify that my usecase still works with your
> patches applied.

Sascha, I have tested your patchset, it works after adding a few of changes
at my otg patchset (we still need to read otgsc for gadget-only function).
I use my mx6q sabrelite board to test.

I have tested all roles with below diff:
diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
index 49527d7..cdbf2ec 100644
--- a/arch/arm/boot/dts/imx6q-sabrelite.dts
+++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
@@ -72,6 +72,10 @@
 				pinctrl-0 = <&pinctrl_usbotg_1>;
 				disable-over-current;
 				otg_id_pin_select_change;
+				/* dr_mode = "peripheral"; */
+				/* dr_mode = "host"; */
+				dr_mode = "otg";
+				phy_type = "utmi_wide";
 				status = "okay";
 			};

So, feel free to add my Tested-by: Peter Chen <peter.chen@freescale.com>

Besides, if you have a mx51 bbg board, you can test id-switch function.
For mx5x, you may need a phy driver.

My git: https://github.com/hzpeterchen/linux-usb.git master

It is based on your tag, and add latest otg patch.
Besides, it has Michael Grzeschik/Marc Kleine-Budde
usb misc patch as well as my platform changes for mx6q board.

> 
> Alex, should the patches work for you and are fine otherwise, could you apply
> them for v3.9?
> 
> Sascha
> 
> 
-- 

Best Regards,
Peter Chen

[PATCH 9/9] USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy

[Sergei Shtylyov]

Hello.

On 04-02-2013 17:24, Sascha Hauer wrote:

> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>   drivers/usb/chipidea/ci13xxx_imx.c |   39 +++++++++++++++++-------------------
>   1 file changed, 18 insertions(+), 21 deletions(-)

> diff --git a/drivers/usb/chipidea/ci13xxx_imx.c b/drivers/usb/chipidea/ci13xxx_imx.c
> index b598bb8f..136869b 100644
> --- a/drivers/usb/chipidea/ci13xxx_imx.c
> +++ b/drivers/usb/chipidea/ci13xxx_imx.c
[...]
> @@ -147,19 +146,21 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
>   		return ret;
>   	}
>
> +	phy = devm_usb_get_phy_by_phandle(&pdev->dev, "fsl,usbphy", 0);
> +

    No need for emoty line here. Keep the style as it was please.

> +	if (PTR_ERR(phy) == -EPROBE_DEFER) {

    Is it valid to call PTR_ERR() on non-error pointer? Isn't it better to do 
this check under *else* clause below the next *if*.

> +		ret = -EPROBE_DEFER;
> +		goto err_clk;
> +	}
> +

    This empty line is also not needed, I think.

> +	if (!IS_ERR(phy)) {
> +		ret = usb_phy_init(phy);
> +		if (ret) {
> +			dev_err(&pdev->dev, "unable to init phy: %d\n", ret);
> +			goto err_clk;
>   		}
> +
> +		data->phy = phy;
>   	}
>
>   	/* we only support host now, so enable vbus here */

WBR, Sergei

[PATCH 9/9] USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy

[Sascha Hauer]

On Tue, Feb 05, 2013 at 03:45:12PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 04-02-2013 17:24, Sascha Hauer wrote:
> 
> >Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> >---
> >  drivers/usb/chipidea/ci13xxx_imx.c |   39 +++++++++++++++++-------------------
> >  1 file changed, 18 insertions(+), 21 deletions(-)
> 
> >diff --git a/drivers/usb/chipidea/ci13xxx_imx.c b/drivers/usb/chipidea/ci13xxx_imx.c
> >index b598bb8f..136869b 100644
> >--- a/drivers/usb/chipidea/ci13xxx_imx.c
> >+++ b/drivers/usb/chipidea/ci13xxx_imx.c
> [...]
> >@@ -147,19 +146,21 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
> >  		return ret;
> >  	}
> >
> >+	phy = devm_usb_get_phy_by_phandle(&pdev->dev, "fsl,usbphy", 0);
> >+
> 
>    No need for emoty line here. Keep the style as it was please.
> 
> >+	if (PTR_ERR(phy) == -EPROBE_DEFER) {
> 
>    Is it valid to call PTR_ERR() on non-error pointer?

Why shouldn't it?

> Isn't it
> better to do this check under *else* clause below the next *if*.

For better readability, yes.

Sascha

> 
> >+		ret = -EPROBE_DEFER;
> >+		goto err_clk;
> >+	}
> >+
> 
>    This empty line is also not needed, I think.
> 
> >+	if (!IS_ERR(phy)) {
> >+		ret = usb_phy_init(phy);
> >+		if (ret) {
> >+			dev_err(&pdev->dev, "unable to init phy: %d\n", ret);
> >+			goto err_clk;
> >  		}
> >+
> >+		data->phy = phy;
> >  	}
> >
> >  	/* we only support host now, so enable vbus here */
> 
> WBR, Sergei
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v4] USB: add devicetree helpers for determining dr_mode and phy_type

[Sascha Hauer]

Alexander,

Did you have a chance to look at this series? Development of this driver
has stalled for too long now, I don't want to miss the next merge
window.

Thanks
 Sascha

On Mon, Feb 04, 2013 at 02:24:26PM +0100, Sascha Hauer wrote:
> 4th round of patches.
> 
> Peter, I would be glad if you could test them before your holiday. I rebased
> your last round of Chipidea OTG patches onto this series which you can pull
> here:
> 
> git://git.pengutronix.de/git/imx/linux-2.6.git tags/usb-chipidea-otg-for-next
> 
> I couldn't really test the otg patches since my current hardware does not have
> the ID pin connected, but I can verify that my usecase still works with your
> patches applied.
> 
> Alex, should the patches work for you and are fine otherwise, could you apply
> them for v3.9?
> 
> Sascha
> 
> 
> 
> changes since v3:
> 
> - add phy patches (which were accidently already part of v2)
> - Use OP_DEVLC instead of OP_PORTSC for lpm case
> - Use enum usb_dr_mode ub ci_hdrc_probe()
> 
> changes since v2:
> 
> - fix adding of GPL Header was in wrong patch
> - add missing hunk for new file of.c
> 
> changes since v1:
> - move phy specific of helper to drivers/usb/phy/of.c
> - use strcmp instead of strcasecmp for matching property values
> - change usb_phy_dr_mode to usb_dr_mode
> - change USBPHY_INTERFACE_MODE_NA to USBPHY_INTERFACE_MODE_UNKNOWN
> - add copyright header to new files
> - chipidea: drop mdelay at end of PTS/PTW setup
> - chipidea: implement lpm core type handling for PTS/PTW
> 
> 
> The following changes since commit 2f0760774711c957c395b31131b848043af98edf:
> 
>   USB: GADGET: optionally force full-speed for net2280 UDC (2013-01-31 10:09:19 +0100)
> 
> are available in the git repository at:
> 
>   git://git.pengutronix.de/git/imx/linux-2.6.git tags/usb-chipidea-for-next
> 
> for you to fetch changes up to 25682afd7be85f1462647d8530dca1bf848074fc:
> 
>   USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy (2013-02-04 12:28:53 +0100)
> 
> ----------------------------------------------------------------
> USB: chipidea patches for v3.9
> 
> These add OF helpers for handling the dr_mode and phy_type property
> and makes use of them in the chipidea driver.
> 
> ----------------------------------------------------------------
> Marc Kleine-Budde (1):
>       usb: otg: use try_module_get in all usb_get_phy functions and add missing module_put
> 
> Michael Grzeschik (3):
>       USB: add devicetree helpers for determining dr_mode and phy_type
>       USB: chipidea: ci13xxx-imx: create dynamic platformdata
>       USB: chipidea: add PTW and PTS handling
> 
> Sascha Hauer (5):
>       USB: move bulk of otg/otg.c to phy/phy.c
>       USB chipidea: introduce dual role mode pdata flags
>       USB chipidea i.MX: introduce dr_mode property
>       USB mxs-phy: Register phy with framework
>       USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy
> 
>  .../devicetree/bindings/usb/ci13xxx-imx.txt        |    6 +
>  drivers/usb/chipidea/bits.h                        |   14 +-
>  drivers/usb/chipidea/ci13xxx_imx.c                 |   68 +--
>  drivers/usb/chipidea/core.c                        |   60 ++-
>  drivers/usb/otg/mxs-phy.c                          |    9 +
>  drivers/usb/otg/otg.c                              |  423 -------------------
>  drivers/usb/phy/Makefile                           |    2 +
>  drivers/usb/phy/of.c                               |   47 +++
>  drivers/usb/phy/phy.c                              |  438 ++++++++++++++++++++
>  drivers/usb/usb-common.c                           |   36 ++
>  include/linux/usb/chipidea.h                       |    3 +-
>  include/linux/usb/of.h                             |   27 ++
>  include/linux/usb/otg.h                            |    7 +
>  include/linux/usb/phy.h                            |    9 +
>  14 files changed, 687 insertions(+), 462 deletions(-)
>  create mode 100644 drivers/usb/phy/of.c
>  create mode 100644 drivers/usb/phy/phy.c
>  create mode 100644 include/linux/usb/of.h
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v4] USB: add devicetree helpers for determining dr_mode and phy_type

[Sascha Hauer]

On Thu, Feb 07, 2013 at 11:56:32AM +0100, Sascha Hauer wrote:
> Alexander,
> 
> Did you have a chance to look at this series? Development of this driver
> has stalled for too long now, I don't want to miss the next merge
> window.

Alexander,

Any comments?

Thanks
 Sascha

> 
> Thanks
>  Sascha
> 
> On Mon, Feb 04, 2013 at 02:24:26PM +0100, Sascha Hauer wrote:
> > 4th round of patches.
> > 
> > Peter, I would be glad if you could test them before your holiday. I rebased
> > your last round of Chipidea OTG patches onto this series which you can pull
> > here:
> > 
> > git://git.pengutronix.de/git/imx/linux-2.6.git tags/usb-chipidea-otg-for-next
> > 
> > I couldn't really test the otg patches since my current hardware does not have
> > the ID pin connected, but I can verify that my usecase still works with your
> > patches applied.
> > 
> > Alex, should the patches work for you and are fine otherwise, could you apply
> > them for v3.9?
> > 
> > Sascha
> > 
> > 
> > 
> > changes since v3:
> > 
> > - add phy patches (which were accidently already part of v2)
> > - Use OP_DEVLC instead of OP_PORTSC for lpm case
> > - Use enum usb_dr_mode ub ci_hdrc_probe()
> > 
> > changes since v2:
> > 
> > - fix adding of GPL Header was in wrong patch
> > - add missing hunk for new file of.c
> > 
> > changes since v1:
> > - move phy specific of helper to drivers/usb/phy/of.c
> > - use strcmp instead of strcasecmp for matching property values
> > - change usb_phy_dr_mode to usb_dr_mode
> > - change USBPHY_INTERFACE_MODE_NA to USBPHY_INTERFACE_MODE_UNKNOWN
> > - add copyright header to new files
> > - chipidea: drop mdelay at end of PTS/PTW setup
> > - chipidea: implement lpm core type handling for PTS/PTW
> > 
> > 
> > The following changes since commit 2f0760774711c957c395b31131b848043af98edf:
> > 
> >   USB: GADGET: optionally force full-speed for net2280 UDC (2013-01-31 10:09:19 +0100)
> > 
> > are available in the git repository at:
> > 
> >   git://git.pengutronix.de/git/imx/linux-2.6.git tags/usb-chipidea-for-next
> > 
> > for you to fetch changes up to 25682afd7be85f1462647d8530dca1bf848074fc:
> > 
> >   USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy (2013-02-04 12:28:53 +0100)
> > 
> > ----------------------------------------------------------------
> > USB: chipidea patches for v3.9
> > 
> > These add OF helpers for handling the dr_mode and phy_type property
> > and makes use of them in the chipidea driver.
> > 
> > ----------------------------------------------------------------
> > Marc Kleine-Budde (1):
> >       usb: otg: use try_module_get in all usb_get_phy functions and add missing module_put
> > 
> > Michael Grzeschik (3):
> >       USB: add devicetree helpers for determining dr_mode and phy_type
> >       USB: chipidea: ci13xxx-imx: create dynamic platformdata
> >       USB: chipidea: add PTW and PTS handling
> > 
> > Sascha Hauer (5):
> >       USB: move bulk of otg/otg.c to phy/phy.c
> >       USB chipidea: introduce dual role mode pdata flags
> >       USB chipidea i.MX: introduce dr_mode property
> >       USB mxs-phy: Register phy with framework
> >       USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy
> > 
> >  .../devicetree/bindings/usb/ci13xxx-imx.txt        |    6 +
> >  drivers/usb/chipidea/bits.h                        |   14 +-
> >  drivers/usb/chipidea/ci13xxx_imx.c                 |   68 +--
> >  drivers/usb/chipidea/core.c                        |   60 ++-
> >  drivers/usb/otg/mxs-phy.c                          |    9 +
> >  drivers/usb/otg/otg.c                              |  423 -------------------
> >  drivers/usb/phy/Makefile                           |    2 +
> >  drivers/usb/phy/of.c                               |   47 +++
> >  drivers/usb/phy/phy.c                              |  438 ++++++++++++++++++++
> >  drivers/usb/usb-common.c                           |   36 ++
> >  include/linux/usb/chipidea.h                       |    3 +-
> >  include/linux/usb/of.h                             |   27 ++
> >  include/linux/usb/otg.h                            |    7 +
> >  include/linux/usb/phy.h                            |    9 +
> >  14 files changed, 687 insertions(+), 462 deletions(-)
> >  create mode 100644 drivers/usb/phy/of.c
> >  create mode 100644 drivers/usb/phy/phy.c
> >  create mode 100644 include/linux/usb/of.h
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type

[Alexander Shishkin]

Sascha Hauer <s.hauer@pengutronix.de> writes:

> From: Michael Grzeschik <m.grzeschik@pengutronix.de>
>
> This adds two little devicetree helper functions for determining the
> dr_mode (host, peripheral, otg) and phy_type (utmi, ulpi,...) from
> the devicetree.
>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/usb/phy/Makefile |    1 +
>  drivers/usb/phy/of.c     |   47 ++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/usb/usb-common.c |   36 +++++++++++++++++++++++++++++++++++
>  include/linux/usb/of.h   |   27 ++++++++++++++++++++++++++
>  include/linux/usb/otg.h  |    7 +++++++
>  include/linux/usb/phy.h  |    9 +++++++++
>  6 files changed, 127 insertions(+)
>  create mode 100644 drivers/usb/phy/of.c
>  create mode 100644 include/linux/usb/of.h
>
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index 9fa6327..e1be1e8 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -5,6 +5,7 @@
>  ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
>  
>  obj-$(CONFIG_USB_OTG_UTILS)		+= phy.o
> +obj-$(CONFIG_OF)			+= of.o
>  obj-$(CONFIG_OMAP_USB2)			+= omap-usb2.o
>  obj-$(CONFIG_OMAP_USB3)			+= omap-usb3.o
>  obj-$(CONFIG_OMAP_CONTROL_USB)		+= omap-control-usb.o
> diff --git a/drivers/usb/phy/of.c b/drivers/usb/phy/of.c
> new file mode 100644
> index 0000000..e6f3b74
> --- /dev/null
> +++ b/drivers/usb/phy/of.c
> @@ -0,0 +1,47 @@
> +/*
> + * USB of helper code
> + *
> + * 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/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/usb/of.h>
> +#include <linux/usb/otg.h>
> +
> +static const char *usbphy_modes[] = {
> +	[USBPHY_INTERFACE_MODE_UNKNOWN]	= "",
> +	[USBPHY_INTERFACE_MODE_UTMI]	= "utmi",
> +	[USBPHY_INTERFACE_MODE_UTMIW]	= "utmi_wide",
> +	[USBPHY_INTERFACE_MODE_ULPI]	= "ulpi",
> +	[USBPHY_INTERFACE_MODE_SERIAL]	= "serial",
> +	[USBPHY_INTERFACE_MODE_HSIC]	= "hsic",
> +};
> +
> +/**
> + * of_usb_get_phy_mode - Get phy mode for given device_node
> + * @np:	Pointer to the given device_node
> + *
> + * The function gets phy interface string from property 'phy_type',
> + * and returns the correspondig enum usb_phy_interface
> + */
> +enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np)
> +{
> +	const char *phy_type;
> +	int err, i;
> +
> +	err = of_property_read_string(np, "phy_type", &phy_type);
> +	if (err < 0)
> +		return USBPHY_INTERFACE_MODE_UNKNOWN;
> +
> +	for (i = 0; i < ARRAY_SIZE(usbphy_modes); i++)
> +		if (!strcmp(phy_type, usbphy_modes[i]))
> +			return i;
> +
> +	return USBPHY_INTERFACE_MODE_UNKNOWN;
> +}
> +EXPORT_SYMBOL_GPL(of_usb_get_phy_mode);
> diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
> index d29503e..ad4d87d 100644
> --- a/drivers/usb/usb-common.c
> +++ b/drivers/usb/usb-common.c
> @@ -14,6 +14,9 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/usb/ch9.h>
> +#include <linux/of.h>
> +#include <linux/usb/of.h>
> +#include <linux/usb/otg.h>
>  
>  const char *usb_speed_string(enum usb_device_speed speed)
>  {
> @@ -32,4 +35,37 @@ const char *usb_speed_string(enum usb_device_speed speed)
>  }
>  EXPORT_SYMBOL_GPL(usb_speed_string);
>  
> +#ifdef CONFIG_OF
> +static const char *usb_dr_modes[] = {
> +	[USB_DR_MODE_UNKNOWN]		= "",
> +	[USB_DR_MODE_HOST]		= "host",
> +	[USB_DR_MODE_PERIPHERAL]	= "peripheral",
> +	[USB_DR_MODE_OTG]		= "otg",
> +};

It turns out this is a problem, especially since this is generic usb
code: we have a chipidea controller (a patchset just arrived) that does
both host and peripheral, but not otg. And I'm told now that dwc3
controller can be synthesized like that too.

Summoning Felipe to the thread, this patch is largely his call anyway.

Regards,
--
Alex

[PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type

[Marc Kleine-Budde]

On 02/14/2013 10:36 AM, Alexander Shishkin wrote:
> Sascha Hauer <s.hauer@pengutronix.de> writes:
> 
>> From: Michael Grzeschik <m.grzeschik@pengutronix.de>
>>
>> This adds two little devicetree helper functions for determining the
>> dr_mode (host, peripheral, otg) and phy_type (utmi, ulpi,...) from
>> the devicetree.
>>
>> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> ---
>>  drivers/usb/phy/Makefile |    1 +
>>  drivers/usb/phy/of.c     |   47 ++++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/usb/usb-common.c |   36 +++++++++++++++++++++++++++++++++++
>>  include/linux/usb/of.h   |   27 ++++++++++++++++++++++++++
>>  include/linux/usb/otg.h  |    7 +++++++
>>  include/linux/usb/phy.h  |    9 +++++++++
>>  6 files changed, 127 insertions(+)
>>  create mode 100644 drivers/usb/phy/of.c
>>  create mode 100644 include/linux/usb/of.h
>>
>> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
>> index 9fa6327..e1be1e8 100644
>> --- a/drivers/usb/phy/Makefile
>> +++ b/drivers/usb/phy/Makefile
>> @@ -5,6 +5,7 @@
>>  ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
>>  
>>  obj-$(CONFIG_USB_OTG_UTILS)		+= phy.o
>> +obj-$(CONFIG_OF)			+= of.o
>>  obj-$(CONFIG_OMAP_USB2)			+= omap-usb2.o
>>  obj-$(CONFIG_OMAP_USB3)			+= omap-usb3.o
>>  obj-$(CONFIG_OMAP_CONTROL_USB)		+= omap-control-usb.o
>> diff --git a/drivers/usb/phy/of.c b/drivers/usb/phy/of.c
>> new file mode 100644
>> index 0000000..e6f3b74
>> --- /dev/null
>> +++ b/drivers/usb/phy/of.c
>> @@ -0,0 +1,47 @@
>> +/*
>> + * USB of helper code
>> + *
>> + * 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/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/usb/of.h>
>> +#include <linux/usb/otg.h>
>> +
>> +static const char *usbphy_modes[] = {
>> +	[USBPHY_INTERFACE_MODE_UNKNOWN]	= "",
>> +	[USBPHY_INTERFACE_MODE_UTMI]	= "utmi",
>> +	[USBPHY_INTERFACE_MODE_UTMIW]	= "utmi_wide",
>> +	[USBPHY_INTERFACE_MODE_ULPI]	= "ulpi",
>> +	[USBPHY_INTERFACE_MODE_SERIAL]	= "serial",
>> +	[USBPHY_INTERFACE_MODE_HSIC]	= "hsic",
>> +};
>> +
>> +/**
>> + * of_usb_get_phy_mode - Get phy mode for given device_node
>> + * @np:	Pointer to the given device_node
>> + *
>> + * The function gets phy interface string from property 'phy_type',
>> + * and returns the correspondig enum usb_phy_interface
>> + */
>> +enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np)
>> +{
>> +	const char *phy_type;
>> +	int err, i;
>> +
>> +	err = of_property_read_string(np, "phy_type", &phy_type);
>> +	if (err < 0)
>> +		return USBPHY_INTERFACE_MODE_UNKNOWN;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(usbphy_modes); i++)
>> +		if (!strcmp(phy_type, usbphy_modes[i]))
>> +			return i;
>> +
>> +	return USBPHY_INTERFACE_MODE_UNKNOWN;
>> +}
>> +EXPORT_SYMBOL_GPL(of_usb_get_phy_mode);
>> diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
>> index d29503e..ad4d87d 100644
>> --- a/drivers/usb/usb-common.c
>> +++ b/drivers/usb/usb-common.c
>> @@ -14,6 +14,9 @@
>>  #include <linux/kernel.h>
>>  #include <linux/module.h>
>>  #include <linux/usb/ch9.h>
>> +#include <linux/of.h>
>> +#include <linux/usb/of.h>
>> +#include <linux/usb/otg.h>
>>  
>>  const char *usb_speed_string(enum usb_device_speed speed)
>>  {
>> @@ -32,4 +35,37 @@ const char *usb_speed_string(enum usb_device_speed speed)
>>  }
>>  EXPORT_SYMBOL_GPL(usb_speed_string);
>>  
>> +#ifdef CONFIG_OF
>> +static const char *usb_dr_modes[] = {
>> +	[USB_DR_MODE_UNKNOWN]		= "",
>> +	[USB_DR_MODE_HOST]		= "host",
>> +	[USB_DR_MODE_PERIPHERAL]	= "peripheral",
>> +	[USB_DR_MODE_OTG]		= "otg",
>> +};
> 
> It turns out this is a problem, especially since this is generic usb
> code: we have a chipidea controller (a patchset just arrived) that does
> both host and peripheral, but not otg. And I'm told now that dwc3
> controller can be synthesized like that too.

You mean a single instance of the controller (i.e. USB port) is host and
peripheral but has no otg registers. This means the mode of the port is
configured by userspace via the debugfs file? Is this possible?

The above property describes a single port not the whole controller. If
there is a controller with one host and one peripheral port the code in
this patch should be sufficient, as you have a property in the DT for
each port.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130214/5667bd71/attachment.sig>

[PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type

[Felipe Balbi]

Hi,

On Thu, Feb 14, 2013 at 10:49:44AM +0100, Marc Kleine-Budde wrote:
> >> diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
> >> index d29503e..ad4d87d 100644
> >> --- a/drivers/usb/usb-common.c
> >> +++ b/drivers/usb/usb-common.c
> >> @@ -14,6 +14,9 @@
> >>  #include <linux/kernel.h>
> >>  #include <linux/module.h>
> >>  #include <linux/usb/ch9.h>
> >> +#include <linux/of.h>
> >> +#include <linux/usb/of.h>
> >> +#include <linux/usb/otg.h>
> >>  
> >>  const char *usb_speed_string(enum usb_device_speed speed)
> >>  {
> >> @@ -32,4 +35,37 @@ const char *usb_speed_string(enum usb_device_speed speed)
> >>  }
> >>  EXPORT_SYMBOL_GPL(usb_speed_string);
> >>  
> >> +#ifdef CONFIG_OF
> >> +static const char *usb_dr_modes[] = {
> >> +	[USB_DR_MODE_UNKNOWN]		= "",
> >> +	[USB_DR_MODE_HOST]		= "host",
> >> +	[USB_DR_MODE_PERIPHERAL]	= "peripheral",
> >> +	[USB_DR_MODE_OTG]		= "otg",
> >> +};
> > 
> > It turns out this is a problem, especially since this is generic usb
> > code: we have a chipidea controller (a patchset just arrived) that does
> > both host and peripheral, but not otg. And I'm told now that dwc3
> > controller can be synthesized like that too.

I wonder if this part is really necessary. Usually you would read it
from HW's registers. For dwc3, it's quite recently that we allowed the
driver to be built with host-only, device-only or DRD functionality.

Also, this is likely to create troubles if not done correctly. Imagine
user compiles a host-only driver and board passes dr_mode = peripheral ?

Maybe we can ignore dr_mode in host-only and device-only builds and only
look at it for DRD builds ?

> You mean a single instance of the controller (i.e. USB port) is host and
> peripheral but has no otg registers. This means the mode of the port is
> configured by userspace via the debugfs file? Is this possible?

yes, it is possible. Dual-Role doesn't imply OTG, but OTG implies
Dual-Role.

> The above property describes a single port not the whole controller. If
> there is a controller with one host and one peripheral port the code in
> this patch should be sufficient, as you have a property in the DT for
> each port.

I don't think you can have a single controller like that, but good that
it supports.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130214/23985ced/attachment.sig>

[PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type

[Sascha Hauer]

On Thu, Feb 14, 2013 at 11:58:22AM +0200, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Feb 14, 2013 at 10:49:44AM +0100, Marc Kleine-Budde wrote:
> > >> diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
> > >> index d29503e..ad4d87d 100644
> > >> --- a/drivers/usb/usb-common.c
> > >> +++ b/drivers/usb/usb-common.c
> > >> @@ -14,6 +14,9 @@
> > >>  #include <linux/kernel.h>
> > >>  #include <linux/module.h>
> > >>  #include <linux/usb/ch9.h>
> > >> +#include <linux/of.h>
> > >> +#include <linux/usb/of.h>
> > >> +#include <linux/usb/otg.h>
> > >>  
> > >>  const char *usb_speed_string(enum usb_device_speed speed)
> > >>  {
> > >> @@ -32,4 +35,37 @@ const char *usb_speed_string(enum usb_device_speed speed)
> > >>  }
> > >>  EXPORT_SYMBOL_GPL(usb_speed_string);
> > >>  
> > >> +#ifdef CONFIG_OF
> > >> +static const char *usb_dr_modes[] = {
> > >> +	[USB_DR_MODE_UNKNOWN]		= "",
> > >> +	[USB_DR_MODE_HOST]		= "host",
> > >> +	[USB_DR_MODE_PERIPHERAL]	= "peripheral",
> > >> +	[USB_DR_MODE_OTG]		= "otg",
> > >> +};
> > > 
> > > It turns out this is a problem, especially since this is generic usb
> > > code: we have a chipidea controller (a patchset just arrived) that does
> > > both host and peripheral, but not otg. And I'm told now that dwc3
> > > controller can be synthesized like that too.
> 
> I wonder if this part is really necessary. Usually you would read it
> from HW's registers. For dwc3, it's quite recently that we allowed the
> driver to be built with host-only, device-only or DRD functionality.

We have quite some boards on which the ID pin is not wired up, so if a
core is both host and device capable there is no way to detect the
wanted mode if not given from the devicetree.

> 
> Also, this is likely to create troubles if not done correctly. Imagine
> user compiles a host-only driver and board passes dr_mode = peripheral ?

Either nothing will happen or some "you messed it up" printk should show
the user what's going on.

> 
> Maybe we can ignore dr_mode in host-only and device-only builds and only
> look at it for DRD builds ?

If something is or is not compiled in the kernel this doesn't mean the kernel
is not started on boards with a different situation.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type

[Marc Kleine-Budde]

On 02/14/2013 10:58 AM, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Feb 14, 2013 at 10:49:44AM +0100, Marc Kleine-Budde wrote:
>>>> diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
>>>> index d29503e..ad4d87d 100644
>>>> --- a/drivers/usb/usb-common.c
>>>> +++ b/drivers/usb/usb-common.c
>>>> @@ -14,6 +14,9 @@
>>>>  #include <linux/kernel.h>
>>>>  #include <linux/module.h>
>>>>  #include <linux/usb/ch9.h>
>>>> +#include <linux/of.h>
>>>> +#include <linux/usb/of.h>
>>>> +#include <linux/usb/otg.h>
>>>>  
>>>>  const char *usb_speed_string(enum usb_device_speed speed)
>>>>  {
>>>> @@ -32,4 +35,37 @@ const char *usb_speed_string(enum usb_device_speed speed)
>>>>  }
>>>>  EXPORT_SYMBOL_GPL(usb_speed_string);
>>>>  
>>>> +#ifdef CONFIG_OF
>>>> +static const char *usb_dr_modes[] = {
>>>> +	[USB_DR_MODE_UNKNOWN]		= "",
>>>> +	[USB_DR_MODE_HOST]		= "host",
>>>> +	[USB_DR_MODE_PERIPHERAL]	= "peripheral",
>>>> +	[USB_DR_MODE_OTG]		= "otg",
>>>> +};
>>>
>>> It turns out this is a problem, especially since this is generic usb
>>> code: we have a chipidea controller (a patchset just arrived) that does
>>> both host and peripheral, but not otg. And I'm told now that dwc3
>>> controller can be synthesized like that too.
> 
> I wonder if this part is really necessary. Usually you would read it
> from HW's registers. For dwc3, it's quite recently that we allowed the
> driver to be built with host-only, device-only or DRD functionality.

The imx25 USB version of the chipidea IP Core get's really confused if
you read from the CAP_DCCPARAMS register on the host only port. In
freescale's documentation the register is marked as reserved. The host
port will not work then.

> Also, this is likely to create troubles if not done correctly. Imagine
> user compiles a host-only driver and board passes dr_mode = peripheral ?

Use dev_err() and inform the user.

> Maybe we can ignore dr_mode in host-only and device-only builds and only
> look at it for DRD builds ?
> 
>> You mean a single instance of the controller (i.e. USB port) is host and
>> peripheral but has no otg registers. This means the mode of the port is
>> configured by userspace via the debugfs file? Is this possible?
> 
> yes, it is possible. Dual-Role doesn't imply OTG, but OTG implies
> Dual-Role.

Thanks for the info. Then we need a fourth value for the helper code.
What's a sensible sting: "dual-role", "dr"?

>> The above property describes a single port not the whole controller. If
>> there is a controller with one host and one peripheral port the code in
>> this patch should be sufficient, as you have a property in the DT for
>> each port.
> 
> I don't think you can have a single controller like that, but good that
> it supports.

The DT binding is per USB port, so that's a by product of good
abstraction :)

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130214/2e0c3ad9/attachment.sig>

[PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type

[Felipe Balbi]

Hi,

On Thu, Feb 14, 2013 at 11:07:22AM +0100, Sascha Hauer wrote:
> > > >> @@ -32,4 +35,37 @@ const char *usb_speed_string(enum usb_device_speed speed)
> > > >>  }
> > > >>  EXPORT_SYMBOL_GPL(usb_speed_string);
> > > >>  
> > > >> +#ifdef CONFIG_OF
> > > >> +static const char *usb_dr_modes[] = {
> > > >> +	[USB_DR_MODE_UNKNOWN]		= "",
> > > >> +	[USB_DR_MODE_HOST]		= "host",
> > > >> +	[USB_DR_MODE_PERIPHERAL]	= "peripheral",
> > > >> +	[USB_DR_MODE_OTG]		= "otg",
> > > >> +};
> > > > 
> > > > It turns out this is a problem, especially since this is generic usb
> > > > code: we have a chipidea controller (a patchset just arrived) that does
> > > > both host and peripheral, but not otg. And I'm told now that dwc3
> > > > controller can be synthesized like that too.
> > 
> > I wonder if this part is really necessary. Usually you would read it
> > from HW's registers. For dwc3, it's quite recently that we allowed the
> > driver to be built with host-only, device-only or DRD functionality.
> 
> We have quite some boards on which the ID pin is not wired up, so if a
> core is both host and device capable there is no way to detect the
> wanted mode if not given from the devicetree.

right, that's fair. But that doesn't mean board can't work as both,
right. The IP is still the same, just the board is wired differently ;-)

> > Maybe we can ignore dr_mode in host-only and device-only builds and only
> > look at it for DRD builds ?
> 
> If something is or is not compiled in the kernel this doesn't mean the kernel
> is not started on boards with a different situation.

who said kernel wouldn't start ? If you request a host-only build, you
need to force your IP into working as host, since that's all you have,
either that or you bail out on probe().

If board wants to work as a device, well tough luck, go figure how to
setup Kconfig properly.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130214/dd70023d/attachment.sig>

[PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type

[Felipe Balbi]

On Thu, Feb 14, 2013 at 11:11:55AM +0100, Marc Kleine-Budde wrote:
> >>>> @@ -32,4 +35,37 @@ const char *usb_speed_string(enum usb_device_speed speed)
> >>>>  }
> >>>>  EXPORT_SYMBOL_GPL(usb_speed_string);
> >>>>  
> >>>> +#ifdef CONFIG_OF
> >>>> +static const char *usb_dr_modes[] = {
> >>>> +	[USB_DR_MODE_UNKNOWN]		= "",
> >>>> +	[USB_DR_MODE_HOST]		= "host",
> >>>> +	[USB_DR_MODE_PERIPHERAL]	= "peripheral",
> >>>> +	[USB_DR_MODE_OTG]		= "otg",
> >>>> +};
> >>>
> >>> It turns out this is a problem, especially since this is generic usb
> >>> code: we have a chipidea controller (a patchset just arrived) that does
> >>> both host and peripheral, but not otg. And I'm told now that dwc3
> >>> controller can be synthesized like that too.
> > 
> > I wonder if this part is really necessary. Usually you would read it
> > from HW's registers. For dwc3, it's quite recently that we allowed the
> > driver to be built with host-only, device-only or DRD functionality.
> 
> The imx25 USB version of the chipidea IP Core get's really confused if
> you read from the CAP_DCCPARAMS register on the host only port. In
> freescale's documentation the register is marked as reserved. The host
> port will not work then.

heh, nicely done.

> > Maybe we can ignore dr_mode in host-only and device-only builds and only
> > look at it for DRD builds ?
> > 
> >> You mean a single instance of the controller (i.e. USB port) is host and
> >> peripheral but has no otg registers. This means the mode of the port is
> >> configured by userspace via the debugfs file? Is this possible?
> > 
> > yes, it is possible. Dual-Role doesn't imply OTG, but OTG implies
> > Dual-Role.
> 
> Thanks for the info. Then we need a fourth value for the helper code.
> What's a sensible sting: "dual-role", "dr"?

dual-role.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130214/c09ab693/attachment.sig>

[PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type

[Sascha Hauer]

On Thu, Feb 14, 2013 at 12:15:10PM +0200, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Feb 14, 2013 at 11:07:22AM +0100, Sascha Hauer wrote:
> > > > >> @@ -32,4 +35,37 @@ const char *usb_speed_string(enum usb_device_speed speed)
> > > > >>  }
> > > > >>  EXPORT_SYMBOL_GPL(usb_speed_string);
> > > > >>  
> > > > >> +#ifdef CONFIG_OF
> > > > >> +static const char *usb_dr_modes[] = {
> > > > >> +	[USB_DR_MODE_UNKNOWN]		= "",
> > > > >> +	[USB_DR_MODE_HOST]		= "host",
> > > > >> +	[USB_DR_MODE_PERIPHERAL]	= "peripheral",
> > > > >> +	[USB_DR_MODE_OTG]		= "otg",
> > > > >> +};
> > > > > 
> > > > > It turns out this is a problem, especially since this is generic usb
> > > > > code: we have a chipidea controller (a patchset just arrived) that does
> > > > > both host and peripheral, but not otg. And I'm told now that dwc3
> > > > > controller can be synthesized like that too.
> > > 
> > > I wonder if this part is really necessary. Usually you would read it
> > > from HW's registers. For dwc3, it's quite recently that we allowed the
> > > driver to be built with host-only, device-only or DRD functionality.
> > 
> > We have quite some boards on which the ID pin is not wired up, so if a
> > core is both host and device capable there is no way to detect the
> > wanted mode if not given from the devicetree.
> 
> right, that's fair. But that doesn't mean board can't work as both,
> right. The IP is still the same, just the board is wired differently ;-)

Yes, it is. Usually I run kernels with both host and device support enabled.
Consider the IP is OTG capable, the chipidea driver will initialize
both roles. Now if a board only supports one role and does not have an
ID pin, how do I make sure the driver is in the correct mode? I need to
specify it somehow. Otherwise I may end up on a host-only board with the
driver sitting in device mode, or with a device-only board with the
driver sitting in host mode.

> 
> > > Maybe we can ignore dr_mode in host-only and device-only builds and only
> > > look at it for DRD builds ?
> > 
> > If something is or is not compiled in the kernel this doesn't mean the kernel
> > is not started on boards with a different situation.
> 
> who said kernel wouldn't start ? If you request a host-only build, you
> need to force your IP into working as host, since that's all you have,
> either that or you bail out on probe().

Let me clarify, I don't want to use Kconfig to specify my boards
capabilities. If a kernel is compiled for host mode only and the
devicetree specifies a port is device-only, then yes, the driver
should bail out on probe, maybe leaving a message that it found
a device for which the suitable role is not compiled in.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 5/9] USB: chipidea: add PTW and PTS handling

[Alexander Shishkin]

Sascha Hauer <s.hauer@pengutronix.de> writes:

> From: Michael Grzeschik <m.grzeschik@pengutronix.de>
>
> This patch makes it possible to configure the PTW and PTS bits inside
> the portsc register for host and device mode before the driver starts
> and the phy can be addressed as hardware implementation is designed.
>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  .../devicetree/bindings/usb/ci13xxx-imx.txt        |    5 +++
>  drivers/usb/chipidea/bits.h                        |   14 ++++++-
>  drivers/usb/chipidea/ci13xxx_imx.c                 |    3 ++
>  drivers/usb/chipidea/core.c                        |   39 ++++++++++++++++++++
>  include/linux/usb/chipidea.h                       |    1 +
>  5 files changed, 61 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
> index 5778b9c..dd42ccd 100644
> --- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
> +++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
> @@ -5,6 +5,11 @@ Required properties:
>  - reg: Should contain registers location and length
>  - interrupts: Should contain controller interrupt
>  
> +Recommended properies:
> +- phy_type: the type of the phy connected to the core. Should be one
> +  of "utmi", "utmi_wide", "ulpi", "serial" or "hsic". Without this
> +  property the PORTSC register won't be touched
> +

Looks like this bit belongs to patch 3/9, where you're adding devicetree
hooks. Otherwise looks good.

Regards,
--
Alex

[PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type

[Felipe Balbi]

On Thu, Feb 14, 2013 at 12:24:49PM +0100, Sascha Hauer wrote:
> On Thu, Feb 14, 2013 at 12:15:10PM +0200, Felipe Balbi wrote:
> > Hi,
> > 
> > On Thu, Feb 14, 2013 at 11:07:22AM +0100, Sascha Hauer wrote:
> > > > > >> @@ -32,4 +35,37 @@ const char *usb_speed_string(enum usb_device_speed speed)
> > > > > >>  }
> > > > > >>  EXPORT_SYMBOL_GPL(usb_speed_string);
> > > > > >>  
> > > > > >> +#ifdef CONFIG_OF
> > > > > >> +static const char *usb_dr_modes[] = {
> > > > > >> +	[USB_DR_MODE_UNKNOWN]		= "",
> > > > > >> +	[USB_DR_MODE_HOST]		= "host",
> > > > > >> +	[USB_DR_MODE_PERIPHERAL]	= "peripheral",
> > > > > >> +	[USB_DR_MODE_OTG]		= "otg",
> > > > > >> +};
> > > > > > 
> > > > > > It turns out this is a problem, especially since this is generic usb
> > > > > > code: we have a chipidea controller (a patchset just arrived) that does
> > > > > > both host and peripheral, but not otg. And I'm told now that dwc3
> > > > > > controller can be synthesized like that too.
> > > > 
> > > > I wonder if this part is really necessary. Usually you would read it
> > > > from HW's registers. For dwc3, it's quite recently that we allowed the
> > > > driver to be built with host-only, device-only or DRD functionality.
> > > 
> > > We have quite some boards on which the ID pin is not wired up, so if a
> > > core is both host and device capable there is no way to detect the
> > > wanted mode if not given from the devicetree.
> > 
> > right, that's fair. But that doesn't mean board can't work as both,
> > right. The IP is still the same, just the board is wired differently ;-)
> 
> Yes, it is. Usually I run kernels with both host and device support enabled.
> Consider the IP is OTG capable, the chipidea driver will initialize
> both roles. Now if a board only supports one role and does not have an
> ID pin, how do I make sure the driver is in the correct mode? I need to
> specify it somehow. Otherwise I may end up on a host-only board with the
> driver sitting in device mode, or with a device-only board with the
> driver sitting in host mode.

right.

> > > > Maybe we can ignore dr_mode in host-only and device-only builds and only
> > > > look at it for DRD builds ?
> > > 
> > > If something is or is not compiled in the kernel this doesn't mean the kernel
> > > is not started on boards with a different situation.
> > 
> > who said kernel wouldn't start ? If you request a host-only build, you
> > need to force your IP into working as host, since that's all you have,
> > either that or you bail out on probe().
> 
> Let me clarify, I don't want to use Kconfig to specify my boards
> capabilities. If a kernel is compiled for host mode only and the
> devicetree specifies a port is device-only, then yes, the driver
> should bail out on probe, maybe leaving a message that it found
> a device for which the suitable role is not compiled in.

yeah, this is why I said we should ignore dr_mode (or bail out) when
!OTG.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130214/49850d5d/attachment.sig>

[PATCH v4] USB: add devicetree helpers for determining dr_mode and phy_type

[Alexander Shishkin]

Sascha Hauer <s.hauer@pengutronix.de> writes:

> 4th round of patches.
>
> Peter, I would be glad if you could test them before your holiday. I rebased
> your last round of Chipidea OTG patches onto this series which you can pull
> here:
>
> git://git.pengutronix.de/git/imx/linux-2.6.git tags/usb-chipidea-otg-for-next
>
> I couldn't really test the otg patches since my current hardware does not have
> the ID pin connected, but I can verify that my usecase still works with your
> patches applied.
>
> Alex, should the patches work for you and are fine otherwise, could you apply
> them for v3.9?

Looks good in general. I just realised the comment I made on 5/9 is not
entirely valid, but the dr_mode setting needs one extra option as was
discussed. Also, all the phy/otg patches (1/9, 2/9, 3/9, 8/9) need
Felipe's ack, then I can apply the series.

Regards,
--
Alex

[PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type

[Sascha Hauer]

On Thu, Feb 14, 2013 at 03:10:15PM +0200, Felipe Balbi wrote:
> On Thu, Feb 14, 2013 at 12:24:49PM +0100, Sascha Hauer wrote:
> > On Thu, Feb 14, 2013 at 12:15:10PM +0200, Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Thu, Feb 14, 2013 at 11:07:22AM +0100, Sascha Hauer wrote:
> > > > > > >> @@ -32,4 +35,37 @@ const char *usb_speed_string(enum usb_device_speed speed)
> > > > > > >>  }
> > > > > > >>  EXPORT_SYMBOL_GPL(usb_speed_string);
> > > > > > >>  
> > > > > > >> +#ifdef CONFIG_OF
> > > > > > >> +static const char *usb_dr_modes[] = {
> > > > > > >> +	[USB_DR_MODE_UNKNOWN]		= "",
> > > > > > >> +	[USB_DR_MODE_HOST]		= "host",
> > > > > > >> +	[USB_DR_MODE_PERIPHERAL]	= "peripheral",
> > > > > > >> +	[USB_DR_MODE_OTG]		= "otg",
> > > > > > >> +};
> > > > > > > 
> > > > > > > It turns out this is a problem, especially since this is generic usb
> > > > > > > code: we have a chipidea controller (a patchset just arrived) that does
> > > > > > > both host and peripheral, but not otg. And I'm told now that dwc3
> > > > > > > controller can be synthesized like that too.
> > > > > 
> > > > > I wonder if this part is really necessary. Usually you would read it
> > > > > from HW's registers. For dwc3, it's quite recently that we allowed the
> > > > > driver to be built with host-only, device-only or DRD functionality.
> > > > 
> > > > We have quite some boards on which the ID pin is not wired up, so if a
> > > > core is both host and device capable there is no way to detect the
> > > > wanted mode if not given from the devicetree.
> > > 
> > > right, that's fair. But that doesn't mean board can't work as both,
> > > right. The IP is still the same, just the board is wired differently ;-)
> > 
> > Yes, it is. Usually I run kernels with both host and device support enabled.
> > Consider the IP is OTG capable, the chipidea driver will initialize
> > both roles. Now if a board only supports one role and does not have an
> > ID pin, how do I make sure the driver is in the correct mode? I need to
> > specify it somehow. Otherwise I may end up on a host-only board with the
> > driver sitting in device mode, or with a device-only board with the
> > driver sitting in host mode.
> 
> right.
> 
> > > > > Maybe we can ignore dr_mode in host-only and device-only builds and only
> > > > > look at it for DRD builds ?
> > > > 
> > > > If something is or is not compiled in the kernel this doesn't mean the kernel
> > > > is not started on boards with a different situation.
> > > 
> > > who said kernel wouldn't start ? If you request a host-only build, you
> > > need to force your IP into working as host, since that's all you have,
> > > either that or you bail out on probe().
> > 
> > Let me clarify, I don't want to use Kconfig to specify my boards
> > capabilities. If a kernel is compiled for host mode only and the
> > devicetree specifies a port is device-only, then yes, the driver
> > should bail out on probe, maybe leaving a message that it found
> > a device for which the suitable role is not compiled in.
> 
> yeah, this is why I said we should ignore dr_mode (or bail out) when
> !OTG.

Ok, that's what the patch effectively does. We have this in chipidea/core.c:

| 	dr_mode = ci->platdata->dr_mode;
| 	if (dr_mode == USB_DR_MODE_UNKNOWN)
| 		dr_mode = USB_DR_MODE_OTG;

default to otg if nothing specified.

| 
| 	/* initialize role(s) before the interrupt is requested */
| 	if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
| 		ret = ci_hdrc_host_init(ci);
| 		if (ret)
| 			dev_info(dev, "doesn't support host\n");
| 	}

If we can do OTG or host, try to initialize host role,

| 
| 	if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) {
| 		ret = ci_hdrc_gadget_init(ci);
| 		if (ret)
| 			dev_info(dev, "doesn't support gadget\n");
| 	}

if we can do otg or peripheral, try to initialize device role,

| 
| 	if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) {
| 		dev_err(dev, "no supported roles\n");
| 		ret = -ENODEV;
| 		goto rm_wq;
| 	}

If we have no roles, complain and bail out.

ci_hdrc_host_init / ci_hdrc_gadget_init are static inline noops when
gadget or host are disabled.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type

[Felipe Balbi]

Hi,

On Thu, Feb 14, 2013 at 05:06:55PM +0100, Sascha Hauer wrote:
> > > > > > Maybe we can ignore dr_mode in host-only and device-only builds and only
> > > > > > look at it for DRD builds ?
> > > > > 
> > > > > If something is or is not compiled in the kernel this doesn't mean the kernel
> > > > > is not started on boards with a different situation.
> > > > 
> > > > who said kernel wouldn't start ? If you request a host-only build, you
> > > > need to force your IP into working as host, since that's all you have,
> > > > either that or you bail out on probe().
> > > 
> > > Let me clarify, I don't want to use Kconfig to specify my boards
> > > capabilities. If a kernel is compiled for host mode only and the
> > > devicetree specifies a port is device-only, then yes, the driver
> > > should bail out on probe, maybe leaving a message that it found
> > > a device for which the suitable role is not compiled in.
> > 
> > yeah, this is why I said we should ignore dr_mode (or bail out) when
> > !OTG.
> 
> Ok, that's what the patch effectively does. We have this in chipidea/core.c:
> 
> | 	dr_mode = ci->platdata->dr_mode;
> | 	if (dr_mode == USB_DR_MODE_UNKNOWN)
> | 		dr_mode = USB_DR_MODE_OTG;
> 
> default to otg if nothing specified.

you missed my point. I wanted something like:

dr_mode = ci->platdata->dr_mode;
if ((dr_mode == USB_DR_MODE_UNKNOWN) || !IS_ENABLED(CONFIG_USB_CHIPIDEA_OTG)
	dr_mode = USB_DR_MODE_OTG;

this copes with the situation where dr_mode == USB_DR_MODE_HOST but
kernel is gadget-only.


-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130214/4c161c47/attachment.sig>

[PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type

[Sascha Hauer]

On Thu, Feb 14, 2013 at 08:04:44PM +0200, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Feb 14, 2013 at 05:06:55PM +0100, Sascha Hauer wrote:
> > > > > > > Maybe we can ignore dr_mode in host-only and device-only builds and only
> > > > > > > look at it for DRD builds ?
> > > > > > 
> > > > > > If something is or is not compiled in the kernel this doesn't mean the kernel
> > > > > > is not started on boards with a different situation.
> > > > > 
> > > > > who said kernel wouldn't start ? If you request a host-only build, you
> > > > > need to force your IP into working as host, since that's all you have,
> > > > > either that or you bail out on probe().
> > > > 
> > > > Let me clarify, I don't want to use Kconfig to specify my boards
> > > > capabilities. If a kernel is compiled for host mode only and the
> > > > devicetree specifies a port is device-only, then yes, the driver
> > > > should bail out on probe, maybe leaving a message that it found
> > > > a device for which the suitable role is not compiled in.
> > > 
> > > yeah, this is why I said we should ignore dr_mode (or bail out) when
> > > !OTG.
> > 
> > Ok, that's what the patch effectively does. We have this in chipidea/core.c:
> > 
> > | 	dr_mode = ci->platdata->dr_mode;
> > | 	if (dr_mode == USB_DR_MODE_UNKNOWN)
> > | 		dr_mode = USB_DR_MODE_OTG;
> > 
> > default to otg if nothing specified.
> 
> you missed my point. I wanted something like:
> 
> dr_mode = ci->platdata->dr_mode;
> if ((dr_mode == USB_DR_MODE_UNKNOWN) || !IS_ENABLED(CONFIG_USB_CHIPIDEA_OTG)
> 	dr_mode = USB_DR_MODE_OTG;

So everytime the chipidea driver cannot do OTG the driver falls back to
exactly this mode?

> 
> this copes with the situation where dr_mode == USB_DR_MODE_HOST but
> kernel is gadget-only.

When I specify dr_mode = USB_DR_MODE_HOST in the devicetree indicating
that my board is only host capable I exactly want the driver to be in
host mode for this device, or to bail out if the kernel does not have
host support compiled in.

Sorry, I still don't get it.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type

[Felipe Balbi]

Hi,

On Thu, Feb 14, 2013 at 07:30:26PM +0100, Sascha Hauer wrote:
> > > > yeah, this is why I said we should ignore dr_mode (or bail out) when
> > > > !OTG.
> > > 
> > > Ok, that's what the patch effectively does. We have this in chipidea/core.c:
> > > 
> > > | 	dr_mode = ci->platdata->dr_mode;
> > > | 	if (dr_mode == USB_DR_MODE_UNKNOWN)
> > > | 		dr_mode = USB_DR_MODE_OTG;
> > > 
> > > default to otg if nothing specified.
> > 
> > you missed my point. I wanted something like:
> > 
> > dr_mode = ci->platdata->dr_mode;
> > if ((dr_mode == USB_DR_MODE_UNKNOWN) || !IS_ENABLED(CONFIG_USB_CHIPIDEA_OTG)
> > 	dr_mode = USB_DR_MODE_OTG;
> 
> So everytime the chipidea driver cannot do OTG the driver falls back to
> exactly this mode?

hehe, that was me being stupid I meant something like:

if (!IS_ENABLED(HOST) && dr_mode == HOST)

or

if (!IS_ENABLED(GADGET) && dr_mode == GADGET)

in those cases, it's best to either force OTG (then driver will work
with whatever it has) or bail out.

Thinking a bit harder, it's best to bail as you can't be sure udc.c or
host.c have been compiled in.

> > this copes with the situation where dr_mode == USB_DR_MODE_HOST but
> > kernel is gadget-only.
> 
> When I specify dr_mode = USB_DR_MODE_HOST in the devicetree indicating
> that my board is only host capable I exactly want the driver to be in
> host mode for this device, or to bail out if the kernel does not have
> host support compiled in.

right.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130214/73f94c12/attachment.sig>

[PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type

[Sascha Hauer]

On Thu, Feb 14, 2013 at 09:36:26PM +0200, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Feb 14, 2013 at 07:30:26PM +0100, Sascha Hauer wrote:
> > > > > yeah, this is why I said we should ignore dr_mode (or bail out) when
> > > > > !OTG.
> > > > 
> > > > Ok, that's what the patch effectively does. We have this in chipidea/core.c:
> > > > 
> > > > | 	dr_mode = ci->platdata->dr_mode;
> > > > | 	if (dr_mode == USB_DR_MODE_UNKNOWN)
> > > > | 		dr_mode = USB_DR_MODE_OTG;
> > > > 
> > > > default to otg if nothing specified.
> > > 
> > > you missed my point. I wanted something like:
> > > 
> > > dr_mode = ci->platdata->dr_mode;
> > > if ((dr_mode == USB_DR_MODE_UNKNOWN) || !IS_ENABLED(CONFIG_USB_CHIPIDEA_OTG)
> > > 	dr_mode = USB_DR_MODE_OTG;
> > 
> > So everytime the chipidea driver cannot do OTG the driver falls back to
> > exactly this mode?
> 
> hehe, that was me being stupid I meant something like:
> 
> if (!IS_ENABLED(HOST) && dr_mode == HOST)
> 
> or
> 
> if (!IS_ENABLED(GADGET) && dr_mode == GADGET)
> 
> in those cases, it's best to either force OTG (then driver will work
> with whatever it has) or bail out.
> 
> Thinking a bit harder, it's best to bail as you can't be sure udc.c or
> host.c have been compiled in.

Look again, that's what the current code does. Maybe it's not coded as
explicit as you describe above.

| 	/* initialize role(s) before the interrupt is requested */
| 	if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
| 		ret = ci_hdrc_host_init(ci);
| 		if (ret)
| 			dev_info(dev, "doesn't support host\n");
| 	}

if IS_ENABLED(HOST), then ci_hdrc_host_init() will set
ci->roles[CI_ROLE_HOST] in case it manages to initialize the host driver.

if !IS_ENABLED(HOST), then ci_hdrc_host_init() will be a static inline
noop function, so ci->roles[CI_ROLE_HOST] ends up being NULL.

| 
| 	if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) {
| 		ret = ci_hdrc_gadget_init(ci);
| 		if (ret)
| 			dev_info(dev, "doesn't support gadget\n");
| 	}

Same applies for gadget.

| 
| 	if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) {
| 		dev_err(dev, "no supported roles\n");
| 		ret = -ENODEV;
| 		goto rm_wq;
| 	}

At this point (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET])
means that the intersection of what is requested and what we are able to
do is empty. The reasons could be That the requested mode is not compiled
into the kernel, or the requested role failed to initialize.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type

[Peter Chen]

On Fri, Feb 15, 2013 at 11:54:10AM +0100, Sascha Hauer wrote:
> On Thu, Feb 14, 2013 at 09:36:26PM +0200, Felipe Balbi wrote:
> > Hi,
> > 
> > On Thu, Feb 14, 2013 at 07:30:26PM +0100, Sascha Hauer wrote:
> > > > > > yeah, this is why I said we should ignore dr_mode (or bail out) when
> > > > > > !OTG.
> > > > > 
> > > > > Ok, that's what the patch effectively does. We have this in chipidea/core.c:
> > > > > 
> > > > > | 	dr_mode = ci->platdata->dr_mode;
> > > > > | 	if (dr_mode == USB_DR_MODE_UNKNOWN)
> > > > > | 		dr_mode = USB_DR_MODE_OTG;
> > > > > 
> > > > > default to otg if nothing specified.
> > > > 
> > > > you missed my point. I wanted something like:
> > > > 
> > > > dr_mode = ci->platdata->dr_mode;
> > > > if ((dr_mode == USB_DR_MODE_UNKNOWN) || !IS_ENABLED(CONFIG_USB_CHIPIDEA_OTG)
> > > > 	dr_mode = USB_DR_MODE_OTG;
> > > 
> > > So everytime the chipidea driver cannot do OTG the driver falls back to
> > > exactly this mode?
> > 
> > hehe, that was me being stupid I meant something like:
> > 
> > if (!IS_ENABLED(HOST) && dr_mode == HOST)
> > 
> > or
> > 
> > if (!IS_ENABLED(GADGET) && dr_mode == GADGET)
> > 
> > in those cases, it's best to either force OTG (then driver will work
> > with whatever it has) or bail out.
> > 
> > Thinking a bit harder, it's best to bail as you can't be sure udc.c or
> > host.c have been compiled in.
> 
> Look again, that's what the current code does. Maybe it's not coded as
> explicit as you describe above.
> 
> | 	/* initialize role(s) before the interrupt is requested */
> | 	if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
> | 		ret = ci_hdrc_host_init(ci);
> | 		if (ret)
> | 			dev_info(dev, "doesn't support host\n");
> | 	}
> 
> if IS_ENABLED(HOST), then ci_hdrc_host_init() will set
> ci->roles[CI_ROLE_HOST] in case it manages to initialize the host driver.
> 
> if !IS_ENABLED(HOST), then ci_hdrc_host_init() will be a static inline
> noop function, so ci->roles[CI_ROLE_HOST] ends up being NULL.
> 
> | 
> | 	if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) {
> | 		ret = ci_hdrc_gadget_init(ci);
> | 		if (ret)
> | 			dev_info(dev, "doesn't support gadget\n");
> | 	}
> 
> Same applies for gadget.
> 
> | 
> | 	if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) {
> | 		dev_err(dev, "no supported roles\n");
> | 		ret = -ENODEV;
> | 		goto rm_wq;
> | 	}
> 
> At this point (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET])
> means that the intersection of what is requested and what we are able to
> do is empty. The reasons could be That the requested mode is not compiled
> into the kernel, or the requested role failed to initialize.
I have tested host-only capable port (host 1) and dr_mode = "host" DT setting
(OTG port) as well as only compile chipidea gadget, the "no support roles"
will be reported, and fails probe, see below. (The platform layer probe's
error needs to my another patch in my otg patchset)

ci_hdrc ci_hdrc.0: ChipIdea HDRC found, lpm: 0; cap: c08a6100 op: c08a6140
ci_hdrc ci_hdrc.0: doesn't support host
ci_hdrc ci_hdrc.0: no supported roles
imx_usb 2184000.usb: Can't register ci_hdrc platform device, err=-19
imx_usb 2184200.usb: pinctrl get/select failed, err=-19
ci_hdrc ci_hdrc.1: ChipIdea HDRC found, lpm: 0; cap: c08ae300 op: c08ae340
ci_hdrc ci_hdrc.1: doesn't support host
ci_hdrc ci_hdrc.1: doesn't support gadget
ci_hdrc ci_hdrc.1: no supported roles
imx_usb 2184200.usb: Can't register ci_hdrc platform device, err=-19

> 
> Sascha
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 

Best Regards,
Peter Chen

[PATCH 2/9] USB: move bulk of otg/otg.c to phy/phy.c

[Felipe Balbi]

On Mon, Feb 04, 2013 at 02:24:28PM +0100, Sascha Hauer wrote:
> Most of otg/otg.c is not otg specific, but phy specific, so move it
> to the phy directory.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Reported-by: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Felipe Balbi <balbi@ti.com>

this is pretty ok, I'll start queueing patches for v3.10 in about a week
or two.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130219/99c3723e/attachment.sig>

[PATCH 2/9] USB: move bulk of otg/otg.c to phy/phy.c

[Sascha Hauer]

On Tue, Feb 19, 2013 at 11:30:19AM +0200, Felipe Balbi wrote:
> On Mon, Feb 04, 2013 at 02:24:28PM +0100, Sascha Hauer wrote:
> > Most of otg/otg.c is not otg specific, but phy specific, so move it
> > to the phy directory.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > Reported-by: Kishon Vijay Abraham I <kishon@ti.com>
> > Cc: Felipe Balbi <balbi@ti.com>
> 
> this is pretty ok, I'll start queueing patches for v3.10 in about a week
> or two.

Ok, thanks. I'll resend this after the merge window.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 2/9] USB: move bulk of otg/otg.c to phy/phy.c

[Felipe Balbi]

On Tue, Feb 19, 2013 at 08:06:35PM +0100, Sascha Hauer wrote:
> On Tue, Feb 19, 2013 at 11:30:19AM +0200, Felipe Balbi wrote:
> > On Mon, Feb 04, 2013 at 02:24:28PM +0100, Sascha Hauer wrote:
> > > Most of otg/otg.c is not otg specific, but phy specific, so move it
> > > to the phy directory.
> > > 
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > Reported-by: Kishon Vijay Abraham I <kishon@ti.com>
> > > Cc: Felipe Balbi <balbi@ti.com>
> > 
> > this is pretty ok, I'll start queueing patches for v3.10 in about a week
> > or two.
> 
> Ok, thanks. I'll resend this after the merge window.

cool great, that lets me wait for v3.9-rc1 before queueing, much
better ;-)

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130219/0d6f7277/attachment-0001.sig>

[PATCH 6/9] USB chipidea: introduce dual role mode pdata flags

[Peter Chen]

On Mon, Feb 04, 2013 at 02:24:32PM +0100, Sascha Hauer wrote:
> Even if a chipidea core is otg capable the board may not. This allows
> to explicitly set the core to host/peripheral mode. Without these
> flags the driver falls back to the old behaviour.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/usb/chipidea/core.c  |   21 +++++++++++++++------
>  include/linux/usb/chipidea.h |    2 +-
>  2 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 04d68cb..c89f2aa 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -435,6 +435,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  	struct resource	*res;
>  	void __iomem	*base;
>  	int		ret;
> +	enum usb_dr_mode dr_mode;
>  
>  	if (!dev->platform_data) {
>  		dev_err(dev, "platform data missing\n");
> @@ -487,14 +488,22 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> -	ret = ci_hdrc_gadget_init(ci);
> -	if (ret)
> -		dev_info(dev, "doesn't support gadget\n");
> +	if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) {

Can we change "USB_DR_MODE_PERIPHERAL" to "USB_DR_MODE_GADGET", since we always
use gadget to stands for device or peripheral mode at code (like below
CI_ROLE_GADGET), it may make code uniform.

> +		ret = ci_hdrc_gadget_init(ci);
> +		if (ret)
> +			dev_info(dev, "doesn't support gadget\n");
> +	}
>  
>  	if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) {
>  		dev_err(dev, "no supported roles\n");

-- 

Best Regards,
Peter Chen

[PATCH 5/9] USB: chipidea: add PTW and PTS handling

[Marc Kleine-Budde]

On 02/14/2013 02:07 PM, Alexander Shishkin wrote:
> Sascha Hauer <s.hauer@pengutronix.de> writes:
> 
>> From: Michael Grzeschik <m.grzeschik@pengutronix.de>
>>
>> This patch makes it possible to configure the PTW and PTS bits inside
>> the portsc register for host and device mode before the driver starts
>> and the phy can be addressed as hardware implementation is designed.
>>
>> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> ---
>>  .../devicetree/bindings/usb/ci13xxx-imx.txt        |    5 +++
>>  drivers/usb/chipidea/bits.h                        |   14 ++++++-
>>  drivers/usb/chipidea/ci13xxx_imx.c                 |    3 ++
>>  drivers/usb/chipidea/core.c                        |   39 ++++++++++++++++++++
>>  include/linux/usb/chipidea.h                       |    1 +
>>  5 files changed, 61 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
>> index 5778b9c..dd42ccd 100644
>> --- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
>> +++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
>> @@ -5,6 +5,11 @@ Required properties:
>>  - reg: Should contain registers location and length
>>  - interrupts: Should contain controller interrupt
>>  
>> +Recommended properies:
>> +- phy_type: the type of the phy connected to the core. Should be one
>> +  of "utmi", "utmi_wide", "ulpi", "serial" or "hsic". Without this
>> +  property the PORTSC register won't be touched
>> +
> 
> Looks like this bit belongs to patch 3/9, where you're adding devicetree
> hooks. Otherwise looks good.

Nope. Patch 3/9 adds the device tree helper code. But this patch adds
the functionality to the chipidea imx glue code, so the update of the
devicetree docs belongs into this patch.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130227/196b967c/attachment.sig>

[PATCH 6/9] USB chipidea: introduce dual role mode pdata flags

[Marc Kleine-Budde]

On 02/22/2013 03:09 AM, Peter Chen wrote:
> On Mon, Feb 04, 2013 at 02:24:32PM +0100, Sascha Hauer wrote:
>> Even if a chipidea core is otg capable the board may not. This allows
>> to explicitly set the core to host/peripheral mode. Without these
>> flags the driver falls back to the old behaviour.
>>
>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> ---
>>  drivers/usb/chipidea/core.c  |   21 +++++++++++++++------
>>  include/linux/usb/chipidea.h |    2 +-
>>  2 files changed, 16 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
>> index 04d68cb..c89f2aa 100644
>> --- a/drivers/usb/chipidea/core.c
>> +++ b/drivers/usb/chipidea/core.c
>> @@ -435,6 +435,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>>  	struct resource	*res;
>>  	void __iomem	*base;
>>  	int		ret;
>> +	enum usb_dr_mode dr_mode;
>>  
>>  	if (!dev->platform_data) {
>>  		dev_err(dev, "platform data missing\n");
>> @@ -487,14 +488,22 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>>  		return -ENODEV;
>>  	}
>>  
>> -	ret = ci_hdrc_gadget_init(ci);
>> -	if (ret)
>> -		dev_info(dev, "doesn't support gadget\n");
>> +	if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) {
> 
> Can we change "USB_DR_MODE_PERIPHERAL" to "USB_DR_MODE_GADGET", since we always
> use gadget to stands for device or peripheral mode at code (like below
> CI_ROLE_GADGET), it may make code uniform.

Peripheral mode seems to be the more official name compared to gadget. I
vote for keeping peripheral in the DT and changing the chipidea to use
peripheral instead of gadget (in a later patch(es)).

Marc


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130227/539165de/attachment.sig>

[PATCH 3/9] USB: add devicetree helpers for determining dr_mode and phy_type

[Peter Chen]

On Mon, Feb 04, 2013 at 02:24:29PM +0100, Sascha Hauer wrote:
> From: Michael Grzeschik <m.grzeschik@pengutronix.de>
> 
> This adds two little devicetree helper functions for determining the
> dr_mode (host, peripheral, otg) and phy_type (utmi, ulpi,...) from
> the devicetree.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/usb/phy/Makefile |    1 +
>  drivers/usb/phy/of.c     |   47 ++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/usb/usb-common.c |   36 +++++++++++++++++++++++++++++++++++
>  include/linux/usb/of.h   |   27 ++++++++++++++++++++++++++
>  include/linux/usb/otg.h  |    7 +++++++
>  include/linux/usb/phy.h  |    9 +++++++++
>  6 files changed, 127 insertions(+)
>  create mode 100644 drivers/usb/phy/of.c
>  create mode 100644 include/linux/usb/of.h
> 
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index 9fa6327..e1be1e8 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -5,6 +5,7 @@
>  ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
>  
>  obj-$(CONFIG_USB_OTG_UTILS)		+= phy.o
> +obj-$(CONFIG_OF)			+= of.o
>  obj-$(CONFIG_OMAP_USB2)			+= omap-usb2.o
>  obj-$(CONFIG_OMAP_USB3)			+= omap-usb3.o
>  obj-$(CONFIG_OMAP_CONTROL_USB)		+= omap-control-usb.o
> diff --git a/drivers/usb/phy/of.c b/drivers/usb/phy/of.c
> new file mode 100644
> index 0000000..e6f3b74
> --- /dev/null
> +++ b/drivers/usb/phy/of.c
> @@ -0,0 +1,47 @@
> +/*
> + * USB of helper code
> + *
> + * 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/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/usb/of.h>
> +#include <linux/usb/otg.h>
> +
> +static const char *usbphy_modes[] = {
> +	[USBPHY_INTERFACE_MODE_UNKNOWN]	= "",
> +	[USBPHY_INTERFACE_MODE_UTMI]	= "utmi",
> +	[USBPHY_INTERFACE_MODE_UTMIW]	= "utmi_wide",
> +	[USBPHY_INTERFACE_MODE_ULPI]	= "ulpi",
> +	[USBPHY_INTERFACE_MODE_SERIAL]	= "serial",
> +	[USBPHY_INTERFACE_MODE_HSIC]	= "hsic",
> +};
> +
> +/**
> + * of_usb_get_phy_mode - Get phy mode for given device_node
> + * @np:	Pointer to the given device_node
> + *
> + * The function gets phy interface string from property 'phy_type',
> + * and returns the correspondig enum usb_phy_interface
> + */
> +enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np)
> +{
> +	const char *phy_type;
> +	int err, i;
> +
> +	err = of_property_read_string(np, "phy_type", &phy_type);
> +	if (err < 0)
> +		return USBPHY_INTERFACE_MODE_UNKNOWN;
> +
> +	for (i = 0; i < ARRAY_SIZE(usbphy_modes); i++)
> +		if (!strcmp(phy_type, usbphy_modes[i]))
> +			return i;
> +
> +	return USBPHY_INTERFACE_MODE_UNKNOWN;
> +}
> +EXPORT_SYMBOL_GPL(of_usb_get_phy_mode);
> diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
> index d29503e..ad4d87d 100644
> --- a/drivers/usb/usb-common.c
> +++ b/drivers/usb/usb-common.c
> @@ -14,6 +14,9 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/usb/ch9.h>
> +#include <linux/of.h>
> +#include <linux/usb/of.h>
> +#include <linux/usb/otg.h>
>  
>  const char *usb_speed_string(enum usb_device_speed speed)
>  {
> @@ -32,4 +35,37 @@ const char *usb_speed_string(enum usb_device_speed speed)
>  }
>  EXPORT_SYMBOL_GPL(usb_speed_string);
>  
> +#ifdef CONFIG_OF
> +static const char *usb_dr_modes[] = {
> +	[USB_DR_MODE_UNKNOWN]		= "",
> +	[USB_DR_MODE_HOST]		= "host",
> +	[USB_DR_MODE_PERIPHERAL]	= "peripheral",
> +	[USB_DR_MODE_OTG]		= "otg",
> +};
> +
> +/**
> + * of_usb_get_dr_mode - Get dual role mode for given device_node
> + * @np:	Pointer to the given device_node
> + *
> + * The function gets phy interface string from property 'dr_mode',
> + * and returns the correspondig enum usb_dr_mode
> + */
> +enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
> +{
> +	const char *dr_mode;
> +	int err, i;
> +
> +	err = of_property_read_string(np, "dr_mode", &dr_mode);
> +	if (err < 0)
> +		return USB_DR_MODE_UNKNOWN;
> +
> +	for (i = 0; i < ARRAY_SIZE(usb_dr_modes); i++)
> +		if (!strcmp(dr_mode, usb_dr_modes[i]))
> +			return i;
> +
> +	return USB_DR_MODE_UNKNOWN;
> +}
> +EXPORT_SYMBOL_GPL(of_usb_get_dr_mode);
> +#endif
> +
>  MODULE_LICENSE("GPL");
> diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h
> new file mode 100644
> index 0000000..4681a20
> --- /dev/null
> +++ b/include/linux/usb/of.h
> @@ -0,0 +1,27 @@
> +/*
> + * OF helpers for usb devices.
> + *
> + * This file is released under the GPLv2
> + */
> +
> +#ifndef __LINUX_USB_OF_H
> +#define __LINUX_USB_OF_H
> +
> +#include <linux/usb/phy.h>

Hi Michael,

When I use allmodconfig to build all usb module, I meet below
error. After changing #include <linux/usb/otg.h>, below
error is disappeared.

 GEN     /home/b29397/work/code/git/linus/linux-2.6/outout/all/Makefile
 scripts/kconfig/conf --allmodconfig Kconfig
#
# configuration written to .config
#
 /home/b29397/work/code/git/linus/linux-2.6/arch/x86/Makefile:92: stack protector enabled but no compiler support
 /home/b29397/work/code/git/linus/linux-2.6/arch/x86/Makefile:107: CONFIG_X86_X32 enabled but no binutils support
   GEN     /home/b29397/work/code/git/linus/linux-2.6/outout/all/Makefile
   scripts/kconfig/conf --silentoldconfig Kconfig
   /home/b29397/work/code/git/linus/linux-2.6/arch/x86/Makefile:92: stack protector enabled but no compiler support
   /home/b29397/work/code/git/linus/linux-2.6/arch/x86/Makefile:107: CONFIG_X86_X32 enabled but no binutils support
   make[2]: Nothing to be done for `all'.
     GEN     /home/b29397/work/code/git/linus/linux-2.6/outout/all/Makefile
       CHK     include/generated/uapi/linux/version.h
       make[2]: Nothing to be done for `relocs'.
         Using /home/b29397/work/code/git/linus/linux-2.6 as source for kernel
	   CHK     include/generated/utsrelease.h
	     CALL    /home/b29397/work/code/git/linus/linux-2.6/scripts/checksyscalls.sh
	       CC [M]  drivers/usb/usb-common.o
	       In file included from /home/b29397/work/code/git/linus/linux-2.6/drivers/usb/usb-common.c:18:
	       /home/b29397/work/code/git/linus/linux-2.6/include/linux/usb/of.h:22: error: return type is an incomplete type
	       /home/b29397/work/code/git/linus/linux-2.6/include/linux/usb/of.h: In function ?of_usb_get_dr_mode?:
	       /home/b29397/work/code/git/linus/linux-2.6/include/linux/usb/of.h:23: error: ?USB_DR_MODE_UNKNOWN? undeclared (first use in this function)
	/home/b29397/work/code/git/linus/linux-2.6/include/linux/usb/of.h:23: error: (Each undeclared identifier is reported only once
			/home/b29397/work/code/git/linus/linux-2.6/include/linux/usb/of.h:23: error: for each function it appears in.)
	/home/b29397/work/code/git/linus/linux-2.6/include/linux/usb/of.h:23: warning: ?return? with a value, in function returning void
	make[2]: *** [drivers/usb/usb-common.o] Error 1
	make[2]: *** Waiting for unfinished jobs....
-- 

Best Regards,
Peter Chen