[PATCH 0/5] phy: remove the need for the phys to know about their users

[PATCH 0/5] phy: remove the need for the phys to know about their users

[Heikki Krogerus]

Hi,

This replaces the consumer & init_data structures with a lookup table
that contains complete associations with the phys and their users,
removing the need for the phy drivers themselves to care about their
users even when not using DT.

The lookup method is copied from the way the gpio descriptor lookup is
now handled in gpiolib.c.


Heikki Krogerus (5):
  phy: unify the phy name parameters
  phy: add support for indexed lookup
  phy: improved lookup method
  arm: omap3: twl: use the new lookup method with usb phy
  phy: remove the old lookup method

 arch/arm/mach-omap2/twl-common.c    |  15 ++-
 drivers/phy/phy-core.c              | 209 ++++++++++++++++++++++++++----------
 drivers/phy/phy-exynos-dp-video.c   |   2 +-
 drivers/phy/phy-exynos-mipi-video.c |   2 +-
 drivers/phy/phy-omap-usb2.c         |   2 +-
 drivers/phy/phy-twl4030-usb.c       |   4 +-
 include/linux/phy/phy.h             |  86 ++++++++++-----
 7 files changed, 221 insertions(+), 99 deletions(-)

-- 
1.8.5.1

[PATCH 0/5] phy: remove the need for the phys to know about their users

[Heikki Krogerus]

Hi,

This replaces the consumer & init_data structures with a lookup table
that contains complete associations with the phys and their users,
removing the need for the phy drivers themselves to care about their
users even when not using DT.

The lookup method is copied from the way the gpio descriptor lookup is
now handled in gpiolib.c.


Heikki Krogerus (5):
  phy: unify the phy name parameters
  phy: add support for indexed lookup
  phy: improved lookup method
  arm: omap3: twl: use the new lookup method with usb phy
  phy: remove the old lookup method

 arch/arm/mach-omap2/twl-common.c    |  15 ++-
 drivers/phy/phy-core.c              | 209 ++++++++++++++++++++++++++----------
 drivers/phy/phy-exynos-dp-video.c   |   2 +-
 drivers/phy/phy-exynos-mipi-video.c |   2 +-
 drivers/phy/phy-omap-usb2.c         |   2 +-
 drivers/phy/phy-twl4030-usb.c       |   4 +-
 include/linux/phy/phy.h             |  86 ++++++++++-----
 7 files changed, 221 insertions(+), 99 deletions(-)

-- 
1.8.5.1

[PATCH 1/5] phy: unify the phy name parameters

[Heikki Krogerus]

Replace "string" and "port" that are used as phy name
parameter for various functions with "con_id" which is
commonly used in other frameworks.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/phy/phy-core.c  | 22 ++++++++++------------
 include/linux/phy/phy.h |  8 ++++----
 2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 03cf8fb..1102aef 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -53,7 +53,7 @@ static int devm_phy_match(struct device *dev, void *res, void *match_data)
 	return res == match_data;
 }
 
-static struct phy *phy_lookup(struct device *device, const char *port)
+static struct phy *phy_lookup(struct device *device, const char *con_id)
 {
 	unsigned int count;
 	struct phy *phy;
@@ -68,7 +68,7 @@ static struct phy *phy_lookup(struct device *device, const char *port)
 		consumers = phy->init_data->consumers;
 		while (count--) {
 			if (!strcmp(consumers->dev_name, dev_name(device)) &&
-					!strcmp(consumers->port, port)) {
+					!strcmp(consumers->port, con_id)) {
 				class_dev_iter_exit(&iter);
 				return phy;
 			}
@@ -350,33 +350,32 @@ EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
 /**
  * phy_get() - lookup and obtain a reference to a phy.
  * @dev: device that requests this phy
- * @string: the phy name as given in the dt data or the name of the controller
- * port for non-dt case
+ * @con_id: name of the phy from device's point of view
  *
  * Returns the phy driver, after getting a refcount to it; or
  * -ENODEV if there is no such phy.  The caller is responsible for
  * calling phy_put() to release that count.
  */
-struct phy *phy_get(struct device *dev, const char *string)
+struct phy *phy_get(struct device *dev, const char *con_id)
 {
 	int index = 0;
 	struct phy *phy = NULL;
 
-	if (string == NULL) {
+	if (con_id == NULL) {
 		dev_WARN(dev, "missing string\n");
 		return ERR_PTR(-EINVAL);
 	}
 
 	if (dev->of_node) {
 		index = of_property_match_string(dev->of_node, "phy-names",
-			string);
+						 con_id);
 		phy = of_phy_get(dev, index);
 		if (IS_ERR(phy)) {
 			dev_err(dev, "unable to find phy\n");
 			return phy;
 		}
 	} else {
-		phy = phy_lookup(dev, string);
+		phy = phy_lookup(dev, con_id);
 		if (IS_ERR(phy)) {
 			dev_err(dev, "unable to find phy\n");
 			return phy;
@@ -395,14 +394,13 @@ EXPORT_SYMBOL_GPL(phy_get);
 /**
  * devm_phy_get() - lookup and obtain a reference to a phy.
  * @dev: device that requests this phy
- * @string: the phy name as given in the dt data or phy device name
- * for non-dt case
+ * @con_id: name of the phy from device's point of view
  *
  * Gets the phy using phy_get(), and associates a device with it using
  * devres. On driver detach, release function is invoked on the devres data,
  * then, devres data is freed.
  */
-struct phy *devm_phy_get(struct device *dev, const char *string)
+struct phy *devm_phy_get(struct device *dev, const char *con_id)
 {
 	struct phy **ptr, *phy;
 
@@ -410,7 +408,7 @@ struct phy *devm_phy_get(struct device *dev, const char *string)
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	phy = phy_get(dev, string);
+	phy = phy_get(dev, con_id);
 	if (!IS_ERR(phy)) {
 		*ptr = phy;
 		devres_add(dev, ptr);
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 6d72269..d67dcbf 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -127,8 +127,8 @@ int phy_init(struct phy *phy);
 int phy_exit(struct phy *phy);
 int phy_power_on(struct phy *phy);
 int phy_power_off(struct phy *phy);
-struct phy *phy_get(struct device *dev, const char *string);
-struct phy *devm_phy_get(struct device *dev, const char *string);
+struct phy *phy_get(struct device *dev, const char *con_id);
+struct phy *devm_phy_get(struct device *dev, const char *con_id);
 void phy_put(struct phy *phy);
 void devm_phy_put(struct device *dev, struct phy *phy);
 struct phy *of_phy_simple_xlate(struct device *dev,
@@ -199,12 +199,12 @@ static inline int phy_power_off(struct phy *phy)
 	return -ENOSYS;
 }
 
-static inline struct phy *phy_get(struct device *dev, const char *string)
+static inline struct phy *phy_get(struct device *dev, const char *con_id)
 {
 	return ERR_PTR(-ENOSYS);
 }
 
-static inline struct phy *devm_phy_get(struct device *dev, const char *string)
+static inline struct phy *devm_phy_get(struct device *dev, const char *con_id)
 {
 	return ERR_PTR(-ENOSYS);
 }
-- 
1.8.5.1

[PATCH 1/5] phy: unify the phy name parameters

[Heikki Krogerus]

Replace "string" and "port" that are used as phy name
parameter for various functions with "con_id" which is
commonly used in other frameworks.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/phy/phy-core.c  | 22 ++++++++++------------
 include/linux/phy/phy.h |  8 ++++----
 2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 03cf8fb..1102aef 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -53,7 +53,7 @@ static int devm_phy_match(struct device *dev, void *res, void *match_data)
 	return res == match_data;
 }
 
-static struct phy *phy_lookup(struct device *device, const char *port)
+static struct phy *phy_lookup(struct device *device, const char *con_id)
 {
 	unsigned int count;
 	struct phy *phy;
@@ -68,7 +68,7 @@ static struct phy *phy_lookup(struct device *device, const char *port)
 		consumers = phy->init_data->consumers;
 		while (count--) {
 			if (!strcmp(consumers->dev_name, dev_name(device)) &&
-					!strcmp(consumers->port, port)) {
+					!strcmp(consumers->port, con_id)) {
 				class_dev_iter_exit(&iter);
 				return phy;
 			}
@@ -350,33 +350,32 @@ EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
 /**
  * phy_get() - lookup and obtain a reference to a phy.
  * @dev: device that requests this phy
- * @string: the phy name as given in the dt data or the name of the controller
- * port for non-dt case
+ * @con_id: name of the phy from device's point of view
  *
  * Returns the phy driver, after getting a refcount to it; or
  * -ENODEV if there is no such phy.  The caller is responsible for
  * calling phy_put() to release that count.
  */
-struct phy *phy_get(struct device *dev, const char *string)
+struct phy *phy_get(struct device *dev, const char *con_id)
 {
 	int index = 0;
 	struct phy *phy = NULL;
 
-	if (string == NULL) {
+	if (con_id == NULL) {
 		dev_WARN(dev, "missing string\n");
 		return ERR_PTR(-EINVAL);
 	}
 
 	if (dev->of_node) {
 		index = of_property_match_string(dev->of_node, "phy-names",
-			string);
+						 con_id);
 		phy = of_phy_get(dev, index);
 		if (IS_ERR(phy)) {
 			dev_err(dev, "unable to find phy\n");
 			return phy;
 		}
 	} else {
-		phy = phy_lookup(dev, string);
+		phy = phy_lookup(dev, con_id);
 		if (IS_ERR(phy)) {
 			dev_err(dev, "unable to find phy\n");
 			return phy;
@@ -395,14 +394,13 @@ EXPORT_SYMBOL_GPL(phy_get);
 /**
  * devm_phy_get() - lookup and obtain a reference to a phy.
  * @dev: device that requests this phy
- * @string: the phy name as given in the dt data or phy device name
- * for non-dt case
+ * @con_id: name of the phy from device's point of view
  *
  * Gets the phy using phy_get(), and associates a device with it using
  * devres. On driver detach, release function is invoked on the devres data,
  * then, devres data is freed.
  */
-struct phy *devm_phy_get(struct device *dev, const char *string)
+struct phy *devm_phy_get(struct device *dev, const char *con_id)
 {
 	struct phy **ptr, *phy;
 
@@ -410,7 +408,7 @@ struct phy *devm_phy_get(struct device *dev, const char *string)
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	phy = phy_get(dev, string);
+	phy = phy_get(dev, con_id);
 	if (!IS_ERR(phy)) {
 		*ptr = phy;
 		devres_add(dev, ptr);
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 6d72269..d67dcbf 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -127,8 +127,8 @@ int phy_init(struct phy *phy);
 int phy_exit(struct phy *phy);
 int phy_power_on(struct phy *phy);
 int phy_power_off(struct phy *phy);
-struct phy *phy_get(struct device *dev, const char *string);
-struct phy *devm_phy_get(struct device *dev, const char *string);
+struct phy *phy_get(struct device *dev, const char *con_id);
+struct phy *devm_phy_get(struct device *dev, const char *con_id);
 void phy_put(struct phy *phy);
 void devm_phy_put(struct device *dev, struct phy *phy);
 struct phy *of_phy_simple_xlate(struct device *dev,
@@ -199,12 +199,12 @@ static inline int phy_power_off(struct phy *phy)
 	return -ENOSYS;
 }
 
-static inline struct phy *phy_get(struct device *dev, const char *string)
+static inline struct phy *phy_get(struct device *dev, const char *con_id)
 {
 	return ERR_PTR(-ENOSYS);
 }
 
-static inline struct phy *devm_phy_get(struct device *dev, const char *string)
+static inline struct phy *devm_phy_get(struct device *dev, const char *con_id)
 {
 	return ERR_PTR(-ENOSYS);
 }
-- 
1.8.5.1

[PATCH 1/5] phy: unify the phy name parameters

[Heikki Krogerus]

Replace "string" and "port" that are used as phy name
parameter for various functions with "con_id" which is
commonly used in other frameworks.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/phy/phy-core.c  | 22 ++++++++++------------
 include/linux/phy/phy.h |  8 ++++----
 2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 03cf8fb..1102aef 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -53,7 +53,7 @@ static int devm_phy_match(struct device *dev, void *res, void *match_data)
 	return res == match_data;
 }
 
-static struct phy *phy_lookup(struct device *device, const char *port)
+static struct phy *phy_lookup(struct device *device, const char *con_id)
 {
 	unsigned int count;
 	struct phy *phy;
@@ -68,7 +68,7 @@ static struct phy *phy_lookup(struct device *device, const char *port)
 		consumers = phy->init_data->consumers;
 		while (count--) {
 			if (!strcmp(consumers->dev_name, dev_name(device)) &&
-					!strcmp(consumers->port, port)) {
+					!strcmp(consumers->port, con_id)) {
 				class_dev_iter_exit(&iter);
 				return phy;
 			}
@@ -350,33 +350,32 @@ EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
 /**
  * phy_get() - lookup and obtain a reference to a phy.
  * @dev: device that requests this phy
- * @string: the phy name as given in the dt data or the name of the controller
- * port for non-dt case
+ * @con_id: name of the phy from device's point of view
  *
  * Returns the phy driver, after getting a refcount to it; or
  * -ENODEV if there is no such phy.  The caller is responsible for
  * calling phy_put() to release that count.
  */
-struct phy *phy_get(struct device *dev, const char *string)
+struct phy *phy_get(struct device *dev, const char *con_id)
 {
 	int index = 0;
 	struct phy *phy = NULL;
 
-	if (string == NULL) {
+	if (con_id == NULL) {
 		dev_WARN(dev, "missing string\n");
 		return ERR_PTR(-EINVAL);
 	}
 
 	if (dev->of_node) {
 		index = of_property_match_string(dev->of_node, "phy-names",
-			string);
+						 con_id);
 		phy = of_phy_get(dev, index);
 		if (IS_ERR(phy)) {
 			dev_err(dev, "unable to find phy\n");
 			return phy;
 		}
 	} else {
-		phy = phy_lookup(dev, string);
+		phy = phy_lookup(dev, con_id);
 		if (IS_ERR(phy)) {
 			dev_err(dev, "unable to find phy\n");
 			return phy;
@@ -395,14 +394,13 @@ EXPORT_SYMBOL_GPL(phy_get);
 /**
  * devm_phy_get() - lookup and obtain a reference to a phy.
  * @dev: device that requests this phy
- * @string: the phy name as given in the dt data or phy device name
- * for non-dt case
+ * @con_id: name of the phy from device's point of view
  *
  * Gets the phy using phy_get(), and associates a device with it using
  * devres. On driver detach, release function is invoked on the devres data,
  * then, devres data is freed.
  */
-struct phy *devm_phy_get(struct device *dev, const char *string)
+struct phy *devm_phy_get(struct device *dev, const char *con_id)
 {
 	struct phy **ptr, *phy;
 
@@ -410,7 +408,7 @@ struct phy *devm_phy_get(struct device *dev, const char *string)
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	phy = phy_get(dev, string);
+	phy = phy_get(dev, con_id);
 	if (!IS_ERR(phy)) {
 		*ptr = phy;
 		devres_add(dev, ptr);
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 6d72269..d67dcbf 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -127,8 +127,8 @@ int phy_init(struct phy *phy);
 int phy_exit(struct phy *phy);
 int phy_power_on(struct phy *phy);
 int phy_power_off(struct phy *phy);
-struct phy *phy_get(struct device *dev, const char *string);
-struct phy *devm_phy_get(struct device *dev, const char *string);
+struct phy *phy_get(struct device *dev, const char *con_id);
+struct phy *devm_phy_get(struct device *dev, const char *con_id);
 void phy_put(struct phy *phy);
 void devm_phy_put(struct device *dev, struct phy *phy);
 struct phy *of_phy_simple_xlate(struct device *dev,
@@ -199,12 +199,12 @@ static inline int phy_power_off(struct phy *phy)
 	return -ENOSYS;
 }
 
-static inline struct phy *phy_get(struct device *dev, const char *string)
+static inline struct phy *phy_get(struct device *dev, const char *con_id)
 {
 	return ERR_PTR(-ENOSYS);
 }
 
-static inline struct phy *devm_phy_get(struct device *dev, const char *string)
+static inline struct phy *devm_phy_get(struct device *dev, const char *con_id)
 {
 	return ERR_PTR(-ENOSYS);
 }
-- 
1.8.5.1

[PATCH 2/5] phy: add support for indexed lookup

[Heikki Krogerus]

Removes the need for the consumer drivers requesting the
phys to provide name for the phy. This should ease the use
of the framework considerable when using only one phy, which
is usually the case when except with USB, but it can also
be useful with multiple phys.

This will also reduce noise from the framework when there is
no phy by changing warnings to debug messages.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/phy/phy-core.c  | 106 ++++++++++++++++++++++++++++++++++--------------
 include/linux/phy/phy.h |  14 +++++++
 2 files changed, 89 insertions(+), 31 deletions(-)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 1102aef..99dc046 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -53,7 +53,8 @@ static int devm_phy_match(struct device *dev, void *res, void *match_data)
 	return res == match_data;
 }
 
-static struct phy *phy_lookup(struct device *device, const char *con_id)
+static struct phy *phy_lookup(struct device *device, const char *con_id,
+			      unsigned int idx)
 {
 	unsigned int count;
 	struct phy *phy;
@@ -67,6 +68,10 @@ static struct phy *phy_lookup(struct device *device, const char *con_id)
 		count = phy->init_data->num_consumers;
 		consumers = phy->init_data->consumers;
 		while (count--) {
+			/* index must always match exactly */
+			if (!con_id)
+				if (idx != count)
+					continue;
 			if (!strcmp(consumers->dev_name, dev_name(device)) &&
 					!strcmp(consumers->port, con_id)) {
 				class_dev_iter_exit(&iter);
@@ -242,7 +247,8 @@ EXPORT_SYMBOL_GPL(phy_power_off);
 /**
  * of_phy_get() - lookup and obtain a reference to a phy by phandle
  * @dev: device that requests this phy
- * @index: the index of the phy
+ * @con_id: name of the phy from device's point of view
+ * @idx: the index of the phy if name is not used
  *
  * Returns the phy associated with the given phandle value,
  * after getting a refcount to it or -ENODEV if there is no such phy or
@@ -250,12 +256,20 @@ EXPORT_SYMBOL_GPL(phy_power_off);
  * not yet loaded. This function uses of_xlate call back function provided
  * while registering the phy_provider to find the phy instance.
  */
-static struct phy *of_phy_get(struct device *dev, int index)
+static struct phy *of_phy_get(struct device *dev, const char *con_id,
+			      unsigned int idx)
 {
 	int ret;
 	struct phy_provider *phy_provider;
 	struct phy *phy = NULL;
 	struct of_phandle_args args;
+	int index;
+
+	if (!con_id)
+		index = idx;
+	else
+		index = of_property_match_string(dev->of_node, "phy-names",
+						 con_id);
 
 	ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
 		index, &args);
@@ -348,38 +362,36 @@ struct phy *of_phy_simple_xlate(struct device *dev, struct of_phandle_args
 EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
 
 /**
- * phy_get() - lookup and obtain a reference to a phy.
+ * phy_get_index() - obtain a phy based on index
  * @dev: device that requests this phy
  * @con_id: name of the phy from device's point of view
+ * @idx: index of the phy to obtain in the consumer
  *
- * Returns the phy driver, after getting a refcount to it; or
- * -ENODEV if there is no such phy.  The caller is responsible for
- * calling phy_put() to release that count.
+ * This variant of phy_get() allows to access PHYs other than the first
+ * defined one for functions that define several PHYs.
  */
-struct phy *phy_get(struct device *dev, const char *con_id)
+struct phy *phy_get_index(struct device *dev, const char *con_id,
+			  unsigned int idx)
 {
-	int index = 0;
 	struct phy *phy = NULL;
 
-	if (con_id == NULL) {
-		dev_WARN(dev, "missing string\n");
-		return ERR_PTR(-EINVAL);
+	if (dev->of_node) {
+		dev_dbg(dev, "using device tree for PHY lookup\n");
+		phy = of_phy_get(dev, con_id, idx);
 	}
 
-	if (dev->of_node) {
-		index = of_property_match_string(dev->of_node, "phy-names",
-						 con_id);
-		phy = of_phy_get(dev, index);
-		if (IS_ERR(phy)) {
-			dev_err(dev, "unable to find phy\n");
-			return phy;
-		}
-	} else {
-		phy = phy_lookup(dev, con_id);
-		if (IS_ERR(phy)) {
-			dev_err(dev, "unable to find phy\n");
-			return phy;
-		}
+	/**
+	 * Either we are not using DT, or their lookup did not return
+	 * a result. In that case, use platform lookup as a fallback.
+	 */
+	if (!phy || IS_ERR(phy)) {
+		dev_dbg(dev, "using lookup tables for PHY lookup");
+		phy = phy_lookup(dev, con_id, idx);
+	}
+
+	if (IS_ERR(phy)) {
+		dev_dbg(dev, "unable to find phy\n");
+		return phy;
 	}
 
 	if (!try_module_get(phy->ops->owner))
@@ -389,18 +401,20 @@ struct phy *phy_get(struct device *dev, const char *con_id)
 
 	return phy;
 }
-EXPORT_SYMBOL_GPL(phy_get);
+EXPORT_SYMBOL_GPL(phy_get_index);
 
 /**
- * devm_phy_get() - lookup and obtain a reference to a phy.
+ * devm_phy_get_index() - obtain a phy based on index
  * @dev: device that requests this phy
  * @con_id: name of the phy from device's point of view
+ * @idx: index of the phy to obtain in the consumer
  *
- * Gets the phy using phy_get(), and associates a device with it using
+ * Gets the phy using phy_get_index(), and associates a device with it using
  * devres. On driver detach, release function is invoked on the devres data,
  * then, devres data is freed.
  */
-struct phy *devm_phy_get(struct device *dev, const char *con_id)
+struct phy *devm_phy_get_index(struct device *dev, const char *con_id,
+			       unsigned int idx)
 {
 	struct phy **ptr, *phy;
 
@@ -408,7 +422,7 @@ struct phy *devm_phy_get(struct device *dev, const char *con_id)
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	phy = phy_get(dev, con_id);
+	phy = phy_get_index(dev, con_id, idx);
 	if (!IS_ERR(phy)) {
 		*ptr = phy;
 		devres_add(dev, ptr);
@@ -418,6 +432,36 @@ struct phy *devm_phy_get(struct device *dev, const char *con_id)
 
 	return phy;
 }
+EXPORT_SYMBOL_GPL(devm_phy_get_index);
+
+/**
+ * phy_get() - lookup and obtain a reference to a phy.
+ * @dev: device that requests this phy
+ * @con_id: name of the phy from device's point of view
+ *
+ * Returns the phy driver, after getting a refcount to it; or
+ * -ENODEV if there is no such phy.  The caller is responsible for
+ * calling phy_put() to release that count.
+ */
+struct phy *phy_get(struct device *dev, const char *con_id)
+{
+	return phy_get_index(dev, con_id, 0);
+}
+EXPORT_SYMBOL_GPL(phy_get);
+
+/**
+ * devm_phy_get() - lookup and obtain a reference to a phy.
+ * @dev: device that requests this phy
+ * @con_id: name of the phy from device's point of view
+ *
+ * Gets the phy using phy_get_index(), and associates a device with it using
+ * devres. On driver detach, release function is invoked on the devres data,
+ * then, devres data is freed.
+ */
+struct phy *devm_phy_get(struct device *dev, const char *con_id)
+{
+	return devm_phy_get_index(dev, con_id, 0);
+}
 EXPORT_SYMBOL_GPL(devm_phy_get);
 
 /**
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index d67dcbf..43d1a23 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -127,6 +127,8 @@ int phy_init(struct phy *phy);
 int phy_exit(struct phy *phy);
 int phy_power_on(struct phy *phy);
 int phy_power_off(struct phy *phy);
+struct phy *phy_get_index(struct device *, const char *, unsigned int);
+struct phy *devm_phy_get_index(struct device *, const char *, unsigned int);
 struct phy *phy_get(struct device *dev, const char *con_id);
 struct phy *devm_phy_get(struct device *dev, const char *con_id);
 void phy_put(struct phy *phy);
@@ -199,6 +201,18 @@ static inline int phy_power_off(struct phy *phy)
 	return -ENOSYS;
 }
 
+static inline struct phy *phy_get_index(struct device *dev, const char *id,
+					unsigned int idx)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
+static inline struct phy *devm_phy_get_index(struct device *dev,
+					     const char *id, unsigned int idx)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
 static inline struct phy *phy_get(struct device *dev, const char *con_id)
 {
 	return ERR_PTR(-ENOSYS);
-- 
1.8.5.1

[PATCH 2/5] phy: add support for indexed lookup

[Heikki Krogerus]

Removes the need for the consumer drivers requesting the
phys to provide name for the phy. This should ease the use
of the framework considerable when using only one phy, which
is usually the case when except with USB, but it can also
be useful with multiple phys.

This will also reduce noise from the framework when there is
no phy by changing warnings to debug messages.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/phy/phy-core.c  | 106 ++++++++++++++++++++++++++++++++++--------------
 include/linux/phy/phy.h |  14 +++++++
 2 files changed, 89 insertions(+), 31 deletions(-)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 1102aef..99dc046 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -53,7 +53,8 @@ static int devm_phy_match(struct device *dev, void *res, void *match_data)
 	return res == match_data;
 }
 
-static struct phy *phy_lookup(struct device *device, const char *con_id)
+static struct phy *phy_lookup(struct device *device, const char *con_id,
+			      unsigned int idx)
 {
 	unsigned int count;
 	struct phy *phy;
@@ -67,6 +68,10 @@ static struct phy *phy_lookup(struct device *device, const char *con_id)
 		count = phy->init_data->num_consumers;
 		consumers = phy->init_data->consumers;
 		while (count--) {
+			/* index must always match exactly */
+			if (!con_id)
+				if (idx != count)
+					continue;
 			if (!strcmp(consumers->dev_name, dev_name(device)) &&
 					!strcmp(consumers->port, con_id)) {
 				class_dev_iter_exit(&iter);
@@ -242,7 +247,8 @@ EXPORT_SYMBOL_GPL(phy_power_off);
 /**
  * of_phy_get() - lookup and obtain a reference to a phy by phandle
  * @dev: device that requests this phy
- * @index: the index of the phy
+ * @con_id: name of the phy from device's point of view
+ * @idx: the index of the phy if name is not used
  *
  * Returns the phy associated with the given phandle value,
  * after getting a refcount to it or -ENODEV if there is no such phy or
@@ -250,12 +256,20 @@ EXPORT_SYMBOL_GPL(phy_power_off);
  * not yet loaded. This function uses of_xlate call back function provided
  * while registering the phy_provider to find the phy instance.
  */
-static struct phy *of_phy_get(struct device *dev, int index)
+static struct phy *of_phy_get(struct device *dev, const char *con_id,
+			      unsigned int idx)
 {
 	int ret;
 	struct phy_provider *phy_provider;
 	struct phy *phy = NULL;
 	struct of_phandle_args args;
+	int index;
+
+	if (!con_id)
+		index = idx;
+	else
+		index = of_property_match_string(dev->of_node, "phy-names",
+						 con_id);
 
 	ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
 		index, &args);
@@ -348,38 +362,36 @@ struct phy *of_phy_simple_xlate(struct device *dev, struct of_phandle_args
 EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
 
 /**
- * phy_get() - lookup and obtain a reference to a phy.
+ * phy_get_index() - obtain a phy based on index
  * @dev: device that requests this phy
  * @con_id: name of the phy from device's point of view
+ * @idx: index of the phy to obtain in the consumer
  *
- * Returns the phy driver, after getting a refcount to it; or
- * -ENODEV if there is no such phy.  The caller is responsible for
- * calling phy_put() to release that count.
+ * This variant of phy_get() allows to access PHYs other than the first
+ * defined one for functions that define several PHYs.
  */
-struct phy *phy_get(struct device *dev, const char *con_id)
+struct phy *phy_get_index(struct device *dev, const char *con_id,
+			  unsigned int idx)
 {
-	int index = 0;
 	struct phy *phy = NULL;
 
-	if (con_id == NULL) {
-		dev_WARN(dev, "missing string\n");
-		return ERR_PTR(-EINVAL);
+	if (dev->of_node) {
+		dev_dbg(dev, "using device tree for PHY lookup\n");
+		phy = of_phy_get(dev, con_id, idx);
 	}
 
-	if (dev->of_node) {
-		index = of_property_match_string(dev->of_node, "phy-names",
-						 con_id);
-		phy = of_phy_get(dev, index);
-		if (IS_ERR(phy)) {
-			dev_err(dev, "unable to find phy\n");
-			return phy;
-		}
-	} else {
-		phy = phy_lookup(dev, con_id);
-		if (IS_ERR(phy)) {
-			dev_err(dev, "unable to find phy\n");
-			return phy;
-		}
+	/**
+	 * Either we are not using DT, or their lookup did not return
+	 * a result. In that case, use platform lookup as a fallback.
+	 */
+	if (!phy || IS_ERR(phy)) {
+		dev_dbg(dev, "using lookup tables for PHY lookup");
+		phy = phy_lookup(dev, con_id, idx);
+	}
+
+	if (IS_ERR(phy)) {
+		dev_dbg(dev, "unable to find phy\n");
+		return phy;
 	}
 
 	if (!try_module_get(phy->ops->owner))
@@ -389,18 +401,20 @@ struct phy *phy_get(struct device *dev, const char *con_id)
 
 	return phy;
 }
-EXPORT_SYMBOL_GPL(phy_get);
+EXPORT_SYMBOL_GPL(phy_get_index);
 
 /**
- * devm_phy_get() - lookup and obtain a reference to a phy.
+ * devm_phy_get_index() - obtain a phy based on index
  * @dev: device that requests this phy
  * @con_id: name of the phy from device's point of view
+ * @idx: index of the phy to obtain in the consumer
  *
- * Gets the phy using phy_get(), and associates a device with it using
+ * Gets the phy using phy_get_index(), and associates a device with it using
  * devres. On driver detach, release function is invoked on the devres data,
  * then, devres data is freed.
  */
-struct phy *devm_phy_get(struct device *dev, const char *con_id)
+struct phy *devm_phy_get_index(struct device *dev, const char *con_id,
+			       unsigned int idx)
 {
 	struct phy **ptr, *phy;
 
@@ -408,7 +422,7 @@ struct phy *devm_phy_get(struct device *dev, const char *con_id)
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	phy = phy_get(dev, con_id);
+	phy = phy_get_index(dev, con_id, idx);
 	if (!IS_ERR(phy)) {
 		*ptr = phy;
 		devres_add(dev, ptr);
@@ -418,6 +432,36 @@ struct phy *devm_phy_get(struct device *dev, const char *con_id)
 
 	return phy;
 }
+EXPORT_SYMBOL_GPL(devm_phy_get_index);
+
+/**
+ * phy_get() - lookup and obtain a reference to a phy.
+ * @dev: device that requests this phy
+ * @con_id: name of the phy from device's point of view
+ *
+ * Returns the phy driver, after getting a refcount to it; or
+ * -ENODEV if there is no such phy.  The caller is responsible for
+ * calling phy_put() to release that count.
+ */
+struct phy *phy_get(struct device *dev, const char *con_id)
+{
+	return phy_get_index(dev, con_id, 0);
+}
+EXPORT_SYMBOL_GPL(phy_get);
+
+/**
+ * devm_phy_get() - lookup and obtain a reference to a phy.
+ * @dev: device that requests this phy
+ * @con_id: name of the phy from device's point of view
+ *
+ * Gets the phy using phy_get_index(), and associates a device with it using
+ * devres. On driver detach, release function is invoked on the devres data,
+ * then, devres data is freed.
+ */
+struct phy *devm_phy_get(struct device *dev, const char *con_id)
+{
+	return devm_phy_get_index(dev, con_id, 0);
+}
 EXPORT_SYMBOL_GPL(devm_phy_get);
 
 /**
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index d67dcbf..43d1a23 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -127,6 +127,8 @@ int phy_init(struct phy *phy);
 int phy_exit(struct phy *phy);
 int phy_power_on(struct phy *phy);
 int phy_power_off(struct phy *phy);
+struct phy *phy_get_index(struct device *, const char *, unsigned int);
+struct phy *devm_phy_get_index(struct device *, const char *, unsigned int);
 struct phy *phy_get(struct device *dev, const char *con_id);
 struct phy *devm_phy_get(struct device *dev, const char *con_id);
 void phy_put(struct phy *phy);
@@ -199,6 +201,18 @@ static inline int phy_power_off(struct phy *phy)
 	return -ENOSYS;
 }
 
+static inline struct phy *phy_get_index(struct device *dev, const char *id,
+					unsigned int idx)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
+static inline struct phy *devm_phy_get_index(struct device *dev,
+					     const char *id, unsigned int idx)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
 static inline struct phy *phy_get(struct device *dev, const char *con_id)
 {
 	return ERR_PTR(-ENOSYS);
-- 
1.8.5.1

Re: [PATCH 2/5] phy: add support for indexed lookup

[Kishon Vijay Abraham I]

Hi,

On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
> Removes the need for the consumer drivers requesting the
> phys to provide name for the phy. This should ease the use
> of the framework considerable when using only one phy, which
> is usually the case when except with USB, but it can also
> be useful with multiple phys.

If index has to be used with multiple PHYs, the controller should be aware of
the order in which it is populated in dt data. That's not good.
> This will also reduce noise from the framework when there is
> no phy by changing warnings to debug messages.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/phy/phy-core.c  | 106 ++++++++++++++++++++++++++++++++++--------------
>  include/linux/phy/phy.h |  14 +++++++
>  2 files changed, 89 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 1102aef..99dc046 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -53,7 +53,8 @@ static int devm_phy_match(struct device *dev, void *res, void *match_data)
>  	return res == match_data;
>  }
>  
> -static struct phy *phy_lookup(struct device *device, const char *con_id)
> +static struct phy *phy_lookup(struct device *device, const char *con_id,
> +			      unsigned int idx)
>  {
>  	unsigned int count;
>  	struct phy *phy;
> @@ -67,6 +68,10 @@ static struct phy *phy_lookup(struct device *device, const char *con_id)
>  		count = phy->init_data->num_consumers;
>  		consumers = phy->init_data->consumers;
>  		while (count--) {
> +			/* index must always match exactly */
> +			if (!con_id)
> +				if (idx != count)
> +					continue;
>  			if (!strcmp(consumers->dev_name, dev_name(device)) &&
>  					!strcmp(consumers->port, con_id)) {
>  				class_dev_iter_exit(&iter);
> @@ -242,7 +247,8 @@ EXPORT_SYMBOL_GPL(phy_power_off);
>  /**
>   * of_phy_get() - lookup and obtain a reference to a phy by phandle
>   * @dev: device that requests this phy
> - * @index: the index of the phy
> + * @con_id: name of the phy from device's point of view
> + * @idx: the index of the phy if name is not used
>   *
>   * Returns the phy associated with the given phandle value,
>   * after getting a refcount to it or -ENODEV if there is no such phy or
> @@ -250,12 +256,20 @@ EXPORT_SYMBOL_GPL(phy_power_off);
>   * not yet loaded. This function uses of_xlate call back function provided
>   * while registering the phy_provider to find the phy instance.
>   */
> -static struct phy *of_phy_get(struct device *dev, int index)
> +static struct phy *of_phy_get(struct device *dev, const char *con_id,
> +			      unsigned int idx)
>  {
>  	int ret;
>  	struct phy_provider *phy_provider;
>  	struct phy *phy = NULL;
>  	struct of_phandle_args args;
> +	int index;
> +
> +	if (!con_id)
> +		index = idx;
> +	else
> +		index = of_property_match_string(dev->of_node, "phy-names",
> +						 con_id);
>  
>  	ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
>  		index, &args);
> @@ -348,38 +362,36 @@ struct phy *of_phy_simple_xlate(struct device *dev, struct of_phandle_args
>  EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
>  
>  /**
> - * phy_get() - lookup and obtain a reference to a phy.
> + * phy_get_index() - obtain a phy based on index

NAK. It still takes a 'char' argument and the name is misleading.
Btw are you replacing phy_get() or adding a new API in addition to phy_get()?
>   * @dev: device that requests this phy
>   * @con_id: name of the phy from device's point of view
> + * @idx: index of the phy to obtain in the consumer
>   *
> - * Returns the phy driver, after getting a refcount to it; or
> - * -ENODEV if there is no such phy.  The caller is responsible for
> - * calling phy_put() to release that count.
> + * This variant of phy_get() allows to access PHYs other than the first
> + * defined one for functions that define several PHYs.
>   */
> -struct phy *phy_get(struct device *dev, const char *con_id)
> +struct phy *phy_get_index(struct device *dev, const char *con_id,
> +			  unsigned int idx)
>  {
> -	int index = 0;
>  	struct phy *phy = NULL;
>  
> -	if (con_id == NULL) {
> -		dev_WARN(dev, "missing string\n");
> -		return ERR_PTR(-EINVAL);
> +	if (dev->of_node) {
> +		dev_dbg(dev, "using device tree for PHY lookup\n");
> +		phy = of_phy_get(dev, con_id, idx);
>  	}
>  
> -	if (dev->of_node) {
> -		index = of_property_match_string(dev->of_node, "phy-names",
> -						 con_id);
> -		phy = of_phy_get(dev, index);
> -		if (IS_ERR(phy)) {
> -			dev_err(dev, "unable to find phy\n");
> -			return phy;
> -		}
> -	} else {
> -		phy = phy_lookup(dev, con_id);
> -		if (IS_ERR(phy)) {
> -			dev_err(dev, "unable to find phy\n");
> -			return phy;
> -		}
> +	/**
> +	 * Either we are not using DT, or their lookup did not return
> +	 * a result. In that case, use platform lookup as a fallback.
> +	 */

In a dt boot, if it has not returned a result how will platform lookup help
since there wouldn't be be any board file in the case of dt boot (to populate
PHY consumers). Maybe your later patch will handle that.
> +	if (!phy || IS_ERR(phy)) {
> +		dev_dbg(dev, "using lookup tables for PHY lookup");
> +		phy = phy_lookup(dev, con_id, idx);
> +	}
> +
> +	if (IS_ERR(phy)) {
> +		dev_dbg(dev, "unable to find phy\n");
> +		return phy;
>  	}
>  
>  	if (!try_module_get(phy->ops->owner))
> @@ -389,18 +401,20 @@ struct phy *phy_get(struct device *dev, const char *con_id)
>  
>  	return phy;
>  }
> -EXPORT_SYMBOL_GPL(phy_get);
> +EXPORT_SYMBOL_GPL(phy_get_index);
>  
>  /**
> - * devm_phy_get() - lookup and obtain a reference to a phy.
> + * devm_phy_get_index() - obtain a phy based on index

I don't see the need of these 2 APIs.

Thanks
Kishon

[PATCH 2/5] phy: add support for indexed lookup

[Kishon Vijay Abraham I]

Hi,

On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
> Removes the need for the consumer drivers requesting the
> phys to provide name for the phy. This should ease the use
> of the framework considerable when using only one phy, which
> is usually the case when except with USB, but it can also
> be useful with multiple phys.

If index has to be used with multiple PHYs, the controller should be aware of
the order in which it is populated in dt data. That's not good.
> This will also reduce noise from the framework when there is
> no phy by changing warnings to debug messages.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/phy/phy-core.c  | 106 ++++++++++++++++++++++++++++++++++--------------
>  include/linux/phy/phy.h |  14 +++++++
>  2 files changed, 89 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 1102aef..99dc046 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -53,7 +53,8 @@ static int devm_phy_match(struct device *dev, void *res, void *match_data)
>  	return res == match_data;
>  }
>  
> -static struct phy *phy_lookup(struct device *device, const char *con_id)
> +static struct phy *phy_lookup(struct device *device, const char *con_id,
> +			      unsigned int idx)
>  {
>  	unsigned int count;
>  	struct phy *phy;
> @@ -67,6 +68,10 @@ static struct phy *phy_lookup(struct device *device, const char *con_id)
>  		count = phy->init_data->num_consumers;
>  		consumers = phy->init_data->consumers;
>  		while (count--) {
> +			/* index must always match exactly */
> +			if (!con_id)
> +				if (idx != count)
> +					continue;
>  			if (!strcmp(consumers->dev_name, dev_name(device)) &&
>  					!strcmp(consumers->port, con_id)) {
>  				class_dev_iter_exit(&iter);
> @@ -242,7 +247,8 @@ EXPORT_SYMBOL_GPL(phy_power_off);
>  /**
>   * of_phy_get() - lookup and obtain a reference to a phy by phandle
>   * @dev: device that requests this phy
> - * @index: the index of the phy
> + * @con_id: name of the phy from device's point of view
> + * @idx: the index of the phy if name is not used
>   *
>   * Returns the phy associated with the given phandle value,
>   * after getting a refcount to it or -ENODEV if there is no such phy or
> @@ -250,12 +256,20 @@ EXPORT_SYMBOL_GPL(phy_power_off);
>   * not yet loaded. This function uses of_xlate call back function provided
>   * while registering the phy_provider to find the phy instance.
>   */
> -static struct phy *of_phy_get(struct device *dev, int index)
> +static struct phy *of_phy_get(struct device *dev, const char *con_id,
> +			      unsigned int idx)
>  {
>  	int ret;
>  	struct phy_provider *phy_provider;
>  	struct phy *phy = NULL;
>  	struct of_phandle_args args;
> +	int index;
> +
> +	if (!con_id)
> +		index = idx;
> +	else
> +		index = of_property_match_string(dev->of_node, "phy-names",
> +						 con_id);
>  
>  	ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
>  		index, &args);
> @@ -348,38 +362,36 @@ struct phy *of_phy_simple_xlate(struct device *dev, struct of_phandle_args
>  EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
>  
>  /**
> - * phy_get() - lookup and obtain a reference to a phy.
> + * phy_get_index() - obtain a phy based on index

NAK. It still takes a 'char' argument and the name is misleading.
Btw are you replacing phy_get() or adding a new API in addition to phy_get()?
>   * @dev: device that requests this phy
>   * @con_id: name of the phy from device's point of view
> + * @idx: index of the phy to obtain in the consumer
>   *
> - * Returns the phy driver, after getting a refcount to it; or
> - * -ENODEV if there is no such phy.  The caller is responsible for
> - * calling phy_put() to release that count.
> + * This variant of phy_get() allows to access PHYs other than the first
> + * defined one for functions that define several PHYs.
>   */
> -struct phy *phy_get(struct device *dev, const char *con_id)
> +struct phy *phy_get_index(struct device *dev, const char *con_id,
> +			  unsigned int idx)
>  {
> -	int index = 0;
>  	struct phy *phy = NULL;
>  
> -	if (con_id == NULL) {
> -		dev_WARN(dev, "missing string\n");
> -		return ERR_PTR(-EINVAL);
> +	if (dev->of_node) {
> +		dev_dbg(dev, "using device tree for PHY lookup\n");
> +		phy = of_phy_get(dev, con_id, idx);
>  	}
>  
> -	if (dev->of_node) {
> -		index = of_property_match_string(dev->of_node, "phy-names",
> -						 con_id);
> -		phy = of_phy_get(dev, index);
> -		if (IS_ERR(phy)) {
> -			dev_err(dev, "unable to find phy\n");
> -			return phy;
> -		}
> -	} else {
> -		phy = phy_lookup(dev, con_id);
> -		if (IS_ERR(phy)) {
> -			dev_err(dev, "unable to find phy\n");
> -			return phy;
> -		}
> +	/**
> +	 * Either we are not using DT, or their lookup did not return
> +	 * a result. In that case, use platform lookup as a fallback.
> +	 */

In a dt boot, if it has not returned a result how will platform lookup help
since there wouldn't be be any board file in the case of dt boot (to populate
PHY consumers). Maybe your later patch will handle that.
> +	if (!phy || IS_ERR(phy)) {
> +		dev_dbg(dev, "using lookup tables for PHY lookup");
> +		phy = phy_lookup(dev, con_id, idx);
> +	}
> +
> +	if (IS_ERR(phy)) {
> +		dev_dbg(dev, "unable to find phy\n");
> +		return phy;
>  	}
>  
>  	if (!try_module_get(phy->ops->owner))
> @@ -389,18 +401,20 @@ struct phy *phy_get(struct device *dev, const char *con_id)
>  
>  	return phy;
>  }
> -EXPORT_SYMBOL_GPL(phy_get);
> +EXPORT_SYMBOL_GPL(phy_get_index);
>  
>  /**
> - * devm_phy_get() - lookup and obtain a reference to a phy.
> + * devm_phy_get_index() - obtain a phy based on index

I don't see the need of these 2 APIs.

Thanks
Kishon

Re: [PATCH 2/5] phy: add support for indexed lookup

[Kishon Vijay Abraham I]

Hi,

On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
> Removes the need for the consumer drivers requesting the
> phys to provide name for the phy. This should ease the use
> of the framework considerable when using only one phy, which
> is usually the case when except with USB, but it can also
> be useful with multiple phys.

If index has to be used with multiple PHYs, the controller should be aware of
the order in which it is populated in dt data. That's not good.
> This will also reduce noise from the framework when there is
> no phy by changing warnings to debug messages.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/phy/phy-core.c  | 106 ++++++++++++++++++++++++++++++++++--------------
>  include/linux/phy/phy.h |  14 +++++++
>  2 files changed, 89 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 1102aef..99dc046 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -53,7 +53,8 @@ static int devm_phy_match(struct device *dev, void *res, void *match_data)
>  	return res == match_data;
>  }
>  
> -static struct phy *phy_lookup(struct device *device, const char *con_id)
> +static struct phy *phy_lookup(struct device *device, const char *con_id,
> +			      unsigned int idx)
>  {
>  	unsigned int count;
>  	struct phy *phy;
> @@ -67,6 +68,10 @@ static struct phy *phy_lookup(struct device *device, const char *con_id)
>  		count = phy->init_data->num_consumers;
>  		consumers = phy->init_data->consumers;
>  		while (count--) {
> +			/* index must always match exactly */
> +			if (!con_id)
> +				if (idx != count)
> +					continue;
>  			if (!strcmp(consumers->dev_name, dev_name(device)) &&
>  					!strcmp(consumers->port, con_id)) {
>  				class_dev_iter_exit(&iter);
> @@ -242,7 +247,8 @@ EXPORT_SYMBOL_GPL(phy_power_off);
>  /**
>   * of_phy_get() - lookup and obtain a reference to a phy by phandle
>   * @dev: device that requests this phy
> - * @index: the index of the phy
> + * @con_id: name of the phy from device's point of view
> + * @idx: the index of the phy if name is not used
>   *
>   * Returns the phy associated with the given phandle value,
>   * after getting a refcount to it or -ENODEV if there is no such phy or
> @@ -250,12 +256,20 @@ EXPORT_SYMBOL_GPL(phy_power_off);
>   * not yet loaded. This function uses of_xlate call back function provided
>   * while registering the phy_provider to find the phy instance.
>   */
> -static struct phy *of_phy_get(struct device *dev, int index)
> +static struct phy *of_phy_get(struct device *dev, const char *con_id,
> +			      unsigned int idx)
>  {
>  	int ret;
>  	struct phy_provider *phy_provider;
>  	struct phy *phy = NULL;
>  	struct of_phandle_args args;
> +	int index;
> +
> +	if (!con_id)
> +		index = idx;
> +	else
> +		index = of_property_match_string(dev->of_node, "phy-names",
> +						 con_id);
>  
>  	ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
>  		index, &args);
> @@ -348,38 +362,36 @@ struct phy *of_phy_simple_xlate(struct device *dev, struct of_phandle_args
>  EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
>  
>  /**
> - * phy_get() - lookup and obtain a reference to a phy.
> + * phy_get_index() - obtain a phy based on index

NAK. It still takes a 'char' argument and the name is misleading.
Btw are you replacing phy_get() or adding a new API in addition to phy_get()?
>   * @dev: device that requests this phy
>   * @con_id: name of the phy from device's point of view
> + * @idx: index of the phy to obtain in the consumer
>   *
> - * Returns the phy driver, after getting a refcount to it; or
> - * -ENODEV if there is no such phy.  The caller is responsible for
> - * calling phy_put() to release that count.
> + * This variant of phy_get() allows to access PHYs other than the first
> + * defined one for functions that define several PHYs.
>   */
> -struct phy *phy_get(struct device *dev, const char *con_id)
> +struct phy *phy_get_index(struct device *dev, const char *con_id,
> +			  unsigned int idx)
>  {
> -	int index = 0;
>  	struct phy *phy = NULL;
>  
> -	if (con_id == NULL) {
> -		dev_WARN(dev, "missing string\n");
> -		return ERR_PTR(-EINVAL);
> +	if (dev->of_node) {
> +		dev_dbg(dev, "using device tree for PHY lookup\n");
> +		phy = of_phy_get(dev, con_id, idx);
>  	}
>  
> -	if (dev->of_node) {
> -		index = of_property_match_string(dev->of_node, "phy-names",
> -						 con_id);
> -		phy = of_phy_get(dev, index);
> -		if (IS_ERR(phy)) {
> -			dev_err(dev, "unable to find phy\n");
> -			return phy;
> -		}
> -	} else {
> -		phy = phy_lookup(dev, con_id);
> -		if (IS_ERR(phy)) {
> -			dev_err(dev, "unable to find phy\n");
> -			return phy;
> -		}
> +	/**
> +	 * Either we are not using DT, or their lookup did not return
> +	 * a result. In that case, use platform lookup as a fallback.
> +	 */

In a dt boot, if it has not returned a result how will platform lookup help
since there wouldn't be be any board file in the case of dt boot (to populate
PHY consumers). Maybe your later patch will handle that.
> +	if (!phy || IS_ERR(phy)) {
> +		dev_dbg(dev, "using lookup tables for PHY lookup");
> +		phy = phy_lookup(dev, con_id, idx);
> +	}
> +
> +	if (IS_ERR(phy)) {
> +		dev_dbg(dev, "unable to find phy\n");
> +		return phy;
>  	}
>  
>  	if (!try_module_get(phy->ops->owner))
> @@ -389,18 +401,20 @@ struct phy *phy_get(struct device *dev, const char *con_id)
>  
>  	return phy;
>  }
> -EXPORT_SYMBOL_GPL(phy_get);
> +EXPORT_SYMBOL_GPL(phy_get_index);
>  
>  /**
> - * devm_phy_get() - lookup and obtain a reference to a phy.
> + * devm_phy_get_index() - obtain a phy based on index

I don't see the need of these 2 APIs.

Thanks
Kishon

Re: [PATCH 2/5] phy: add support for indexed lookup

[Heikki Krogerus]

Hi Kishon,

On Mon, Dec 16, 2013 at 04:32:58PM +0530, Kishon Vijay Abraham I wrote:
> On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
> > Removes the need for the consumer drivers requesting the
> > phys to provide name for the phy. This should ease the use
> > of the framework considerable when using only one phy, which
> > is usually the case when except with USB, but it can also
> > be useful with multiple phys.
> 
> If index has to be used with multiple PHYs, the controller should be aware of
> the order in which it is populated in dt data. That's not good.

The Idea is not to replace the name based lookup. Just to provide
possibility for index based lookup.

With ACPI, if we get the device entries for PHYs, the order will be
fixed and we will not have any other reference to the phys. In case
of USB, the first one should always be USB2 PHY and the second the
USB3 PHY.

> > This will also reduce noise from the framework when there is
> > no phy by changing warnings to debug messages.
> > 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  drivers/phy/phy-core.c  | 106 ++++++++++++++++++++++++++++++++++--------------
> >  include/linux/phy/phy.h |  14 +++++++
> >  2 files changed, 89 insertions(+), 31 deletions(-)
> > 
> > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> > index 1102aef..99dc046 100644
> > --- a/drivers/phy/phy-core.c
> > +++ b/drivers/phy/phy-core.c
> > @@ -53,7 +53,8 @@ static int devm_phy_match(struct device *dev, void *res, void *match_data)
> >  	return res == match_data;
> >  }
> >  
> > -static struct phy *phy_lookup(struct device *device, const char *con_id)
> > +static struct phy *phy_lookup(struct device *device, const char *con_id,
> > +			      unsigned int idx)
> >  {
> >  	unsigned int count;
> >  	struct phy *phy;
> > @@ -67,6 +68,10 @@ static struct phy *phy_lookup(struct device *device, const char *con_id)
> >  		count = phy->init_data->num_consumers;
> >  		consumers = phy->init_data->consumers;
> >  		while (count--) {
> > +			/* index must always match exactly */
> > +			if (!con_id)
> > +				if (idx != count)
> > +					continue;
> >  			if (!strcmp(consumers->dev_name, dev_name(device)) &&
> >  					!strcmp(consumers->port, con_id)) {
> >  				class_dev_iter_exit(&iter);
> > @@ -242,7 +247,8 @@ EXPORT_SYMBOL_GPL(phy_power_off);
> >  /**
> >   * of_phy_get() - lookup and obtain a reference to a phy by phandle
> >   * @dev: device that requests this phy
> > - * @index: the index of the phy
> > + * @con_id: name of the phy from device's point of view
> > + * @idx: the index of the phy if name is not used
> >   *
> >   * Returns the phy associated with the given phandle value,
> >   * after getting a refcount to it or -ENODEV if there is no such phy or
> > @@ -250,12 +256,20 @@ EXPORT_SYMBOL_GPL(phy_power_off);
> >   * not yet loaded. This function uses of_xlate call back function provided
> >   * while registering the phy_provider to find the phy instance.
> >   */
> > -static struct phy *of_phy_get(struct device *dev, int index)
> > +static struct phy *of_phy_get(struct device *dev, const char *con_id,
> > +			      unsigned int idx)
> >  {
> >  	int ret;
> >  	struct phy_provider *phy_provider;
> >  	struct phy *phy = NULL;
> >  	struct of_phandle_args args;
> > +	int index;
> > +
> > +	if (!con_id)
> > +		index = idx;
> > +	else
> > +		index = of_property_match_string(dev->of_node, "phy-names",
> > +						 con_id);
> >  
> >  	ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
> >  		index, &args);
> > @@ -348,38 +362,36 @@ struct phy *of_phy_simple_xlate(struct device *dev, struct of_phandle_args
> >  EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
> >  
> >  /**
> > - * phy_get() - lookup and obtain a reference to a phy.
> > + * phy_get_index() - obtain a phy based on index
> 
> NAK. It still takes a 'char' argument and the name is misleading.
> Btw are you replacing phy_get() or adding a new API in addition to phy_get()?

Additional API. The phy_get() would in practice act as a wrapper after
this. It could actually be just a #define macro in the include file.
The function naming I just copied straight from gpiolib.c. I did not
have the imagination for anything fancier.

I would like to be able to use some function like phy_get_index() and
be able to deliver it both the name and the index. With DT you guys
will always be able to use the name (and the string will always
supersede the index if we do it like this), but with ACPI, and possibly
the platform lookup tables, the index can be used...

phy2 = phy_get_index(dev, "usb2-phy", 0);
phy3 = phy_get_index(dev, "usb3-phy", 1);

> >   * @dev: device that requests this phy
> >   * @con_id: name of the phy from device's point of view
> > + * @idx: index of the phy to obtain in the consumer
> >   *
> > - * Returns the phy driver, after getting a refcount to it; or
> > - * -ENODEV if there is no such phy.  The caller is responsible for
> > - * calling phy_put() to release that count.
> > + * This variant of phy_get() allows to access PHYs other than the first
> > + * defined one for functions that define several PHYs.
> >   */
> > -struct phy *phy_get(struct device *dev, const char *con_id)
> > +struct phy *phy_get_index(struct device *dev, const char *con_id,
> > +			  unsigned int idx)
> >  {
> > -	int index = 0;
> >  	struct phy *phy = NULL;
> >  
> > -	if (con_id == NULL) {
> > -		dev_WARN(dev, "missing string\n");
> > -		return ERR_PTR(-EINVAL);
> > +	if (dev->of_node) {
> > +		dev_dbg(dev, "using device tree for PHY lookup\n");
> > +		phy = of_phy_get(dev, con_id, idx);
> >  	}
> >  
> > -	if (dev->of_node) {
> > -		index = of_property_match_string(dev->of_node, "phy-names",
> > -						 con_id);
> > -		phy = of_phy_get(dev, index);
> > -		if (IS_ERR(phy)) {
> > -			dev_err(dev, "unable to find phy\n");
> > -			return phy;
> > -		}
> > -	} else {
> > -		phy = phy_lookup(dev, con_id);
> > -		if (IS_ERR(phy)) {
> > -			dev_err(dev, "unable to find phy\n");
> > -			return phy;
> > -		}
> > +	/**
> > +	 * Either we are not using DT, or their lookup did not return
> > +	 * a result. In that case, use platform lookup as a fallback.
> > +	 */
> 
> In a dt boot, if it has not returned a result how will platform lookup help
> since there wouldn't be be any board file in the case of dt boot (to populate
> PHY consumers). Maybe your later patch will handle that.

This is something that I copy pasted from gpiolib.c. It felt like a
good idea to have possibility to create lookup tables even when the
device has of_node. I don't have strong feelings about this, so we can
drop it if you like.

> > +	if (!phy || IS_ERR(phy)) {
> > +		dev_dbg(dev, "using lookup tables for PHY lookup");
> > +		phy = phy_lookup(dev, con_id, idx);
> > +	}
> > +
> > +	if (IS_ERR(phy)) {
> > +		dev_dbg(dev, "unable to find phy\n");
> > +		return phy;
> >  	}
> >  
> >  	if (!try_module_get(phy->ops->owner))
> > @@ -389,18 +401,20 @@ struct phy *phy_get(struct device *dev, const char *con_id)
> >  
> >  	return phy;
> >  }
> > -EXPORT_SYMBOL_GPL(phy_get);
> > +EXPORT_SYMBOL_GPL(phy_get_index);
> >  
> >  /**
> > - * devm_phy_get() - lookup and obtain a reference to a phy.
> > + * devm_phy_get_index() - obtain a phy based on index
> 
> I don't see the need of these 2 APIs.

My goal is really just to prepare for ACPI with this one. I don't know
if my commit message was not clear enough.

Thanks,

-- 
heikki

[PATCH 2/5] phy: add support for indexed lookup

[Heikki Krogerus]

Hi Kishon,

On Mon, Dec 16, 2013 at 04:32:58PM +0530, Kishon Vijay Abraham I wrote:
> On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
> > Removes the need for the consumer drivers requesting the
> > phys to provide name for the phy. This should ease the use
> > of the framework considerable when using only one phy, which
> > is usually the case when except with USB, but it can also
> > be useful with multiple phys.
> 
> If index has to be used with multiple PHYs, the controller should be aware of
> the order in which it is populated in dt data. That's not good.

The Idea is not to replace the name based lookup. Just to provide
possibility for index based lookup.

With ACPI, if we get the device entries for PHYs, the order will be
fixed and we will not have any other reference to the phys. In case
of USB, the first one should always be USB2 PHY and the second the
USB3 PHY.

> > This will also reduce noise from the framework when there is
> > no phy by changing warnings to debug messages.
> > 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  drivers/phy/phy-core.c  | 106 ++++++++++++++++++++++++++++++++++--------------
> >  include/linux/phy/phy.h |  14 +++++++
> >  2 files changed, 89 insertions(+), 31 deletions(-)
> > 
> > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> > index 1102aef..99dc046 100644
> > --- a/drivers/phy/phy-core.c
> > +++ b/drivers/phy/phy-core.c
> > @@ -53,7 +53,8 @@ static int devm_phy_match(struct device *dev, void *res, void *match_data)
> >  	return res == match_data;
> >  }
> >  
> > -static struct phy *phy_lookup(struct device *device, const char *con_id)
> > +static struct phy *phy_lookup(struct device *device, const char *con_id,
> > +			      unsigned int idx)
> >  {
> >  	unsigned int count;
> >  	struct phy *phy;
> > @@ -67,6 +68,10 @@ static struct phy *phy_lookup(struct device *device, const char *con_id)
> >  		count = phy->init_data->num_consumers;
> >  		consumers = phy->init_data->consumers;
> >  		while (count--) {
> > +			/* index must always match exactly */
> > +			if (!con_id)
> > +				if (idx != count)
> > +					continue;
> >  			if (!strcmp(consumers->dev_name, dev_name(device)) &&
> >  					!strcmp(consumers->port, con_id)) {
> >  				class_dev_iter_exit(&iter);
> > @@ -242,7 +247,8 @@ EXPORT_SYMBOL_GPL(phy_power_off);
> >  /**
> >   * of_phy_get() - lookup and obtain a reference to a phy by phandle
> >   * @dev: device that requests this phy
> > - * @index: the index of the phy
> > + * @con_id: name of the phy from device's point of view
> > + * @idx: the index of the phy if name is not used
> >   *
> >   * Returns the phy associated with the given phandle value,
> >   * after getting a refcount to it or -ENODEV if there is no such phy or
> > @@ -250,12 +256,20 @@ EXPORT_SYMBOL_GPL(phy_power_off);
> >   * not yet loaded. This function uses of_xlate call back function provided
> >   * while registering the phy_provider to find the phy instance.
> >   */
> > -static struct phy *of_phy_get(struct device *dev, int index)
> > +static struct phy *of_phy_get(struct device *dev, const char *con_id,
> > +			      unsigned int idx)
> >  {
> >  	int ret;
> >  	struct phy_provider *phy_provider;
> >  	struct phy *phy = NULL;
> >  	struct of_phandle_args args;
> > +	int index;
> > +
> > +	if (!con_id)
> > +		index = idx;
> > +	else
> > +		index = of_property_match_string(dev->of_node, "phy-names",
> > +						 con_id);
> >  
> >  	ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
> >  		index, &args);
> > @@ -348,38 +362,36 @@ struct phy *of_phy_simple_xlate(struct device *dev, struct of_phandle_args
> >  EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
> >  
> >  /**
> > - * phy_get() - lookup and obtain a reference to a phy.
> > + * phy_get_index() - obtain a phy based on index
> 
> NAK. It still takes a 'char' argument and the name is misleading.
> Btw are you replacing phy_get() or adding a new API in addition to phy_get()?

Additional API. The phy_get() would in practice act as a wrapper after
this. It could actually be just a #define macro in the include file.
The function naming I just copied straight from gpiolib.c. I did not
have the imagination for anything fancier.

I would like to be able to use some function like phy_get_index() and
be able to deliver it both the name and the index. With DT you guys
will always be able to use the name (and the string will always
supersede the index if we do it like this), but with ACPI, and possibly
the platform lookup tables, the index can be used...

phy2 = phy_get_index(dev, "usb2-phy", 0);
phy3 = phy_get_index(dev, "usb3-phy", 1);

> >   * @dev: device that requests this phy
> >   * @con_id: name of the phy from device's point of view
> > + * @idx: index of the phy to obtain in the consumer
> >   *
> > - * Returns the phy driver, after getting a refcount to it; or
> > - * -ENODEV if there is no such phy.  The caller is responsible for
> > - * calling phy_put() to release that count.
> > + * This variant of phy_get() allows to access PHYs other than the first
> > + * defined one for functions that define several PHYs.
> >   */
> > -struct phy *phy_get(struct device *dev, const char *con_id)
> > +struct phy *phy_get_index(struct device *dev, const char *con_id,
> > +			  unsigned int idx)
> >  {
> > -	int index = 0;
> >  	struct phy *phy = NULL;
> >  
> > -	if (con_id == NULL) {
> > -		dev_WARN(dev, "missing string\n");
> > -		return ERR_PTR(-EINVAL);
> > +	if (dev->of_node) {
> > +		dev_dbg(dev, "using device tree for PHY lookup\n");
> > +		phy = of_phy_get(dev, con_id, idx);
> >  	}
> >  
> > -	if (dev->of_node) {
> > -		index = of_property_match_string(dev->of_node, "phy-names",
> > -						 con_id);
> > -		phy = of_phy_get(dev, index);
> > -		if (IS_ERR(phy)) {
> > -			dev_err(dev, "unable to find phy\n");
> > -			return phy;
> > -		}
> > -	} else {
> > -		phy = phy_lookup(dev, con_id);
> > -		if (IS_ERR(phy)) {
> > -			dev_err(dev, "unable to find phy\n");
> > -			return phy;
> > -		}
> > +	/**
> > +	 * Either we are not using DT, or their lookup did not return
> > +	 * a result. In that case, use platform lookup as a fallback.
> > +	 */
> 
> In a dt boot, if it has not returned a result how will platform lookup help
> since there wouldn't be be any board file in the case of dt boot (to populate
> PHY consumers). Maybe your later patch will handle that.

This is something that I copy pasted from gpiolib.c. It felt like a
good idea to have possibility to create lookup tables even when the
device has of_node. I don't have strong feelings about this, so we can
drop it if you like.

> > +	if (!phy || IS_ERR(phy)) {
> > +		dev_dbg(dev, "using lookup tables for PHY lookup");
> > +		phy = phy_lookup(dev, con_id, idx);
> > +	}
> > +
> > +	if (IS_ERR(phy)) {
> > +		dev_dbg(dev, "unable to find phy\n");
> > +		return phy;
> >  	}
> >  
> >  	if (!try_module_get(phy->ops->owner))
> > @@ -389,18 +401,20 @@ struct phy *phy_get(struct device *dev, const char *con_id)
> >  
> >  	return phy;
> >  }
> > -EXPORT_SYMBOL_GPL(phy_get);
> > +EXPORT_SYMBOL_GPL(phy_get_index);
> >  
> >  /**
> > - * devm_phy_get() - lookup and obtain a reference to a phy.
> > + * devm_phy_get_index() - obtain a phy based on index
> 
> I don't see the need of these 2 APIs.

My goal is really just to prepare for ACPI with this one. I don't know
if my commit message was not clear enough.

Thanks,

-- 
heikki

Re: [PATCH 2/5] phy: add support for indexed lookup

[Kishon Vijay Abraham I]

Hi,

On Monday 16 December 2013 08:02 PM, Heikki Krogerus wrote:
> Hi Kishon,
> 
> On Mon, Dec 16, 2013 at 04:32:58PM +0530, Kishon Vijay Abraham I wrote:
>> On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
>>> Removes the need for the consumer drivers requesting the
>>> phys to provide name for the phy. This should ease the use
>>> of the framework considerable when using only one phy, which
>>> is usually the case when except with USB, but it can also
>>> be useful with multiple phys.
>>
>> If index has to be used with multiple PHYs, the controller should be aware of
>> the order in which it is populated in dt data. That's not good.
> 
> The Idea is not to replace the name based lookup. Just to provide
> possibility for index based lookup.
> 
> With ACPI, if we get the device entries for PHYs, the order will be
> fixed and we will not have any other reference to the phys. In case
> of USB, the first one should always be USB2 PHY and the second the
> USB3 PHY.
> 
>>> This will also reduce noise from the framework when there is
>>> no phy by changing warnings to debug messages.
>>>
>>> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>>> ---
>>>  drivers/phy/phy-core.c  | 106 ++++++++++++++++++++++++++++++++++--------------
>>>  include/linux/phy/phy.h |  14 +++++++
>>>  2 files changed, 89 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>>> index 1102aef..99dc046 100644
>>> --- a/drivers/phy/phy-core.c
>>> +++ b/drivers/phy/phy-core.c
>>> @@ -53,7 +53,8 @@ static int devm_phy_match(struct device *dev, void *res, void *match_data)
>>>  	return res == match_data;
>>>  }
>>>  
>>> -static struct phy *phy_lookup(struct device *device, const char *con_id)
>>> +static struct phy *phy_lookup(struct device *device, const char *con_id,
>>> +			      unsigned int idx)
>>>  {
>>>  	unsigned int count;
>>>  	struct phy *phy;
>>> @@ -67,6 +68,10 @@ static struct phy *phy_lookup(struct device *device, const char *con_id)
>>>  		count = phy->init_data->num_consumers;
>>>  		consumers = phy->init_data->consumers;
>>>  		while (count--) {
>>> +			/* index must always match exactly */
>>> +			if (!con_id)
>>> +				if (idx != count)
>>> +					continue;
>>>  			if (!strcmp(consumers->dev_name, dev_name(device)) &&
>>>  					!strcmp(consumers->port, con_id)) {
>>>  				class_dev_iter_exit(&iter);
>>> @@ -242,7 +247,8 @@ EXPORT_SYMBOL_GPL(phy_power_off);
>>>  /**
>>>   * of_phy_get() - lookup and obtain a reference to a phy by phandle
>>>   * @dev: device that requests this phy
>>> - * @index: the index of the phy
>>> + * @con_id: name of the phy from device's point of view
>>> + * @idx: the index of the phy if name is not used
>>>   *
>>>   * Returns the phy associated with the given phandle value,
>>>   * after getting a refcount to it or -ENODEV if there is no such phy or
>>> @@ -250,12 +256,20 @@ EXPORT_SYMBOL_GPL(phy_power_off);
>>>   * not yet loaded. This function uses of_xlate call back function provided
>>>   * while registering the phy_provider to find the phy instance.
>>>   */
>>> -static struct phy *of_phy_get(struct device *dev, int index)
>>> +static struct phy *of_phy_get(struct device *dev, const char *con_id,
>>> +			      unsigned int idx)
>>>  {
>>>  	int ret;
>>>  	struct phy_provider *phy_provider;
>>>  	struct phy *phy = NULL;
>>>  	struct of_phandle_args args;
>>> +	int index;
>>> +
>>> +	if (!con_id)
>>> +		index = idx;
>>> +	else
>>> +		index = of_property_match_string(dev->of_node, "phy-names",
>>> +						 con_id);
>>>  
>>>  	ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
>>>  		index, &args);
>>> @@ -348,38 +362,36 @@ struct phy *of_phy_simple_xlate(struct device *dev, struct of_phandle_args
>>>  EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
>>>  
>>>  /**
>>> - * phy_get() - lookup and obtain a reference to a phy.
>>> + * phy_get_index() - obtain a phy based on index
>>
>> NAK. It still takes a 'char' argument and the name is misleading.
>> Btw are you replacing phy_get() or adding a new API in addition to phy_get()?
> 
> Additional API. The phy_get() would in practice act as a wrapper after

In this patch it looks like you've replaced phy_get().
> this. It could actually be just a #define macro in the include file.
> The function naming I just copied straight from gpiolib.c. I did not
> have the imagination for anything fancier.
> 
> I would like to be able to use some function like phy_get_index() and
> be able to deliver it both the name and the index. With DT you guys
> will always be able to use the name (and the string will always
> supersede the index if we do it like this), but with ACPI, and possibly
> the platform lookup tables, the index can be used...

I think in that case, we should drop the 'string' from phy_get_index since we
have the other API to handle that? I don't know about ACPI, but is it not
possible to use strings with ACPI?

Thanks
Kishon

[PATCH 2/5] phy: add support for indexed lookup

[Kishon Vijay Abraham I]

Hi,

On Monday 16 December 2013 08:02 PM, Heikki Krogerus wrote:
> Hi Kishon,
> 
> On Mon, Dec 16, 2013 at 04:32:58PM +0530, Kishon Vijay Abraham I wrote:
>> On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
>>> Removes the need for the consumer drivers requesting the
>>> phys to provide name for the phy. This should ease the use
>>> of the framework considerable when using only one phy, which
>>> is usually the case when except with USB, but it can also
>>> be useful with multiple phys.
>>
>> If index has to be used with multiple PHYs, the controller should be aware of
>> the order in which it is populated in dt data. That's not good.
> 
> The Idea is not to replace the name based lookup. Just to provide
> possibility for index based lookup.
> 
> With ACPI, if we get the device entries for PHYs, the order will be
> fixed and we will not have any other reference to the phys. In case
> of USB, the first one should always be USB2 PHY and the second the
> USB3 PHY.
> 
>>> This will also reduce noise from the framework when there is
>>> no phy by changing warnings to debug messages.
>>>
>>> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>>> ---
>>>  drivers/phy/phy-core.c  | 106 ++++++++++++++++++++++++++++++++++--------------
>>>  include/linux/phy/phy.h |  14 +++++++
>>>  2 files changed, 89 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>>> index 1102aef..99dc046 100644
>>> --- a/drivers/phy/phy-core.c
>>> +++ b/drivers/phy/phy-core.c
>>> @@ -53,7 +53,8 @@ static int devm_phy_match(struct device *dev, void *res, void *match_data)
>>>  	return res == match_data;
>>>  }
>>>  
>>> -static struct phy *phy_lookup(struct device *device, const char *con_id)
>>> +static struct phy *phy_lookup(struct device *device, const char *con_id,
>>> +			      unsigned int idx)
>>>  {
>>>  	unsigned int count;
>>>  	struct phy *phy;
>>> @@ -67,6 +68,10 @@ static struct phy *phy_lookup(struct device *device, const char *con_id)
>>>  		count = phy->init_data->num_consumers;
>>>  		consumers = phy->init_data->consumers;
>>>  		while (count--) {
>>> +			/* index must always match exactly */
>>> +			if (!con_id)
>>> +				if (idx != count)
>>> +					continue;
>>>  			if (!strcmp(consumers->dev_name, dev_name(device)) &&
>>>  					!strcmp(consumers->port, con_id)) {
>>>  				class_dev_iter_exit(&iter);
>>> @@ -242,7 +247,8 @@ EXPORT_SYMBOL_GPL(phy_power_off);
>>>  /**
>>>   * of_phy_get() - lookup and obtain a reference to a phy by phandle
>>>   * @dev: device that requests this phy
>>> - * @index: the index of the phy
>>> + * @con_id: name of the phy from device's point of view
>>> + * @idx: the index of the phy if name is not used
>>>   *
>>>   * Returns the phy associated with the given phandle value,
>>>   * after getting a refcount to it or -ENODEV if there is no such phy or
>>> @@ -250,12 +256,20 @@ EXPORT_SYMBOL_GPL(phy_power_off);
>>>   * not yet loaded. This function uses of_xlate call back function provided
>>>   * while registering the phy_provider to find the phy instance.
>>>   */
>>> -static struct phy *of_phy_get(struct device *dev, int index)
>>> +static struct phy *of_phy_get(struct device *dev, const char *con_id,
>>> +			      unsigned int idx)
>>>  {
>>>  	int ret;
>>>  	struct phy_provider *phy_provider;
>>>  	struct phy *phy = NULL;
>>>  	struct of_phandle_args args;
>>> +	int index;
>>> +
>>> +	if (!con_id)
>>> +		index = idx;
>>> +	else
>>> +		index = of_property_match_string(dev->of_node, "phy-names",
>>> +						 con_id);
>>>  
>>>  	ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
>>>  		index, &args);
>>> @@ -348,38 +362,36 @@ struct phy *of_phy_simple_xlate(struct device *dev, struct of_phandle_args
>>>  EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
>>>  
>>>  /**
>>> - * phy_get() - lookup and obtain a reference to a phy.
>>> + * phy_get_index() - obtain a phy based on index
>>
>> NAK. It still takes a 'char' argument and the name is misleading.
>> Btw are you replacing phy_get() or adding a new API in addition to phy_get()?
> 
> Additional API. The phy_get() would in practice act as a wrapper after

In this patch it looks like you've replaced phy_get().
> this. It could actually be just a #define macro in the include file.
> The function naming I just copied straight from gpiolib.c. I did not
> have the imagination for anything fancier.
> 
> I would like to be able to use some function like phy_get_index() and
> be able to deliver it both the name and the index. With DT you guys
> will always be able to use the name (and the string will always
> supersede the index if we do it like this), but with ACPI, and possibly
> the platform lookup tables, the index can be used...

I think in that case, we should drop the 'string' from phy_get_index since we
have the other API to handle that? I don't know about ACPI, but is it not
possible to use strings with ACPI?

Thanks
Kishon

Re: [PATCH 2/5] phy: add support for indexed lookup

[Kishon Vijay Abraham I]

Hi,

On Monday 16 December 2013 08:02 PM, Heikki Krogerus wrote:
> Hi Kishon,
> 
> On Mon, Dec 16, 2013 at 04:32:58PM +0530, Kishon Vijay Abraham I wrote:
>> On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
>>> Removes the need for the consumer drivers requesting the
>>> phys to provide name for the phy. This should ease the use
>>> of the framework considerable when using only one phy, which
>>> is usually the case when except with USB, but it can also
>>> be useful with multiple phys.
>>
>> If index has to be used with multiple PHYs, the controller should be aware of
>> the order in which it is populated in dt data. That's not good.
> 
> The Idea is not to replace the name based lookup. Just to provide
> possibility for index based lookup.
> 
> With ACPI, if we get the device entries for PHYs, the order will be
> fixed and we will not have any other reference to the phys. In case
> of USB, the first one should always be USB2 PHY and the second the
> USB3 PHY.
> 
>>> This will also reduce noise from the framework when there is
>>> no phy by changing warnings to debug messages.
>>>
>>> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>>> ---
>>>  drivers/phy/phy-core.c  | 106 ++++++++++++++++++++++++++++++++++--------------
>>>  include/linux/phy/phy.h |  14 +++++++
>>>  2 files changed, 89 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>>> index 1102aef..99dc046 100644
>>> --- a/drivers/phy/phy-core.c
>>> +++ b/drivers/phy/phy-core.c
>>> @@ -53,7 +53,8 @@ static int devm_phy_match(struct device *dev, void *res, void *match_data)
>>>  	return res == match_data;
>>>  }
>>>  
>>> -static struct phy *phy_lookup(struct device *device, const char *con_id)
>>> +static struct phy *phy_lookup(struct device *device, const char *con_id,
>>> +			      unsigned int idx)
>>>  {
>>>  	unsigned int count;
>>>  	struct phy *phy;
>>> @@ -67,6 +68,10 @@ static struct phy *phy_lookup(struct device *device, const char *con_id)
>>>  		count = phy->init_data->num_consumers;
>>>  		consumers = phy->init_data->consumers;
>>>  		while (count--) {
>>> +			/* index must always match exactly */
>>> +			if (!con_id)
>>> +				if (idx != count)
>>> +					continue;
>>>  			if (!strcmp(consumers->dev_name, dev_name(device)) &&
>>>  					!strcmp(consumers->port, con_id)) {
>>>  				class_dev_iter_exit(&iter);
>>> @@ -242,7 +247,8 @@ EXPORT_SYMBOL_GPL(phy_power_off);
>>>  /**
>>>   * of_phy_get() - lookup and obtain a reference to a phy by phandle
>>>   * @dev: device that requests this phy
>>> - * @index: the index of the phy
>>> + * @con_id: name of the phy from device's point of view
>>> + * @idx: the index of the phy if name is not used
>>>   *
>>>   * Returns the phy associated with the given phandle value,
>>>   * after getting a refcount to it or -ENODEV if there is no such phy or
>>> @@ -250,12 +256,20 @@ EXPORT_SYMBOL_GPL(phy_power_off);
>>>   * not yet loaded. This function uses of_xlate call back function provided
>>>   * while registering the phy_provider to find the phy instance.
>>>   */
>>> -static struct phy *of_phy_get(struct device *dev, int index)
>>> +static struct phy *of_phy_get(struct device *dev, const char *con_id,
>>> +			      unsigned int idx)
>>>  {
>>>  	int ret;
>>>  	struct phy_provider *phy_provider;
>>>  	struct phy *phy = NULL;
>>>  	struct of_phandle_args args;
>>> +	int index;
>>> +
>>> +	if (!con_id)
>>> +		index = idx;
>>> +	else
>>> +		index = of_property_match_string(dev->of_node, "phy-names",
>>> +						 con_id);
>>>  
>>>  	ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
>>>  		index, &args);
>>> @@ -348,38 +362,36 @@ struct phy *of_phy_simple_xlate(struct device *dev, struct of_phandle_args
>>>  EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
>>>  
>>>  /**
>>> - * phy_get() - lookup and obtain a reference to a phy.
>>> + * phy_get_index() - obtain a phy based on index
>>
>> NAK. It still takes a 'char' argument and the name is misleading.
>> Btw are you replacing phy_get() or adding a new API in addition to phy_get()?
> 
> Additional API. The phy_get() would in practice act as a wrapper after

In this patch it looks like you've replaced phy_get().
> this. It could actually be just a #define macro in the include file.
> The function naming I just copied straight from gpiolib.c. I did not
> have the imagination for anything fancier.
> 
> I would like to be able to use some function like phy_get_index() and
> be able to deliver it both the name and the index. With DT you guys
> will always be able to use the name (and the string will always
> supersede the index if we do it like this), but with ACPI, and possibly
> the platform lookup tables, the index can be used...

I think in that case, we should drop the 'string' from phy_get_index since we
have the other API to handle that? I don't know about ACPI, but is it not
possible to use strings with ACPI?

Thanks
Kishon

Re: [PATCH 2/5] phy: add support for indexed lookup

[Heikki Krogerus]

Hi Kishon,

And happy new year..

On Tue, Jan 07, 2014 at 07:10:36PM +0530, Kishon Vijay Abraham I wrote:
> >>>  /**
> >>> - * phy_get() - lookup and obtain a reference to a phy.
> >>> + * phy_get_index() - obtain a phy based on index
> >>
> >> NAK. It still takes a 'char' argument and the name is misleading.
> >> Btw are you replacing phy_get() or adding a new API in addition to phy_get()?
> > 
> > Additional API. The phy_get() would in practice act as a wrapper after
> 
> In this patch it looks like you've replaced phy_get().
> > this. It could actually be just a #define macro in the include file.
> > The function naming I just copied straight from gpiolib.c. I did not
> > have the imagination for anything fancier.
> > 
> > I would like to be able to use some function like phy_get_index() and
> > be able to deliver it both the name and the index. With DT you guys
> > will always be able to use the name (and the string will always
> > supersede the index if we do it like this), but with ACPI, and possibly
> > the platform lookup tables, the index can be used...
> 
> I think in that case, we should drop the 'string' from phy_get_index since we
> have the other API to handle that? I don't know about ACPI, but is it not
> possible to use strings with ACPI?

No unfortunately. We just have what the ACPI tables provide. The PHYs
would be "child" device entries under the controller and we can only
get handle to them based on the index.

I think I'll skip this patch from this set. Let's wait until we have
an actual ACPI DSDT describing some PHYs.


Thanks,

-- 
heikki

[PATCH 2/5] phy: add support for indexed lookup

[Heikki Krogerus]

Hi Kishon,

And happy new year..

On Tue, Jan 07, 2014 at 07:10:36PM +0530, Kishon Vijay Abraham I wrote:
> >>>  /**
> >>> - * phy_get() - lookup and obtain a reference to a phy.
> >>> + * phy_get_index() - obtain a phy based on index
> >>
> >> NAK. It still takes a 'char' argument and the name is misleading.
> >> Btw are you replacing phy_get() or adding a new API in addition to phy_get()?
> > 
> > Additional API. The phy_get() would in practice act as a wrapper after
> 
> In this patch it looks like you've replaced phy_get().
> > this. It could actually be just a #define macro in the include file.
> > The function naming I just copied straight from gpiolib.c. I did not
> > have the imagination for anything fancier.
> > 
> > I would like to be able to use some function like phy_get_index() and
> > be able to deliver it both the name and the index. With DT you guys
> > will always be able to use the name (and the string will always
> > supersede the index if we do it like this), but with ACPI, and possibly
> > the platform lookup tables, the index can be used...
> 
> I think in that case, we should drop the 'string' from phy_get_index since we
> have the other API to handle that? I don't know about ACPI, but is it not
> possible to use strings with ACPI?

No unfortunately. We just have what the ACPI tables provide. The PHYs
would be "child" device entries under the controller and we can only
get handle to them based on the index.

I think I'll skip this patch from this set. Let's wait until we have
an actual ACPI DSDT describing some PHYs.


Thanks,

-- 
heikki

Re: [PATCH 2/5] phy: add support for indexed lookup

[Kishon Vijay Abraham I]

Hi Heikki,

On Tuesday 14 January 2014 07:53 PM, Heikki Krogerus wrote:
> Hi Kishon,
> 
> And happy new year..

Happy new year :-)
> 
> On Tue, Jan 07, 2014 at 07:10:36PM +0530, Kishon Vijay Abraham I wrote:
>>>>>  /**
>>>>> - * phy_get() - lookup and obtain a reference to a phy.
>>>>> + * phy_get_index() - obtain a phy based on index
>>>>
>>>> NAK. It still takes a 'char' argument and the name is misleading.
>>>> Btw are you replacing phy_get() or adding a new API in addition to phy_get()?
>>>
>>> Additional API. The phy_get() would in practice act as a wrapper after
>>
>> In this patch it looks like you've replaced phy_get().
>>> this. It could actually be just a #define macro in the include file.
>>> The function naming I just copied straight from gpiolib.c. I did not
>>> have the imagination for anything fancier.
>>>
>>> I would like to be able to use some function like phy_get_index() and
>>> be able to deliver it both the name and the index. With DT you guys
>>> will always be able to use the name (and the string will always
>>> supersede the index if we do it like this), but with ACPI, and possibly
>>> the platform lookup tables, the index can be used...
>>
>> I think in that case, we should drop the 'string' from phy_get_index since we
>> have the other API to handle that? I don't know about ACPI, but is it not
>> possible to use strings with ACPI?
> 
> No unfortunately. We just have what the ACPI tables provide. The PHYs
> would be "child" device entries under the controller and we can only
> get handle to them based on the index.
> 
> I think I'll skip this patch from this set. Let's wait until we have
> an actual ACPI DSDT describing some PHYs.

yeah.. sure.

Cheers
Kishon

[PATCH 2/5] phy: add support for indexed lookup

[Kishon Vijay Abraham I]

Hi Heikki,

On Tuesday 14 January 2014 07:53 PM, Heikki Krogerus wrote:
> Hi Kishon,
> 
> And happy new year..

Happy new year :-)
> 
> On Tue, Jan 07, 2014 at 07:10:36PM +0530, Kishon Vijay Abraham I wrote:
>>>>>  /**
>>>>> - * phy_get() - lookup and obtain a reference to a phy.
>>>>> + * phy_get_index() - obtain a phy based on index
>>>>
>>>> NAK. It still takes a 'char' argument and the name is misleading.
>>>> Btw are you replacing phy_get() or adding a new API in addition to phy_get()?
>>>
>>> Additional API. The phy_get() would in practice act as a wrapper after
>>
>> In this patch it looks like you've replaced phy_get().
>>> this. It could actually be just a #define macro in the include file.
>>> The function naming I just copied straight from gpiolib.c. I did not
>>> have the imagination for anything fancier.
>>>
>>> I would like to be able to use some function like phy_get_index() and
>>> be able to deliver it both the name and the index. With DT you guys
>>> will always be able to use the name (and the string will always
>>> supersede the index if we do it like this), but with ACPI, and possibly
>>> the platform lookup tables, the index can be used...
>>
>> I think in that case, we should drop the 'string' from phy_get_index since we
>> have the other API to handle that? I don't know about ACPI, but is it not
>> possible to use strings with ACPI?
> 
> No unfortunately. We just have what the ACPI tables provide. The PHYs
> would be "child" device entries under the controller and we can only
> get handle to them based on the index.
> 
> I think I'll skip this patch from this set. Let's wait until we have
> an actual ACPI DSDT describing some PHYs.

yeah.. sure.

Cheers
Kishon

Re: [PATCH 2/5] phy: add support for indexed lookup

[Kishon Vijay Abraham I]

Hi Heikki,

On Tuesday 14 January 2014 07:53 PM, Heikki Krogerus wrote:
> Hi Kishon,
> 
> And happy new year..

Happy new year :-)
> 
> On Tue, Jan 07, 2014 at 07:10:36PM +0530, Kishon Vijay Abraham I wrote:
>>>>>  /**
>>>>> - * phy_get() - lookup and obtain a reference to a phy.
>>>>> + * phy_get_index() - obtain a phy based on index
>>>>
>>>> NAK. It still takes a 'char' argument and the name is misleading.
>>>> Btw are you replacing phy_get() or adding a new API in addition to phy_get()?
>>>
>>> Additional API. The phy_get() would in practice act as a wrapper after
>>
>> In this patch it looks like you've replaced phy_get().
>>> this. It could actually be just a #define macro in the include file.
>>> The function naming I just copied straight from gpiolib.c. I did not
>>> have the imagination for anything fancier.
>>>
>>> I would like to be able to use some function like phy_get_index() and
>>> be able to deliver it both the name and the index. With DT you guys
>>> will always be able to use the name (and the string will always
>>> supersede the index if we do it like this), but with ACPI, and possibly
>>> the platform lookup tables, the index can be used...
>>
>> I think in that case, we should drop the 'string' from phy_get_index since we
>> have the other API to handle that? I don't know about ACPI, but is it not
>> possible to use strings with ACPI?
> 
> No unfortunately. We just have what the ACPI tables provide. The PHYs
> would be "child" device entries under the controller and we can only
> get handle to them based on the index.
> 
> I think I'll skip this patch from this set. Let's wait until we have
> an actual ACPI DSDT describing some PHYs.

yeah.. sure.

Cheers
Kishon

[PATCH 3/5] phy: improved lookup method

[Heikki Krogerus]

Removes the need for the phys to be aware of their users
even when not using DT. The method is copied from gpiolib.c.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/phy/phy-core.c  | 91 ++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/phy/phy.h | 48 ++++++++++++++++++++++++++
 2 files changed, 138 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 99dc046..05792d0 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -25,6 +25,7 @@
 static struct class *phy_class;
 static DEFINE_MUTEX(phy_provider_mutex);
 static LIST_HEAD(phy_provider_list);
+static LIST_HEAD(phy_lookup_list);
 static DEFINE_IDA(phy_ida);
 
 static void devm_phy_release(struct device *dev, void *res)
@@ -85,6 +86,94 @@ static struct phy *phy_lookup(struct device *device, const char *con_id,
 	return ERR_PTR(-ENODEV);
 }
 
+/**
+ * phy_add_table() - register PHY/device association to the lookup list
+ * @table: association to register
+ */
+void phy_add_lookup_table(struct phy_lookup_table *table)
+{
+	mutex_lock(&phy_provider_mutex);
+	list_add_tail(&table->list, &phy_lookup_list);
+	mutex_unlock(&phy_provider_mutex);
+}
+
+/**
+ * phy_add_table() - remove PHY/device association from the lookup list
+ * @table: association to be removed
+ */
+void phy_del_lookup_table(struct phy_lookup_table *table)
+{
+	mutex_lock(&phy_provider_mutex);
+	list_del(&table->list);
+	mutex_unlock(&phy_provider_mutex);
+}
+
+static struct phy *find_phy_by_name(const char *name)
+{
+	struct class_dev_iter iter;
+	struct phy *phy = NULL;
+	struct device *dev;
+
+	class_dev_iter_init(&iter, phy_class, NULL, NULL);
+	while ((dev = class_dev_iter_next(&iter))) {
+		if (!strcmp(dev_name(dev), name)) {
+			phy = to_phy(dev);
+			break;
+		}
+	}
+	class_dev_iter_exit(&iter);
+
+	return phy;
+}
+
+static struct phy_lookup_table *phy_get_lookup_table(struct device *dev)
+{
+	const char *dev_id = dev ? dev_name(dev) : NULL;
+	struct phy_lookup_table *table;
+
+	mutex_lock(&phy_provider_mutex);
+	list_for_each_entry(table, &phy_lookup_list, list)
+		if (!strcmp(table->dev_id, dev_id))
+			goto out;
+	table = NULL;
+out:
+	mutex_unlock(&phy_provider_mutex);
+	return table;
+}
+
+static struct phy *phy_find(struct device *dev, const char *con_id,
+			    unsigned int idx)
+{
+	struct phy_lookup_table *table;
+	struct phy_lookup *p;
+	struct phy *phy;
+
+	table = phy_get_lookup_table(dev);
+	if (!table)
+		/* fall-back to the old lookup method for now */
+		return phy_lookup(dev, con_id, idx);
+
+	for (p = &table->table[0]; p->phy_name; p++) {
+		/* index must always match exactly */
+		if (idx != p->idx)
+			continue;
+
+		/* If the lookup entry has a con_id, require exact match */
+		if (p->con_id && (!con_id || strcmp(p->con_id, con_id)))
+			continue;
+
+		phy = find_phy_by_name(p->phy_name);
+		if (!phy) {
+			dev_warn(dev, "no PHY by the name %s\n", p->phy_name);
+			return ERR_PTR(-ENODEV);
+		}
+
+		return phy;
+	}
+
+	return ERR_PTR(-ENODEV);
+}
+
 static struct phy_provider *of_phy_provider_lookup(struct device_node *node)
 {
 	struct phy_provider *phy_provider;
@@ -386,7 +475,7 @@ struct phy *phy_get_index(struct device *dev, const char *con_id,
 	 */
 	if (!phy || IS_ERR(phy)) {
 		dev_dbg(dev, "using lookup tables for PHY lookup");
-		phy = phy_lookup(dev, con_id, idx);
+		phy = phy_find(dev, con_id, idx);
 	}
 
 	if (IS_ERR(phy)) {
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 43d1a23..cca363a 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -98,6 +98,54 @@ struct phy_init_data {
 	.port		= _port,				\
 }
 
+/**
+ * struct phy_lookup - Lookup entry for associating PHYs
+ * @phy_name: device name of the PHY
+ * @con_id: name of the PHY from device's point of view
+ * @idx: index of the PHY when name is not used
+ */
+struct phy_lookup {
+	const char *phy_name;
+	const char *con_id;
+	unsigned int idx;
+};
+
+/**
+ * struct phy_lookup_table - association of PHYs to specific device
+ * @list: entry in the lookup list
+ * @dev_id: the name of the device
+ * @table: table of PHYs attached to this device
+ */
+struct phy_lookup_table {
+	struct list_head list;
+	const char *dev_id;
+	struct phy_lookup table[];
+};
+
+/**
+ * Simple definition of a single PHY under a consumer
+ */
+#define PHY_LOOKUP(_phy_name, _con_id)		\
+{						\
+	PHY_LOOKUP_IDX(_phy_name, _con_id, 0),	\
+	{ },					\
+}
+
+/**
+ * Use this macro if you need to have several PHYs under the same con_id.
+ * Each PHY needs to use a different index and can be accessed using
+ * phy_get_index()
+ */
+#define PHY_LOOKUP_IDX(_phy_name, _con_id, _idx)	\
+{							\
+	.phy_name = _phy_name,				\
+	.con_id = _con_id,				\
+	.idx = _idx,					\
+}
+
+void phy_add_lookup_table(struct phy_lookup_table *);
+void phy_del_lookup_table(struct phy_lookup_table *);
+
 #define	to_phy(dev)	(container_of((dev), struct phy, dev))
 
 #define	of_phy_provider_register(dev, xlate)	\
-- 
1.8.5.1

[PATCH 3/5] phy: improved lookup method

[Heikki Krogerus]

Removes the need for the phys to be aware of their users
even when not using DT. The method is copied from gpiolib.c.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/phy/phy-core.c  | 91 ++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/phy/phy.h | 48 ++++++++++++++++++++++++++
 2 files changed, 138 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 99dc046..05792d0 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -25,6 +25,7 @@
 static struct class *phy_class;
 static DEFINE_MUTEX(phy_provider_mutex);
 static LIST_HEAD(phy_provider_list);
+static LIST_HEAD(phy_lookup_list);
 static DEFINE_IDA(phy_ida);
 
 static void devm_phy_release(struct device *dev, void *res)
@@ -85,6 +86,94 @@ static struct phy *phy_lookup(struct device *device, const char *con_id,
 	return ERR_PTR(-ENODEV);
 }
 
+/**
+ * phy_add_table() - register PHY/device association to the lookup list
+ * @table: association to register
+ */
+void phy_add_lookup_table(struct phy_lookup_table *table)
+{
+	mutex_lock(&phy_provider_mutex);
+	list_add_tail(&table->list, &phy_lookup_list);
+	mutex_unlock(&phy_provider_mutex);
+}
+
+/**
+ * phy_add_table() - remove PHY/device association from the lookup list
+ * @table: association to be removed
+ */
+void phy_del_lookup_table(struct phy_lookup_table *table)
+{
+	mutex_lock(&phy_provider_mutex);
+	list_del(&table->list);
+	mutex_unlock(&phy_provider_mutex);
+}
+
+static struct phy *find_phy_by_name(const char *name)
+{
+	struct class_dev_iter iter;
+	struct phy *phy = NULL;
+	struct device *dev;
+
+	class_dev_iter_init(&iter, phy_class, NULL, NULL);
+	while ((dev = class_dev_iter_next(&iter))) {
+		if (!strcmp(dev_name(dev), name)) {
+			phy = to_phy(dev);
+			break;
+		}
+	}
+	class_dev_iter_exit(&iter);
+
+	return phy;
+}
+
+static struct phy_lookup_table *phy_get_lookup_table(struct device *dev)
+{
+	const char *dev_id = dev ? dev_name(dev) : NULL;
+	struct phy_lookup_table *table;
+
+	mutex_lock(&phy_provider_mutex);
+	list_for_each_entry(table, &phy_lookup_list, list)
+		if (!strcmp(table->dev_id, dev_id))
+			goto out;
+	table = NULL;
+out:
+	mutex_unlock(&phy_provider_mutex);
+	return table;
+}
+
+static struct phy *phy_find(struct device *dev, const char *con_id,
+			    unsigned int idx)
+{
+	struct phy_lookup_table *table;
+	struct phy_lookup *p;
+	struct phy *phy;
+
+	table = phy_get_lookup_table(dev);
+	if (!table)
+		/* fall-back to the old lookup method for now */
+		return phy_lookup(dev, con_id, idx);
+
+	for (p = &table->table[0]; p->phy_name; p++) {
+		/* index must always match exactly */
+		if (idx != p->idx)
+			continue;
+
+		/* If the lookup entry has a con_id, require exact match */
+		if (p->con_id && (!con_id || strcmp(p->con_id, con_id)))
+			continue;
+
+		phy = find_phy_by_name(p->phy_name);
+		if (!phy) {
+			dev_warn(dev, "no PHY by the name %s\n", p->phy_name);
+			return ERR_PTR(-ENODEV);
+		}
+
+		return phy;
+	}
+
+	return ERR_PTR(-ENODEV);
+}
+
 static struct phy_provider *of_phy_provider_lookup(struct device_node *node)
 {
 	struct phy_provider *phy_provider;
@@ -386,7 +475,7 @@ struct phy *phy_get_index(struct device *dev, const char *con_id,
 	 */
 	if (!phy || IS_ERR(phy)) {
 		dev_dbg(dev, "using lookup tables for PHY lookup");
-		phy = phy_lookup(dev, con_id, idx);
+		phy = phy_find(dev, con_id, idx);
 	}
 
 	if (IS_ERR(phy)) {
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 43d1a23..cca363a 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -98,6 +98,54 @@ struct phy_init_data {
 	.port		= _port,				\
 }
 
+/**
+ * struct phy_lookup - Lookup entry for associating PHYs
+ * @phy_name: device name of the PHY
+ * @con_id: name of the PHY from device's point of view
+ * @idx: index of the PHY when name is not used
+ */
+struct phy_lookup {
+	const char *phy_name;
+	const char *con_id;
+	unsigned int idx;
+};
+
+/**
+ * struct phy_lookup_table - association of PHYs to specific device
+ * @list: entry in the lookup list
+ * @dev_id: the name of the device
+ * @table: table of PHYs attached to this device
+ */
+struct phy_lookup_table {
+	struct list_head list;
+	const char *dev_id;
+	struct phy_lookup table[];
+};
+
+/**
+ * Simple definition of a single PHY under a consumer
+ */
+#define PHY_LOOKUP(_phy_name, _con_id)		\
+{						\
+	PHY_LOOKUP_IDX(_phy_name, _con_id, 0),	\
+	{ },					\
+}
+
+/**
+ * Use this macro if you need to have several PHYs under the same con_id.
+ * Each PHY needs to use a different index and can be accessed using
+ * phy_get_index()
+ */
+#define PHY_LOOKUP_IDX(_phy_name, _con_id, _idx)	\
+{							\
+	.phy_name = _phy_name,				\
+	.con_id = _con_id,				\
+	.idx = _idx,					\
+}
+
+void phy_add_lookup_table(struct phy_lookup_table *);
+void phy_del_lookup_table(struct phy_lookup_table *);
+
 #define	to_phy(dev)	(container_of((dev), struct phy, dev))
 
 #define	of_phy_provider_register(dev, xlate)	\
-- 
1.8.5.1

[PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Heikki Krogerus]

Provide a complete association for the phy and it's user
(musb) with the new phy_lookup_table.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 arch/arm/mach-omap2/twl-common.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
index b0d54da..972430b 100644
--- a/arch/arm/mach-omap2/twl-common.c
+++ b/arch/arm/mach-omap2/twl-common.c
@@ -91,18 +91,13 @@ void __init omap_pmic_late_init(void)
 }
 
 #if defined(CONFIG_ARCH_OMAP3)
-struct phy_consumer consumers[] = {
-	PHY_CONSUMER("musb-hdrc.0", "usb"),
-};
-
-struct phy_init_data init_data = {
-	.consumers = consumers,
-	.num_consumers = ARRAY_SIZE(consumers),
+static struct phy_lookup_table twl4030_usb_lookup = {
+	.dev_id = "musb-hdrc.0",
+	.table = PHY_LOOKUP("phy-twl4030_usb.0", NULL),
 };
 
 static struct twl4030_usb_data omap3_usb_pdata = {
 	.usb_mode	= T2_USB_MODE_ULPI,
-	.init_data	= &init_data,
 };
 
 static int omap3_batt_table[] = {
@@ -225,8 +220,10 @@ void __init omap3_pmic_get_config(struct twl4030_platform_data *pmic_data,
 	}
 
 	/* Common platform data configurations */
-	if (pdata_flags & TWL_COMMON_PDATA_USB && !pmic_data->usb)
+	if (pdata_flags & TWL_COMMON_PDATA_USB && !pmic_data->usb) {
+		phy_add_lookup_table(&twl4030_usb_lookup);
 		pmic_data->usb = &omap3_usb_pdata;
+	}
 
 	if (pdata_flags & TWL_COMMON_PDATA_BCI && !pmic_data->bci)
 		pmic_data->bci = &omap3_bci_pdata;
-- 
1.8.5.1

[PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Heikki Krogerus]

Provide a complete association for the phy and it's user
(musb) with the new phy_lookup_table.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 arch/arm/mach-omap2/twl-common.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
index b0d54da..972430b 100644
--- a/arch/arm/mach-omap2/twl-common.c
+++ b/arch/arm/mach-omap2/twl-common.c
@@ -91,18 +91,13 @@ void __init omap_pmic_late_init(void)
 }
 
 #if defined(CONFIG_ARCH_OMAP3)
-struct phy_consumer consumers[] = {
-	PHY_CONSUMER("musb-hdrc.0", "usb"),
-};
-
-struct phy_init_data init_data = {
-	.consumers = consumers,
-	.num_consumers = ARRAY_SIZE(consumers),
+static struct phy_lookup_table twl4030_usb_lookup = {
+	.dev_id = "musb-hdrc.0",
+	.table = PHY_LOOKUP("phy-twl4030_usb.0", NULL),
 };
 
 static struct twl4030_usb_data omap3_usb_pdata = {
 	.usb_mode	= T2_USB_MODE_ULPI,
-	.init_data	= &init_data,
 };
 
 static int omap3_batt_table[] = {
@@ -225,8 +220,10 @@ void __init omap3_pmic_get_config(struct twl4030_platform_data *pmic_data,
 	}
 
 	/* Common platform data configurations */
-	if (pdata_flags & TWL_COMMON_PDATA_USB && !pmic_data->usb)
+	if (pdata_flags & TWL_COMMON_PDATA_USB && !pmic_data->usb) {
+		phy_add_lookup_table(&twl4030_usb_lookup);
 		pmic_data->usb = &omap3_usb_pdata;
+	}
 
 	if (pdata_flags & TWL_COMMON_PDATA_BCI && !pmic_data->bci)
 		pmic_data->bci = &omap3_bci_pdata;
-- 
1.8.5.1

Re: [PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Kishon Vijay Abraham I]

Hi,

On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
> Provide a complete association for the phy and it's user
> (musb) with the new phy_lookup_table.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  arch/arm/mach-omap2/twl-common.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
> index b0d54da..972430b 100644
> --- a/arch/arm/mach-omap2/twl-common.c
> +++ b/arch/arm/mach-omap2/twl-common.c
> @@ -91,18 +91,13 @@ void __init omap_pmic_late_init(void)
>  }
>  
>  #if defined(CONFIG_ARCH_OMAP3)
> -struct phy_consumer consumers[] = {
> -	PHY_CONSUMER("musb-hdrc.0", "usb"),
> -};
> -
> -struct phy_init_data init_data = {
> -	.consumers = consumers,
> -	.num_consumers = ARRAY_SIZE(consumers),
> +static struct phy_lookup_table twl4030_usb_lookup = {
> +	.dev_id = "musb-hdrc.0",
> +	.table = PHY_LOOKUP("phy-twl4030_usb.0", NULL),
>  };

had a similar approach during the initial version of phy framework [1] (see
phy_bind) but changed it later [2] . Here you should know the device names of
two devices and even if one of them started using DEVID_AUTO, it'll start
breaking. Such an approach needs to reviewed carefully.

[1] -> https://lkml.org/lkml/2013/4/3/258
[2] -> http://www.spinics.net/lists/linux-usb/msg85299.html (removed phy_bind)

Cheers
Kishon
>  
>  static struct twl4030_usb_data omap3_usb_pdata = {
>  	.usb_mode	= T2_USB_MODE_ULPI,
> -	.init_data	= &init_data,
>  };
>  
>  static int omap3_batt_table[] = {
> @@ -225,8 +220,10 @@ void __init omap3_pmic_get_config(struct twl4030_platform_data *pmic_data,
>  	}
>  
>  	/* Common platform data configurations */
> -	if (pdata_flags & TWL_COMMON_PDATA_USB && !pmic_data->usb)
> +	if (pdata_flags & TWL_COMMON_PDATA_USB && !pmic_data->usb) {
> +		phy_add_lookup_table(&twl4030_usb_lookup);
>  		pmic_data->usb = &omap3_usb_pdata;
> +	}
>  
>  	if (pdata_flags & TWL_COMMON_PDATA_BCI && !pmic_data->bci)
>  		pmic_data->bci = &omap3_bci_pdata;
> 

[PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Kishon Vijay Abraham I]

Hi,

On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
> Provide a complete association for the phy and it's user
> (musb) with the new phy_lookup_table.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  arch/arm/mach-omap2/twl-common.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
> index b0d54da..972430b 100644
> --- a/arch/arm/mach-omap2/twl-common.c
> +++ b/arch/arm/mach-omap2/twl-common.c
> @@ -91,18 +91,13 @@ void __init omap_pmic_late_init(void)
>  }
>  
>  #if defined(CONFIG_ARCH_OMAP3)
> -struct phy_consumer consumers[] = {
> -	PHY_CONSUMER("musb-hdrc.0", "usb"),
> -};
> -
> -struct phy_init_data init_data = {
> -	.consumers = consumers,
> -	.num_consumers = ARRAY_SIZE(consumers),
> +static struct phy_lookup_table twl4030_usb_lookup = {
> +	.dev_id = "musb-hdrc.0",
> +	.table = PHY_LOOKUP("phy-twl4030_usb.0", NULL),
>  };

had a similar approach during the initial version of phy framework [1] (see
phy_bind) but changed it later [2] . Here you should know the device names of
two devices and even if one of them started using DEVID_AUTO, it'll start
breaking. Such an approach needs to reviewed carefully.

[1] -> https://lkml.org/lkml/2013/4/3/258
[2] -> http://www.spinics.net/lists/linux-usb/msg85299.html (removed phy_bind)

Cheers
Kishon
>  
>  static struct twl4030_usb_data omap3_usb_pdata = {
>  	.usb_mode	= T2_USB_MODE_ULPI,
> -	.init_data	= &init_data,
>  };
>  
>  static int omap3_batt_table[] = {
> @@ -225,8 +220,10 @@ void __init omap3_pmic_get_config(struct twl4030_platform_data *pmic_data,
>  	}
>  
>  	/* Common platform data configurations */
> -	if (pdata_flags & TWL_COMMON_PDATA_USB && !pmic_data->usb)
> +	if (pdata_flags & TWL_COMMON_PDATA_USB && !pmic_data->usb) {
> +		phy_add_lookup_table(&twl4030_usb_lookup);
>  		pmic_data->usb = &omap3_usb_pdata;
> +	}
>  
>  	if (pdata_flags & TWL_COMMON_PDATA_BCI && !pmic_data->bci)
>  		pmic_data->bci = &omap3_bci_pdata;
> 

Re: [PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Kishon Vijay Abraham I]

Hi,

On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
> Provide a complete association for the phy and it's user
> (musb) with the new phy_lookup_table.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  arch/arm/mach-omap2/twl-common.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
> index b0d54da..972430b 100644
> --- a/arch/arm/mach-omap2/twl-common.c
> +++ b/arch/arm/mach-omap2/twl-common.c
> @@ -91,18 +91,13 @@ void __init omap_pmic_late_init(void)
>  }
>  
>  #if defined(CONFIG_ARCH_OMAP3)
> -struct phy_consumer consumers[] = {
> -	PHY_CONSUMER("musb-hdrc.0", "usb"),
> -};
> -
> -struct phy_init_data init_data = {
> -	.consumers = consumers,
> -	.num_consumers = ARRAY_SIZE(consumers),
> +static struct phy_lookup_table twl4030_usb_lookup = {
> +	.dev_id = "musb-hdrc.0",
> +	.table = PHY_LOOKUP("phy-twl4030_usb.0", NULL),
>  };

had a similar approach during the initial version of phy framework [1] (see
phy_bind) but changed it later [2] . Here you should know the device names of
two devices and even if one of them started using DEVID_AUTO, it'll start
breaking. Such an approach needs to reviewed carefully.

[1] -> https://lkml.org/lkml/2013/4/3/258
[2] -> http://www.spinics.net/lists/linux-usb/msg85299.html (removed phy_bind)

Cheers
Kishon
>  
>  static struct twl4030_usb_data omap3_usb_pdata = {
>  	.usb_mode	= T2_USB_MODE_ULPI,
> -	.init_data	= &init_data,
>  };
>  
>  static int omap3_batt_table[] = {
> @@ -225,8 +220,10 @@ void __init omap3_pmic_get_config(struct twl4030_platform_data *pmic_data,
>  	}
>  
>  	/* Common platform data configurations */
> -	if (pdata_flags & TWL_COMMON_PDATA_USB && !pmic_data->usb)
> +	if (pdata_flags & TWL_COMMON_PDATA_USB && !pmic_data->usb) {
> +		phy_add_lookup_table(&twl4030_usb_lookup);
>  		pmic_data->usb = &omap3_usb_pdata;
> +	}
>  
>  	if (pdata_flags & TWL_COMMON_PDATA_BCI && !pmic_data->bci)
>  		pmic_data->bci = &omap3_bci_pdata;
> 

Re: [PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Heikki Krogerus]

Hi,

On Mon, Dec 16, 2013 at 04:34:35PM +0530, Kishon Vijay Abraham I wrote:
> On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
> > Provide a complete association for the phy and it's user
> > (musb) with the new phy_lookup_table.
> > 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  arch/arm/mach-omap2/twl-common.c | 15 ++++++---------
> >  1 file changed, 6 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
> > index b0d54da..972430b 100644
> > --- a/arch/arm/mach-omap2/twl-common.c
> > +++ b/arch/arm/mach-omap2/twl-common.c
> > @@ -91,18 +91,13 @@ void __init omap_pmic_late_init(void)
> >  }
> >  
> >  #if defined(CONFIG_ARCH_OMAP3)
> > -struct phy_consumer consumers[] = {
> > -	PHY_CONSUMER("musb-hdrc.0", "usb"),
> > -};
> > -
> > -struct phy_init_data init_data = {
> > -	.consumers = consumers,
> > -	.num_consumers = ARRAY_SIZE(consumers),
> > +static struct phy_lookup_table twl4030_usb_lookup = {
> > +	.dev_id = "musb-hdrc.0",
> > +	.table = PHY_LOOKUP("phy-twl4030_usb.0", NULL),
> >  };
> 
> had a similar approach during the initial version of phy framework [1] (see
> phy_bind) but changed it later [2] . Here you should know the device names of
> two devices and even if one of them started using DEVID_AUTO, it'll start
> breaking. Such an approach needs to reviewed carefully.

If somebody creates a regression by changing something like the
platform device id without checking all the users, he deserves to get
into big trouble. I don't see why an individual framework should
provide protection against such cases.

In any case, having two device names to deal with does not add any
more risk. These associations should always be made in the place where
the phy device is created so you will always know it's device name.
Normally the platform code creates these associations in the same
place it creates the platform devices, so you definitely know what the
device names will be.

In this case it's actually created in drivers/mfd/twl-core.c, so I do
need to update this and move the lookup table there. We can still
deliver the user name as platform data, though I believe it's always
"musb". Maybe we could actually skip that and just hard code the name?
The name of the phy we will of course know as the platform device is
created there and then, so the two device names don't create any more
risk even in this case.


Thanks,

-- 
heikki

[PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Heikki Krogerus]

Hi,

On Mon, Dec 16, 2013 at 04:34:35PM +0530, Kishon Vijay Abraham I wrote:
> On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
> > Provide a complete association for the phy and it's user
> > (musb) with the new phy_lookup_table.
> > 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  arch/arm/mach-omap2/twl-common.c | 15 ++++++---------
> >  1 file changed, 6 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
> > index b0d54da..972430b 100644
> > --- a/arch/arm/mach-omap2/twl-common.c
> > +++ b/arch/arm/mach-omap2/twl-common.c
> > @@ -91,18 +91,13 @@ void __init omap_pmic_late_init(void)
> >  }
> >  
> >  #if defined(CONFIG_ARCH_OMAP3)
> > -struct phy_consumer consumers[] = {
> > -	PHY_CONSUMER("musb-hdrc.0", "usb"),
> > -};
> > -
> > -struct phy_init_data init_data = {
> > -	.consumers = consumers,
> > -	.num_consumers = ARRAY_SIZE(consumers),
> > +static struct phy_lookup_table twl4030_usb_lookup = {
> > +	.dev_id = "musb-hdrc.0",
> > +	.table = PHY_LOOKUP("phy-twl4030_usb.0", NULL),
> >  };
> 
> had a similar approach during the initial version of phy framework [1] (see
> phy_bind) but changed it later [2] . Here you should know the device names of
> two devices and even if one of them started using DEVID_AUTO, it'll start
> breaking. Such an approach needs to reviewed carefully.

If somebody creates a regression by changing something like the
platform device id without checking all the users, he deserves to get
into big trouble. I don't see why an individual framework should
provide protection against such cases.

In any case, having two device names to deal with does not add any
more risk. These associations should always be made in the place where
the phy device is created so you will always know it's device name.
Normally the platform code creates these associations in the same
place it creates the platform devices, so you definitely know what the
device names will be.

In this case it's actually created in drivers/mfd/twl-core.c, so I do
need to update this and move the lookup table there. We can still
deliver the user name as platform data, though I believe it's always
"musb". Maybe we could actually skip that and just hard code the name?
The name of the phy we will of course know as the platform device is
created there and then, so the two device names don't create any more
risk even in this case.


Thanks,

-- 
heikki

Re: [PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Kishon Vijay Abraham I]

Hi,

On Monday 16 December 2013 08:13 PM, Heikki Krogerus wrote:
> Hi,
> 
> On Mon, Dec 16, 2013 at 04:34:35PM +0530, Kishon Vijay Abraham I wrote:
>> On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
>>> Provide a complete association for the phy and it's user
>>> (musb) with the new phy_lookup_table.
>>>
>>> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>>> ---
>>>  arch/arm/mach-omap2/twl-common.c | 15 ++++++---------
>>>  1 file changed, 6 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
>>> index b0d54da..972430b 100644
>>> --- a/arch/arm/mach-omap2/twl-common.c
>>> +++ b/arch/arm/mach-omap2/twl-common.c
>>> @@ -91,18 +91,13 @@ void __init omap_pmic_late_init(void)
>>>  }
>>>  
>>>  #if defined(CONFIG_ARCH_OMAP3)
>>> -struct phy_consumer consumers[] = {
>>> -	PHY_CONSUMER("musb-hdrc.0", "usb"),
>>> -};
>>> -
>>> -struct phy_init_data init_data = {
>>> -	.consumers = consumers,
>>> -	.num_consumers = ARRAY_SIZE(consumers),
>>> +static struct phy_lookup_table twl4030_usb_lookup = {
>>> +	.dev_id = "musb-hdrc.0",
>>> +	.table = PHY_LOOKUP("phy-twl4030_usb.0", NULL),
>>>  };
>>
>> had a similar approach during the initial version of phy framework [1] (see
>> phy_bind) but changed it later [2] . Here you should know the device names of
>> two devices and even if one of them started using DEVID_AUTO, it'll start
>> breaking. Such an approach needs to reviewed carefully.
> 
> If somebody creates a regression by changing something like the
> platform device id without checking all the users, he deserves to get
> into big trouble. I don't see why an individual framework should
> provide protection against such cases.
> 
> In any case, having two device names to deal with does not add any
> more risk. These associations should always be made in the place where
> the phy device is created so you will always know it's device name.

huh.. we should also know the 'controller' device name while defining these
associations and in some cases the controller device and phy device are created
in entirely different places.
> Normally the platform code creates these associations in the same
> place it creates the platform devices, so you definitely know what the
> device names will be.
> 
> In this case it's actually created in drivers/mfd/twl-core.c, so I do
> need to update this and move the lookup table there. We can still
> deliver the user name as platform data, though I believe it's always
> "musb". Maybe we could actually skip that and just hard code the name?

I would rather leave the way it is modelled now.

Thanks
Kishon

[PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Kishon Vijay Abraham I]

Hi,

On Monday 16 December 2013 08:13 PM, Heikki Krogerus wrote:
> Hi,
> 
> On Mon, Dec 16, 2013 at 04:34:35PM +0530, Kishon Vijay Abraham I wrote:
>> On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
>>> Provide a complete association for the phy and it's user
>>> (musb) with the new phy_lookup_table.
>>>
>>> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>>> ---
>>>  arch/arm/mach-omap2/twl-common.c | 15 ++++++---------
>>>  1 file changed, 6 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
>>> index b0d54da..972430b 100644
>>> --- a/arch/arm/mach-omap2/twl-common.c
>>> +++ b/arch/arm/mach-omap2/twl-common.c
>>> @@ -91,18 +91,13 @@ void __init omap_pmic_late_init(void)
>>>  }
>>>  
>>>  #if defined(CONFIG_ARCH_OMAP3)
>>> -struct phy_consumer consumers[] = {
>>> -	PHY_CONSUMER("musb-hdrc.0", "usb"),
>>> -};
>>> -
>>> -struct phy_init_data init_data = {
>>> -	.consumers = consumers,
>>> -	.num_consumers = ARRAY_SIZE(consumers),
>>> +static struct phy_lookup_table twl4030_usb_lookup = {
>>> +	.dev_id = "musb-hdrc.0",
>>> +	.table = PHY_LOOKUP("phy-twl4030_usb.0", NULL),
>>>  };
>>
>> had a similar approach during the initial version of phy framework [1] (see
>> phy_bind) but changed it later [2] . Here you should know the device names of
>> two devices and even if one of them started using DEVID_AUTO, it'll start
>> breaking. Such an approach needs to reviewed carefully.
> 
> If somebody creates a regression by changing something like the
> platform device id without checking all the users, he deserves to get
> into big trouble. I don't see why an individual framework should
> provide protection against such cases.
> 
> In any case, having two device names to deal with does not add any
> more risk. These associations should always be made in the place where
> the phy device is created so you will always know it's device name.

huh.. we should also know the 'controller' device name while defining these
associations and in some cases the controller device and phy device are created
in entirely different places.
> Normally the platform code creates these associations in the same
> place it creates the platform devices, so you definitely know what the
> device names will be.
> 
> In this case it's actually created in drivers/mfd/twl-core.c, so I do
> need to update this and move the lookup table there. We can still
> deliver the user name as platform data, though I believe it's always
> "musb". Maybe we could actually skip that and just hard code the name?

I would rather leave the way it is modelled now.

Thanks
Kishon

Re: [PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Kishon Vijay Abraham I]

Hi,

On Monday 16 December 2013 08:13 PM, Heikki Krogerus wrote:
> Hi,
> 
> On Mon, Dec 16, 2013 at 04:34:35PM +0530, Kishon Vijay Abraham I wrote:
>> On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
>>> Provide a complete association for the phy and it's user
>>> (musb) with the new phy_lookup_table.
>>>
>>> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>>> ---
>>>  arch/arm/mach-omap2/twl-common.c | 15 ++++++---------
>>>  1 file changed, 6 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
>>> index b0d54da..972430b 100644
>>> --- a/arch/arm/mach-omap2/twl-common.c
>>> +++ b/arch/arm/mach-omap2/twl-common.c
>>> @@ -91,18 +91,13 @@ void __init omap_pmic_late_init(void)
>>>  }
>>>  
>>>  #if defined(CONFIG_ARCH_OMAP3)
>>> -struct phy_consumer consumers[] = {
>>> -	PHY_CONSUMER("musb-hdrc.0", "usb"),
>>> -};
>>> -
>>> -struct phy_init_data init_data = {
>>> -	.consumers = consumers,
>>> -	.num_consumers = ARRAY_SIZE(consumers),
>>> +static struct phy_lookup_table twl4030_usb_lookup = {
>>> +	.dev_id = "musb-hdrc.0",
>>> +	.table = PHY_LOOKUP("phy-twl4030_usb.0", NULL),
>>>  };
>>
>> had a similar approach during the initial version of phy framework [1] (see
>> phy_bind) but changed it later [2] . Here you should know the device names of
>> two devices and even if one of them started using DEVID_AUTO, it'll start
>> breaking. Such an approach needs to reviewed carefully.
> 
> If somebody creates a regression by changing something like the
> platform device id without checking all the users, he deserves to get
> into big trouble. I don't see why an individual framework should
> provide protection against such cases.
> 
> In any case, having two device names to deal with does not add any
> more risk. These associations should always be made in the place where
> the phy device is created so you will always know it's device name.

huh.. we should also know the 'controller' device name while defining these
associations and in some cases the controller device and phy device are created
in entirely different places.
> Normally the platform code creates these associations in the same
> place it creates the platform devices, so you definitely know what the
> device names will be.
> 
> In this case it's actually created in drivers/mfd/twl-core.c, so I do
> need to update this and move the lookup table there. We can still
> deliver the user name as platform data, though I believe it's always
> "musb". Maybe we could actually skip that and just hard code the name?

I would rather leave the way it is modelled now.

Thanks
Kishon

Re: [PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Heikki Krogerus]

On Tue, Jan 07, 2014 at 06:31:52PM +0530, Kishon Vijay Abraham I wrote:
> > In any case, having two device names to deal with does not add any
> > more risk. These associations should always be made in the place where
> > the phy device is created so you will always know it's device name.
> 
> huh.. we should also know the 'controller' device name while defining these
> associations and in some cases the controller device and phy device are created
> in entirely different places.

If you don't want to use the controller device name to do the
matching, we can use the con_id string as well. I believe the lookup
method I use in this set just needs a small change.

Is that OK with you?


> > Normally the platform code creates these associations in the same
> > place it creates the platform devices, so you definitely know what the
> > device names will be.
> > 
> > In this case it's actually created in drivers/mfd/twl-core.c, so I do
> > need to update this and move the lookup table there. We can still
> > deliver the user name as platform data, though I believe it's always
> > "musb". Maybe we could actually skip that and just hard code the name?
> 
> I would rather leave the way it is modelled now.

Do you mean, leave it in the platform code? Why? We would reduce the
platform code by moving it to the mfd driver. Or did I misunderstood
something?

Hope I'm not forgetting something we have talked before my vacation.

Thanks,

-- 
heikki

[PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Heikki Krogerus]

On Tue, Jan 07, 2014 at 06:31:52PM +0530, Kishon Vijay Abraham I wrote:
> > In any case, having two device names to deal with does not add any
> > more risk. These associations should always be made in the place where
> > the phy device is created so you will always know it's device name.
> 
> huh.. we should also know the 'controller' device name while defining these
> associations and in some cases the controller device and phy device are created
> in entirely different places.

If you don't want to use the controller device name to do the
matching, we can use the con_id string as well. I believe the lookup
method I use in this set just needs a small change.

Is that OK with you?


> > Normally the platform code creates these associations in the same
> > place it creates the platform devices, so you definitely know what the
> > device names will be.
> > 
> > In this case it's actually created in drivers/mfd/twl-core.c, so I do
> > need to update this and move the lookup table there. We can still
> > deliver the user name as platform data, though I believe it's always
> > "musb". Maybe we could actually skip that and just hard code the name?
> 
> I would rather leave the way it is modelled now.

Do you mean, leave it in the platform code? Why? We would reduce the
platform code by moving it to the mfd driver. Or did I misunderstood
something?

Hope I'm not forgetting something we have talked before my vacation.

Thanks,

-- 
heikki

Re: [PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Kishon Vijay Abraham I]

Hi,

On Tuesday 14 January 2014 08:08 PM, Heikki Krogerus wrote:
> On Tue, Jan 07, 2014 at 06:31:52PM +0530, Kishon Vijay Abraham I wrote:
>>> In any case, having two device names to deal with does not add any
>>> more risk. These associations should always be made in the place where
>>> the phy device is created so you will always know it's device name.
>>
>> huh.. we should also know the 'controller' device name while defining these
>> associations and in some cases the controller device and phy device are created
>> in entirely different places.
> 
> If you don't want to use the controller device name to do the
> matching, we can use the con_id string as well. I believe the lookup
> method I use in this set just needs a small change.

The point I'm trying to make is that we won't 'always' know the device names in
advance.
Btw how are you planning to differentiate between multiple controllers of the
same type with con_id?

Maybe I'm missing the actual intention of this patch. Do you face problem if
the PHY's have to know about it's consumers in ACPI?
> 
> Is that OK with you?
> 
> 
>>> Normally the platform code creates these associations in the same
>>> place it creates the platform devices, so you definitely know what the
>>> device names will be.
>>>
>>> In this case it's actually created in drivers/mfd/twl-core.c, so I do
>>> need to update this and move the lookup table there. We can still
>>> deliver the user name as platform data, though I believe it's always
>>> "musb". Maybe we could actually skip that and just hard code the name?
>>
>> I would rather leave the way it is modelled now.
> 
> Do you mean, leave it in the platform code? Why? We would reduce the
> platform code by moving it to the mfd driver. Or did I misunderstood
> something?

In this case, the lookup table will be used only for non-dt boot so don't see
much use in moving to mfd driver.
I meant if the new method is not solving any problem then I would prefer the
way it is modelled now where the PHY's know about it's consumers.

Btw where does device gets created in ACPI boot?

Cheers
Kishon

[PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Kishon Vijay Abraham I]

Hi,

On Tuesday 14 January 2014 08:08 PM, Heikki Krogerus wrote:
> On Tue, Jan 07, 2014 at 06:31:52PM +0530, Kishon Vijay Abraham I wrote:
>>> In any case, having two device names to deal with does not add any
>>> more risk. These associations should always be made in the place where
>>> the phy device is created so you will always know it's device name.
>>
>> huh.. we should also know the 'controller' device name while defining these
>> associations and in some cases the controller device and phy device are created
>> in entirely different places.
> 
> If you don't want to use the controller device name to do the
> matching, we can use the con_id string as well. I believe the lookup
> method I use in this set just needs a small change.

The point I'm trying to make is that we won't 'always' know the device names in
advance.
Btw how are you planning to differentiate between multiple controllers of the
same type with con_id?

Maybe I'm missing the actual intention of this patch. Do you face problem if
the PHY's have to know about it's consumers in ACPI?
> 
> Is that OK with you?
> 
> 
>>> Normally the platform code creates these associations in the same
>>> place it creates the platform devices, so you definitely know what the
>>> device names will be.
>>>
>>> In this case it's actually created in drivers/mfd/twl-core.c, so I do
>>> need to update this and move the lookup table there. We can still
>>> deliver the user name as platform data, though I believe it's always
>>> "musb". Maybe we could actually skip that and just hard code the name?
>>
>> I would rather leave the way it is modelled now.
> 
> Do you mean, leave it in the platform code? Why? We would reduce the
> platform code by moving it to the mfd driver. Or did I misunderstood
> something?

In this case, the lookup table will be used only for non-dt boot so don't see
much use in moving to mfd driver.
I meant if the new method is not solving any problem then I would prefer the
way it is modelled now where the PHY's know about it's consumers.

Btw where does device gets created in ACPI boot?

Cheers
Kishon

Re: [PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Kishon Vijay Abraham I]

Hi,

On Tuesday 14 January 2014 08:08 PM, Heikki Krogerus wrote:
> On Tue, Jan 07, 2014 at 06:31:52PM +0530, Kishon Vijay Abraham I wrote:
>>> In any case, having two device names to deal with does not add any
>>> more risk. These associations should always be made in the place where
>>> the phy device is created so you will always know it's device name.
>>
>> huh.. we should also know the 'controller' device name while defining these
>> associations and in some cases the controller device and phy device are created
>> in entirely different places.
> 
> If you don't want to use the controller device name to do the
> matching, we can use the con_id string as well. I believe the lookup
> method I use in this set just needs a small change.

The point I'm trying to make is that we won't 'always' know the device names in
advance.
Btw how are you planning to differentiate between multiple controllers of the
same type with con_id?

Maybe I'm missing the actual intention of this patch. Do you face problem if
the PHY's have to know about it's consumers in ACPI?
> 
> Is that OK with you?
> 
> 
>>> Normally the platform code creates these associations in the same
>>> place it creates the platform devices, so you definitely know what the
>>> device names will be.
>>>
>>> In this case it's actually created in drivers/mfd/twl-core.c, so I do
>>> need to update this and move the lookup table there. We can still
>>> deliver the user name as platform data, though I believe it's always
>>> "musb". Maybe we could actually skip that and just hard code the name?
>>
>> I would rather leave the way it is modelled now.
> 
> Do you mean, leave it in the platform code? Why? We would reduce the
> platform code by moving it to the mfd driver. Or did I misunderstood
> something?

In this case, the lookup table will be used only for non-dt boot so don't see
much use in moving to mfd driver.
I meant if the new method is not solving any problem then I would prefer the
way it is modelled now where the PHY's know about it's consumers.

Btw where does device gets created in ACPI boot?

Cheers
Kishon

Re: [PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Heikki Krogerus]

On Wed, Jan 15, 2014 at 07:41:55PM +0530, Kishon Vijay Abraham I wrote:
> The point I'm trying to make is that we won't 'always' know the device names in
> advance.

In which cases do we not know the device names, and please note, cases
where we would need to use the lookup? The normal cases we would use
it are..

1) A driver creating a child device, like dwc3 when it creates the
xhci. There the parent just delivers the phys it has to the child, so
both the device name and the phys are known.

2) Platform code. Hopefully we can get rid of the platform code one
day, but in any case, when we use it, we always know at least how many
users a phy has, and if there is only a singe user, we can rely con_id
to do the matching. Though I still don't really see much risk in using
the controller device name also there. The lookup table should in any
case be made in the place where the phy devices are created, so the
phy name is definitely always known.

3) Phys part of something like mfd. This is in practice the same as
the platform code case. For example, with twl we always know there is
only one user for the phy, which is musb. So we can just use musb's
con_id if it's to scary to use "musb-hdrc.0".

In any other case, you get the phy from DT, and there is no need to
use the lookup when linking the phys and the users.

> Btw how are you planning to differentiate between multiple controllers of the
> same type with con_id?

You don't rely on the con_id alone in those cases. You then use the
device name. The con_id is just one parameter you can use to do the
matching.

> Maybe I'm missing the actual intention of this patch. Do you face problem if
> the PHY's have to know about it's consumers in ACPI?

<snip>

> >> I would rather leave the way it is modelled now.
> > 
> > Do you mean, leave it in the platform code? Why? We would reduce the
> > platform code by moving it to the mfd driver. Or did I misunderstood
> > something?
> 
> In this case, the lookup table will be used only for non-dt boot so don't see
> much use in moving to mfd driver.
> I meant if the new method is not solving any problem then I would prefer the
> way it is modelled now where the PHY's know about it's consumers.

OK, so that's what you meant. Things like that init_data promotes use
of platform data in the drivers, something that we really should get
rid of. Is that not a problem to you?

The phy drivers simply should not need to care about the consumers.
They should not need to care about anything DT, ACPI, platform
specific if it's possible, and here there is nothing preventing it.
Let me clarify..

The plan is that ultimately the drivers (meaning all the drivers, not
just phy) don't need to care about things like DT properties, ACPI
properties or anything DT, ACPI, platform specific. We will have
generic driver properties and capabilities and the upper layers will
take care of delivering the correct values from the source at hand.
The drivers can be made truly platform agnostic.

That is the goal, and we should make everything new by keeping that in
mind. Many existing frameworks are being converted to that direction,
like the gpio framework and DMA Engine API. The way you force the phy
drivers the deliver the consumers is taking a step backwards.

> Btw where does device gets created in ACPI boot?

drivers/acpi/acpi_platform.c

Thanks,

-- 
heikki

[PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Heikki Krogerus]

On Wed, Jan 15, 2014 at 07:41:55PM +0530, Kishon Vijay Abraham I wrote:
> The point I'm trying to make is that we won't 'always' know the device names in
> advance.

In which cases do we not know the device names, and please note, cases
where we would need to use the lookup? The normal cases we would use
it are..

1) A driver creating a child device, like dwc3 when it creates the
xhci. There the parent just delivers the phys it has to the child, so
both the device name and the phys are known.

2) Platform code. Hopefully we can get rid of the platform code one
day, but in any case, when we use it, we always know at least how many
users a phy has, and if there is only a singe user, we can rely con_id
to do the matching. Though I still don't really see much risk in using
the controller device name also there. The lookup table should in any
case be made in the place where the phy devices are created, so the
phy name is definitely always known.

3) Phys part of something like mfd. This is in practice the same as
the platform code case. For example, with twl we always know there is
only one user for the phy, which is musb. So we can just use musb's
con_id if it's to scary to use "musb-hdrc.0".

In any other case, you get the phy from DT, and there is no need to
use the lookup when linking the phys and the users.

> Btw how are you planning to differentiate between multiple controllers of the
> same type with con_id?

You don't rely on the con_id alone in those cases. You then use the
device name. The con_id is just one parameter you can use to do the
matching.

> Maybe I'm missing the actual intention of this patch. Do you face problem if
> the PHY's have to know about it's consumers in ACPI?

<snip>

> >> I would rather leave the way it is modelled now.
> > 
> > Do you mean, leave it in the platform code? Why? We would reduce the
> > platform code by moving it to the mfd driver. Or did I misunderstood
> > something?
> 
> In this case, the lookup table will be used only for non-dt boot so don't see
> much use in moving to mfd driver.
> I meant if the new method is not solving any problem then I would prefer the
> way it is modelled now where the PHY's know about it's consumers.

OK, so that's what you meant. Things like that init_data promotes use
of platform data in the drivers, something that we really should get
rid of. Is that not a problem to you?

The phy drivers simply should not need to care about the consumers.
They should not need to care about anything DT, ACPI, platform
specific if it's possible, and here there is nothing preventing it.
Let me clarify..

The plan is that ultimately the drivers (meaning all the drivers, not
just phy) don't need to care about things like DT properties, ACPI
properties or anything DT, ACPI, platform specific. We will have
generic driver properties and capabilities and the upper layers will
take care of delivering the correct values from the source at hand.
The drivers can be made truly platform agnostic.

That is the goal, and we should make everything new by keeping that in
mind. Many existing frameworks are being converted to that direction,
like the gpio framework and DMA Engine API. The way you force the phy
drivers the deliver the consumers is taking a step backwards.

> Btw where does device gets created in ACPI boot?

drivers/acpi/acpi_platform.c

Thanks,

-- 
heikki

Re: [PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Tony Lindgren]

* Heikki Krogerus <heikki.krogerus@linux.intel.com> [131209 07:10]:
> Provide a complete association for the phy and it's user
> (musb) with the new phy_lookup_table.

This seems safe to queue via the USB list:

Acked-by: Tony Lindgren <tony@atomide.com>
 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  arch/arm/mach-omap2/twl-common.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
> index b0d54da..972430b 100644
> --- a/arch/arm/mach-omap2/twl-common.c
> +++ b/arch/arm/mach-omap2/twl-common.c
> @@ -91,18 +91,13 @@ void __init omap_pmic_late_init(void)
>  }
>  
>  #if defined(CONFIG_ARCH_OMAP3)
> -struct phy_consumer consumers[] = {
> -	PHY_CONSUMER("musb-hdrc.0", "usb"),
> -};
> -
> -struct phy_init_data init_data = {
> -	.consumers = consumers,
> -	.num_consumers = ARRAY_SIZE(consumers),
> +static struct phy_lookup_table twl4030_usb_lookup = {
> +	.dev_id = "musb-hdrc.0",
> +	.table = PHY_LOOKUP("phy-twl4030_usb.0", NULL),
>  };
>  
>  static struct twl4030_usb_data omap3_usb_pdata = {
>  	.usb_mode	= T2_USB_MODE_ULPI,
> -	.init_data	= &init_data,
>  };
>  
>  static int omap3_batt_table[] = {
> @@ -225,8 +220,10 @@ void __init omap3_pmic_get_config(struct twl4030_platform_data *pmic_data,
>  	}
>  
>  	/* Common platform data configurations */
> -	if (pdata_flags & TWL_COMMON_PDATA_USB && !pmic_data->usb)
> +	if (pdata_flags & TWL_COMMON_PDATA_USB && !pmic_data->usb) {
> +		phy_add_lookup_table(&twl4030_usb_lookup);
>  		pmic_data->usb = &omap3_usb_pdata;
> +	}
>  
>  	if (pdata_flags & TWL_COMMON_PDATA_BCI && !pmic_data->bci)
>  		pmic_data->bci = &omap3_bci_pdata;
> -- 
> 1.8.5.1
> 

[PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy

[Tony Lindgren]

* Heikki Krogerus <heikki.krogerus@linux.intel.com> [131209 07:10]:
> Provide a complete association for the phy and it's user
> (musb) with the new phy_lookup_table.

This seems safe to queue via the USB list:

Acked-by: Tony Lindgren <tony@atomide.com>
 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  arch/arm/mach-omap2/twl-common.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
> index b0d54da..972430b 100644
> --- a/arch/arm/mach-omap2/twl-common.c
> +++ b/arch/arm/mach-omap2/twl-common.c
> @@ -91,18 +91,13 @@ void __init omap_pmic_late_init(void)
>  }
>  
>  #if defined(CONFIG_ARCH_OMAP3)
> -struct phy_consumer consumers[] = {
> -	PHY_CONSUMER("musb-hdrc.0", "usb"),
> -};
> -
> -struct phy_init_data init_data = {
> -	.consumers = consumers,
> -	.num_consumers = ARRAY_SIZE(consumers),
> +static struct phy_lookup_table twl4030_usb_lookup = {
> +	.dev_id = "musb-hdrc.0",
> +	.table = PHY_LOOKUP("phy-twl4030_usb.0", NULL),
>  };
>  
>  static struct twl4030_usb_data omap3_usb_pdata = {
>  	.usb_mode	= T2_USB_MODE_ULPI,
> -	.init_data	= &init_data,
>  };
>  
>  static int omap3_batt_table[] = {
> @@ -225,8 +220,10 @@ void __init omap3_pmic_get_config(struct twl4030_platform_data *pmic_data,
>  	}
>  
>  	/* Common platform data configurations */
> -	if (pdata_flags & TWL_COMMON_PDATA_USB && !pmic_data->usb)
> +	if (pdata_flags & TWL_COMMON_PDATA_USB && !pmic_data->usb) {
> +		phy_add_lookup_table(&twl4030_usb_lookup);
>  		pmic_data->usb = &omap3_usb_pdata;
> +	}
>  
>  	if (pdata_flags & TWL_COMMON_PDATA_BCI && !pmic_data->bci)
>  		pmic_data->bci = &omap3_bci_pdata;
> -- 
> 1.8.5.1
> 

[PATCH] phy: remove the old lookup method

[Heikki Krogerus]

The users of the old method are now converted to the new one.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 Documentation/phy.txt               | 100 ++++++++++++++++++++++--------------
 drivers/phy/phy-core.c              |  46 ++---------------
 drivers/phy/phy-exynos-dp-video.c   |   2 +-
 drivers/phy/phy-exynos-mipi-video.c |   2 +-
 drivers/phy/phy-omap-usb2.c         |   2 +-
 drivers/phy/phy-twl4030-usb.c       |   4 +-
 include/linux/phy/phy.h             |  36 ++-----------
 7 files changed, 73 insertions(+), 119 deletions(-)

diff --git a/Documentation/phy.txt b/Documentation/phy.txt
index 0103e4b..cfff1a2 100644
--- a/Documentation/phy.txt
+++ b/Documentation/phy.txt
@@ -53,17 +53,13 @@ unregister the PHY.
 The PHY driver should create the PHY in order for other peripheral controllers
 to make use of it. The PHY framework provides 2 APIs to create the PHY.
 
-struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
-        struct phy_init_data *init_data);
-struct phy *devm_phy_create(struct device *dev, const struct phy_ops *ops,
-	struct phy_init_data *init_data);
+struct phy *phy_create(struct device *dev, const struct phy_ops *ops);
+struct phy *devm_phy_create(struct device *dev, const struct phy_ops *ops);
 
 The PHY drivers can use one of the above 2 APIs to create the PHY by passing
-the device pointer, phy ops and init_data.
+the device pointer, phy ops.
 phy_ops is a set of function pointers for performing PHY operations such as
-init, exit, power_on and power_off. *init_data* is mandatory to get a reference
-to the PHY in the case of non-dt boot. See section *Board File Initialization*
-on how init_data should be used.
+init, exit, power_on and power_off.
 
 Inorder to dereference the private data (in phy_ops), the phy provider driver
 can use phy_set_drvdata() after creating the PHY and use phy_get_drvdata() in
@@ -74,16 +70,21 @@ phy_ops to get back the private data.
 Before the controller can make use of the PHY, it has to get a reference to
 it. This framework provides the following APIs to get a reference to the PHY.
 
-struct phy *phy_get(struct device *dev, const char *string);
-struct phy *devm_phy_get(struct device *dev, const char *string);
+struct phy *phy_get(struct device *dev, const char *con_id);
+struct phy *devm_phy_get(struct device *dev, const char *con_id);
 
 phy_get and devm_phy_get can be used to get the PHY. In the case of dt boot,
-the string arguments should contain the phy name as given in the dt data and
+the con_id arguments should contain the phy name as given in the dt data and
 in the case of non-dt boot, it should contain the label of the PHY.
 The only difference between the two APIs is that devm_phy_get associates the
 device with the PHY using devres on successful PHY get. On driver detach,
 release function is invoked on the the devres data and devres data is freed.
 
+Indexed lookup is also possible when device has multiple PHYs attached to it.
+The framework provides variants for phy_get() functions called phy_get_index()
+and devm_phy_get_index(). They take the same parameters as do the normal
+phy_get() plus the index number of the PHY as the last parameter.
+
 5. Releasing a reference to the PHY
 
 When the controller no longer needs the PHY, it has to release the reference
@@ -123,42 +124,63 @@ There are exported APIs like phy_pm_runtime_get, phy_pm_runtime_get_sync,
 phy_pm_runtime_put, phy_pm_runtime_put_sync, phy_pm_runtime_allow and
 phy_pm_runtime_forbid for performing PM operations.
 
-8. Board File Initialization
+8. Platform Data binding
 
-Certain board file initialization is necessary in order to get a reference
-to the PHY in the case of non-dt boot.
-Say we have a single device that implements 3 PHYs that of USB, SATA and PCIe,
-then in the board file the following initialization should be done.
+PHY's are mapped by the means of tables of lookups, containing instances of the
+phy_lookup structure. Two macros are defined to help declaring such mappings:
 
-struct phy_consumer consumers[] = {
-	PHY_CONSUMER("dwc3.0", "usb"),
-	PHY_CONSUMER("pcie.0", "pcie"),
-	PHY_CONSUMER("sata.0", "sata"),
-};
-PHY_CONSUMER takes 2 parameters, first is the device name of the controller
-(PHY consumer) and second is the port name.
+	PHY_LOOKUP(phy_name, con_id)
+	PHY_LOOKUP_IDX(phy_name, con_id, idx)
+
+where
 
-struct phy_init_data init_data = {
-	.consumers = consumers,
-	.num_consumers = ARRAY_SIZE(consumers),
+  - phy_name identifiers of the phy device
+  - con_id is the name of the PHY from the device point of view. It can be NULL.
+  - idx is the index of the PHY within the function.
+
+A lookup table can then be defined as follows, with an empty entry defining its
+end:
+
+struct phy_lookup_table phys_table = {
+       .dev_id = "usb_controller.0",
+       .table = {
+               PHY_LOOKUP_IDX("phy-usb2phy.0", "usb2-phy", 0),
+               PHY_LOOKUP_IDX("phy-usb3phy.0", "usb3-phy", 1),
+               { },
+       },
 };
 
-static const struct platform_device pipe3_phy_dev = {
-	.name = "pipe3-phy",
-	.id = -1,
-	.dev = {
-		.platform_data = {
-			.init_data = &init_data,
-		},
-	},
+or when there is only single PHY:
+
+struct phy_lookup_table phy_table = {
+       .dev_id = "foo.0",
+       .table = PHY_LOOKUP("phy.0", NULL),
 };
 
-then, while doing phy_create, the PHY driver should pass this init_data
-	phy_create(dev, ops, pdata->init_data);
+And the table can be added by the board code as follows:
+
+	phy_add_table(&phys_table);
+and
+	phy_add_table(&phy_table);
+
+The driver controlling "usb_controller.0" will then be able to obtain its PHYs
+as follows:
+
+	struct phy *usb2_phy, usb3_phy;
+
+	usb2_phy = phy_get(dev, "usb2-phy");
+	usb3_phy = phy_get(dev, "usb3-phy");
+or
+	usb2_phy = phy_get_index(dev, "usb2-phy", 0);
+	usb3_phy = phy_get_index(dev, "usb3-phy", 1);
+
+The driver controlling "foo.0" can obtain its PHY as follows:
+
+	struct phy *phy;
 
-and the controller driver (phy consumer) should pass the port name along with
-the device to get a reference to the PHY
-	phy_get(dev, "pcie");
+	phy = phy_get(dev, NULL);
+or
+	phy = phy_get_index(dev, NULL, 0);
 
 9. DeviceTree Binding
 
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 05792d0..26f19ee 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -54,38 +54,6 @@ static int devm_phy_match(struct device *dev, void *res, void *match_data)
 	return res == match_data;
 }
 
-static struct phy *phy_lookup(struct device *device, const char *con_id,
-			      unsigned int idx)
-{
-	unsigned int count;
-	struct phy *phy;
-	struct device *dev;
-	struct phy_consumer *consumers;
-	struct class_dev_iter iter;
-
-	class_dev_iter_init(&iter, phy_class, NULL, NULL);
-	while ((dev = class_dev_iter_next(&iter))) {
-		phy = to_phy(dev);
-		count = phy->init_data->num_consumers;
-		consumers = phy->init_data->consumers;
-		while (count--) {
-			/* index must always match exactly */
-			if (!con_id)
-				if (idx != count)
-					continue;
-			if (!strcmp(consumers->dev_name, dev_name(device)) &&
-					!strcmp(consumers->port, con_id)) {
-				class_dev_iter_exit(&iter);
-				return phy;
-			}
-			consumers++;
-		}
-	}
-
-	class_dev_iter_exit(&iter);
-	return ERR_PTR(-ENODEV);
-}
-
 /**
  * phy_add_table() - register PHY/device association to the lookup list
  * @table: association to register
@@ -150,8 +118,7 @@ static struct phy *phy_find(struct device *dev, const char *con_id,
 
 	table = phy_get_lookup_table(dev);
 	if (!table)
-		/* fall-back to the old lookup method for now */
-		return phy_lookup(dev, con_id, idx);
+		return ERR_PTR(-ENODEV);
 
 	for (p = &table->table[0]; p->phy_name; p++) {
 		/* index must always match exactly */
@@ -557,12 +524,10 @@ EXPORT_SYMBOL_GPL(devm_phy_get);
  * phy_create() - create a new phy
  * @dev: device that is creating the new phy
  * @ops: function pointers for performing phy operations
- * @init_data: contains the list of PHY consumers or NULL
  *
  * Called to create a phy using phy framework.
  */
-struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
-	struct phy_init_data *init_data)
+struct phy *phy_create(struct device *dev, const struct phy_ops *ops)
 {
 	int ret;
 	int id;
@@ -595,7 +560,6 @@ struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
 	phy->dev.of_node = dev->of_node;
 	phy->id = id;
 	phy->ops = ops;
-	phy->init_data = init_data;
 
 	ret = dev_set_name(&phy->dev, "phy-%s.%d", dev_name(dev), id);
 	if (ret)
@@ -626,15 +590,13 @@ EXPORT_SYMBOL_GPL(phy_create);
  * devm_phy_create() - create a new phy
  * @dev: device that is creating the new phy
  * @ops: function pointers for performing phy operations
- * @init_data: contains the list of PHY consumers or NULL
  *
  * Creates a new PHY device adding it to the PHY class.
  * 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.
  */
-struct phy *devm_phy_create(struct device *dev, const struct phy_ops *ops,
-	struct phy_init_data *init_data)
+struct phy *devm_phy_create(struct device *dev, const struct phy_ops *ops)
 {
 	struct phy **ptr, *phy;
 
@@ -642,7 +604,7 @@ struct phy *devm_phy_create(struct device *dev, const struct phy_ops *ops,
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	phy = phy_create(dev, ops, init_data);
+	phy = phy_create(dev, ops);
 	if (!IS_ERR(phy)) {
 		*ptr = phy;
 		devres_add(dev, ptr);
diff --git a/drivers/phy/phy-exynos-dp-video.c b/drivers/phy/phy-exynos-dp-video.c
index 1dbe6ce..e6c5065 100644
--- a/drivers/phy/phy-exynos-dp-video.c
+++ b/drivers/phy/phy-exynos-dp-video.c
@@ -80,7 +80,7 @@ static int exynos_dp_video_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(phy_provider))
 		return PTR_ERR(phy_provider);
 
-	phy = devm_phy_create(dev, &exynos_dp_video_phy_ops, NULL);
+	phy = devm_phy_create(dev, &exynos_dp_video_phy_ops);
 	if (IS_ERR(phy)) {
 		dev_err(dev, "failed to create Display Port PHY\n");
 		return PTR_ERR(phy);
diff --git a/drivers/phy/phy-exynos-mipi-video.c b/drivers/phy/phy-exynos-mipi-video.c
index 0c5efab..900a14d 100644
--- a/drivers/phy/phy-exynos-mipi-video.c
+++ b/drivers/phy/phy-exynos-mipi-video.c
@@ -141,7 +141,7 @@ static int exynos_mipi_video_phy_probe(struct platform_device *pdev)
 
 	for (i = 0; i < EXYNOS_MIPI_PHYS_NUM; i++) {
 		struct phy *phy = devm_phy_create(dev,
-					&exynos_mipi_video_phy_ops, NULL);
+						  &exynos_mipi_video_phy_ops);
 		if (IS_ERR(phy)) {
 			dev_err(dev, "failed to create PHY %d\n", i);
 			return PTR_ERR(phy);
diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
index bfc5c33..87c9d8b 100644
--- a/drivers/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -208,7 +208,7 @@ static int omap_usb2_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, phy);
 	pm_runtime_enable(phy->dev);
 
-	generic_phy = devm_phy_create(phy->dev, &ops, NULL);
+	generic_phy = devm_phy_create(phy->dev, &ops);
 	if (IS_ERR(generic_phy))
 		return PTR_ERR(generic_phy);
 
diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index daf65e6..bcc2f7d 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -659,7 +659,6 @@ static int twl4030_usb_probe(struct platform_device *pdev)
 	struct usb_otg		*otg;
 	struct device_node	*np = pdev->dev.of_node;
 	struct phy_provider	*phy_provider;
-	struct phy_init_data	*init_data = NULL;
 
 	twl = devm_kzalloc(&pdev->dev, sizeof *twl, GFP_KERNEL);
 	if (!twl)
@@ -670,7 +669,6 @@ static int twl4030_usb_probe(struct platform_device *pdev)
 				(enum twl4030_usb_mode *)&twl->usb_mode);
 	else if (pdata) {
 		twl->usb_mode = pdata->usb_mode;
-		init_data = pdata->init_data;
 	} else {
 		dev_err(&pdev->dev, "twl4030 initialized without pdata\n");
 		return -EINVAL;
@@ -700,7 +698,7 @@ static int twl4030_usb_probe(struct platform_device *pdev)
 	if (IS_ERR(phy_provider))
 		return PTR_ERR(phy_provider);
 
-	phy = devm_phy_create(twl->dev, &ops, init_data);
+	phy = devm_phy_create(twl->dev, &ops);
 	if (IS_ERR(phy)) {
 		dev_dbg(&pdev->dev, "Failed to create PHY\n");
 		return PTR_ERR(phy);
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index cca363a..a0fd30f 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -73,32 +73,6 @@ struct phy_provider {
 };
 
 /**
- * struct phy_consumer - represents the phy consumer
- * @dev_name: the device name of the controller that will use this PHY device
- * @port: name given to the consumer port
- */
-struct phy_consumer {
-	const char *dev_name;
-	const char *port;
-};
-
-/**
- * struct phy_init_data - contains the list of PHY consumers
- * @num_consumers: number of consumers for this PHY device
- * @consumers: list of PHY consumers
- */
-struct phy_init_data {
-	unsigned int num_consumers;
-	struct phy_consumer *consumers;
-};
-
-#define PHY_CONSUMER(_dev_name, _port)				\
-{								\
-	.dev_name	= _dev_name,				\
-	.port		= _port,				\
-}
-
-/**
  * struct phy_lookup - Lookup entry for associating PHYs
  * @phy_name: device name of the PHY
  * @con_id: name of the PHY from device's point of view
@@ -183,10 +157,8 @@ void phy_put(struct phy *phy);
 void devm_phy_put(struct device *dev, struct phy *phy);
 struct phy *of_phy_simple_xlate(struct device *dev,
 	struct of_phandle_args *args);
-struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
-	struct phy_init_data *init_data);
-struct phy *devm_phy_create(struct device *dev,
-	const struct phy_ops *ops, struct phy_init_data *init_data);
+struct phy *phy_create(struct device *dev, const struct phy_ops *ops);
+struct phy *devm_phy_create(struct device *dev, const struct phy_ops *ops);
 void phy_destroy(struct phy *phy);
 void devm_phy_destroy(struct device *dev, struct phy *phy);
 struct phy_provider *__of_phy_provider_register(struct device *dev,
@@ -286,13 +258,13 @@ static inline struct phy *of_phy_simple_xlate(struct device *dev,
 }
 
 static inline struct phy *phy_create(struct device *dev,
-	const struct phy_ops *ops, struct phy_init_data *init_data)
+				     const struct phy_ops *ops)
 {
 	return ERR_PTR(-ENOSYS);
 }
 
 static inline struct phy *devm_phy_create(struct device *dev,
-	const struct phy_ops *ops, struct phy_init_data *init_data)
+					  const struct phy_ops *ops)
 {
 	return ERR_PTR(-ENOSYS);
 }
-- 
1.8.5.1

[PATCH] phy: remove the old lookup method

[Heikki Krogerus]

The users of the old method are now converted to the new one.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 Documentation/phy.txt               | 100 ++++++++++++++++++++++--------------
 drivers/phy/phy-core.c              |  46 ++---------------
 drivers/phy/phy-exynos-dp-video.c   |   2 +-
 drivers/phy/phy-exynos-mipi-video.c |   2 +-
 drivers/phy/phy-omap-usb2.c         |   2 +-
 drivers/phy/phy-twl4030-usb.c       |   4 +-
 include/linux/phy/phy.h             |  36 ++-----------
 7 files changed, 73 insertions(+), 119 deletions(-)

diff --git a/Documentation/phy.txt b/Documentation/phy.txt
index 0103e4b..cfff1a2 100644
--- a/Documentation/phy.txt
+++ b/Documentation/phy.txt
@@ -53,17 +53,13 @@ unregister the PHY.
 The PHY driver should create the PHY in order for other peripheral controllers
 to make use of it. The PHY framework provides 2 APIs to create the PHY.
 
-struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
-        struct phy_init_data *init_data);
-struct phy *devm_phy_create(struct device *dev, const struct phy_ops *ops,
-	struct phy_init_data *init_data);
+struct phy *phy_create(struct device *dev, const struct phy_ops *ops);
+struct phy *devm_phy_create(struct device *dev, const struct phy_ops *ops);
 
 The PHY drivers can use one of the above 2 APIs to create the PHY by passing
-the device pointer, phy ops and init_data.
+the device pointer, phy ops.
 phy_ops is a set of function pointers for performing PHY operations such as
-init, exit, power_on and power_off. *init_data* is mandatory to get a reference
-to the PHY in the case of non-dt boot. See section *Board File Initialization*
-on how init_data should be used.
+init, exit, power_on and power_off.
 
 Inorder to dereference the private data (in phy_ops), the phy provider driver
 can use phy_set_drvdata() after creating the PHY and use phy_get_drvdata() in
@@ -74,16 +70,21 @@ phy_ops to get back the private data.
 Before the controller can make use of the PHY, it has to get a reference to
 it. This framework provides the following APIs to get a reference to the PHY.
 
-struct phy *phy_get(struct device *dev, const char *string);
-struct phy *devm_phy_get(struct device *dev, const char *string);
+struct phy *phy_get(struct device *dev, const char *con_id);
+struct phy *devm_phy_get(struct device *dev, const char *con_id);
 
 phy_get and devm_phy_get can be used to get the PHY. In the case of dt boot,
-the string arguments should contain the phy name as given in the dt data and
+the con_id arguments should contain the phy name as given in the dt data and
 in the case of non-dt boot, it should contain the label of the PHY.
 The only difference between the two APIs is that devm_phy_get associates the
 device with the PHY using devres on successful PHY get. On driver detach,
 release function is invoked on the the devres data and devres data is freed.
 
+Indexed lookup is also possible when device has multiple PHYs attached to it.
+The framework provides variants for phy_get() functions called phy_get_index()
+and devm_phy_get_index(). They take the same parameters as do the normal
+phy_get() plus the index number of the PHY as the last parameter.
+
 5. Releasing a reference to the PHY
 
 When the controller no longer needs the PHY, it has to release the reference
@@ -123,42 +124,63 @@ There are exported APIs like phy_pm_runtime_get, phy_pm_runtime_get_sync,
 phy_pm_runtime_put, phy_pm_runtime_put_sync, phy_pm_runtime_allow and
 phy_pm_runtime_forbid for performing PM operations.
 
-8. Board File Initialization
+8. Platform Data binding
 
-Certain board file initialization is necessary in order to get a reference
-to the PHY in the case of non-dt boot.
-Say we have a single device that implements 3 PHYs that of USB, SATA and PCIe,
-then in the board file the following initialization should be done.
+PHY's are mapped by the means of tables of lookups, containing instances of the
+phy_lookup structure. Two macros are defined to help declaring such mappings:
 
-struct phy_consumer consumers[] = {
-	PHY_CONSUMER("dwc3.0", "usb"),
-	PHY_CONSUMER("pcie.0", "pcie"),
-	PHY_CONSUMER("sata.0", "sata"),
-};
-PHY_CONSUMER takes 2 parameters, first is the device name of the controller
-(PHY consumer) and second is the port name.
+	PHY_LOOKUP(phy_name, con_id)
+	PHY_LOOKUP_IDX(phy_name, con_id, idx)
+
+where
 
-struct phy_init_data init_data = {
-	.consumers = consumers,
-	.num_consumers = ARRAY_SIZE(consumers),
+  - phy_name identifiers of the phy device
+  - con_id is the name of the PHY from the device point of view. It can be NULL.
+  - idx is the index of the PHY within the function.
+
+A lookup table can then be defined as follows, with an empty entry defining its
+end:
+
+struct phy_lookup_table phys_table = {
+       .dev_id = "usb_controller.0",
+       .table = {
+               PHY_LOOKUP_IDX("phy-usb2phy.0", "usb2-phy", 0),
+               PHY_LOOKUP_IDX("phy-usb3phy.0", "usb3-phy", 1),
+               { },
+       },
 };
 
-static const struct platform_device pipe3_phy_dev = {
-	.name = "pipe3-phy",
-	.id = -1,
-	.dev = {
-		.platform_data = {
-			.init_data = &init_data,
-		},
-	},
+or when there is only single PHY:
+
+struct phy_lookup_table phy_table = {
+       .dev_id = "foo.0",
+       .table = PHY_LOOKUP("phy.0", NULL),
 };
 
-then, while doing phy_create, the PHY driver should pass this init_data
-	phy_create(dev, ops, pdata->init_data);
+And the table can be added by the board code as follows:
+
+	phy_add_table(&phys_table);
+and
+	phy_add_table(&phy_table);
+
+The driver controlling "usb_controller.0" will then be able to obtain its PHYs
+as follows:
+
+	struct phy *usb2_phy, usb3_phy;
+
+	usb2_phy = phy_get(dev, "usb2-phy");
+	usb3_phy = phy_get(dev, "usb3-phy");
+or
+	usb2_phy = phy_get_index(dev, "usb2-phy", 0);
+	usb3_phy = phy_get_index(dev, "usb3-phy", 1);
+
+The driver controlling "foo.0" can obtain its PHY as follows:
+
+	struct phy *phy;
 
-and the controller driver (phy consumer) should pass the port name along with
-the device to get a reference to the PHY
-	phy_get(dev, "pcie");
+	phy = phy_get(dev, NULL);
+or
+	phy = phy_get_index(dev, NULL, 0);
 
 9. DeviceTree Binding
 
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 05792d0..26f19ee 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -54,38 +54,6 @@ static int devm_phy_match(struct device *dev, void *res, void *match_data)
 	return res == match_data;
 }
 
-static struct phy *phy_lookup(struct device *device, const char *con_id,
-			      unsigned int idx)
-{
-	unsigned int count;
-	struct phy *phy;
-	struct device *dev;
-	struct phy_consumer *consumers;
-	struct class_dev_iter iter;
-
-	class_dev_iter_init(&iter, phy_class, NULL, NULL);
-	while ((dev = class_dev_iter_next(&iter))) {
-		phy = to_phy(dev);
-		count = phy->init_data->num_consumers;
-		consumers = phy->init_data->consumers;
-		while (count--) {
-			/* index must always match exactly */
-			if (!con_id)
-				if (idx != count)
-					continue;
-			if (!strcmp(consumers->dev_name, dev_name(device)) &&
-					!strcmp(consumers->port, con_id)) {
-				class_dev_iter_exit(&iter);
-				return phy;
-			}
-			consumers++;
-		}
-	}
-
-	class_dev_iter_exit(&iter);
-	return ERR_PTR(-ENODEV);
-}
-
 /**
  * phy_add_table() - register PHY/device association to the lookup list
  * @table: association to register
@@ -150,8 +118,7 @@ static struct phy *phy_find(struct device *dev, const char *con_id,
 
 	table = phy_get_lookup_table(dev);
 	if (!table)
-		/* fall-back to the old lookup method for now */
-		return phy_lookup(dev, con_id, idx);
+		return ERR_PTR(-ENODEV);
 
 	for (p = &table->table[0]; p->phy_name; p++) {
 		/* index must always match exactly */
@@ -557,12 +524,10 @@ EXPORT_SYMBOL_GPL(devm_phy_get);
  * phy_create() - create a new phy
  * @dev: device that is creating the new phy
  * @ops: function pointers for performing phy operations
- * @init_data: contains the list of PHY consumers or NULL
  *
  * Called to create a phy using phy framework.
  */
-struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
-	struct phy_init_data *init_data)
+struct phy *phy_create(struct device *dev, const struct phy_ops *ops)
 {
 	int ret;
 	int id;
@@ -595,7 +560,6 @@ struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
 	phy->dev.of_node = dev->of_node;
 	phy->id = id;
 	phy->ops = ops;
-	phy->init_data = init_data;
 
 	ret = dev_set_name(&phy->dev, "phy-%s.%d", dev_name(dev), id);
 	if (ret)
@@ -626,15 +590,13 @@ EXPORT_SYMBOL_GPL(phy_create);
  * devm_phy_create() - create a new phy
  * @dev: device that is creating the new phy
  * @ops: function pointers for performing phy operations
- * @init_data: contains the list of PHY consumers or NULL
  *
  * Creates a new PHY device adding it to the PHY class.
  * 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.
  */
-struct phy *devm_phy_create(struct device *dev, const struct phy_ops *ops,
-	struct phy_init_data *init_data)
+struct phy *devm_phy_create(struct device *dev, const struct phy_ops *ops)
 {
 	struct phy **ptr, *phy;
 
@@ -642,7 +604,7 @@ struct phy *devm_phy_create(struct device *dev, const struct phy_ops *ops,
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	phy = phy_create(dev, ops, init_data);
+	phy = phy_create(dev, ops);
 	if (!IS_ERR(phy)) {
 		*ptr = phy;
 		devres_add(dev, ptr);
diff --git a/drivers/phy/phy-exynos-dp-video.c b/drivers/phy/phy-exynos-dp-video.c
index 1dbe6ce..e6c5065 100644
--- a/drivers/phy/phy-exynos-dp-video.c
+++ b/drivers/phy/phy-exynos-dp-video.c
@@ -80,7 +80,7 @@ static int exynos_dp_video_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(phy_provider))
 		return PTR_ERR(phy_provider);
 
-	phy = devm_phy_create(dev, &exynos_dp_video_phy_ops, NULL);
+	phy = devm_phy_create(dev, &exynos_dp_video_phy_ops);
 	if (IS_ERR(phy)) {
 		dev_err(dev, "failed to create Display Port PHY\n");
 		return PTR_ERR(phy);
diff --git a/drivers/phy/phy-exynos-mipi-video.c b/drivers/phy/phy-exynos-mipi-video.c
index 0c5efab..900a14d 100644
--- a/drivers/phy/phy-exynos-mipi-video.c
+++ b/drivers/phy/phy-exynos-mipi-video.c
@@ -141,7 +141,7 @@ static int exynos_mipi_video_phy_probe(struct platform_device *pdev)
 
 	for (i = 0; i < EXYNOS_MIPI_PHYS_NUM; i++) {
 		struct phy *phy = devm_phy_create(dev,
-					&exynos_mipi_video_phy_ops, NULL);
+						  &exynos_mipi_video_phy_ops);
 		if (IS_ERR(phy)) {
 			dev_err(dev, "failed to create PHY %d\n", i);
 			return PTR_ERR(phy);
diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
index bfc5c33..87c9d8b 100644
--- a/drivers/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -208,7 +208,7 @@ static int omap_usb2_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, phy);
 	pm_runtime_enable(phy->dev);
 
-	generic_phy = devm_phy_create(phy->dev, &ops, NULL);
+	generic_phy = devm_phy_create(phy->dev, &ops);
 	if (IS_ERR(generic_phy))
 		return PTR_ERR(generic_phy);
 
diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index daf65e6..bcc2f7d 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -659,7 +659,6 @@ static int twl4030_usb_probe(struct platform_device *pdev)
 	struct usb_otg		*otg;
 	struct device_node	*np = pdev->dev.of_node;
 	struct phy_provider	*phy_provider;
-	struct phy_init_data	*init_data = NULL;
 
 	twl = devm_kzalloc(&pdev->dev, sizeof *twl, GFP_KERNEL);
 	if (!twl)
@@ -670,7 +669,6 @@ static int twl4030_usb_probe(struct platform_device *pdev)
 				(enum twl4030_usb_mode *)&twl->usb_mode);
 	else if (pdata) {
 		twl->usb_mode = pdata->usb_mode;
-		init_data = pdata->init_data;
 	} else {
 		dev_err(&pdev->dev, "twl4030 initialized without pdata\n");
 		return -EINVAL;
@@ -700,7 +698,7 @@ static int twl4030_usb_probe(struct platform_device *pdev)
 	if (IS_ERR(phy_provider))
 		return PTR_ERR(phy_provider);
 
-	phy = devm_phy_create(twl->dev, &ops, init_data);
+	phy = devm_phy_create(twl->dev, &ops);
 	if (IS_ERR(phy)) {
 		dev_dbg(&pdev->dev, "Failed to create PHY\n");
 		return PTR_ERR(phy);
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index cca363a..a0fd30f 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -73,32 +73,6 @@ struct phy_provider {
 };
 
 /**
- * struct phy_consumer - represents the phy consumer
- * @dev_name: the device name of the controller that will use this PHY device
- * @port: name given to the consumer port
- */
-struct phy_consumer {
-	const char *dev_name;
-	const char *port;
-};
-
-/**
- * struct phy_init_data - contains the list of PHY consumers
- * @num_consumers: number of consumers for this PHY device
- * @consumers: list of PHY consumers
- */
-struct phy_init_data {
-	unsigned int num_consumers;
-	struct phy_consumer *consumers;
-};
-
-#define PHY_CONSUMER(_dev_name, _port)				\
-{								\
-	.dev_name	= _dev_name,				\
-	.port		= _port,				\
-}
-
-/**
  * struct phy_lookup - Lookup entry for associating PHYs
  * @phy_name: device name of the PHY
  * @con_id: name of the PHY from device's point of view
@@ -183,10 +157,8 @@ void phy_put(struct phy *phy);
 void devm_phy_put(struct device *dev, struct phy *phy);
 struct phy *of_phy_simple_xlate(struct device *dev,
 	struct of_phandle_args *args);
-struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
-	struct phy_init_data *init_data);
-struct phy *devm_phy_create(struct device *dev,
-	const struct phy_ops *ops, struct phy_init_data *init_data);
+struct phy *phy_create(struct device *dev, const struct phy_ops *ops);
+struct phy *devm_phy_create(struct device *dev, const struct phy_ops *ops);
 void phy_destroy(struct phy *phy);
 void devm_phy_destroy(struct device *dev, struct phy *phy);
 struct phy_provider *__of_phy_provider_register(struct device *dev,
@@ -286,13 +258,13 @@ static inline struct phy *of_phy_simple_xlate(struct device *dev,
 }
 
 static inline struct phy *phy_create(struct device *dev,
-	const struct phy_ops *ops, struct phy_init_data *init_data)
+				     const struct phy_ops *ops)
 {
 	return ERR_PTR(-ENOSYS);
 }
 
 static inline struct phy *devm_phy_create(struct device *dev,
-	const struct phy_ops *ops, struct phy_init_data *init_data)
+					  const struct phy_ops *ops)
 {
 	return ERR_PTR(-ENOSYS);
 }
-- 
1.8.5.1