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 v3:
- We can't rely on the order in which the phys are registered, so
using the name of the parent of the phy instance for matching
instead of the phy itself. The parent device is always the actual
physical device.
- Using PHY_LOOKUP macro in twl-common.c as suggested by Kishon.
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 | 17 ++--
drivers/base/platform.c | 69 +++++++++-------
drivers/phy/phy-bcm-kona-usb2.c | 2 +-
drivers/phy/phy-berlin-sata.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-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-rcar-gen2.c | 2 +-
drivers/phy/phy-samsung-usb2.c | 3 +-
drivers/phy/phy-spear1310-miphy.c | 2 +-
drivers/phy/phy-spear1340-miphy.c | 2 +-
drivers/phy/phy-stih407-usb.c | 2 +-
drivers/phy/phy-stih41x-usb.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 +-
drivers/usb/dwc3/host.c | 22 +++--
include/linux/phy/phy.h | 61 +++++++-------
29 files changed, 263 insertions(+), 182 deletions(-)
--
2.1.1
This makes to_phy() macro work with other variable names
besides "dev".
Signed-off-by: Heikki Krogerus <[email protected]>
Tested-by: Vivek Gautam <[email protected]>
---
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.1
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 <[email protected]>
Tested-by: Vivek Gautam <[email protected]>
---
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..c8d0f66 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.parent);
+ 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.parent)) &&
+ !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->parent), 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.1
The users of the old method are now converted to the new one.
Signed-off-by: Heikki Krogerus <[email protected]>
Tested-by: Vivek Gautam <[email protected]>
---
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-rcar-gen2.c | 2 +-
drivers/phy/phy-samsung-usb2.c | 3 +--
drivers/phy/phy-spear1310-miphy.c | 2 +-
drivers/phy/phy-spear1340-miphy.c | 2 +-
drivers/phy/phy-stih407-usb.c | 2 +-
drivers/phy/phy-stih41x-usb.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 ++++--------------------------
25 files changed, 31 insertions(+), 106 deletions(-)
diff --git a/drivers/phy/phy-bcm-kona-usb2.c b/drivers/phy/phy-bcm-kona-usb2.c
index c1e0ca3..ef2dc1a 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 69ced52..99bbf91 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 c8d0f66..0b279d3 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 84f49e5..f86cbe6 100644
--- a/drivers/phy/phy-exynos-dp-video.c
+++ b/drivers/phy/phy-exynos-dp-video.c
@@ -112,7 +112,7 @@ static int exynos_dp_video_phy_probe(struct platform_device *pdev)
match = of_match_node(exynos_dp_video_phy_of_match, dev->of_node);
state->drvdata = match->data;
- 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 6a9bef1..943e0f8 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 f756aca..b3ca3bc 100644
--- a/drivers/phy/phy-exynos5-usbdrd.c
+++ b/drivers/phy/phy-exynos5-usbdrd.c
@@ -637,8 +637,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 54cf4ae..bc858cc 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 d5d9780..a80ff9d 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 801afaf..239930e 100644
--- a/drivers/phy/phy-miphy365x.c
+++ b/drivers/phy/phy-miphy365x.c
@@ -593,7 +593,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 d395558..03b94f9 100644
--- a/drivers/phy/phy-mvebu-sata.c
+++ b/drivers/phy/phy-mvebu-sata.c
@@ -101,7 +101,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 8c84298..3fcc420 100644
--- a/drivers/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -259,7 +259,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 7b3ddfb..4b243f7 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 759b0bf..6f2fe26 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-rcar-gen2.c b/drivers/phy/phy-rcar-gen2.c
index 2793af1..778276a 100644
--- a/drivers/phy/phy-rcar-gen2.c
+++ b/drivers/phy/phy-rcar-gen2.c
@@ -304,7 +304,7 @@ static int rcar_gen2_phy_probe(struct platform_device *pdev)
phy->select_value = select_value[channel_num][n];
phy->phy = devm_phy_create(dev, NULL,
- &rcar_gen2_phy_ops, NULL);
+ &rcar_gen2_phy_ops);
if (IS_ERR(phy->phy)) {
dev_err(dev, "Failed to create PHY\n");
return PTR_ERR(phy->phy);
diff --git a/drivers/phy/phy-samsung-usb2.c b/drivers/phy/phy-samsung-usb2.c
index 908949d..4a12f66 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 5f4c586..9f47fae 100644
--- a/drivers/phy/phy-spear1310-miphy.c
+++ b/drivers/phy/phy-spear1310-miphy.c
@@ -227,7 +227,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 1ecd094..e42bc20 100644
--- a/drivers/phy/phy-spear1340-miphy.c
+++ b/drivers/phy/phy-spear1340-miphy.c
@@ -259,7 +259,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-stih407-usb.c b/drivers/phy/phy-stih407-usb.c
index 42428d4..74f0fab 100644
--- a/drivers/phy/phy-stih407-usb.c
+++ b/drivers/phy/phy-stih407-usb.c
@@ -137,7 +137,7 @@ static int stih407_usb2_picophy_probe(struct platform_device *pdev)
}
phy_dev->param = res->start;
- phy = devm_phy_create(dev, NULL, &stih407_usb2_picophy_data, NULL);
+ phy = devm_phy_create(dev, NULL, &stih407_usb2_picophy_data);
if (IS_ERR(phy)) {
dev_err(dev, "failed to create Display Port PHY\n");
return PTR_ERR(phy);
diff --git a/drivers/phy/phy-stih41x-usb.c b/drivers/phy/phy-stih41x-usb.c
index 9f16cb8..4ab581e 100644
--- a/drivers/phy/phy-stih41x-usb.c
+++ b/drivers/phy/phy-stih41x-usb.c
@@ -148,7 +148,7 @@ static int stih41x_usb_phy_probe(struct platform_device *pdev)
return PTR_ERR(phy_dev->clk);
}
- phy = devm_phy_create(dev, NULL, &stih41x_usb_phy_ops, NULL);
+ phy = devm_phy_create(dev, NULL, &stih41x_usb_phy_ops);
if (IS_ERR(phy)) {
dev_err(dev, "failed to create phy\n");
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 0baf5ef..28df50d 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 ab1e22d..c297b7a 100644
--- a/drivers/phy/phy-ti-pipe3.c
+++ b/drivers/phy/phy-ti-pipe3.c
@@ -399,7 +399,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 7b04bef..d658d8c 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -644,7 +644,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)
@@ -655,7 +654,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;
@@ -680,7 +678,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 f8a51b1..29214a3 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 1631ec9..a84299b 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.1
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 <[email protected]>
Tested-by: Vivek Gautam <[email protected]>
Acked-by: Felipe Balbi <[email protected]>
---
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.1
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 <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
---
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 b2afc29..97b6da3 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -195,11 +195,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);
@@ -228,6 +258,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;
@@ -308,28 +342,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];
@@ -372,7 +384,6 @@ int platform_device_add(struct platform_device *pdev)
release_resource(r);
}
- err_out:
return ret;
}
EXPORT_SYMBOL_GPL(platform_device_add);
@@ -392,11 +403,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);
@@ -414,8 +420,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.1
Provide complete association for the phy and it's user
(musb) with the new phy lookup method.
Signed-off-by: Heikki Krogerus <[email protected]>
---
arch/arm/mach-omap2/twl-common.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
index b0d54da..0e4f3e3 100644
--- a/arch/arm/mach-omap2/twl-common.c
+++ b/arch/arm/mach-omap2/twl-common.c
@@ -91,18 +91,11 @@ 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_LOOKUP("twl4030_usb",
+ "musb-hdrc.0", "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 +218,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.1
On Fri, Oct 17, 2014 at 05:39:11PM +0300, Heikki Krogerus wrote:
> This makes to_phy() macro work with other variable names
> besides "dev".
>
> Signed-off-by: Heikki Krogerus <[email protected]>
> Tested-by: Vivek Gautam <[email protected]>
Acked-by: Felipe Balbi <[email protected]>
> ---
> 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.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
balbi
Hi Heikki,
On Fri, Oct 17, 2014 at 8:09 PM, Heikki Krogerus
<[email protected]> 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 <[email protected]>
> Tested-by: Vivek Gautam <[email protected]>
> ---
> 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..c8d0f66 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.parent);
> + 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.parent)) &&
> + !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))) {
We have the case with phy-exynos5-usbdrd driver, which registers two
phys usb2-phy and usb3-phy
both being used by xhci (after dwc3 creates a lookup table).
The phy_dev is coming same for both the PHYs, and that's the reason
when i try to get
"usb2-phy" and "usb3-phy" i end up getting only usb2-phy.
This is happening with V4 of this patch; V3 seems fine. The only
differnece i see is
we are creating the phy_lookup_table using phy_dev->parent.
Is there something that i am missing ?
> + if (!strcmp(dev_name(phy_dev->parent), 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.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
* Heikki Krogerus <[email protected]> [141017 07:43]:
> Provide complete association for the phy and it's user
> (musb) with the new phy lookup method.
>
> Signed-off-by: Heikki Krogerus <[email protected]>
Kishon, this looks OK for you to queue with other PHY patches:
Acked-by: Tony Lindgren <[email protected]>
> ---
> arch/arm/mach-omap2/twl-common.c | 17 ++++++-----------
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
> index b0d54da..0e4f3e3 100644
> --- a/arch/arm/mach-omap2/twl-common.c
> +++ b/arch/arm/mach-omap2/twl-common.c
> @@ -91,18 +91,11 @@ 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_LOOKUP("twl4030_usb",
> + "musb-hdrc.0", "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 +218,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.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
Hi Felipe,
On Friday 17 October 2014 08:09 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 <[email protected]>
> Tested-by: Vivek Gautam <[email protected]>
> Acked-by: Felipe Balbi <[email protected]>
Already see you acked-by, so I'm queueing this in my linux-phy tree.
Thanks
Kishon
> ---
> 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);
> }
>
Hi,
On Friday 31 October 2014 06:03 PM, Vivek Gautam wrote:
> Hi Heikki,
>
>
> On Fri, Oct 17, 2014 at 8:09 PM, Heikki Krogerus
> <[email protected]> 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 <[email protected]>
>> Tested-by: Vivek Gautam <[email protected]>
>> ---
>> 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..c8d0f66 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.parent);
>> + 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.parent)) &&
>> + !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))) {
>
> We have the case with phy-exynos5-usbdrd driver, which registers two
> phys usb2-phy and usb3-phy
> both being used by xhci (after dwc3 creates a lookup table).
>
> The phy_dev is coming same for both the PHYs, and that's the reason
> when i try to get
> "usb2-phy" and "usb3-phy" i end up getting only usb2-phy.
> This is happening with V4 of this patch; V3 seems fine. The only
> differnece i see is
> we are creating the phy_lookup_table using phy_dev->parent.
>
> Is there something that i am missing ?
looks like a genuine problem.
>
>> + if (!strcmp(dev_name(phy_dev->parent), pl->phy_name)) {
here there are two phys which has the same parent and the first one that
matches will be returned. Hence you always get "usb2-phy".
IIUC just with device names of parent, we won't be able to get the PHY. We need
another 'variable' to differentiate it's children. Or have *phy* pointer
directly in the lookup table like how clk driver does?
Thanks
Kishon
>> + 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.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
On Tue, Nov 11, 2014 at 12:12 PM, Kishon Vijay Abraham I <[email protected]> wrote:
> Hi,
>
> On Friday 31 October 2014 06:03 PM, Vivek Gautam wrote:
>> Hi Heikki,
>>
>>
>> On Fri, Oct 17, 2014 at 8:09 PM, Heikki Krogerus
>> <[email protected]> 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 <[email protected]>
>>> Tested-by: Vivek Gautam <[email protected]>
>>> ---
>>> 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..c8d0f66 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.parent);
>>> + 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.parent)) &&
>>> + !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))) {
>>
>> We have the case with phy-exynos5-usbdrd driver, which registers two
>> phys usb2-phy and usb3-phy
>> both being used by xhci (after dwc3 creates a lookup table).
>>
>> The phy_dev is coming same for both the PHYs, and that's the reason
>> when i try to get
>> "usb2-phy" and "usb3-phy" i end up getting only usb2-phy.
>> This is happening with V4 of this patch; V3 seems fine. The only
>> differnece i see is
>> we are creating the phy_lookup_table using phy_dev->parent.
>>
>> Is there something that i am missing ?
>
> looks like a genuine problem.
>>
>>> + if (!strcmp(dev_name(phy_dev->parent), pl->phy_name)) {
>
> here there are two phys which has the same parent and the first one that
> matches will be returned. Hence you always get "usb2-phy".
>
> IIUC just with device names of parent, we won't be able to get the PHY. We need
> another 'variable' to differentiate it's children.
> Or have *phy* pointer directly in the lookup table like how clk driver does?
We do create the lookup table with actual *phy* pointer isn't it ?
Like if you see Heikki's last patch in this series:
[PATCHv4 6/6] usb: dwc3: host: convey the PHYs to xhci
We do pass the usb2_phy and usb3_phy pointers to phy_create_lookup().
But while finding the phy (using phy_find()) we don't seem to be
utilizing these.
>
> Thanks
> Kishon
>>> + 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.1
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
Hi,
On Tuesday 11 November 2014 02:07 PM, Vivek Gautam wrote:
> On Tue, Nov 11, 2014 at 12:12 PM, Kishon Vijay Abraham I <[email protected]> wrote:
>> Hi,
>>
>> On Friday 31 October 2014 06:03 PM, Vivek Gautam wrote:
>>> Hi Heikki,
>>>
>>>
>>> On Fri, Oct 17, 2014 at 8:09 PM, Heikki Krogerus
>>> <[email protected]> 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 <[email protected]>
>>>> Tested-by: Vivek Gautam <[email protected]>
>>>> ---
>>>> 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..c8d0f66 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.parent);
>>>> + 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.parent)) &&
>>>> + !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))) {
>>>
>>> We have the case with phy-exynos5-usbdrd driver, which registers two
>>> phys usb2-phy and usb3-phy
>>> both being used by xhci (after dwc3 creates a lookup table).
>>>
>>> The phy_dev is coming same for both the PHYs, and that's the reason
>>> when i try to get
>>> "usb2-phy" and "usb3-phy" i end up getting only usb2-phy.
>>> This is happening with V4 of this patch; V3 seems fine. The only
>>> differnece i see is
>>> we are creating the phy_lookup_table using phy_dev->parent.
>>>
>>> Is there something that i am missing ?
>>
>> looks like a genuine problem.
>>>
>>>> + if (!strcmp(dev_name(phy_dev->parent), pl->phy_name)) {
>>
>> here there are two phys which has the same parent and the first one that
>> matches will be returned. Hence you always get "usb2-phy".
>>
>> IIUC just with device names of parent, we won't be able to get the PHY. We need
>> another 'variable' to differentiate it's children.
>
>> Or have *phy* pointer directly in the lookup table like how clk driver does?
>
> We do create the lookup table with actual *phy* pointer isn't it ?
> Like if you see Heikki's last patch in this series:
> [PATCHv4 6/6] usb: dwc3: host: convey the PHYs to xhci
> We do pass the usb2_phy and usb3_phy pointers to phy_create_lookup().
right, but phy_create_lookup doesn't store the *phy* pointer in the lookup table.
Thanks
Kishon
Hi Kishon,
On Tue, Nov 11, 2014 at 2:20 PM, Kishon Vijay Abraham I <[email protected]> wrote:
> Hi,
>
> On Tuesday 11 November 2014 02:07 PM, Vivek Gautam wrote:
>> On Tue, Nov 11, 2014 at 12:12 PM, Kishon Vijay Abraham I <[email protected]> wrote:
>>> Hi,
>>>
>>> On Friday 31 October 2014 06:03 PM, Vivek Gautam wrote:
>>>> Hi Heikki,
>>>>
>>>>
>>>> On Fri, Oct 17, 2014 at 8:09 PM, Heikki Krogerus
>>>> <[email protected]> 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 <[email protected]>
>>>>> Tested-by: Vivek Gautam <[email protected]>
>>>>> ---
>>>>> 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..c8d0f66 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.parent);
>>>>> + 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.parent)) &&
>>>>> + !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))) {
>>>>
>>>> We have the case with phy-exynos5-usbdrd driver, which registers two
>>>> phys usb2-phy and usb3-phy
>>>> both being used by xhci (after dwc3 creates a lookup table).
>>>>
>>>> The phy_dev is coming same for both the PHYs, and that's the reason
>>>> when i try to get
>>>> "usb2-phy" and "usb3-phy" i end up getting only usb2-phy.
>>>> This is happening with V4 of this patch; V3 seems fine. The only
>>>> differnece i see is
>>>> we are creating the phy_lookup_table using phy_dev->parent.
>>>>
>>>> Is there something that i am missing ?
>>>
>>> looks like a genuine problem.
>>>>
>>>>> + if (!strcmp(dev_name(phy_dev->parent), pl->phy_name)) {
>>>
>>> here there are two phys which has the same parent and the first one that
>>> matches will be returned. Hence you always get "usb2-phy".
>>>
>>> IIUC just with device names of parent, we won't be able to get the PHY. We need
>>> another 'variable' to differentiate it's children.
>>
>>> Or have *phy* pointer directly in the lookup table like how clk driver does?
>>
>> We do create the lookup table with actual *phy* pointer isn't it ?
>> Like if you see Heikki's last patch in this series:
>> [PATCHv4 6/6] usb: dwc3: host: convey the PHYs to xhci
>> We do pass the usb2_phy and usb3_phy pointers to phy_create_lookup().
>
> right, but phy_create_lookup doesn't store the *phy* pointer in the lookup table.
Right, so i think that's the place where we should rework to get the
correct phy.
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
Hi Heikki, Kishon,
On Tue, Nov 11, 2014 at 2:23 PM, Vivek Gautam <[email protected]> wrote:
> Hi Kishon,
>
>
> On Tue, Nov 11, 2014 at 2:20 PM, Kishon Vijay Abraham I <[email protected]> wrote:
>> Hi,
>>
>> On Tuesday 11 November 2014 02:07 PM, Vivek Gautam wrote:
>>> On Tue, Nov 11, 2014 at 12:12 PM, Kishon Vijay Abraham I <[email protected]> wrote:
>>>> Hi,
>>>>
>>>> On Friday 31 October 2014 06:03 PM, Vivek Gautam wrote:
>>>>> Hi Heikki,
>>>>>
>>>>>
>>>>> On Fri, Oct 17, 2014 at 8:09 PM, Heikki Krogerus
>>>>> <[email protected]> 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 <[email protected]>
>>>>>> Tested-by: Vivek Gautam <[email protected]>
>>>>>> ---
>>>>>> 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..c8d0f66 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.parent);
>>>>>> + 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.parent)) &&
>>>>>> + !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))) {
>>>>>
>>>>> We have the case with phy-exynos5-usbdrd driver, which registers two
>>>>> phys usb2-phy and usb3-phy
>>>>> both being used by xhci (after dwc3 creates a lookup table).
>>>>>
>>>>> The phy_dev is coming same for both the PHYs, and that's the reason
>>>>> when i try to get
>>>>> "usb2-phy" and "usb3-phy" i end up getting only usb2-phy.
>>>>> This is happening with V4 of this patch; V3 seems fine. The only
>>>>> differnece i see is
>>>>> we are creating the phy_lookup_table using phy_dev->parent.
>>>>>
>>>>> Is there something that i am missing ?
>>>>
>>>> looks like a genuine problem.
>>>>>
>>>>>> + if (!strcmp(dev_name(phy_dev->parent), pl->phy_name)) {
>>>>
>>>> here there are two phys which has the same parent and the first one that
>>>> matches will be returned. Hence you always get "usb2-phy".
>>>>
>>>> IIUC just with device names of parent, we won't be able to get the PHY. We need
>>>> another 'variable' to differentiate it's children.
>>>
>>>> Or have *phy* pointer directly in the lookup table like how clk driver does?
>>>
>>> We do create the lookup table with actual *phy* pointer isn't it ?
>>> Like if you see Heikki's last patch in this series:
>>> [PATCHv4 6/6] usb: dwc3: host: convey the PHYs to xhci
>>> We do pass the usb2_phy and usb3_phy pointers to phy_create_lookup().
>>
>> right, but phy_create_lookup doesn't store the *phy* pointer in the lookup table.
>
> Right, so i think that's the place where we should rework to get the
> correct phy.
How about adding the change in attached patch [1] on top of this patch.
Just introduced the phy pointer in "phy_lookup" structure, and
modified phy_find() accordingly.
[1] Attachment:
0001-phy-core-Use-phy-pointer-with-phy_lookup_table.patch
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
Hi Vivek,
On Thu, Nov 13, 2014 at 07:03:01PM +0530, Vivek Gautam wrote:
> Hi Heikki, Kishon,
>
> How about adding the change in attached patch [1] on top of this patch.
> Just introduced the phy pointer in "phy_lookup" structure, and
> modified phy_find() accordingly.
>
> [1] Attachment:
> 0001-phy-core-Use-phy-pointer-with-phy_lookup_table.patch
Just to let you know, I'm finally able to work on this again.
I was actually thinking that we should just roll back to the previous
version where I didn't yet use the parent name, and rethink how to
solve the problem Kishon pointed out.
But, I'll take a look at this properly on Monday.
Cheers,
--
heikki
On Fri, Nov 14, 2014 at 7:45 PM, Heikki Krogerus
<[email protected]> wrote:
> Hi Vivek,
>
> On Thu, Nov 13, 2014 at 07:03:01PM +0530, Vivek Gautam wrote:
>> Hi Heikki, Kishon,
>>
>> How about adding the change in attached patch [1] on top of this patch.
>> Just introduced the phy pointer in "phy_lookup" structure, and
>> modified phy_find() accordingly.
>>
>> [1] Attachment:
>> 0001-phy-core-Use-phy-pointer-with-phy_lookup_table.patch
>
> Just to let you know, I'm finally able to work on this again.
Great !!
>
> I was actually thinking that we should just roll back to the previous
> version where I didn't yet use the parent name, and rethink how to
> solve the problem Kishon pointed out.
>
> But, I'll take a look at this properly on Monday.
Cool, was just throwing some ideas, similar to clk driver, like Kishon
suggested.
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
On Thu, Nov 13, 2014 at 07:03:01PM +0530, Vivek Gautam wrote:
> How about adding the change in attached patch [1] on top of this patch.
> Just introduced the phy pointer in "phy_lookup" structure, and
> modified phy_find() accordingly.
I would be fine if we used the phy pointer to match, but if we have
the pointer to the phy in the lookup, then I would say we need to
remove the phy_name member.
Otherwise we would have to support two ways of finding the lookups
(one for the "static" lookups where we match to the phy_name and other
where we match to the pointer). That means we will also not be able to
create "static" lookups, but those would be only needed if we wanted to
create them in board files. I don't think there will ever be need to
create the lookups in board files, so I'm more then happy to remove
the static way of creating the lookups.
Kishon, if you are OK with this change, I'll prepare new version of
this set and use the idea. Vivek, you'll get the credit for it.
Thanks!
--
heikki
Hi,
On Monday 17 November 2014 09:10 PM, Heikki Krogerus wrote:
> On Thu, Nov 13, 2014 at 07:03:01PM +0530, Vivek Gautam wrote:
>> How about adding the change in attached patch [1] on top of this patch.
>> Just introduced the phy pointer in "phy_lookup" structure, and
>> modified phy_find() accordingly.
>
> I would be fine if we used the phy pointer to match, but if we have
> the pointer to the phy in the lookup, then I would say we need to
> remove the phy_name member.
>
> Otherwise we would have to support two ways of finding the lookups
> (one for the "static" lookups where we match to the phy_name and other
> where we match to the pointer). That means we will also not be able to
> create "static" lookups, but those would be only needed if we wanted to
> create them in board files. I don't think there will ever be need to
> create the lookups in board files, so I'm more then happy to remove
> the static way of creating the lookups.
Just using the phy pointer sounds good. But interested to know where the phy
pointer will be added to the lookup table.
Thanks
Kishon
On Tue, Nov 18, 2014 at 10:49:18AM +0530, Kishon Vijay Abraham I wrote:
> On Monday 17 November 2014 09:10 PM, Heikki Krogerus wrote:
> > On Thu, Nov 13, 2014 at 07:03:01PM +0530, Vivek Gautam wrote:
> >> How about adding the change in attached patch [1] on top of this patch.
> >> Just introduced the phy pointer in "phy_lookup" structure, and
> >> modified phy_find() accordingly.
> >
> > I would be fine if we used the phy pointer to match, but if we have
> > the pointer to the phy in the lookup, then I would say we need to
> > remove the phy_name member.
> >
> > Otherwise we would have to support two ways of finding the lookups
> > (one for the "static" lookups where we match to the phy_name and other
> > where we match to the pointer). That means we will also not be able to
> > create "static" lookups, but those would be only needed if we wanted to
> > create them in board files. I don't think there will ever be need to
> > create the lookups in board files, so I'm more then happy to remove
> > the static way of creating the lookups.
>
> Just using the phy pointer sounds good. But interested to know where the phy
> pointer will be added to the lookup table.
I'm making assumption that there will not be any (new) platforms where
the bindings are not provided in HW descriptions like dt or ACPI
tables. That leaves the lookups to be useful only in cases where a
driver has to, for whatever reason, pass it's phy's forward, like
dwc3 host. ULPI will be an other case where we will need to use them.
Now the only user for the lookups is twl4030-usb, but since it's
actually just ULPI type PHY in cases where we have platform data to
deal with (please correct me if I'm wrong), I think we could later
remove it's need for platform data completely with the help of the
ULPI bus I'm working with.
Cheers,
--
heikki
Hi,
On Tuesday 18 November 2014 09:06 PM, Heikki Krogerus wrote:
> On Tue, Nov 18, 2014 at 10:49:18AM +0530, Kishon Vijay Abraham I wrote:
>> On Monday 17 November 2014 09:10 PM, Heikki Krogerus wrote:
>>> On Thu, Nov 13, 2014 at 07:03:01PM +0530, Vivek Gautam wrote:
>>>> How about adding the change in attached patch [1] on top of this patch.
>>>> Just introduced the phy pointer in "phy_lookup" structure, and
>>>> modified phy_find() accordingly.
>>>
>>> I would be fine if we used the phy pointer to match, but if we have
>>> the pointer to the phy in the lookup, then I would say we need to
>>> remove the phy_name member.
>>>
>>> Otherwise we would have to support two ways of finding the lookups
>>> (one for the "static" lookups where we match to the phy_name and other
>>> where we match to the pointer). That means we will also not be able to
>>> create "static" lookups, but those would be only needed if we wanted to
>>> create them in board files. I don't think there will ever be need to
>>> create the lookups in board files, so I'm more then happy to remove
>>> the static way of creating the lookups.
>>
>> Just using the phy pointer sounds good. But interested to know where the phy
>> pointer will be added to the lookup table.
>
> I'm making assumption that there will not be any (new) platforms where
> the bindings are not provided in HW descriptions like dt or ACPI
> tables. That leaves the lookups to be useful only in cases where a
> driver has to, for whatever reason, pass it's phy's forward, like
> dwc3 host. ULPI will be an other case where we will need to use them.
right.
>
> Now the only user for the lookups is twl4030-usb, but since it's
> actually just ULPI type PHY in cases where we have platform data to
> deal with (please correct me if I'm wrong), I think we could later
you mean passing consumer information using platform data?
> remove it's need for platform data completely with the help of the
> ULPI bus I'm working with.
Sure.
Thanks
Kishon
Hi,
On Tuesday 18 November 2014 09:06 PM, Heikki Krogerus wrote:
> On Tue, Nov 18, 2014 at 10:49:18AM +0530, Kishon Vijay Abraham I wrote:
>> On Monday 17 November 2014 09:10 PM, Heikki Krogerus wrote:
>>> On Thu, Nov 13, 2014 at 07:03:01PM +0530, Vivek Gautam wrote:
>>>> How about adding the change in attached patch [1] on top of this patch.
>>>> Just introduced the phy pointer in "phy_lookup" structure, and
>>>> modified phy_find() accordingly.
>>>
>>> I would be fine if we used the phy pointer to match, but if we have
>>> the pointer to the phy in the lookup, then I would say we need to
>>> remove the phy_name member.
>>>
>>> Otherwise we would have to support two ways of finding the lookups
>>> (one for the "static" lookups where we match to the phy_name and other
>>> where we match to the pointer). That means we will also not be able to
>>> create "static" lookups, but those would be only needed if we wanted to
>>> create them in board files. I don't think there will ever be need to
>>> create the lookups in board files, so I'm more then happy to remove
>>> the static way of creating the lookups.
>>
>> Just using the phy pointer sounds good. But interested to know where the phy
>> pointer will be added to the lookup table.
>
> I'm making assumption that there will not be any (new) platforms where
> the bindings are not provided in HW descriptions like dt or ACPI
> tables. That leaves the lookups to be useful only in cases where a
> driver has to, for whatever reason, pass it's phy's forward, like
> dwc3 host. ULPI will be an other case where we will need to use them.
right.
>
> Now the only user for the lookups is twl4030-usb, but since it's
> actually just ULPI type PHY in cases where we have platform data to
> deal with (please correct me if I'm wrong), I think we could later
You mean passing the consumer info using platform data?
> remove it's need for platform data completely with the help of the
> ULPI bus I'm working with.
Sure.
I'll be closing my phy tree by friday, so would be great if you can send your
patches before that with "Tested-by" from Vivek.
Thanks
Kishon