linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv3 0/6] phy: simplified phy lookup
@ 2014-08-21 11:33 Heikki Krogerus
  2014-08-21 11:33 ` [PATCH 1/6] phy: safer to_phy() macro Heikki Krogerus
                   ` (5 more replies)
  0 siblings, 6 replies; 31+ messages in thread
From: Heikki Krogerus @ 2014-08-21 11:33 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

Hi,

So the idea with these is that they should help to make it possible to
request phys without caring about how they are mapped to the
consumers, meaning, was is the mapping done in DT, ACPI, etc. Mapping
phys to consumers can be now done with lookups similarly how clocks
can be mapped in clkdev.c.

Vivek needs to handle the phys of dwc3 also in xhci driver on
Exynos5420 SoC, so I'm resending these now.

Changes since v2:
- Calling ida_simple_remove in release function as pointed out by Greg


Heikki Krogerus (6):
  phy: safer to_phy() macro
  phy: improved lookup method
  arm: omap3: twl: use the new lookup method with usb phy
  phy: remove the old lookup method
  base: platform: name the device already during allocation
  usb: dwc3: host: convey the PHYs to xhci

 Documentation/phy.txt               |  66 +++++----------
 arch/arm/mach-omap2/twl-common.c    |  18 ++---
 drivers/base/platform.c             |  69 +++++++++-------
 drivers/phy/phy-bcm-kona-usb2.c     |   2 +-
 drivers/phy/phy-core.c              | 156 +++++++++++++++++++++++++++++-------
 drivers/phy/phy-exynos-dp-video.c   |   2 +-
 drivers/phy/phy-exynos-mipi-video.c |   2 +-
 drivers/phy/phy-exynos5-usbdrd.c    |   3 +-
 drivers/phy/phy-exynos5250-sata.c   |   2 +-
 drivers/phy/phy-mvebu-sata.c        |   2 +-
 drivers/phy/phy-omap-usb2.c         |   2 +-
 drivers/phy/phy-samsung-usb2.c      |   3 +-
 drivers/phy/phy-sun4i-usb.c         |   2 +-
 drivers/phy/phy-ti-pipe3.c          |   2 +-
 drivers/phy/phy-twl4030-usb.c       |   4 +-
 drivers/phy/phy-xgene.c             |   2 +-
 drivers/usb/dwc3/host.c             |  22 +++--
 include/linux/phy/phy.h             |  61 +++++++-------
 18 files changed, 253 insertions(+), 167 deletions(-)

-- 
2.1.0


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 1/6] phy: safer to_phy() macro
  2014-08-21 11:33 [PATCHv3 0/6] phy: simplified phy lookup Heikki Krogerus
@ 2014-08-21 11:33 ` Heikki Krogerus
  2014-08-21 11:33 ` [PATCH 2/6] phy: improved lookup method Heikki Krogerus
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 31+ messages in thread
From: Heikki Krogerus @ 2014-08-21 11:33 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

This makes to_phy() macro work with other variable names
besides "dev".

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Tested-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 include/linux/phy/phy.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 8cb6f81..9fda683 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -110,7 +110,7 @@ struct phy_init_data {
 	.port		= _port,				\
 }
 
-#define	to_phy(dev)	(container_of((dev), struct phy, dev))
+#define	to_phy(a)	(container_of((a), struct phy, dev))
 
 #define	of_phy_provider_register(dev, xlate)	\
 	__of_phy_provider_register((dev), THIS_MODULE, (xlate))
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 2/6] phy: improved lookup method
  2014-08-21 11:33 [PATCHv3 0/6] phy: simplified phy lookup Heikki Krogerus
  2014-08-21 11:33 ` [PATCH 1/6] phy: safer to_phy() macro Heikki Krogerus
@ 2014-08-21 11:33 ` Heikki Krogerus
  2014-09-11 15:33   ` Kishon Vijay Abraham I
  2014-08-21 11:33 ` [PATCH 3/6] arm: omap3: twl: use the new lookup method with usb phy Heikki Krogerus
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 31+ messages in thread
From: Heikki Krogerus @ 2014-08-21 11:33 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

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

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Tested-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 Documentation/phy.txt   |  66 ++++++++---------------
 drivers/phy/phy-core.c  | 135 +++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/phy/phy.h |  27 ++++++++++
 3 files changed, 183 insertions(+), 45 deletions(-)

diff --git a/Documentation/phy.txt b/Documentation/phy.txt
index c6594af..8add515 100644
--- a/Documentation/phy.txt
+++ b/Documentation/phy.txt
@@ -54,18 +54,14 @@ 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, struct device_node *node,
-		       const struct phy_ops *ops,
-		       struct phy_init_data *init_data);
+		       const struct phy_ops *ops);
 struct phy *devm_phy_create(struct device *dev, struct device_node *node,
-			    const struct phy_ops *ops,
-			    struct phy_init_data *init_data);
+			    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 and 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
@@ -137,42 +133,24 @@ 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
-
-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.
-
-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.
-
-struct phy_init_data init_data = {
-	.consumers = consumers,
-	.num_consumers = ARRAY_SIZE(consumers),
-};
-
-static const struct platform_device pipe3_phy_dev = {
-	.name = "pipe3-phy",
-	.id = -1,
-	.dev = {
-		.platform_data = {
-			.init_data = &init_data,
-		},
-	},
-};
-
-then, while doing phy_create, the PHY driver should pass this init_data
-	phy_create(dev, ops, pdata->init_data);
-
-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");
+8. PHY Mappings
+
+In order to get reference to a PHY without help from DeviceTree, the framework
+offers lookups which can be compared to clkdev that allow clk structures to be
+bound to devices. A lookup can be made statically by directly registering
+phy_lookup structure which contains the name of the PHY device, the name of the
+device which the PHY will be bind to and Connection ID string. Alternatively a
+lookup can be made during runtime when a handle to the struct phy already
+exists.
+
+The framework offers the following APIs for registering and unregistering the
+lookups.
+
+void phy_register_lookup(struct phy_lookup *pl);
+int phy_create_lookup(struct phy *phy, const char *con_id, const char *dev_id);
+
+void phy_unregister_lookup(struct phy_lookup *pl);
+void phy_remove_lookup(struct phy *phy, const char *con_id, const char *dev_id);
 
 9. DeviceTree Binding
 
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index ff5eec5..67a8c726 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -26,6 +26,7 @@
 static struct class *phy_class;
 static DEFINE_MUTEX(phy_provider_mutex);
 static LIST_HEAD(phy_provider_list);
+static LIST_HEAD(phys);
 static DEFINE_IDA(phy_ida);
 
 static void devm_phy_release(struct device *dev, void *res)
@@ -84,6 +85,138 @@ static struct phy *phy_lookup(struct device *device, const char *port)
 	return ERR_PTR(-ENODEV);
 }
 
+/**
+ * phy_register_lookup() - register PHY/device association
+ * @pl: association to register
+ */
+void phy_register_lookup(struct phy_lookup *pl)
+{
+	mutex_lock(&phy_provider_mutex);
+	list_add_tail(&pl->node, &phys);
+	mutex_unlock(&phy_provider_mutex);
+}
+
+/**
+ * phy_unregister_lookup() - remove PHY/device association
+ * @pl: association to be removed
+ */
+void phy_unregister_lookup(struct phy_lookup *pl)
+{
+	mutex_lock(&phy_provider_mutex);
+	list_del(&pl->node);
+	mutex_unlock(&phy_provider_mutex);
+}
+
+/**
+ * phy_create_lookup() - allocate and register PHY/device association
+ * @phy: the phy of the association
+ * @con_id: connection ID string on device
+ * @dev_id: the device of the association
+ *
+ * Creates and registers phy_lookup entry.
+ */
+int phy_create_lookup(struct phy *phy, const char *con_id, const char *dev_id)
+{
+	struct phy_lookup *pl;
+
+	if (!phy || (!dev_id && !con_id))
+		return -EINVAL;
+
+	pl = kzalloc(sizeof(*pl), GFP_KERNEL);
+	if (!pl)
+		return -ENOMEM;
+
+	pl->phy_name = dev_name(&phy->dev);
+	pl->dev_id = dev_id;
+	pl->con_id = con_id;
+
+	phy_register_lookup(pl);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(phy_create_lookup);
+
+/**
+ * phy_remove_lookup() - find and remove PHY/device association
+ * @phy: the phy of the association
+ * @con_id: connection ID string on device
+ * @dev_id: the device of the association
+ *
+ * Finds and unregisters phy_lookup entry that was created with
+ * phy_create_lookup().
+ */
+void phy_remove_lookup(struct phy *phy, const char *con_id, const char *dev_id)
+{
+	struct phy_lookup *pl;
+
+	if (!phy || (!dev_id && !con_id))
+		return;
+
+	list_for_each_entry(pl, &phys, node)
+		if (!strcmp(pl->phy_name, dev_name(&phy->dev)) &&
+		    !strcmp(pl->dev_id, dev_id) &&
+		    !strcmp(pl->con_id, con_id)) {
+			phy_unregister_lookup(pl);
+			kfree(pl);
+			return;
+		}
+}
+EXPORT_SYMBOL_GPL(phy_remove_lookup);
+
+static struct phy *phy_find(struct device *dev, const char *con_id)
+{
+	const char *dev_id = dev ? dev_name(dev) : NULL;
+	int match, best_found = 0, best_possible = 0;
+	struct phy *phy = ERR_PTR(-ENODEV);
+	struct phy_lookup *p, *pl = NULL;
+
+	if (dev_id)
+		best_possible += 2;
+	if (con_id)
+		best_possible += 1;
+
+	list_for_each_entry(p, &phys, node) {
+		match = 0;
+		if (p->dev_id) {
+			if (!dev_id || strcmp(p->dev_id, dev_id))
+				continue;
+			match += 2;
+		}
+		if (p->con_id) {
+			if (!con_id || strcmp(p->con_id, con_id))
+				continue;
+			match += 1;
+		}
+
+		if (match > best_found) {
+			pl = p;
+			if (match != best_possible)
+				best_found = match;
+			else
+				break;
+		}
+	}
+
+	if (pl) {
+		struct class_dev_iter iter;
+		struct device *phy_dev;
+
+		class_dev_iter_init(&iter, phy_class, NULL, NULL);
+		while ((phy_dev = class_dev_iter_next(&iter))) {
+			if (!strcmp(dev_name(phy_dev), pl->phy_name)) {
+				phy = to_phy(phy_dev);
+				break;
+			}
+		}
+		class_dev_iter_exit(&iter);
+	}
+
+	/* fall-back to the old lookup method for now */
+	if (IS_ERR(phy))
+		phy = phy_lookup(dev, con_id);
+	return phy;
+}
+
 static struct phy_provider *of_phy_provider_lookup(struct device_node *node)
 {
 	struct phy_provider *phy_provider;
@@ -463,7 +596,7 @@ struct phy *phy_get(struct device *dev, const char *string)
 			string);
 		phy = _of_phy_get(dev->of_node, index);
 	} else {
-		phy = phy_lookup(dev, string);
+		phy = phy_find(dev, string);
 	}
 	if (IS_ERR(phy))
 		return phy;
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 9fda683..2696b95 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -110,6 +110,20 @@ struct phy_init_data {
 	.port		= _port,				\
 }
 
+struct phy_lookup {
+	struct list_head node;
+	const char *phy_name;
+	const char *dev_id;
+	const char *con_id;
+};
+
+#define PHY_LOOKUP(a, b, c)	\
+	{				\
+		.phy_name = a,		\
+		.dev_id = b,		\
+		.con_id = c,		\
+	}
+
 #define	to_phy(a)	(container_of((a), struct phy, dev))
 
 #define	of_phy_provider_register(dev, xlate)	\
@@ -174,6 +188,10 @@ struct phy_provider *__devm_of_phy_provider_register(struct device *dev,
 void of_phy_provider_unregister(struct phy_provider *phy_provider);
 void devm_of_phy_provider_unregister(struct device *dev,
 	struct phy_provider *phy_provider);
+void phy_register_lookup(struct phy_lookup *pl);
+void phy_unregister_lookup(struct phy_lookup *pl);
+int phy_create_lookup(struct phy *phy, const char *con_id, const char *dev_id);
+void phy_remove_lookup(struct phy *phy, const char *con_id, const char *dev_id);
 #else
 static inline int phy_pm_runtime_get(struct phy *phy)
 {
@@ -345,6 +363,15 @@ static inline void devm_of_phy_provider_unregister(struct device *dev,
 	struct phy_provider *phy_provider)
 {
 }
+static inline void phy_register_lookup(struct phy_lookup *pl) { }
+static inline void phy_unregister_lookup(struct phy_lookup *pl) { }
+static inline int
+phy_create_lookup(struct phy *phy, const char *con_id, const char *dev_id)
+{
+	return 0;
+}
+static inline void phy_remove_lookup(struct phy *phy, const char *con_id,
+				     const char *dev_id) { }
 #endif
 
 #endif /* __DRIVERS_PHY_H */
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 3/6] arm: omap3: twl: use the new lookup method with usb phy
  2014-08-21 11:33 [PATCHv3 0/6] phy: simplified phy lookup Heikki Krogerus
  2014-08-21 11:33 ` [PATCH 1/6] phy: safer to_phy() macro Heikki Krogerus
  2014-08-21 11:33 ` [PATCH 2/6] phy: improved lookup method Heikki Krogerus
@ 2014-08-21 11:33 ` Heikki Krogerus
  2014-09-11 15:26   ` Kishon Vijay Abraham I
  2014-08-21 11:33 ` [PATCH 4/6] phy: remove the old lookup method Heikki Krogerus
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 31+ messages in thread
From: Heikki Krogerus @ 2014-08-21 11:33 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

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

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

diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
index b0d54da..b78383c 100644
--- a/arch/arm/mach-omap2/twl-common.c
+++ b/arch/arm/mach-omap2/twl-common.c
@@ -91,18 +91,14 @@ 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 twl4030_usb_lookup = {
+	.phy_name	= "phy-twl4030_usb.0",
+	.dev_id		= "musb-hdrc.0",
+	.con_id		= "usb",
 };
 
 static struct twl4030_usb_data omap3_usb_pdata = {
-	.usb_mode	= T2_USB_MODE_ULPI,
-	.init_data	= &init_data,
+	.usb_mode = T2_USB_MODE_ULPI,
 };
 
 static int omap3_batt_table[] = {
@@ -225,8 +221,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_register_lookup(&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;
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 4/6] phy: remove the old lookup method
  2014-08-21 11:33 [PATCHv3 0/6] phy: simplified phy lookup Heikki Krogerus
                   ` (2 preceding siblings ...)
  2014-08-21 11:33 ` [PATCH 3/6] arm: omap3: twl: use the new lookup method with usb phy Heikki Krogerus
@ 2014-08-21 11:33 ` Heikki Krogerus
  2014-08-25  7:41   ` Vivek Gautam
  2014-08-21 11:33 ` [PATCH 5/6] base: platform: name the device already during allocation Heikki Krogerus
  2014-08-21 11:33 ` [PATCH 6/6] usb: dwc3: host: convey the PHYs to xhci Heikki Krogerus
  5 siblings, 1 reply; 31+ messages in thread
From: Heikki Krogerus @ 2014-08-21 11:33 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

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

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Tested-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 drivers/phy/phy-bcm-kona-usb2.c     |  2 +-
 drivers/phy/phy-core.c              | 45 +++----------------------------------
 drivers/phy/phy-exynos-dp-video.c   |  2 +-
 drivers/phy/phy-exynos-mipi-video.c |  2 +-
 drivers/phy/phy-exynos5-usbdrd.c    |  3 +--
 drivers/phy/phy-exynos5250-sata.c   |  2 +-
 drivers/phy/phy-mvebu-sata.c        |  2 +-
 drivers/phy/phy-omap-usb2.c         |  2 +-
 drivers/phy/phy-samsung-usb2.c      |  3 +--
 drivers/phy/phy-sun4i-usb.c         |  2 +-
 drivers/phy/phy-ti-pipe3.c          |  2 +-
 drivers/phy/phy-twl4030-usb.c       |  4 +---
 drivers/phy/phy-xgene.c             |  2 +-
 include/linux/phy/phy.h             | 38 ++++---------------------------
 14 files changed, 19 insertions(+), 92 deletions(-)

diff --git a/drivers/phy/phy-bcm-kona-usb2.c b/drivers/phy/phy-bcm-kona-usb2.c
index 894fe74..3463983 100644
--- a/drivers/phy/phy-bcm-kona-usb2.c
+++ b/drivers/phy/phy-bcm-kona-usb2.c
@@ -117,7 +117,7 @@ static int bcm_kona_usb2_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, phy);
 
-	gphy = devm_phy_create(dev, NULL, &ops, NULL);
+	gphy = devm_phy_create(dev, NULL, &ops);
 	if (IS_ERR(gphy))
 		return PTR_ERR(gphy);
 
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 67a8c726..834b337 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -55,36 +55,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 *port)
-{
-	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);
-
-		if (!phy->init_data)
-			continue;
-		count = phy->init_data->num_consumers;
-		consumers = phy->init_data->consumers;
-		while (count--) {
-			if (!strcmp(consumers->dev_name, dev_name(device)) &&
-					!strcmp(consumers->port, port)) {
-				class_dev_iter_exit(&iter);
-				return phy;
-			}
-			consumers++;
-		}
-	}
-
-	class_dev_iter_exit(&iter);
-	return ERR_PTR(-ENODEV);
-}
-
 /**
  * phy_register_lookup() - register PHY/device association
  * @pl: association to register
@@ -210,10 +180,6 @@ static struct phy *phy_find(struct device *dev, const char *con_id)
 		}
 		class_dev_iter_exit(&iter);
 	}
-
-	/* fall-back to the old lookup method for now */
-	if (IS_ERR(phy))
-		phy = phy_lookup(dev, con_id);
 	return phy;
 }
 
@@ -721,13 +687,11 @@ EXPORT_SYMBOL_GPL(devm_of_phy_get);
  * @dev: device that is creating the new phy
  * @node: device node of the 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, struct device_node *node,
-		       const struct phy_ops *ops,
-		       struct phy_init_data *init_data)
+		       const struct phy_ops *ops)
 {
 	int ret;
 	int id;
@@ -765,7 +729,6 @@ struct phy *phy_create(struct device *dev, struct device_node *node,
 	phy->dev.of_node = 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)
@@ -800,7 +763,6 @@ EXPORT_SYMBOL_GPL(phy_create);
  * @dev: device that is creating the new phy
  * @node: device node of the 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.
@@ -808,8 +770,7 @@ EXPORT_SYMBOL_GPL(phy_create);
  * then, devres data is freed.
  */
 struct phy *devm_phy_create(struct device *dev, struct device_node *node,
-			    const struct phy_ops *ops,
-			    struct phy_init_data *init_data)
+			    const struct phy_ops *ops)
 {
 	struct phy **ptr, *phy;
 
@@ -817,7 +778,7 @@ struct phy *devm_phy_create(struct device *dev, struct device_node *node,
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	phy = phy_create(dev, node, ops, init_data);
+	phy = phy_create(dev, node, 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 8b3026e..c667d2b 100644
--- a/drivers/phy/phy-exynos-dp-video.c
+++ b/drivers/phy/phy-exynos-dp-video.c
@@ -77,7 +77,7 @@ static int exynos_dp_video_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(state->regs))
 		return PTR_ERR(state->regs);
 
-	phy = devm_phy_create(dev, NULL, &exynos_dp_video_phy_ops, NULL);
+	phy = devm_phy_create(dev, NULL, &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 b55a92e..2807f95 100644
--- a/drivers/phy/phy-exynos-mipi-video.c
+++ b/drivers/phy/phy-exynos-mipi-video.c
@@ -137,7 +137,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, NULL,
-					&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-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c
index b05302b..47f47fe 100644
--- a/drivers/phy/phy-exynos5-usbdrd.c
+++ b/drivers/phy/phy-exynos5-usbdrd.c
@@ -636,8 +636,7 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
 
 	for (i = 0; i < EXYNOS5_DRDPHYS_NUM; i++) {
 		struct phy *phy = devm_phy_create(dev, NULL,
-						  &exynos5_usbdrd_phy_ops,
-						  NULL);
+						  &exynos5_usbdrd_phy_ops);
 		if (IS_ERR(phy)) {
 			dev_err(dev, "Failed to create usbdrd_phy phy\n");
 			return PTR_ERR(phy);
diff --git a/drivers/phy/phy-exynos5250-sata.c b/drivers/phy/phy-exynos5250-sata.c
index 19a679a..3b0b4af7 100644
--- a/drivers/phy/phy-exynos5250-sata.c
+++ b/drivers/phy/phy-exynos5250-sata.c
@@ -210,7 +210,7 @@ static int exynos_sata_phy_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	sata_phy->phy = devm_phy_create(dev, NULL, &exynos_sata_phy_ops, NULL);
+	sata_phy->phy = devm_phy_create(dev, NULL, &exynos_sata_phy_ops);
 	if (IS_ERR(sata_phy->phy)) {
 		clk_disable_unprepare(sata_phy->phyclk);
 		dev_err(dev, "failed to create PHY\n");
diff --git a/drivers/phy/phy-mvebu-sata.c b/drivers/phy/phy-mvebu-sata.c
index cc3c0e1..0387e7b 100644
--- a/drivers/phy/phy-mvebu-sata.c
+++ b/drivers/phy/phy-mvebu-sata.c
@@ -99,7 +99,7 @@ static int phy_mvebu_sata_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->clk))
 		return PTR_ERR(priv->clk);
 
-	phy = devm_phy_create(&pdev->dev, NULL, &phy_mvebu_sata_ops, NULL);
+	phy = devm_phy_create(&pdev->dev, NULL, &phy_mvebu_sata_ops);
 	if (IS_ERR(phy))
 		return PTR_ERR(phy);
 
diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
index 93d7835..a4a93e3 100644
--- a/drivers/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -263,7 +263,7 @@ static int omap_usb2_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, phy);
 
-	generic_phy = devm_phy_create(phy->dev, NULL, &ops, NULL);
+	generic_phy = devm_phy_create(phy->dev, NULL, &ops);
 	if (IS_ERR(generic_phy))
 		return PTR_ERR(generic_phy);
 
diff --git a/drivers/phy/phy-samsung-usb2.c b/drivers/phy/phy-samsung-usb2.c
index 3732ca2..e2a364d 100644
--- a/drivers/phy/phy-samsung-usb2.c
+++ b/drivers/phy/phy-samsung-usb2.c
@@ -202,8 +202,7 @@ static int samsung_usb2_phy_probe(struct platform_device *pdev)
 		struct samsung_usb2_phy_instance *p = &drv->instances[i];
 
 		dev_dbg(dev, "Creating phy \"%s\"\n", label);
-		p->phy = devm_phy_create(dev, NULL, &samsung_usb2_phy_ops,
-					 NULL);
+		p->phy = devm_phy_create(dev, NULL, &samsung_usb2_phy_ops);
 		if (IS_ERR(p->phy)) {
 			dev_err(drv->dev, "Failed to create usb2_phy \"%s\"\n",
 				label);
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 61ebea4..56e7f49 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -295,7 +295,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
 				return PTR_ERR(phy->pmu);
 		}
 
-		phy->phy = devm_phy_create(dev, NULL, &sun4i_usb_phy_ops, NULL);
+		phy->phy = devm_phy_create(dev, NULL, &sun4i_usb_phy_ops);
 		if (IS_ERR(phy->phy)) {
 			dev_err(dev, "failed to create PHY %d\n", i);
 			return PTR_ERR(phy->phy);
diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c
index b964aa9..9f3a1d20 100644
--- a/drivers/phy/phy-ti-pipe3.c
+++ b/drivers/phy/phy-ti-pipe3.c
@@ -400,7 +400,7 @@ static int ti_pipe3_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, phy);
 	pm_runtime_enable(phy->dev);
 
-	generic_phy = devm_phy_create(phy->dev, NULL, &ops, NULL);
+	generic_phy = devm_phy_create(phy->dev, NULL, &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 e1a6623..851f126 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;
@@ -695,7 +693,7 @@ static int twl4030_usb_probe(struct platform_device *pdev)
 	otg->set_host		= twl4030_set_host;
 	otg->set_peripheral	= twl4030_set_peripheral;
 
-	phy = devm_phy_create(twl->dev, NULL, &ops, init_data);
+	phy = devm_phy_create(twl->dev, NULL, &ops);
 	if (IS_ERR(phy)) {
 		dev_dbg(&pdev->dev, "Failed to create PHY\n");
 		return PTR_ERR(phy);
diff --git a/drivers/phy/phy-xgene.c b/drivers/phy/phy-xgene.c
index db809b9..a239310 100644
--- a/drivers/phy/phy-xgene.c
+++ b/drivers/phy/phy-xgene.c
@@ -1707,7 +1707,7 @@ static int xgene_phy_probe(struct platform_device *pdev)
 	ctx->dev = &pdev->dev;
 	platform_set_drvdata(pdev, ctx);
 
-	ctx->phy = devm_phy_create(ctx->dev, NULL, &xgene_phy_ops, NULL);
+	ctx->phy = devm_phy_create(ctx->dev, NULL, &xgene_phy_ops);
 	if (IS_ERR(ctx->phy)) {
 		dev_dbg(&pdev->dev, "Failed to create PHY\n");
 		rc = PTR_ERR(ctx->phy);
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 2696b95..d983051 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -61,7 +61,6 @@ struct phy {
 	struct device		dev;
 	int			id;
 	const struct phy_ops	*ops;
-	struct phy_init_data	*init_data;
 	struct mutex		mutex;
 	int			init_count;
 	int			power_count;
@@ -84,32 +83,6 @@ struct phy_provider {
 		struct of_phandle_args *args);
 };
 
-/**
- * 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 {
 	struct list_head node;
 	const char *phy_name;
@@ -173,10 +146,9 @@ struct phy *of_phy_get(struct device_node *np, const char *con_id);
 struct phy *of_phy_simple_xlate(struct device *dev,
 	struct of_phandle_args *args);
 struct phy *phy_create(struct device *dev, struct device_node *node,
-		       const struct phy_ops *ops,
-		       struct phy_init_data *init_data);
+		       const struct phy_ops *ops);
 struct phy *devm_phy_create(struct device *dev, struct device_node *node,
-	const struct phy_ops *ops, struct phy_init_data *init_data);
+			    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,
@@ -319,16 +291,14 @@ static inline struct phy *of_phy_simple_xlate(struct device *dev,
 
 static inline struct phy *phy_create(struct device *dev,
 				     struct device_node *node,
-				     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,
 					  struct device_node *node,
-					  const struct phy_ops *ops,
-					  struct phy_init_data *init_data)
+					  const struct phy_ops *ops)
 {
 	return ERR_PTR(-ENOSYS);
 }
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 5/6] base: platform: name the device already during allocation
  2014-08-21 11:33 [PATCHv3 0/6] phy: simplified phy lookup Heikki Krogerus
                   ` (3 preceding siblings ...)
  2014-08-21 11:33 ` [PATCH 4/6] phy: remove the old lookup method Heikki Krogerus
@ 2014-08-21 11:33 ` Heikki Krogerus
  2014-08-21 11:33 ` [PATCH 6/6] usb: dwc3: host: convey the PHYs to xhci Heikki Krogerus
  5 siblings, 0 replies; 31+ messages in thread
From: Heikki Krogerus @ 2014-08-21 11:33 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

The device name is needed when assigning resources like
clocks or GPIOs to devices using lookups. If
PLATFORM_DEVICE_AUTO is used, the device name is not know
before platform_device_add is called after which it's too
late to assign that kind of resources as the drivers most
likely have already requested them.

By naming the device already in platform_device_alloc, the
resources can be assigned before platform_device_add is
called.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/base/platform.c | 69 +++++++++++++++++++++++++++++--------------------
 1 file changed, 41 insertions(+), 28 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index ab4f4ce..d3a7022 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -194,11 +194,41 @@ void platform_device_put(struct platform_device *pdev)
 }
 EXPORT_SYMBOL_GPL(platform_device_put);
 
+static int pdev_set_name(struct platform_device *pdev)
+{
+	int ret;
+
+	switch (pdev->id) {
+	default:
+		return dev_set_name(&pdev->dev, "%s.%d", pdev->name,  pdev->id);
+	case PLATFORM_DEVID_NONE:
+		return dev_set_name(&pdev->dev, "%s", pdev->name);
+	case PLATFORM_DEVID_AUTO:
+		/*
+		 * Automatically allocated device ID. We mark it as such so
+		 * that we remember it must be freed, and we append a suffix
+		 * to avoid namespace collision with explicit IDs.
+		 */
+		ret = ida_simple_get(&platform_devid_ida, 0, 0, GFP_KERNEL);
+		if (ret < 0)
+			return ret;
+		pdev->id = ret;
+		pdev->id_auto = true;
+		return dev_set_name(&pdev->dev, "%s.%d.auto", pdev->name,
+				    pdev->id);
+	}
+
+	return 0;
+}
+
 static void platform_device_release(struct device *dev)
 {
 	struct platform_object *pa = container_of(dev, struct platform_object,
 						  pdev.dev);
 
+	if (pa->pdev.id_auto)
+		ida_simple_remove(&platform_devid_ida, pa->pdev.id);
+
 	of_device_node_put(&pa->pdev.dev);
 	kfree(pa->pdev.dev.platform_data);
 	kfree(pa->pdev.mfd_cell);
@@ -227,6 +257,10 @@ struct platform_device *platform_device_alloc(const char *name, int id)
 		device_initialize(&pa->pdev.dev);
 		pa->pdev.dev.release = platform_device_release;
 		arch_setup_pdev_archdata(&pa->pdev);
+		if (pdev_set_name(&pa->pdev)) {
+			kfree(pa);
+			return NULL;
+		}
 	}
 
 	return pa ? &pa->pdev : NULL;
@@ -307,28 +341,6 @@ int platform_device_add(struct platform_device *pdev)
 
 	pdev->dev.bus = &platform_bus_type;
 
-	switch (pdev->id) {
-	default:
-		dev_set_name(&pdev->dev, "%s.%d", pdev->name,  pdev->id);
-		break;
-	case PLATFORM_DEVID_NONE:
-		dev_set_name(&pdev->dev, "%s", pdev->name);
-		break;
-	case PLATFORM_DEVID_AUTO:
-		/*
-		 * Automatically allocated device ID. We mark it as such so
-		 * that we remember it must be freed, and we append a suffix
-		 * to avoid namespace collision with explicit IDs.
-		 */
-		ret = ida_simple_get(&platform_devid_ida, 0, 0, GFP_KERNEL);
-		if (ret < 0)
-			goto err_out;
-		pdev->id = ret;
-		pdev->id_auto = true;
-		dev_set_name(&pdev->dev, "%s.%d.auto", pdev->name, pdev->id);
-		break;
-	}
-
 	for (i = 0; i < pdev->num_resources; i++) {
 		struct resource *p, *r = &pdev->resource[i];
 
@@ -371,7 +383,6 @@ int platform_device_add(struct platform_device *pdev)
 			release_resource(r);
 	}
 
- err_out:
 	return ret;
 }
 EXPORT_SYMBOL_GPL(platform_device_add);
@@ -391,11 +402,6 @@ void platform_device_del(struct platform_device *pdev)
 	if (pdev) {
 		device_del(&pdev->dev);
 
-		if (pdev->id_auto) {
-			ida_simple_remove(&platform_devid_ida, pdev->id);
-			pdev->id = PLATFORM_DEVID_AUTO;
-		}
-
 		for (i = 0; i < pdev->num_resources; i++) {
 			struct resource *r = &pdev->resource[i];
 			unsigned long type = resource_type(r);
@@ -413,8 +419,15 @@ EXPORT_SYMBOL_GPL(platform_device_del);
  */
 int platform_device_register(struct platform_device *pdev)
 {
+	int ret;
+
 	device_initialize(&pdev->dev);
 	arch_setup_pdev_archdata(pdev);
+
+	ret = pdev_set_name(pdev);
+	if (ret)
+		return ret;
+
 	return platform_device_add(pdev);
 }
 EXPORT_SYMBOL_GPL(platform_device_register);
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 6/6] usb: dwc3: host: convey the PHYs to xhci
  2014-08-21 11:33 [PATCHv3 0/6] phy: simplified phy lookup Heikki Krogerus
                   ` (4 preceding siblings ...)
  2014-08-21 11:33 ` [PATCH 5/6] base: platform: name the device already during allocation Heikki Krogerus
@ 2014-08-21 11:33 ` Heikki Krogerus
  2014-09-11 15:01   ` Kishon Vijay Abraham I
  5 siblings, 1 reply; 31+ messages in thread
From: Heikki Krogerus @ 2014-08-21 11:33 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

On some platforms a PHY may need to be handled also in the
host controller driver. Exynos5420 SoC requires some "PHY
tuning" based on the USB speed. This patch delivers dwc3's
PHYs to the xhci platform device when it's created.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Tested-by: Vivek Gautam <gautam.vivek@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/dwc3/host.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index dcb8ca0..12bfd3c 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -29,8 +29,7 @@ int dwc3_host_init(struct dwc3 *dwc)
 	xhci = platform_device_alloc("xhci-hcd", PLATFORM_DEVID_AUTO);
 	if (!xhci) {
 		dev_err(dwc->dev, "couldn't allocate xHCI device\n");
-		ret = -ENOMEM;
-		goto err0;
+		return -ENOMEM;
 	}
 
 	dma_set_coherent_mask(&xhci->dev, dwc->dev->coherent_dma_mask);
@@ -60,22 +59,33 @@ int dwc3_host_init(struct dwc3 *dwc)
 		goto err1;
 	}
 
+	phy_create_lookup(dwc->usb2_generic_phy, "usb2-phy",
+			  dev_name(&xhci->dev));
+	phy_create_lookup(dwc->usb3_generic_phy, "usb3-phy",
+			  dev_name(&xhci->dev));
+
 	ret = platform_device_add(xhci);
 	if (ret) {
 		dev_err(dwc->dev, "failed to register xHCI device\n");
-		goto err1;
+		goto err2;
 	}
 
 	return 0;
-
+err2:
+	phy_remove_lookup(dwc->usb2_generic_phy, "usb2-phy",
+			  dev_name(&xhci->dev));
+	phy_remove_lookup(dwc->usb3_generic_phy, "usb3-phy",
+			  dev_name(&xhci->dev));
 err1:
 	platform_device_put(xhci);
-
-err0:
 	return ret;
 }
 
 void dwc3_host_exit(struct dwc3 *dwc)
 {
+	phy_remove_lookup(dwc->usb2_generic_phy, "usb2-phy",
+			  dev_name(&dwc->xhci->dev));
+	phy_remove_lookup(dwc->usb3_generic_phy, "usb3-phy",
+			  dev_name(&dwc->xhci->dev));
 	platform_device_unregister(dwc->xhci);
 }
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 4/6] phy: remove the old lookup method
  2014-08-21 11:33 ` [PATCH 4/6] phy: remove the old lookup method Heikki Krogerus
@ 2014-08-25  7:41   ` Vivek Gautam
  2014-08-25  8:17     ` Heikki Krogerus
  0 siblings, 1 reply; 31+ messages in thread
From: Vivek Gautam @ 2014-08-25  7:41 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Kishon Vijay Abraham I, Greg Kroah-Hartman, Felipe Balbi,
	linux-kernel, Linux USB Mailing List

[-- Attachment #1: Type: text/plain, Size: 17135 bytes --]

Hi Heikki,


On Thu, Aug 21, 2014 at 5:03 PM, Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
> The users of the old method are now converted to the new one.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Tested-by: Vivek Gautam <gautam.vivek@samsung.com>
> ---
>  drivers/phy/phy-bcm-kona-usb2.c     |  2 +-
>  drivers/phy/phy-core.c              | 45 +++----------------------------------
>  drivers/phy/phy-exynos-dp-video.c   |  2 +-
>  drivers/phy/phy-exynos-mipi-video.c |  2 +-
>  drivers/phy/phy-exynos5-usbdrd.c    |  3 +--
>  drivers/phy/phy-exynos5250-sata.c   |  2 +-
>  drivers/phy/phy-mvebu-sata.c        |  2 +-
>  drivers/phy/phy-omap-usb2.c         |  2 +-
>  drivers/phy/phy-samsung-usb2.c      |  3 +--
>  drivers/phy/phy-sun4i-usb.c         |  2 +-
>  drivers/phy/phy-ti-pipe3.c          |  2 +-
>  drivers/phy/phy-twl4030-usb.c       |  4 +---
>  drivers/phy/phy-xgene.c             |  2 +-
>  include/linux/phy/phy.h             | 38 ++++---------------------------
>  14 files changed, 19 insertions(+), 92 deletions(-)

Please squash the attached diff which removes the 'init_data' field
from some of the other instances
of devm_phy_create() in few other drivers.
This should prevent any build errors that i could see with multi_v7_defconfig.

>
> diff --git a/drivers/phy/phy-bcm-kona-usb2.c b/drivers/phy/phy-bcm-kona-usb2.c
> index 894fe74..3463983 100644
> --- a/drivers/phy/phy-bcm-kona-usb2.c
> +++ b/drivers/phy/phy-bcm-kona-usb2.c
> @@ -117,7 +117,7 @@ static int bcm_kona_usb2_probe(struct platform_device *pdev)
>
>         platform_set_drvdata(pdev, phy);
>
> -       gphy = devm_phy_create(dev, NULL, &ops, NULL);
> +       gphy = devm_phy_create(dev, NULL, &ops);
>         if (IS_ERR(gphy))
>                 return PTR_ERR(gphy);
>
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 67a8c726..834b337 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -55,36 +55,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 *port)
> -{
> -       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);
> -
> -               if (!phy->init_data)
> -                       continue;
> -               count = phy->init_data->num_consumers;
> -               consumers = phy->init_data->consumers;
> -               while (count--) {
> -                       if (!strcmp(consumers->dev_name, dev_name(device)) &&
> -                                       !strcmp(consumers->port, port)) {
> -                               class_dev_iter_exit(&iter);
> -                               return phy;
> -                       }
> -                       consumers++;
> -               }
> -       }
> -
> -       class_dev_iter_exit(&iter);
> -       return ERR_PTR(-ENODEV);
> -}
> -
>  /**
>   * phy_register_lookup() - register PHY/device association
>   * @pl: association to register
> @@ -210,10 +180,6 @@ static struct phy *phy_find(struct device *dev, const char *con_id)
>                 }
>                 class_dev_iter_exit(&iter);
>         }
> -
> -       /* fall-back to the old lookup method for now */
> -       if (IS_ERR(phy))
> -               phy = phy_lookup(dev, con_id);
>         return phy;
>  }
>
> @@ -721,13 +687,11 @@ EXPORT_SYMBOL_GPL(devm_of_phy_get);
>   * @dev: device that is creating the new phy
>   * @node: device node of the 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, struct device_node *node,
> -                      const struct phy_ops *ops,
> -                      struct phy_init_data *init_data)
> +                      const struct phy_ops *ops)
>  {
>         int ret;
>         int id;
> @@ -765,7 +729,6 @@ struct phy *phy_create(struct device *dev, struct device_node *node,
>         phy->dev.of_node = 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)
> @@ -800,7 +763,6 @@ EXPORT_SYMBOL_GPL(phy_create);
>   * @dev: device that is creating the new phy
>   * @node: device node of the 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.
> @@ -808,8 +770,7 @@ EXPORT_SYMBOL_GPL(phy_create);
>   * then, devres data is freed.
>   */
>  struct phy *devm_phy_create(struct device *dev, struct device_node *node,
> -                           const struct phy_ops *ops,
> -                           struct phy_init_data *init_data)
> +                           const struct phy_ops *ops)
>  {
>         struct phy **ptr, *phy;
>
> @@ -817,7 +778,7 @@ struct phy *devm_phy_create(struct device *dev, struct device_node *node,
>         if (!ptr)
>                 return ERR_PTR(-ENOMEM);
>
> -       phy = phy_create(dev, node, ops, init_data);
> +       phy = phy_create(dev, node, 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 8b3026e..c667d2b 100644
> --- a/drivers/phy/phy-exynos-dp-video.c
> +++ b/drivers/phy/phy-exynos-dp-video.c
> @@ -77,7 +77,7 @@ static int exynos_dp_video_phy_probe(struct platform_device *pdev)
>         if (IS_ERR(state->regs))
>                 return PTR_ERR(state->regs);
>
> -       phy = devm_phy_create(dev, NULL, &exynos_dp_video_phy_ops, NULL);
> +       phy = devm_phy_create(dev, NULL, &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 b55a92e..2807f95 100644
> --- a/drivers/phy/phy-exynos-mipi-video.c
> +++ b/drivers/phy/phy-exynos-mipi-video.c
> @@ -137,7 +137,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, NULL,
> -                                       &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-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c
> index b05302b..47f47fe 100644
> --- a/drivers/phy/phy-exynos5-usbdrd.c
> +++ b/drivers/phy/phy-exynos5-usbdrd.c
> @@ -636,8 +636,7 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
>
>         for (i = 0; i < EXYNOS5_DRDPHYS_NUM; i++) {
>                 struct phy *phy = devm_phy_create(dev, NULL,
> -                                                 &exynos5_usbdrd_phy_ops,
> -                                                 NULL);
> +                                                 &exynos5_usbdrd_phy_ops);
>                 if (IS_ERR(phy)) {
>                         dev_err(dev, "Failed to create usbdrd_phy phy\n");
>                         return PTR_ERR(phy);
> diff --git a/drivers/phy/phy-exynos5250-sata.c b/drivers/phy/phy-exynos5250-sata.c
> index 19a679a..3b0b4af7 100644
> --- a/drivers/phy/phy-exynos5250-sata.c
> +++ b/drivers/phy/phy-exynos5250-sata.c
> @@ -210,7 +210,7 @@ static int exynos_sata_phy_probe(struct platform_device *pdev)
>                 return ret;
>         }
>
> -       sata_phy->phy = devm_phy_create(dev, NULL, &exynos_sata_phy_ops, NULL);
> +       sata_phy->phy = devm_phy_create(dev, NULL, &exynos_sata_phy_ops);
>         if (IS_ERR(sata_phy->phy)) {
>                 clk_disable_unprepare(sata_phy->phyclk);
>                 dev_err(dev, "failed to create PHY\n");
> diff --git a/drivers/phy/phy-mvebu-sata.c b/drivers/phy/phy-mvebu-sata.c
> index cc3c0e1..0387e7b 100644
> --- a/drivers/phy/phy-mvebu-sata.c
> +++ b/drivers/phy/phy-mvebu-sata.c
> @@ -99,7 +99,7 @@ static int phy_mvebu_sata_probe(struct platform_device *pdev)
>         if (IS_ERR(priv->clk))
>                 return PTR_ERR(priv->clk);
>
> -       phy = devm_phy_create(&pdev->dev, NULL, &phy_mvebu_sata_ops, NULL);
> +       phy = devm_phy_create(&pdev->dev, NULL, &phy_mvebu_sata_ops);
>         if (IS_ERR(phy))
>                 return PTR_ERR(phy);
>
> diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
> index 93d7835..a4a93e3 100644
> --- a/drivers/phy/phy-omap-usb2.c
> +++ b/drivers/phy/phy-omap-usb2.c
> @@ -263,7 +263,7 @@ static int omap_usb2_probe(struct platform_device *pdev)
>
>         platform_set_drvdata(pdev, phy);
>
> -       generic_phy = devm_phy_create(phy->dev, NULL, &ops, NULL);
> +       generic_phy = devm_phy_create(phy->dev, NULL, &ops);
>         if (IS_ERR(generic_phy))
>                 return PTR_ERR(generic_phy);
>
> diff --git a/drivers/phy/phy-samsung-usb2.c b/drivers/phy/phy-samsung-usb2.c
> index 3732ca2..e2a364d 100644
> --- a/drivers/phy/phy-samsung-usb2.c
> +++ b/drivers/phy/phy-samsung-usb2.c
> @@ -202,8 +202,7 @@ static int samsung_usb2_phy_probe(struct platform_device *pdev)
>                 struct samsung_usb2_phy_instance *p = &drv->instances[i];
>
>                 dev_dbg(dev, "Creating phy \"%s\"\n", label);
> -               p->phy = devm_phy_create(dev, NULL, &samsung_usb2_phy_ops,
> -                                        NULL);
> +               p->phy = devm_phy_create(dev, NULL, &samsung_usb2_phy_ops);
>                 if (IS_ERR(p->phy)) {
>                         dev_err(drv->dev, "Failed to create usb2_phy \"%s\"\n",
>                                 label);
> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
> index 61ebea4..56e7f49 100644
> --- a/drivers/phy/phy-sun4i-usb.c
> +++ b/drivers/phy/phy-sun4i-usb.c
> @@ -295,7 +295,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
>                                 return PTR_ERR(phy->pmu);
>                 }
>
> -               phy->phy = devm_phy_create(dev, NULL, &sun4i_usb_phy_ops, NULL);
> +               phy->phy = devm_phy_create(dev, NULL, &sun4i_usb_phy_ops);
>                 if (IS_ERR(phy->phy)) {
>                         dev_err(dev, "failed to create PHY %d\n", i);
>                         return PTR_ERR(phy->phy);
> diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c
> index b964aa9..9f3a1d20 100644
> --- a/drivers/phy/phy-ti-pipe3.c
> +++ b/drivers/phy/phy-ti-pipe3.c
> @@ -400,7 +400,7 @@ static int ti_pipe3_probe(struct platform_device *pdev)
>         platform_set_drvdata(pdev, phy);
>         pm_runtime_enable(phy->dev);
>
> -       generic_phy = devm_phy_create(phy->dev, NULL, &ops, NULL);
> +       generic_phy = devm_phy_create(phy->dev, NULL, &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 e1a6623..851f126 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;
> @@ -695,7 +693,7 @@ static int twl4030_usb_probe(struct platform_device *pdev)
>         otg->set_host           = twl4030_set_host;
>         otg->set_peripheral     = twl4030_set_peripheral;
>
> -       phy = devm_phy_create(twl->dev, NULL, &ops, init_data);
> +       phy = devm_phy_create(twl->dev, NULL, &ops);
>         if (IS_ERR(phy)) {
>                 dev_dbg(&pdev->dev, "Failed to create PHY\n");
>                 return PTR_ERR(phy);
> diff --git a/drivers/phy/phy-xgene.c b/drivers/phy/phy-xgene.c
> index db809b9..a239310 100644
> --- a/drivers/phy/phy-xgene.c
> +++ b/drivers/phy/phy-xgene.c
> @@ -1707,7 +1707,7 @@ static int xgene_phy_probe(struct platform_device *pdev)
>         ctx->dev = &pdev->dev;
>         platform_set_drvdata(pdev, ctx);
>
> -       ctx->phy = devm_phy_create(ctx->dev, NULL, &xgene_phy_ops, NULL);
> +       ctx->phy = devm_phy_create(ctx->dev, NULL, &xgene_phy_ops);
>         if (IS_ERR(ctx->phy)) {
>                 dev_dbg(&pdev->dev, "Failed to create PHY\n");
>                 rc = PTR_ERR(ctx->phy);
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 2696b95..d983051 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -61,7 +61,6 @@ struct phy {
>         struct device           dev;
>         int                     id;
>         const struct phy_ops    *ops;
> -       struct phy_init_data    *init_data;
>         struct mutex            mutex;
>         int                     init_count;
>         int                     power_count;
> @@ -84,32 +83,6 @@ struct phy_provider {
>                 struct of_phandle_args *args);
>  };
>
> -/**
> - * 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 {
>         struct list_head node;
>         const char *phy_name;
> @@ -173,10 +146,9 @@ struct phy *of_phy_get(struct device_node *np, const char *con_id);
>  struct phy *of_phy_simple_xlate(struct device *dev,
>         struct of_phandle_args *args);
>  struct phy *phy_create(struct device *dev, struct device_node *node,
> -                      const struct phy_ops *ops,
> -                      struct phy_init_data *init_data);
> +                      const struct phy_ops *ops);
>  struct phy *devm_phy_create(struct device *dev, struct device_node *node,
> -       const struct phy_ops *ops, struct phy_init_data *init_data);
> +                           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,
> @@ -319,16 +291,14 @@ static inline struct phy *of_phy_simple_xlate(struct device *dev,
>
>  static inline struct phy *phy_create(struct device *dev,
>                                      struct device_node *node,
> -                                    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,
>                                           struct device_node *node,
> -                                         const struct phy_ops *ops,
> -                                         struct phy_init_data *init_data)
> +                                         const struct phy_ops *ops)
>  {
>         return ERR_PTR(-ENOSYS);
>  }
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

[-- Attachment #2: phy-fix --]
[-- Type: application/octet-stream, Size: 3931 bytes --]

diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
index 5c3a042..010c463 100644
--- a/drivers/phy/phy-berlin-sata.c
+++ b/drivers/phy/phy-berlin-sata.c
@@ -239,7 +239,7 @@ static int phy_berlin_sata_probe(struct platform_device *pdev)
 		if (!phy_desc)
 			return -ENOMEM;
 
-		phy = devm_phy_create(dev, NULL, &phy_berlin_sata_ops, NULL);
+		phy = devm_phy_create(dev, NULL, &phy_berlin_sata_ops);
 		if (IS_ERR(phy)) {
 			dev_err(dev, "failed to create PHY %d\n", phy_id);
 			return PTR_ERR(phy);
diff --git a/drivers/phy/phy-hix5hd2-sata.c b/drivers/phy/phy-hix5hd2-sata.c
index 6a08fa5..152eaa7 100644
--- a/drivers/phy/phy-hix5hd2-sata.c
+++ b/drivers/phy/phy-hix5hd2-sata.c
@@ -156,7 +156,7 @@ static int hix5hd2_sata_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->peri_ctrl))
 		priv->peri_ctrl = NULL;
 
-	phy = devm_phy_create(dev, NULL, &hix5hd2_sata_phy_ops, NULL);
+	phy = devm_phy_create(dev, NULL, &hix5hd2_sata_phy_ops);
 	if (IS_ERR(phy)) {
 		dev_err(dev, "failed to create PHY\n");
 		return PTR_ERR(phy);
diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
index e111baf..0849844 100644
--- a/drivers/phy/phy-miphy365x.c
+++ b/drivers/phy/phy-miphy365x.c
@@ -592,7 +592,7 @@ static int miphy365x_probe(struct platform_device *pdev)
 
 		miphy_dev->phys[port] = miphy_phy;
 
-		phy = devm_phy_create(&pdev->dev, child, &miphy365x_ops, NULL);
+		phy = devm_phy_create(&pdev->dev, child, &miphy365x_ops);
 		if (IS_ERR(phy)) {
 			dev_err(&pdev->dev, "failed to create PHY\n");
 			return PTR_ERR(phy);
diff --git a/drivers/phy/phy-qcom-apq8064-sata.c b/drivers/phy/phy-qcom-apq8064-sata.c
index b3ef7d8..21fb392 100644
--- a/drivers/phy/phy-qcom-apq8064-sata.c
+++ b/drivers/phy/phy-qcom-apq8064-sata.c
@@ -228,8 +228,7 @@ static int qcom_apq8064_sata_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(phy->mmio))
 		return PTR_ERR(phy->mmio);
 
-	generic_phy = devm_phy_create(dev, NULL, &qcom_apq8064_sata_phy_ops,
-				      NULL);
+	generic_phy = devm_phy_create(dev, NULL, &qcom_apq8064_sata_phy_ops);
 	if (IS_ERR(generic_phy)) {
 		dev_err(dev, "%s: failed to create phy\n", __func__);
 		return PTR_ERR(generic_phy);
diff --git a/drivers/phy/phy-qcom-ipq806x-sata.c b/drivers/phy/phy-qcom-ipq806x-sata.c
index 909b5a8..b3be999 100644
--- a/drivers/phy/phy-qcom-ipq806x-sata.c
+++ b/drivers/phy/phy-qcom-ipq806x-sata.c
@@ -150,8 +150,7 @@ static int qcom_ipq806x_sata_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(phy->mmio))
 		return PTR_ERR(phy->mmio);
 
-	generic_phy = devm_phy_create(dev, NULL, &qcom_ipq806x_sata_phy_ops,
-				      NULL);
+	generic_phy = devm_phy_create(dev, NULL, &qcom_ipq806x_sata_phy_ops);
 	if (IS_ERR(generic_phy)) {
 		dev_err(dev, "%s: failed to create phy\n", __func__);
 		return PTR_ERR(generic_phy);
diff --git a/drivers/phy/phy-spear1310-miphy.c b/drivers/phy/phy-spear1310-miphy.c
index 6dcbfcd..51f372b 100644
--- a/drivers/phy/phy-spear1310-miphy.c
+++ b/drivers/phy/phy-spear1310-miphy.c
@@ -229,7 +229,7 @@ static int spear1310_miphy_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	priv->phy = devm_phy_create(dev, NULL, &spear1310_miphy_ops, NULL);
+	priv->phy = devm_phy_create(dev, NULL, &spear1310_miphy_ops);
 	if (IS_ERR(priv->phy)) {
 		dev_err(dev, "failed to create SATA PCIe PHY\n");
 		return PTR_ERR(priv->phy);
diff --git a/drivers/phy/phy-spear1340-miphy.c b/drivers/phy/phy-spear1340-miphy.c
index 7135ba2..c618150 100644
--- a/drivers/phy/phy-spear1340-miphy.c
+++ b/drivers/phy/phy-spear1340-miphy.c
@@ -261,7 +261,7 @@ static int spear1340_miphy_probe(struct platform_device *pdev)
 		return PTR_ERR(priv->misc);
 	}
 
-	priv->phy = devm_phy_create(dev, NULL, &spear1340_miphy_ops, NULL);
+	priv->phy = devm_phy_create(dev, NULL, &spear1340_miphy_ops);
 	if (IS_ERR(priv->phy)) {
 		dev_err(dev, "failed to create SATA PCIe PHY\n");
 		return PTR_ERR(priv->phy);

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 4/6] phy: remove the old lookup method
  2014-08-25  7:41   ` Vivek Gautam
@ 2014-08-25  8:17     ` Heikki Krogerus
  2014-08-25  8:25       ` Vivek Gautam
  0 siblings, 1 reply; 31+ messages in thread
From: Heikki Krogerus @ 2014-08-25  8:17 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: Kishon Vijay Abraham I, Greg Kroah-Hartman, Felipe Balbi,
	linux-kernel, Linux USB Mailing List

On Mon, Aug 25, 2014 at 01:11:34PM +0530, Vivek Gautam wrote:
> Please squash the attached diff which removes the 'init_data' field
> from some of the other instances
> of devm_phy_create() in few other drivers.
> This should prevent any build errors that i could see with multi_v7_defconfig.

OK, I'll add those changes into this patch, and it seems there
is now at least one more new driver on top of those calling
devm_phy_create.


Thanks!

-- 
heikki

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 4/6] phy: remove the old lookup method
  2014-08-25  8:17     ` Heikki Krogerus
@ 2014-08-25  8:25       ` Vivek Gautam
  2014-08-26  8:27         ` [PATCHv4 " Heikki Krogerus
  0 siblings, 1 reply; 31+ messages in thread
From: Vivek Gautam @ 2014-08-25  8:25 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Kishon Vijay Abraham I, Greg Kroah-Hartman, Felipe Balbi,
	linux-kernel, Linux USB Mailing List

On Mon, Aug 25, 2014 at 1:47 PM, Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
> On Mon, Aug 25, 2014 at 01:11:34PM +0530, Vivek Gautam wrote:
>> Please squash the attached diff which removes the 'init_data' field
>> from some of the other instances
>> of devm_phy_create() in few other drivers.
>> This should prevent any build errors that i could see with multi_v7_defconfig.
>
> OK, I'll add those changes into this patch, and it seems there
> is now at least one more new driver on top of those calling
> devm_phy_create.

Ok, i think i just saw this on 3.17-rc1.




-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCHv4 4/6] phy: remove the old lookup method
  2014-08-25  8:25       ` Vivek Gautam
@ 2014-08-26  8:27         ` Heikki Krogerus
  0 siblings, 0 replies; 31+ messages in thread
From: Heikki Krogerus @ 2014-08-26  8:27 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

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

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Tested-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 drivers/phy/phy-bcm-kona-usb2.c      |  2 +-
 drivers/phy/phy-berlin-sata.c        |  2 +-
 drivers/phy/phy-core.c               | 45 +++---------------------------------
 drivers/phy/phy-exynos-dp-video.c    |  2 +-
 drivers/phy/phy-exynos-mipi-video.c  |  2 +-
 drivers/phy/phy-exynos5-usbdrd.c     |  3 +--
 drivers/phy/phy-exynos5250-sata.c    |  2 +-
 drivers/phy/phy-hix5hd2-sata.c       |  2 +-
 drivers/phy/phy-miphy365x.c          |  2 +-
 drivers/phy/phy-mvebu-sata.c         |  2 +-
 drivers/phy/phy-omap-usb2.c          |  2 +-
 drivers/phy/phy-qcom-apq8064-sata.c  |  3 +--
 drivers/phy/phy-qcom-ipq806x-sata.c  |  3 +--
 drivers/phy/phy-samsung-usb2.c       |  3 +--
 drivers/phy/phy-spear1310-miphy.c    |  2 +-
 drivers/phy/phy-spear1340-miphy.c    |  2 +-
 drivers/phy/phy-sun4i-usb.c          |  2 +-
 drivers/phy/phy-ti-pipe3.c           |  2 +-
 drivers/phy/phy-twl4030-usb.c        |  4 +---
 drivers/phy/phy-xgene.c              |  2 +-
 drivers/pinctrl/pinctrl-tegra-xusb.c |  4 ++--
 include/linux/phy/phy.h              | 38 ++++--------------------------
 22 files changed, 28 insertions(+), 103 deletions(-)

diff --git a/drivers/phy/phy-bcm-kona-usb2.c b/drivers/phy/phy-bcm-kona-usb2.c
index 894fe74..3463983 100644
--- a/drivers/phy/phy-bcm-kona-usb2.c
+++ b/drivers/phy/phy-bcm-kona-usb2.c
@@ -117,7 +117,7 @@ static int bcm_kona_usb2_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, phy);
 
-	gphy = devm_phy_create(dev, NULL, &ops, NULL);
+	gphy = devm_phy_create(dev, NULL, &ops);
 	if (IS_ERR(gphy))
 		return PTR_ERR(gphy);
 
diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
index 5c3a042..010c463 100644
--- a/drivers/phy/phy-berlin-sata.c
+++ b/drivers/phy/phy-berlin-sata.c
@@ -239,7 +239,7 @@ static int phy_berlin_sata_probe(struct platform_device *pdev)
 		if (!phy_desc)
 			return -ENOMEM;
 
-		phy = devm_phy_create(dev, NULL, &phy_berlin_sata_ops, NULL);
+		phy = devm_phy_create(dev, NULL, &phy_berlin_sata_ops);
 		if (IS_ERR(phy)) {
 			dev_err(dev, "failed to create PHY %d\n", phy_id);
 			return PTR_ERR(phy);
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 67a8c726..834b337 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -55,36 +55,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 *port)
-{
-	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);
-
-		if (!phy->init_data)
-			continue;
-		count = phy->init_data->num_consumers;
-		consumers = phy->init_data->consumers;
-		while (count--) {
-			if (!strcmp(consumers->dev_name, dev_name(device)) &&
-					!strcmp(consumers->port, port)) {
-				class_dev_iter_exit(&iter);
-				return phy;
-			}
-			consumers++;
-		}
-	}
-
-	class_dev_iter_exit(&iter);
-	return ERR_PTR(-ENODEV);
-}
-
 /**
  * phy_register_lookup() - register PHY/device association
  * @pl: association to register
@@ -210,10 +180,6 @@ static struct phy *phy_find(struct device *dev, const char *con_id)
 		}
 		class_dev_iter_exit(&iter);
 	}
-
-	/* fall-back to the old lookup method for now */
-	if (IS_ERR(phy))
-		phy = phy_lookup(dev, con_id);
 	return phy;
 }
 
@@ -721,13 +687,11 @@ EXPORT_SYMBOL_GPL(devm_of_phy_get);
  * @dev: device that is creating the new phy
  * @node: device node of the 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, struct device_node *node,
-		       const struct phy_ops *ops,
-		       struct phy_init_data *init_data)
+		       const struct phy_ops *ops)
 {
 	int ret;
 	int id;
@@ -765,7 +729,6 @@ struct phy *phy_create(struct device *dev, struct device_node *node,
 	phy->dev.of_node = 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)
@@ -800,7 +763,6 @@ EXPORT_SYMBOL_GPL(phy_create);
  * @dev: device that is creating the new phy
  * @node: device node of the 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.
@@ -808,8 +770,7 @@ EXPORT_SYMBOL_GPL(phy_create);
  * then, devres data is freed.
  */
 struct phy *devm_phy_create(struct device *dev, struct device_node *node,
-			    const struct phy_ops *ops,
-			    struct phy_init_data *init_data)
+			    const struct phy_ops *ops)
 {
 	struct phy **ptr, *phy;
 
@@ -817,7 +778,7 @@ struct phy *devm_phy_create(struct device *dev, struct device_node *node,
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	phy = phy_create(dev, node, ops, init_data);
+	phy = phy_create(dev, node, 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 8b3026e..c667d2b 100644
--- a/drivers/phy/phy-exynos-dp-video.c
+++ b/drivers/phy/phy-exynos-dp-video.c
@@ -77,7 +77,7 @@ static int exynos_dp_video_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(state->regs))
 		return PTR_ERR(state->regs);
 
-	phy = devm_phy_create(dev, NULL, &exynos_dp_video_phy_ops, NULL);
+	phy = devm_phy_create(dev, NULL, &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 b55a92e..2807f95 100644
--- a/drivers/phy/phy-exynos-mipi-video.c
+++ b/drivers/phy/phy-exynos-mipi-video.c
@@ -137,7 +137,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, NULL,
-					&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-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c
index b05302b..47f47fe 100644
--- a/drivers/phy/phy-exynos5-usbdrd.c
+++ b/drivers/phy/phy-exynos5-usbdrd.c
@@ -636,8 +636,7 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
 
 	for (i = 0; i < EXYNOS5_DRDPHYS_NUM; i++) {
 		struct phy *phy = devm_phy_create(dev, NULL,
-						  &exynos5_usbdrd_phy_ops,
-						  NULL);
+						  &exynos5_usbdrd_phy_ops);
 		if (IS_ERR(phy)) {
 			dev_err(dev, "Failed to create usbdrd_phy phy\n");
 			return PTR_ERR(phy);
diff --git a/drivers/phy/phy-exynos5250-sata.c b/drivers/phy/phy-exynos5250-sata.c
index 19a679a..3b0b4af7 100644
--- a/drivers/phy/phy-exynos5250-sata.c
+++ b/drivers/phy/phy-exynos5250-sata.c
@@ -210,7 +210,7 @@ static int exynos_sata_phy_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	sata_phy->phy = devm_phy_create(dev, NULL, &exynos_sata_phy_ops, NULL);
+	sata_phy->phy = devm_phy_create(dev, NULL, &exynos_sata_phy_ops);
 	if (IS_ERR(sata_phy->phy)) {
 		clk_disable_unprepare(sata_phy->phyclk);
 		dev_err(dev, "failed to create PHY\n");
diff --git a/drivers/phy/phy-hix5hd2-sata.c b/drivers/phy/phy-hix5hd2-sata.c
index 6a08fa5..152eaa7 100644
--- a/drivers/phy/phy-hix5hd2-sata.c
+++ b/drivers/phy/phy-hix5hd2-sata.c
@@ -156,7 +156,7 @@ static int hix5hd2_sata_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->peri_ctrl))
 		priv->peri_ctrl = NULL;
 
-	phy = devm_phy_create(dev, NULL, &hix5hd2_sata_phy_ops, NULL);
+	phy = devm_phy_create(dev, NULL, &hix5hd2_sata_phy_ops);
 	if (IS_ERR(phy)) {
 		dev_err(dev, "failed to create PHY\n");
 		return PTR_ERR(phy);
diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
index e111baf..0849844 100644
--- a/drivers/phy/phy-miphy365x.c
+++ b/drivers/phy/phy-miphy365x.c
@@ -592,7 +592,7 @@ static int miphy365x_probe(struct platform_device *pdev)
 
 		miphy_dev->phys[port] = miphy_phy;
 
-		phy = devm_phy_create(&pdev->dev, child, &miphy365x_ops, NULL);
+		phy = devm_phy_create(&pdev->dev, child, &miphy365x_ops);
 		if (IS_ERR(phy)) {
 			dev_err(&pdev->dev, "failed to create PHY\n");
 			return PTR_ERR(phy);
diff --git a/drivers/phy/phy-mvebu-sata.c b/drivers/phy/phy-mvebu-sata.c
index cc3c0e1..0387e7b 100644
--- a/drivers/phy/phy-mvebu-sata.c
+++ b/drivers/phy/phy-mvebu-sata.c
@@ -99,7 +99,7 @@ static int phy_mvebu_sata_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->clk))
 		return PTR_ERR(priv->clk);
 
-	phy = devm_phy_create(&pdev->dev, NULL, &phy_mvebu_sata_ops, NULL);
+	phy = devm_phy_create(&pdev->dev, NULL, &phy_mvebu_sata_ops);
 	if (IS_ERR(phy))
 		return PTR_ERR(phy);
 
diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
index 93d7835..a4a93e3 100644
--- a/drivers/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -263,7 +263,7 @@ static int omap_usb2_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, phy);
 
-	generic_phy = devm_phy_create(phy->dev, NULL, &ops, NULL);
+	generic_phy = devm_phy_create(phy->dev, NULL, &ops);
 	if (IS_ERR(generic_phy))
 		return PTR_ERR(generic_phy);
 
diff --git a/drivers/phy/phy-qcom-apq8064-sata.c b/drivers/phy/phy-qcom-apq8064-sata.c
index b3ef7d8..21fb392 100644
--- a/drivers/phy/phy-qcom-apq8064-sata.c
+++ b/drivers/phy/phy-qcom-apq8064-sata.c
@@ -228,8 +228,7 @@ static int qcom_apq8064_sata_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(phy->mmio))
 		return PTR_ERR(phy->mmio);
 
-	generic_phy = devm_phy_create(dev, NULL, &qcom_apq8064_sata_phy_ops,
-				      NULL);
+	generic_phy = devm_phy_create(dev, NULL, &qcom_apq8064_sata_phy_ops);
 	if (IS_ERR(generic_phy)) {
 		dev_err(dev, "%s: failed to create phy\n", __func__);
 		return PTR_ERR(generic_phy);
diff --git a/drivers/phy/phy-qcom-ipq806x-sata.c b/drivers/phy/phy-qcom-ipq806x-sata.c
index 909b5a8..b3be999 100644
--- a/drivers/phy/phy-qcom-ipq806x-sata.c
+++ b/drivers/phy/phy-qcom-ipq806x-sata.c
@@ -150,8 +150,7 @@ static int qcom_ipq806x_sata_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(phy->mmio))
 		return PTR_ERR(phy->mmio);
 
-	generic_phy = devm_phy_create(dev, NULL, &qcom_ipq806x_sata_phy_ops,
-				      NULL);
+	generic_phy = devm_phy_create(dev, NULL, &qcom_ipq806x_sata_phy_ops);
 	if (IS_ERR(generic_phy)) {
 		dev_err(dev, "%s: failed to create phy\n", __func__);
 		return PTR_ERR(generic_phy);
diff --git a/drivers/phy/phy-samsung-usb2.c b/drivers/phy/phy-samsung-usb2.c
index 3732ca2..e2a364d 100644
--- a/drivers/phy/phy-samsung-usb2.c
+++ b/drivers/phy/phy-samsung-usb2.c
@@ -202,8 +202,7 @@ static int samsung_usb2_phy_probe(struct platform_device *pdev)
 		struct samsung_usb2_phy_instance *p = &drv->instances[i];
 
 		dev_dbg(dev, "Creating phy \"%s\"\n", label);
-		p->phy = devm_phy_create(dev, NULL, &samsung_usb2_phy_ops,
-					 NULL);
+		p->phy = devm_phy_create(dev, NULL, &samsung_usb2_phy_ops);
 		if (IS_ERR(p->phy)) {
 			dev_err(drv->dev, "Failed to create usb2_phy \"%s\"\n",
 				label);
diff --git a/drivers/phy/phy-spear1310-miphy.c b/drivers/phy/phy-spear1310-miphy.c
index 6dcbfcd..51f372b 100644
--- a/drivers/phy/phy-spear1310-miphy.c
+++ b/drivers/phy/phy-spear1310-miphy.c
@@ -229,7 +229,7 @@ static int spear1310_miphy_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	priv->phy = devm_phy_create(dev, NULL, &spear1310_miphy_ops, NULL);
+	priv->phy = devm_phy_create(dev, NULL, &spear1310_miphy_ops);
 	if (IS_ERR(priv->phy)) {
 		dev_err(dev, "failed to create SATA PCIe PHY\n");
 		return PTR_ERR(priv->phy);
diff --git a/drivers/phy/phy-spear1340-miphy.c b/drivers/phy/phy-spear1340-miphy.c
index 7135ba2..c618150 100644
--- a/drivers/phy/phy-spear1340-miphy.c
+++ b/drivers/phy/phy-spear1340-miphy.c
@@ -261,7 +261,7 @@ static int spear1340_miphy_probe(struct platform_device *pdev)
 		return PTR_ERR(priv->misc);
 	}
 
-	priv->phy = devm_phy_create(dev, NULL, &spear1340_miphy_ops, NULL);
+	priv->phy = devm_phy_create(dev, NULL, &spear1340_miphy_ops);
 	if (IS_ERR(priv->phy)) {
 		dev_err(dev, "failed to create SATA PCIe PHY\n");
 		return PTR_ERR(priv->phy);
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 61ebea4..56e7f49 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -295,7 +295,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
 				return PTR_ERR(phy->pmu);
 		}
 
-		phy->phy = devm_phy_create(dev, NULL, &sun4i_usb_phy_ops, NULL);
+		phy->phy = devm_phy_create(dev, NULL, &sun4i_usb_phy_ops);
 		if (IS_ERR(phy->phy)) {
 			dev_err(dev, "failed to create PHY %d\n", i);
 			return PTR_ERR(phy->phy);
diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c
index b964aa9..9f3a1d20 100644
--- a/drivers/phy/phy-ti-pipe3.c
+++ b/drivers/phy/phy-ti-pipe3.c
@@ -400,7 +400,7 @@ static int ti_pipe3_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, phy);
 	pm_runtime_enable(phy->dev);
 
-	generic_phy = devm_phy_create(phy->dev, NULL, &ops, NULL);
+	generic_phy = devm_phy_create(phy->dev, NULL, &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 e1a6623..851f126 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;
@@ -695,7 +693,7 @@ static int twl4030_usb_probe(struct platform_device *pdev)
 	otg->set_host		= twl4030_set_host;
 	otg->set_peripheral	= twl4030_set_peripheral;
 
-	phy = devm_phy_create(twl->dev, NULL, &ops, init_data);
+	phy = devm_phy_create(twl->dev, NULL, &ops);
 	if (IS_ERR(phy)) {
 		dev_dbg(&pdev->dev, "Failed to create PHY\n");
 		return PTR_ERR(phy);
diff --git a/drivers/phy/phy-xgene.c b/drivers/phy/phy-xgene.c
index db809b9..a239310 100644
--- a/drivers/phy/phy-xgene.c
+++ b/drivers/phy/phy-xgene.c
@@ -1707,7 +1707,7 @@ static int xgene_phy_probe(struct platform_device *pdev)
 	ctx->dev = &pdev->dev;
 	platform_set_drvdata(pdev, ctx);
 
-	ctx->phy = devm_phy_create(ctx->dev, NULL, &xgene_phy_ops, NULL);
+	ctx->phy = devm_phy_create(ctx->dev, NULL, &xgene_phy_ops);
 	if (IS_ERR(ctx->phy)) {
 		dev_dbg(&pdev->dev, "Failed to create PHY\n");
 		rc = PTR_ERR(ctx->phy);
diff --git a/drivers/pinctrl/pinctrl-tegra-xusb.c b/drivers/pinctrl/pinctrl-tegra-xusb.c
index e641b42..ae3c983 100644
--- a/drivers/pinctrl/pinctrl-tegra-xusb.c
+++ b/drivers/pinctrl/pinctrl-tegra-xusb.c
@@ -910,7 +910,7 @@ static int tegra_xusb_padctl_probe(struct platform_device *pdev)
 		goto reset;
 	}
 
-	phy = devm_phy_create(&pdev->dev, NULL, &pcie_phy_ops, NULL);
+	phy = devm_phy_create(&pdev->dev, NULL, &pcie_phy_ops);
 	if (IS_ERR(phy)) {
 		err = PTR_ERR(phy);
 		goto unregister;
@@ -919,7 +919,7 @@ static int tegra_xusb_padctl_probe(struct platform_device *pdev)
 	padctl->phys[TEGRA_XUSB_PADCTL_PCIE] = phy;
 	phy_set_drvdata(phy, padctl);
 
-	phy = devm_phy_create(&pdev->dev, NULL, &sata_phy_ops, NULL);
+	phy = devm_phy_create(&pdev->dev, NULL, &sata_phy_ops);
 	if (IS_ERR(phy)) {
 		err = PTR_ERR(phy);
 		goto unregister;
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 2696b95..d983051 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -61,7 +61,6 @@ struct phy {
 	struct device		dev;
 	int			id;
 	const struct phy_ops	*ops;
-	struct phy_init_data	*init_data;
 	struct mutex		mutex;
 	int			init_count;
 	int			power_count;
@@ -84,32 +83,6 @@ struct phy_provider {
 		struct of_phandle_args *args);
 };
 
-/**
- * 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 {
 	struct list_head node;
 	const char *phy_name;
@@ -173,10 +146,9 @@ struct phy *of_phy_get(struct device_node *np, const char *con_id);
 struct phy *of_phy_simple_xlate(struct device *dev,
 	struct of_phandle_args *args);
 struct phy *phy_create(struct device *dev, struct device_node *node,
-		       const struct phy_ops *ops,
-		       struct phy_init_data *init_data);
+		       const struct phy_ops *ops);
 struct phy *devm_phy_create(struct device *dev, struct device_node *node,
-	const struct phy_ops *ops, struct phy_init_data *init_data);
+			    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,
@@ -319,16 +291,14 @@ static inline struct phy *of_phy_simple_xlate(struct device *dev,
 
 static inline struct phy *phy_create(struct device *dev,
 				     struct device_node *node,
-				     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,
 					  struct device_node *node,
-					  const struct phy_ops *ops,
-					  struct phy_init_data *init_data)
+					  const struct phy_ops *ops)
 {
 	return ERR_PTR(-ENOSYS);
 }
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/6] usb: dwc3: host: convey the PHYs to xhci
  2014-08-21 11:33 ` [PATCH 6/6] usb: dwc3: host: convey the PHYs to xhci Heikki Krogerus
@ 2014-09-11 15:01   ` Kishon Vijay Abraham I
  2014-09-12 13:49     ` Heikki Krogerus
  0 siblings, 1 reply; 31+ messages in thread
From: Kishon Vijay Abraham I @ 2014-09-11 15:01 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

Hi,

On Thursday 21 August 2014 05:03 PM, Heikki Krogerus wrote:
> On some platforms a PHY may need to be handled also in the
> host controller driver. Exynos5420 SoC requires some "PHY
> tuning" based on the USB speed. This patch delivers dwc3's
> PHYs to the xhci platform device when it's created.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Tested-by: Vivek Gautam <gautam.vivek@samsung.com>
> Acked-by: Felipe Balbi <balbi@ti.com>
> ---
>  drivers/usb/dwc3/host.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> index dcb8ca0..12bfd3c 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -29,8 +29,7 @@ int dwc3_host_init(struct dwc3 *dwc)
>  	xhci = platform_device_alloc("xhci-hcd", PLATFORM_DEVID_AUTO);
>  	if (!xhci) {
>  		dev_err(dwc->dev, "couldn't allocate xHCI device\n");
> -		ret = -ENOMEM;
> -		goto err0;
> +		return -ENOMEM;
>  	}
>  
>  	dma_set_coherent_mask(&xhci->dev, dwc->dev->coherent_dma_mask);
> @@ -60,22 +59,33 @@ int dwc3_host_init(struct dwc3 *dwc)
>  		goto err1;
>  	}
>  
> +	phy_create_lookup(dwc->usb2_generic_phy, "usb2-phy",
> +			  dev_name(&xhci->dev));
> +	phy_create_lookup(dwc->usb3_generic_phy, "usb3-phy",
> +			  dev_name(&xhci->dev));
> +

I don't think create lookup should be in host init. If it's dt boot, the
binding should be in dt data or for other boot modes the bindig should be done
in the board file. This just seems hacky to me.

Thanks
Kishon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/6] arm: omap3: twl: use the new lookup method with usb phy
  2014-08-21 11:33 ` [PATCH 3/6] arm: omap3: twl: use the new lookup method with usb phy Heikki Krogerus
@ 2014-09-11 15:26   ` Kishon Vijay Abraham I
  2014-09-12 13:50     ` Heikki Krogerus
  0 siblings, 1 reply; 31+ messages in thread
From: Kishon Vijay Abraham I @ 2014-09-11 15:26 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

Hi,

On Thursday 21 August 2014 05:03 PM, Heikki Krogerus wrote:
> Provide complete association for the phy and it's user
> (musb) with the new phy lookup method.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  arch/arm/mach-omap2/twl-common.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
> index b0d54da..b78383c 100644
> --- a/arch/arm/mach-omap2/twl-common.c
> +++ b/arch/arm/mach-omap2/twl-common.c
> @@ -91,18 +91,14 @@ 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 twl4030_usb_lookup = {
> +	.phy_name	= "phy-twl4030_usb.0",
> +	.dev_id		= "musb-hdrc.0",
> +	.con_id		= "usb",
>  };

Can use PHY_LOOKUP no?

Thanks
Kishon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/6] phy: improved lookup method
  2014-08-21 11:33 ` [PATCH 2/6] phy: improved lookup method Heikki Krogerus
@ 2014-09-11 15:33   ` Kishon Vijay Abraham I
  2014-09-12 14:07     ` Heikki Krogerus
  0 siblings, 1 reply; 31+ messages in thread
From: Kishon Vijay Abraham I @ 2014-09-11 15:33 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

Hi,

On Thursday 21 August 2014 05:03 PM, Heikki Krogerus wrote:
> Removes the need for the phys to be aware of their users
> even when not using DT. The method is copied from clkdev.c.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Tested-by: Vivek Gautam <gautam.vivek@samsung.com>
> ---
>
.
.
<skip>
.
.

>  
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index ff5eec5..67a8c726 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -26,6 +26,7 @@
>  static struct class *phy_class;
>  static DEFINE_MUTEX(phy_provider_mutex);
>  static LIST_HEAD(phy_provider_list);
> +static LIST_HEAD(phys);
>  static DEFINE_IDA(phy_ida);
>  
>  static void devm_phy_release(struct device *dev, void *res)
> @@ -84,6 +85,138 @@ static struct phy *phy_lookup(struct device *device, const char *port)
>  	return ERR_PTR(-ENODEV);
>  }
>  
> +/**
> + * phy_register_lookup() - register PHY/device association
> + * @pl: association to register
> + */
> +void phy_register_lookup(struct phy_lookup *pl)
> +{
> +	mutex_lock(&phy_provider_mutex);
> +	list_add_tail(&pl->node, &phys);
> +	mutex_unlock(&phy_provider_mutex);
> +}
> +
> +/**
> + * phy_unregister_lookup() - remove PHY/device association
> + * @pl: association to be removed
> + */
> +void phy_unregister_lookup(struct phy_lookup *pl)
> +{
> +	mutex_lock(&phy_provider_mutex);
> +	list_del(&pl->node);
> +	mutex_unlock(&phy_provider_mutex);
> +}
> +
> +/**
> + * phy_create_lookup() - allocate and register PHY/device association
> + * @phy: the phy of the association
> + * @con_id: connection ID string on device
> + * @dev_id: the device of the association
> + *
> + * Creates and registers phy_lookup entry.
> + */
> +int phy_create_lookup(struct phy *phy, const char *con_id, const char *dev_id)
> +{
> +	struct phy_lookup *pl;
> +
> +	if (!phy || (!dev_id && !con_id))
> +		return -EINVAL;
> +
> +	pl = kzalloc(sizeof(*pl), GFP_KERNEL);
> +	if (!pl)
> +		return -ENOMEM;
> +
> +	pl->phy_name = dev_name(&phy->dev);
> +	pl->dev_id = dev_id;
> +	pl->con_id = con_id;
> +
> +	phy_register_lookup(pl);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(phy_create_lookup);
> +
> +/**
> + * phy_remove_lookup() - find and remove PHY/device association
> + * @phy: the phy of the association
> + * @con_id: connection ID string on device
> + * @dev_id: the device of the association
> + *
> + * Finds and unregisters phy_lookup entry that was created with
> + * phy_create_lookup().
> + */
> +void phy_remove_lookup(struct phy *phy, const char *con_id, const char *dev_id)
> +{
> +	struct phy_lookup *pl;
> +
> +	if (!phy || (!dev_id && !con_id))
> +		return;
> +
> +	list_for_each_entry(pl, &phys, node)
> +		if (!strcmp(pl->phy_name, dev_name(&phy->dev)) &&
> +		    !strcmp(pl->dev_id, dev_id) &&
> +		    !strcmp(pl->con_id, con_id)) {
> +			phy_unregister_lookup(pl);
> +			kfree(pl);
> +			return;
> +		}
> +}
> +EXPORT_SYMBOL_GPL(phy_remove_lookup);
> +
> +static struct phy *phy_find(struct device *dev, const char *con_id)
> +{
> +	const char *dev_id = dev ? dev_name(dev) : NULL;
> +	int match, best_found = 0, best_possible = 0;
> +	struct phy *phy = ERR_PTR(-ENODEV);
> +	struct phy_lookup *p, *pl = NULL;
> +
> +	if (dev_id)
> +		best_possible += 2;
> +	if (con_id)
> +		best_possible += 1;
> +
> +	list_for_each_entry(p, &phys, node) {
> +		match = 0;
> +		if (p->dev_id) {
> +			if (!dev_id || strcmp(p->dev_id, dev_id))
> +				continue;
> +			match += 2;
> +		}
> +		if (p->con_id) {
> +			if (!con_id || strcmp(p->con_id, con_id))
> +				continue;
> +			match += 1;
> +		}
> +
> +		if (match > best_found) {
> +			pl = p;
> +			if (match != best_possible)
> +				best_found = match;
> +			else
> +				break;
> +		}
> +	}
> +
> +	if (pl) {
> +		struct class_dev_iter iter;
> +		struct device *phy_dev;
> +
> +		class_dev_iter_init(&iter, phy_class, NULL, NULL);
> +		while ((phy_dev = class_dev_iter_next(&iter))) {
> +			if (!strcmp(dev_name(phy_dev), pl->phy_name)) {

I'm not sure how it'll work with systems which has multiple PHYs since the "id"
component of the device is determined purely in runtime.

I'd assume we'll be constantly patching the lookup data for non-dt boot :-/

Thanks
Kishon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/6] usb: dwc3: host: convey the PHYs to xhci
  2014-09-11 15:01   ` Kishon Vijay Abraham I
@ 2014-09-12 13:49     ` Heikki Krogerus
  2014-09-12 14:11       ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 31+ messages in thread
From: Heikki Krogerus @ 2014-09-12 13:49 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

On Thu, Sep 11, 2014 at 08:31:19PM +0530, Kishon Vijay Abraham I wrote:
> > @@ -60,22 +59,33 @@ int dwc3_host_init(struct dwc3 *dwc)
> >  		goto err1;
> >  	}
> >  
> > +	phy_create_lookup(dwc->usb2_generic_phy, "usb2-phy",
> > +			  dev_name(&xhci->dev));
> > +	phy_create_lookup(dwc->usb3_generic_phy, "usb3-phy",
> > +			  dev_name(&xhci->dev));
> > +
> 
> I don't think create lookup should be in host init. If it's dt boot, the
> binding should be in dt data or for other boot modes the bindig should be done
> in the board file. This just seems hacky to me.

So are you now suggesting that instead of using platform independent
solution of sharing the PHYs here, you would have us add platform
specific quirks? That would be totally wrong!

And please don't even consider use of board files especially if there
is an option. They are the one thing that we are meant to avoid if
possible! No?


Cheers,

-- 
heikki

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/6] arm: omap3: twl: use the new lookup method with usb phy
  2014-09-11 15:26   ` Kishon Vijay Abraham I
@ 2014-09-12 13:50     ` Heikki Krogerus
  0 siblings, 0 replies; 31+ messages in thread
From: Heikki Krogerus @ 2014-09-12 13:50 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

On Thu, Sep 11, 2014 at 08:56:15PM +0530, Kishon Vijay Abraham I wrote:
> > +static struct phy_lookup twl4030_usb_lookup = {
> > +	.phy_name	= "phy-twl4030_usb.0",
> > +	.dev_id		= "musb-hdrc.0",
> > +	.con_id		= "usb",
> >  };
> 
> Can use PHY_LOOKUP no?

I'll fix this.

Thanks,

-- 
heikki

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/6] phy: improved lookup method
  2014-09-11 15:33   ` Kishon Vijay Abraham I
@ 2014-09-12 14:07     ` Heikki Krogerus
  2014-09-12 14:46       ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 31+ messages in thread
From: Heikki Krogerus @ 2014-09-12 14:07 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

On Thu, Sep 11, 2014 at 09:03:06PM +0530, Kishon Vijay Abraham I wrote:
> > +static struct phy *phy_find(struct device *dev, const char *con_id)
> > +{
> > +	const char *dev_id = dev ? dev_name(dev) : NULL;
> > +	int match, best_found = 0, best_possible = 0;
> > +	struct phy *phy = ERR_PTR(-ENODEV);
> > +	struct phy_lookup *p, *pl = NULL;
> > +
> > +	if (dev_id)
> > +		best_possible += 2;
> > +	if (con_id)
> > +		best_possible += 1;
> > +
> > +	list_for_each_entry(p, &phys, node) {
> > +		match = 0;
> > +		if (p->dev_id) {
> > +			if (!dev_id || strcmp(p->dev_id, dev_id))
> > +				continue;
> > +			match += 2;
> > +		}
> > +		if (p->con_id) {
> > +			if (!con_id || strcmp(p->con_id, con_id))
> > +				continue;
> > +			match += 1;
> > +		}
> > +
> > +		if (match > best_found) {
> > +			pl = p;
> > +			if (match != best_possible)
> > +				best_found = match;
> > +			else
> > +				break;
> > +		}
> > +	}
> > +
> > +	if (pl) {
> > +		struct class_dev_iter iter;
> > +		struct device *phy_dev;
> > +
> > +		class_dev_iter_init(&iter, phy_class, NULL, NULL);
> > +		while ((phy_dev = class_dev_iter_next(&iter))) {
> > +			if (!strcmp(dev_name(phy_dev), pl->phy_name)) {
> 
> I'm not sure how it'll work with systems which has multiple PHYs since the "id"
> component of the device is determined purely in runtime.
> 
> I'd assume we'll be constantly patching the lookup data for non-dt boot :-/

I'm sorry but I don't think I understand (I must be a bit tired
today)? Could you please elaborate?


Have a nice weekend!

-- 
heikki

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/6] usb: dwc3: host: convey the PHYs to xhci
  2014-09-12 13:49     ` Heikki Krogerus
@ 2014-09-12 14:11       ` Kishon Vijay Abraham I
  2014-09-15 12:06         ` Heikki Krogerus
  0 siblings, 1 reply; 31+ messages in thread
From: Kishon Vijay Abraham I @ 2014-09-12 14:11 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

Hi,

On Friday 12 September 2014 07:19 PM, Heikki Krogerus wrote:
> On Thu, Sep 11, 2014 at 08:31:19PM +0530, Kishon Vijay Abraham I wrote:
>>> @@ -60,22 +59,33 @@ int dwc3_host_init(struct dwc3 *dwc)
>>>  		goto err1;
>>>  	}
>>>  
>>> +	phy_create_lookup(dwc->usb2_generic_phy, "usb2-phy",
>>> +			  dev_name(&xhci->dev));
>>> +	phy_create_lookup(dwc->usb3_generic_phy, "usb3-phy",
>>> +			  dev_name(&xhci->dev));
>>> +
>>
>> I don't think create lookup should be in host init. If it's dt boot, the
>> binding should be in dt data or for other boot modes the bindig should be done
>> in the board file. This just seems hacky to me.
> 
> So are you now suggesting that instead of using platform independent
> solution of sharing the PHYs here, you would have us add platform
> specific quirks? That would be totally wrong!

No. The binding between the controller and the PHY is done in hardware design
and it would be wrong to create such a binding in drivers/* IMO.
> 
> And please don't even consider use of board files especially if there
> is an option. They are the one thing that we are meant to avoid if
> possible! No?

For dt yes, I'm not sure about other modes.

So in the case of dt boot, I'd prefer giving the binding in dt file than
anywhere else.

Thanks
Kishon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/6] phy: improved lookup method
  2014-09-12 14:07     ` Heikki Krogerus
@ 2014-09-12 14:46       ` Kishon Vijay Abraham I
  2014-09-15 12:35         ` Heikki Krogerus
  0 siblings, 1 reply; 31+ messages in thread
From: Kishon Vijay Abraham I @ 2014-09-12 14:46 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

Hi,

On Friday 12 September 2014 07:37 PM, Heikki Krogerus wrote:
> On Thu, Sep 11, 2014 at 09:03:06PM +0530, Kishon Vijay Abraham I wrote:
>>> +static struct phy *phy_find(struct device *dev, const char *con_id)
>>> +{
>>> +	const char *dev_id = dev ? dev_name(dev) : NULL;
>>> +	int match, best_found = 0, best_possible = 0;
>>> +	struct phy *phy = ERR_PTR(-ENODEV);
>>> +	struct phy_lookup *p, *pl = NULL;
>>> +
>>> +	if (dev_id)
>>> +		best_possible += 2;
>>> +	if (con_id)
>>> +		best_possible += 1;
>>> +
>>> +	list_for_each_entry(p, &phys, node) {
>>> +		match = 0;
>>> +		if (p->dev_id) {
>>> +			if (!dev_id || strcmp(p->dev_id, dev_id))
>>> +				continue;
>>> +			match += 2;
>>> +		}
>>> +		if (p->con_id) {
>>> +			if (!con_id || strcmp(p->con_id, con_id))
>>> +				continue;
>>> +			match += 1;
>>> +		}
>>> +
>>> +		if (match > best_found) {
>>> +			pl = p;
>>> +			if (match != best_possible)
>>> +				best_found = match;
>>> +			else
>>> +				break;
>>> +		}
>>> +	}
>>> +
>>> +	if (pl) {
>>> +		struct class_dev_iter iter;
>>> +		struct device *phy_dev;
>>> +
>>> +		class_dev_iter_init(&iter, phy_class, NULL, NULL);
>>> +		while ((phy_dev = class_dev_iter_next(&iter))) {
>>> +			if (!strcmp(dev_name(phy_dev), pl->phy_name)) {
>>
>> I'm not sure how it'll work with systems which has multiple PHYs since the "id"
>> component of the device is determined purely in runtime.
>>
>> I'd assume we'll be constantly patching the lookup data for non-dt boot :-/
> 
> I'm sorry but I don't think I understand (I must be a bit tired
> today)? Could you please elaborate?

Assume you have 2 phys in your system..
static struct phy_lookup usb_lookup = {
	.phy_name	= "phy-usb.0",
	.dev_id		= "usb.0",
	.con_id		= "usb",
};

static struct phy_lookup sata_lookup = {
	.phy_name	= "sata-usb.1",
	.dev_id		= "sata.0",
	.con_id		= "sata",
};

First you do modprobe phy-usb, the probe of USB PHY driver gets invoked and it
creates the PHY. The phy-core will find a free id (now it will be 0) and then
name the phy as phy-usb.0.
Then with modprobe phy-sata, the phy-core will create phy-sata.1.

This is an ideal case where the .phy_name in phy_lookup matches.

Consider if the order is flipped and the user does modprobe phy-sata first. The
phy_names won't match anymore (the sata phy device name would be "sata-usb.0").

Thanks
Kishon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/6] usb: dwc3: host: convey the PHYs to xhci
  2014-09-12 14:11       ` Kishon Vijay Abraham I
@ 2014-09-15 12:06         ` Heikki Krogerus
  2014-09-16  6:37           ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 31+ messages in thread
From: Heikki Krogerus @ 2014-09-15 12:06 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

On Fri, Sep 12, 2014 at 07:41:56PM +0530, Kishon Vijay Abraham I wrote:
> >> I don't think create lookup should be in host init. If it's dt boot, the
> >> binding should be in dt data or for other boot modes the bindig should be done
> >> in the board file. This just seems hacky to me.
> > 
> > So are you now suggesting that instead of using platform independent
> > solution of sharing the PHYs here, you would have us add platform
> > specific quirks? That would be totally wrong!
> 
> No. The binding between the controller and the PHY is done in hardware design
> and it would be wrong to create such a binding in drivers/* IMO.

And kernel of course always knows the hardware design when it's being
booted, wrong!

Firstly, don't assume this kind of controllers are always part of some
SoC or chip set. They could easily be on a PCI card for example.
Secondly, don't assume we could tell all the details about the board
based on some identifiers. Fox example, at least with our SoCs we
won't be able to differentiate the boards. And it's not like every
board using the same SoC uses the same USB2 PHY for example. That kind
of things are up to the manufacturers.

By default we have to rely on hardware descriptions like DT and ACPI
tables or being able to runtime detect (ULPI). If those things don't
work, we live without PHY in this case. We add board specific quirks
_only_ in case of exceptions, where we simply have no other way. If
it's possible to avoid them, especially with something as simple as
this, we avoid them!

> > And please don't even consider use of board files especially if there
> > is an option. They are the one thing that we are meant to avoid if
> > possible! No?
> 
> For dt yes, I'm not sure about other modes.
> 
> So in the case of dt boot, I'd prefer giving the binding in dt file than
> anywhere else.

I don't know how dt works, but at least in case of ACPI we still need
to deliver the PHY to xHCI here, even when dwc3 is provided bindings
to a PHY(s).


-- 
heikki

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/6] phy: improved lookup method
  2014-09-12 14:46       ` Kishon Vijay Abraham I
@ 2014-09-15 12:35         ` Heikki Krogerus
  2014-09-18 10:25           ` Heikki Krogerus
  0 siblings, 1 reply; 31+ messages in thread
From: Heikki Krogerus @ 2014-09-15 12:35 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

On Fri, Sep 12, 2014 at 08:16:01PM +0530, Kishon Vijay Abraham I wrote:
> Assume you have 2 phys in your system..
> static struct phy_lookup usb_lookup = {
> 	.phy_name	= "phy-usb.0",
> 	.dev_id		= "usb.0",
> 	.con_id		= "usb",
> };
> 
> static struct phy_lookup sata_lookup = {
> 	.phy_name	= "sata-usb.1",
> 	.dev_id		= "sata.0",
> 	.con_id		= "sata",
> };
> 
> First you do modprobe phy-usb, the probe of USB PHY driver gets invoked and it
> creates the PHY. The phy-core will find a free id (now it will be 0) and then
> name the phy as phy-usb.0.
> Then with modprobe phy-sata, the phy-core will create phy-sata.1.
> 
> This is an ideal case where the .phy_name in phy_lookup matches.
> 
> Consider if the order is flipped and the user does modprobe phy-sata first. The
> phy_names won't match anymore (the sata phy device name would be "sata-usb.0").

True!

So we can't accept statically created lookups. Which is probable the
best thing to do in any case even if there wasn't this issue.

I think we already talked about this. I know I was going to create the
lookup for twl4030 in twl-core.c instead of the board file at one
point, but forgot about it. I need to do that now.

In any case, I'll fix this by dropping the possibility of creating the
lookups statically. I'll prepare new version of the whole set.


Thanks,

-- 
heikki

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/6] usb: dwc3: host: convey the PHYs to xhci
  2014-09-15 12:06         ` Heikki Krogerus
@ 2014-09-16  6:37           ` Kishon Vijay Abraham I
  2014-09-16 12:13             ` Heikki Krogerus
  0 siblings, 1 reply; 31+ messages in thread
From: Kishon Vijay Abraham I @ 2014-09-16  6:37 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

Hi,

On Monday 15 September 2014 05:36 PM, Heikki Krogerus wrote:
> On Fri, Sep 12, 2014 at 07:41:56PM +0530, Kishon Vijay Abraham I wrote:
>>>> I don't think create lookup should be in host init. If it's dt boot, the
>>>> binding should be in dt data or for other boot modes the bindig should be done
>>>> in the board file. This just seems hacky to me.
>>>
>>> So are you now suggesting that instead of using platform independent
>>> solution of sharing the PHYs here, you would have us add platform
>>> specific quirks? That would be totally wrong!
>>
>> No. The binding between the controller and the PHY is done in hardware design
>> and it would be wrong to create such a binding in drivers/* IMO.
> 
> And kernel of course always knows the hardware design when it's being
> booted, wrong!

Exactly my point. By creating the binding in drivers/*, you are directly
telling the driver of the hardware design whereas it should have come from dt
or platform data or .. (ACPI?).
> 
> Firstly, don't assume this kind of controllers are always part of some
> SoC or chip set. They could easily be on a PCI card for example.

Agreed. Not convinced the current phy-core could handle it well.
> Secondly, don't assume we could tell all the details about the board
> based on some identifiers. Fox example, at least with our SoCs we
> won't be able to differentiate the boards. And it's not like every
> board using the same SoC uses the same USB2 PHY for example. That kind
> of things are up to the manufacturers.

right. In dt, generally we have different dt files for different boards,
similarly we have different board files for different boards where those
bindings can be created. Again not sure about ACPI.
> 
> By default we have to rely on hardware descriptions like DT and ACPI
> tables or being able to runtime detect (ULPI). If those things don't
> work, we live without PHY in this case. We add board specific quirks
> _only_ in case of exceptions, where we simply have no other way. If
> it's possible to avoid them, especially with something as simple as
> this, we avoid them!
> 
>>> And please don't even consider use of board files especially if there
>>> is an option. They are the one thing that we are meant to avoid if
>>> possible! No?
>>
>> For dt yes, I'm not sure about other modes.
>>
>> So in the case of dt boot, I'd prefer giving the binding in dt file than
>> anywhere else.
> 
> I don't know how dt works, but at least in case of ACPI we still need
> to deliver the PHY to xHCI here, even when dwc3 is provided bindings
> to a PHY(s).

Agreed. But can't we have the bindings for xHCI in ACPI itself?

Thanks
Kishon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/6] usb: dwc3: host: convey the PHYs to xhci
  2014-09-16  6:37           ` Kishon Vijay Abraham I
@ 2014-09-16 12:13             ` Heikki Krogerus
  0 siblings, 0 replies; 31+ messages in thread
From: Heikki Krogerus @ 2014-09-16 12:13 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

On Tue, Sep 16, 2014 at 12:07:00PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Monday 15 September 2014 05:36 PM, Heikki Krogerus wrote:
> > On Fri, Sep 12, 2014 at 07:41:56PM +0530, Kishon Vijay Abraham I wrote:
> >>>> I don't think create lookup should be in host init. If it's dt boot, the
> >>>> binding should be in dt data or for other boot modes the bindig should be done
> >>>> in the board file. This just seems hacky to me.
> >>>
> >>> So are you now suggesting that instead of using platform independent
> >>> solution of sharing the PHYs here, you would have us add platform
> >>> specific quirks? That would be totally wrong!
> >>
> >> No. The binding between the controller and the PHY is done in hardware design
> >> and it would be wrong to create such a binding in drivers/* IMO.
> > 
> > And kernel of course always knows the hardware design when it's being
> > booted, wrong!
> 
> Exactly my point. By creating the binding in drivers/*, you are directly
> telling the driver of the hardware design whereas it should have come from dt
> or platform data or .. (ACPI?).

Man, you are not getting this... We want to avoid the damn platform
data!!! If you are not using devicetree it doesn't mean the same as
something like board files or some other platform specific quirks are
suddenly acceptable.

I'm giving you a way of avoiding those things, basically a way
ignoring the platform completely in this case, but you are saying no
no no, we need to have "board files". C'mon!

And I'm pretty sure you misspelled this sentence "..you are directly
telling the driver of the hardware design..". I believe you meant to
say "Wau! With this we newer have to care about the hardware design
with this driver!".

Note. That and some other things that we can always simply ignore here
with this approach include:
- How were we enumerated (DT, ACPI, PCI...)
- Do some other bindings exist (DT)
- Do we even have PHYs to convey
- Does xHCI have any use for the PHYs
- What is the platform we are running on in general

> > Firstly, don't assume this kind of controllers are always part of some
> > SoC or chip set. They could easily be on a PCI card for example.
> 
> Agreed. Not convinced the current phy-core could handle it well.

If there is ever a PHY that needs OS control in this case, it will
almost certainly be possible to runtime detected somehow like ULPI
PHYs can be.

> > Secondly, don't assume we could tell all the details about the board
> > based on some identifiers. Fox example, at least with our SoCs we
> > won't be able to differentiate the boards. And it's not like every
> > board using the same SoC uses the same USB2 PHY for example. That kind
> > of things are up to the manufacturers.
> 
> right. In dt, generally we have different dt files for different boards,
> similarly we have different board files for different boards where those
> bindings can be created. Again not sure about ACPI.

I'll repeat what I said below - If we don't have description of the
hardware from something like DT or ACPI, and that includes the
bindings, or we can't runtime detect the underlying hardware, by
default it means we live without PHYs. Quirks you add only in case of
exceptions, but I actually think there will never be need for them...

If we are not using ACPI, we are using devicetree and otherwise we can
runtime detect the PHYs one way or the other. And again, if no PHY is
found it simply means the PHYs are autonomous.

So as you can see, it's possible we will newer need any kind of
"platform data" in order to provide dwc3 the correct PHYs, no matter
what kind of platform we are running on, so it makes no sense of
having them just for dwc3 host. Do you not agree?

> > I don't know how dt works, but at least in case of ACPI we still need
> > to deliver the PHY to xHCI here, even when dwc3 is provided bindings
> > to a PHY(s).
> 
> Agreed. But can't we have the bindings for xHCI in ACPI itself?

No. ACPI will never provide separate object for the xHCI that is part
of DWC3. There is nothing the PHYs could be bound to.


-- 
heikki

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/6] phy: improved lookup method
  2014-09-15 12:35         ` Heikki Krogerus
@ 2014-09-18 10:25           ` Heikki Krogerus
  2014-09-22 11:37             ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 31+ messages in thread
From: Heikki Krogerus @ 2014-09-18 10:25 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

On Mon, Sep 15, 2014 at 03:35:08PM +0300, Heikki Krogerus wrote:
> On Fri, Sep 12, 2014 at 08:16:01PM +0530, Kishon Vijay Abraham I wrote:
> > Assume you have 2 phys in your system..
> > static struct phy_lookup usb_lookup = {
> > 	.phy_name	= "phy-usb.0",
> > 	.dev_id		= "usb.0",
> > 	.con_id		= "usb",
> > };
> > 
> > static struct phy_lookup sata_lookup = {
> > 	.phy_name	= "sata-usb.1",
> > 	.dev_id		= "sata.0",
> > 	.con_id		= "sata",
> > };
> > 
> > First you do modprobe phy-usb, the probe of USB PHY driver gets invoked and it
> > creates the PHY. The phy-core will find a free id (now it will be 0) and then
> > name the phy as phy-usb.0.
> > Then with modprobe phy-sata, the phy-core will create phy-sata.1.
> > 
> > This is an ideal case where the .phy_name in phy_lookup matches.
> > 
> > Consider if the order is flipped and the user does modprobe phy-sata first. The
> > phy_names won't match anymore (the sata phy device name would be "sata-usb.0").

Actually, I don't think there would be this problem if we used the
name of the actual device which is the parent of phy devices, right?

Cheers,

-- 
heikki

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/6] phy: improved lookup method
  2014-09-18 10:25           ` Heikki Krogerus
@ 2014-09-22 11:37             ` Kishon Vijay Abraham I
  2014-09-23 10:53               ` Heikki Krogerus
  0 siblings, 1 reply; 31+ messages in thread
From: Kishon Vijay Abraham I @ 2014-09-22 11:37 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

Hi,

On Thursday 18 September 2014 03:55 PM, Heikki Krogerus wrote:
> On Mon, Sep 15, 2014 at 03:35:08PM +0300, Heikki Krogerus wrote:
>> On Fri, Sep 12, 2014 at 08:16:01PM +0530, Kishon Vijay Abraham I wrote:
>>> Assume you have 2 phys in your system..
>>> static struct phy_lookup usb_lookup = {
>>> 	.phy_name	= "phy-usb.0",
>>> 	.dev_id		= "usb.0",
>>> 	.con_id		= "usb",
>>> };
>>>
>>> static struct phy_lookup sata_lookup = {
>>> 	.phy_name	= "sata-usb.1",
>>> 	.dev_id		= "sata.0",
>>> 	.con_id		= "sata",
>>> };
>>>
>>> First you do modprobe phy-usb, the probe of USB PHY driver gets invoked and it
>>> creates the PHY. The phy-core will find a free id (now it will be 0) and then
>>> name the phy as phy-usb.0.
>>> Then with modprobe phy-sata, the phy-core will create phy-sata.1.
>>>
>>> This is an ideal case where the .phy_name in phy_lookup matches.
>>>
>>> Consider if the order is flipped and the user does modprobe phy-sata first. The
>>> phy_names won't match anymore (the sata phy device name would be "sata-usb.0").
> 
> Actually, I don't think there would be this problem if we used the
> name of the actual device which is the parent of phy devices, right?

hmm.. but if the parent is a multi-phy phy provider (like pipe3 PHY driver), we
might end up with the same problem.

Thanks
Kishon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/6] phy: improved lookup method
  2014-09-22 11:37             ` Kishon Vijay Abraham I
@ 2014-09-23 10:53               ` Heikki Krogerus
  2014-09-23 11:03                 ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 31+ messages in thread
From: Heikki Krogerus @ 2014-09-23 10:53 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

On Mon, Sep 22, 2014 at 05:07:55PM +0530, Kishon Vijay Abraham I wrote:
> On Thursday 18 September 2014 03:55 PM, Heikki Krogerus wrote:
> > On Mon, Sep 15, 2014 at 03:35:08PM +0300, Heikki Krogerus wrote:
> >> On Fri, Sep 12, 2014 at 08:16:01PM +0530, Kishon Vijay Abraham I wrote:
> >>> Assume you have 2 phys in your system..
> >>> static struct phy_lookup usb_lookup = {
> >>> 	.phy_name	= "phy-usb.0",
> >>> 	.dev_id		= "usb.0",
> >>> 	.con_id		= "usb",
> >>> };
> >>>
> >>> static struct phy_lookup sata_lookup = {
> >>> 	.phy_name	= "sata-usb.1",
> >>> 	.dev_id		= "sata.0",
> >>> 	.con_id		= "sata",
> >>> };
> >>>
> >>> First you do modprobe phy-usb, the probe of USB PHY driver gets invoked and it
> >>> creates the PHY. The phy-core will find a free id (now it will be 0) and then
> >>> name the phy as phy-usb.0.
> >>> Then with modprobe phy-sata, the phy-core will create phy-sata.1.
> >>>
> >>> This is an ideal case where the .phy_name in phy_lookup matches.
> >>>
> >>> Consider if the order is flipped and the user does modprobe phy-sata first. The
> >>> phy_names won't match anymore (the sata phy device name would be "sata-usb.0").
> > 
> > Actually, I don't think there would be this problem if we used the
> > name of the actual device which is the parent of phy devices, right?
> 
> hmm.. but if the parent is a multi-phy phy provider (like pipe3 PHY driver), we
> might end up with the same problem.

I'm not completely sure what you mean? If you are talking about
platforms with multiple instances of a single phy, I don't see how
there could ever be a scenario where we did not know the order in
which they were enumerated. Can you give an example again?


Thanks,

-- 
heikki

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/6] phy: improved lookup method
  2014-09-23 10:53               ` Heikki Krogerus
@ 2014-09-23 11:03                 ` Kishon Vijay Abraham I
  2014-09-23 11:43                   ` Heikki Krogerus
  0 siblings, 1 reply; 31+ messages in thread
From: Kishon Vijay Abraham I @ 2014-09-23 11:03 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

Hi,

On Tuesday 23 September 2014 04:23 PM, Heikki Krogerus wrote:
> On Mon, Sep 22, 2014 at 05:07:55PM +0530, Kishon Vijay Abraham I wrote:
>> On Thursday 18 September 2014 03:55 PM, Heikki Krogerus wrote:
>>> On Mon, Sep 15, 2014 at 03:35:08PM +0300, Heikki Krogerus wrote:
>>>> On Fri, Sep 12, 2014 at 08:16:01PM +0530, Kishon Vijay Abraham I wrote:
>>>>> Assume you have 2 phys in your system..
>>>>> static struct phy_lookup usb_lookup = {
>>>>> 	.phy_name	= "phy-usb.0",
>>>>> 	.dev_id		= "usb.0",
>>>>> 	.con_id		= "usb",
>>>>> };
>>>>>
>>>>> static struct phy_lookup sata_lookup = {
>>>>> 	.phy_name	= "sata-usb.1",
>>>>> 	.dev_id		= "sata.0",
>>>>> 	.con_id		= "sata",
>>>>> };
>>>>>
>>>>> First you do modprobe phy-usb, the probe of USB PHY driver gets invoked and it
>>>>> creates the PHY. The phy-core will find a free id (now it will be 0) and then
>>>>> name the phy as phy-usb.0.
>>>>> Then with modprobe phy-sata, the phy-core will create phy-sata.1.
>>>>>
>>>>> This is an ideal case where the .phy_name in phy_lookup matches.
>>>>>
>>>>> Consider if the order is flipped and the user does modprobe phy-sata first. The
>>>>> phy_names won't match anymore (the sata phy device name would be "sata-usb.0").
>>>
>>> Actually, I don't think there would be this problem if we used the
>>> name of the actual device which is the parent of phy devices, right?
>>
>> hmm.. but if the parent is a multi-phy phy provider (like pipe3 PHY driver), we
>> might end up with the same problem.
> 
> I'm not completely sure what you mean? If you are talking about
> platforms with multiple instances of a single phy, I don't see how
> there could ever be a scenario where we did not know the order in
> which they were enumerated. Can you give an example again?

If a single IP implements multiple PHYs (phy-miphy365x.c in linux-phy next),
the parent for all the phy devices would be the same.

Thanks
Kishon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/6] phy: improved lookup method
  2014-09-23 11:03                 ` Kishon Vijay Abraham I
@ 2014-09-23 11:43                   ` Heikki Krogerus
  2014-09-24  9:44                     ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 31+ messages in thread
From: Heikki Krogerus @ 2014-09-23 11:43 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

On Tue, Sep 23, 2014 at 04:33:09PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Tuesday 23 September 2014 04:23 PM, Heikki Krogerus wrote:
> > On Mon, Sep 22, 2014 at 05:07:55PM +0530, Kishon Vijay Abraham I wrote:
> >> On Thursday 18 September 2014 03:55 PM, Heikki Krogerus wrote:
> >>> On Mon, Sep 15, 2014 at 03:35:08PM +0300, Heikki Krogerus wrote:
> >>>> On Fri, Sep 12, 2014 at 08:16:01PM +0530, Kishon Vijay Abraham I wrote:
> >>>>> Assume you have 2 phys in your system..
> >>>>> static struct phy_lookup usb_lookup = {
> >>>>> 	.phy_name	= "phy-usb.0",
> >>>>> 	.dev_id		= "usb.0",
> >>>>> 	.con_id		= "usb",
> >>>>> };
> >>>>>
> >>>>> static struct phy_lookup sata_lookup = {
> >>>>> 	.phy_name	= "sata-usb.1",
> >>>>> 	.dev_id		= "sata.0",
> >>>>> 	.con_id		= "sata",
> >>>>> };
> >>>>>
> >>>>> First you do modprobe phy-usb, the probe of USB PHY driver gets invoked and it
> >>>>> creates the PHY. The phy-core will find a free id (now it will be 0) and then
> >>>>> name the phy as phy-usb.0.
> >>>>> Then with modprobe phy-sata, the phy-core will create phy-sata.1.
> >>>>>
> >>>>> This is an ideal case where the .phy_name in phy_lookup matches.
> >>>>>
> >>>>> Consider if the order is flipped and the user does modprobe phy-sata first. The
> >>>>> phy_names won't match anymore (the sata phy device name would be "sata-usb.0").
> >>>
> >>> Actually, I don't think there would be this problem if we used the
> >>> name of the actual device which is the parent of phy devices, right?
> >>
> >> hmm.. but if the parent is a multi-phy phy provider (like pipe3 PHY driver), we
> >> might end up with the same problem.
> > 
> > I'm not completely sure what you mean? If you are talking about
> > platforms with multiple instances of a single phy, I don't see how
> > there could ever be a scenario where we did not know the order in
> > which they were enumerated. Can you give an example again?
> 
> If a single IP implements multiple PHYs (phy-miphy365x.c in linux-phy next),
> the parent for all the phy devices would be the same.

OK, got it. So I guess we need to match to the phy dev and to the phy
name. First to the dev and then in case the phy name is defined in the
lookup, to that as well. That should cover both cases.


Thanks,

-- 
heikki

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/6] phy: improved lookup method
  2014-09-23 11:43                   ` Heikki Krogerus
@ 2014-09-24  9:44                     ` Kishon Vijay Abraham I
  2014-09-25  7:00                       ` Heikki Krogerus
  0 siblings, 1 reply; 31+ messages in thread
From: Kishon Vijay Abraham I @ 2014-09-24  9:44 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

Hi,

On Tuesday 23 September 2014 05:13 PM, Heikki Krogerus wrote:
> On Tue, Sep 23, 2014 at 04:33:09PM +0530, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Tuesday 23 September 2014 04:23 PM, Heikki Krogerus wrote:
>>> On Mon, Sep 22, 2014 at 05:07:55PM +0530, Kishon Vijay Abraham I wrote:
>>>> On Thursday 18 September 2014 03:55 PM, Heikki Krogerus wrote:
>>>>> On Mon, Sep 15, 2014 at 03:35:08PM +0300, Heikki Krogerus wrote:
>>>>>> On Fri, Sep 12, 2014 at 08:16:01PM +0530, Kishon Vijay Abraham I wrote:
>>>>>>> Assume you have 2 phys in your system..
>>>>>>> static struct phy_lookup usb_lookup = {
>>>>>>> 	.phy_name	= "phy-usb.0",
>>>>>>> 	.dev_id		= "usb.0",
>>>>>>> 	.con_id		= "usb",
>>>>>>> };
>>>>>>>
>>>>>>> static struct phy_lookup sata_lookup = {
>>>>>>> 	.phy_name	= "sata-usb.1",
>>>>>>> 	.dev_id		= "sata.0",
>>>>>>> 	.con_id		= "sata",
>>>>>>> };
>>>>>>>
>>>>>>> First you do modprobe phy-usb, the probe of USB PHY driver gets invoked and it
>>>>>>> creates the PHY. The phy-core will find a free id (now it will be 0) and then
>>>>>>> name the phy as phy-usb.0.
>>>>>>> Then with modprobe phy-sata, the phy-core will create phy-sata.1.
>>>>>>>
>>>>>>> This is an ideal case where the .phy_name in phy_lookup matches.
>>>>>>>
>>>>>>> Consider if the order is flipped and the user does modprobe phy-sata first. The
>>>>>>> phy_names won't match anymore (the sata phy device name would be "sata-usb.0").
>>>>>
>>>>> Actually, I don't think there would be this problem if we used the
>>>>> name of the actual device which is the parent of phy devices, right?
>>>>
>>>> hmm.. but if the parent is a multi-phy phy provider (like pipe3 PHY driver), we
>>>> might end up with the same problem.
>>>
>>> I'm not completely sure what you mean? If you are talking about
>>> platforms with multiple instances of a single phy, I don't see how
>>> there could ever be a scenario where we did not know the order in
>>> which they were enumerated. Can you give an example again?
>>
>> If a single IP implements multiple PHYs (phy-miphy365x.c in linux-phy next),
>> the parent for all the phy devices would be the same.
> 
> OK, got it. So I guess we need to match to the phy dev and to the phy
> name. First to the dev and then in case the phy name is defined in the
> lookup, to that as well. That should cover both cases.

So what would be the phy name? I mean it's completely user-defined or it's
derived from device name?

Isn't making the PHY to be aware of it's user much simpler?

Thanks
Kishon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/6] phy: improved lookup method
  2014-09-24  9:44                     ` Kishon Vijay Abraham I
@ 2014-09-25  7:00                       ` Heikki Krogerus
  2014-09-29  6:28                         ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 31+ messages in thread
From: Heikki Krogerus @ 2014-09-25  7:00 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb

> >>>>>>> Assume you have 2 phys in your system..
> >>>>>>> static struct phy_lookup usb_lookup = {
> >>>>>>> 	.phy_name	= "phy-usb.0",
> >>>>>>> 	.dev_id		= "usb.0",
> >>>>>>> 	.con_id		= "usb",
> >>>>>>> };
> >>>>>>>
> >>>>>>> static struct phy_lookup sata_lookup = {
> >>>>>>> 	.phy_name	= "sata-usb.1",
> >>>>>>> 	.dev_id		= "sata.0",
> >>>>>>> 	.con_id		= "sata",
> >>>>>>> };
> >>>>>>>
> >>>>>>> First you do modprobe phy-usb, the probe of USB PHY driver gets invoked and it
> >>>>>>> creates the PHY. The phy-core will find a free id (now it will be 0) and then
> >>>>>>> name the phy as phy-usb.0.
> >>>>>>> Then with modprobe phy-sata, the phy-core will create phy-sata.1.
> >>>>>>>
> >>>>>>> This is an ideal case where the .phy_name in phy_lookup matches.
> >>>>>>>
> >>>>>>> Consider if the order is flipped and the user does modprobe phy-sata first. The
> >>>>>>> phy_names won't match anymore (the sata phy device name would be "sata-usb.0").
> >>>>>
> >>>>> Actually, I don't think there would be this problem if we used the
> >>>>> name of the actual device which is the parent of phy devices, right?
> >>>>
> >>>> hmm.. but if the parent is a multi-phy phy provider (like pipe3 PHY driver), we
> >>>> might end up with the same problem.
> >>>
> >>> I'm not completely sure what you mean? If you are talking about
> >>> platforms with multiple instances of a single phy, I don't see how
> >>> there could ever be a scenario where we did not know the order in
> >>> which they were enumerated. Can you give an example again?
> >>
> >> If a single IP implements multiple PHYs (phy-miphy365x.c in linux-phy next),
> >> the parent for all the phy devices would be the same.

Hold on...

Let's take a step back here. Where could we actually have a scenario
where the phy device, the dev_id (consumer) and the con_id would all
be the same? There can't be such a case.

It's not like you could ever have a driver requesting multiple phys
with the same con_id. You would just get the same phy handle even if
you used dt.

        phy1 = phy_get(dev, "phy");
        ...
        phy2 = phy_get(dev, "phy");

And if the drivers requesting those phys are different, your consumers
are different.

> Isn't making the PHY to be aware of it's user much simpler?

No it's not. I'm not going into this again. We have already gone
through this in the past.


Cheers,

-- 
heikki

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/6] phy: improved lookup method
  2014-09-25  7:00                       ` Heikki Krogerus
@ 2014-09-29  6:28                         ` Kishon Vijay Abraham I
  0 siblings, 0 replies; 31+ messages in thread
From: Kishon Vijay Abraham I @ 2014-09-29  6:28 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg Kroah-Hartman, Felipe Balbi, Vivek Gautam, linux-kernel, linux-usb



On Thursday 25 September 2014 12:30 PM, Heikki Krogerus wrote:
>>>>>>>>> Assume you have 2 phys in your system..
>>>>>>>>> static struct phy_lookup usb_lookup = {
>>>>>>>>> 	.phy_name	= "phy-usb.0",
>>>>>>>>> 	.dev_id		= "usb.0",
>>>>>>>>> 	.con_id		= "usb",
>>>>>>>>> };
>>>>>>>>>
>>>>>>>>> static struct phy_lookup sata_lookup = {
>>>>>>>>> 	.phy_name	= "sata-usb.1",
>>>>>>>>> 	.dev_id		= "sata.0",
>>>>>>>>> 	.con_id		= "sata",
>>>>>>>>> };
>>>>>>>>>
>>>>>>>>> First you do modprobe phy-usb, the probe of USB PHY driver gets invoked and it
>>>>>>>>> creates the PHY. The phy-core will find a free id (now it will be 0) and then
>>>>>>>>> name the phy as phy-usb.0.
>>>>>>>>> Then with modprobe phy-sata, the phy-core will create phy-sata.1.
>>>>>>>>>
>>>>>>>>> This is an ideal case where the .phy_name in phy_lookup matches.
>>>>>>>>>
>>>>>>>>> Consider if the order is flipped and the user does modprobe phy-sata first. The
>>>>>>>>> phy_names won't match anymore (the sata phy device name would be "sata-usb.0").
>>>>>>>
>>>>>>> Actually, I don't think there would be this problem if we used the
>>>>>>> name of the actual device which is the parent of phy devices, right?
>>>>>>
>>>>>> hmm.. but if the parent is a multi-phy phy provider (like pipe3 PHY driver), we
>>>>>> might end up with the same problem.
>>>>>
>>>>> I'm not completely sure what you mean? If you are talking about
>>>>> platforms with multiple instances of a single phy, I don't see how
>>>>> there could ever be a scenario where we did not know the order in
>>>>> which they were enumerated. Can you give an example again?
>>>>
>>>> If a single IP implements multiple PHYs (phy-miphy365x.c in linux-phy next),
>>>> the parent for all the phy devices would be the same.
> 
> Hold on...
> 
> Let's take a step back here. Where could we actually have a scenario
> where the phy device, the dev_id (consumer) and the con_id would all
> be the same? There can't be such a case.
> 
> It's not like you could ever have a driver requesting multiple phys
> with the same con_id. You would just get the same phy handle even if
> you used dt.
> 
>         phy1 = phy_get(dev, "phy");
>         ...
>         phy2 = phy_get(dev, "phy");
> 
> And if the drivers requesting those phys are different, your consumers
> are different.

sounds right to me.

Cheers
Kishon

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2014-09-29  6:28 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-21 11:33 [PATCHv3 0/6] phy: simplified phy lookup Heikki Krogerus
2014-08-21 11:33 ` [PATCH 1/6] phy: safer to_phy() macro Heikki Krogerus
2014-08-21 11:33 ` [PATCH 2/6] phy: improved lookup method Heikki Krogerus
2014-09-11 15:33   ` Kishon Vijay Abraham I
2014-09-12 14:07     ` Heikki Krogerus
2014-09-12 14:46       ` Kishon Vijay Abraham I
2014-09-15 12:35         ` Heikki Krogerus
2014-09-18 10:25           ` Heikki Krogerus
2014-09-22 11:37             ` Kishon Vijay Abraham I
2014-09-23 10:53               ` Heikki Krogerus
2014-09-23 11:03                 ` Kishon Vijay Abraham I
2014-09-23 11:43                   ` Heikki Krogerus
2014-09-24  9:44                     ` Kishon Vijay Abraham I
2014-09-25  7:00                       ` Heikki Krogerus
2014-09-29  6:28                         ` Kishon Vijay Abraham I
2014-08-21 11:33 ` [PATCH 3/6] arm: omap3: twl: use the new lookup method with usb phy Heikki Krogerus
2014-09-11 15:26   ` Kishon Vijay Abraham I
2014-09-12 13:50     ` Heikki Krogerus
2014-08-21 11:33 ` [PATCH 4/6] phy: remove the old lookup method Heikki Krogerus
2014-08-25  7:41   ` Vivek Gautam
2014-08-25  8:17     ` Heikki Krogerus
2014-08-25  8:25       ` Vivek Gautam
2014-08-26  8:27         ` [PATCHv4 " Heikki Krogerus
2014-08-21 11:33 ` [PATCH 5/6] base: platform: name the device already during allocation Heikki Krogerus
2014-08-21 11:33 ` [PATCH 6/6] usb: dwc3: host: convey the PHYs to xhci Heikki Krogerus
2014-09-11 15:01   ` Kishon Vijay Abraham I
2014-09-12 13:49     ` Heikki Krogerus
2014-09-12 14:11       ` Kishon Vijay Abraham I
2014-09-15 12:06         ` Heikki Krogerus
2014-09-16  6:37           ` Kishon Vijay Abraham I
2014-09-16 12:13             ` Heikki Krogerus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).