[PATCH v7 0/9] Generic PHY Framework

[PATCH v7 0/9] Generic PHY Framework

[Kishon Vijay Abraham I]

Added a generic PHY framework that provides a set of APIs for the PHY drivers
to create/destroy a PHY and APIs for the PHY users to obtain a reference to
the PHY with or without using phandle.

This framework will be of use only to devices that uses external PHY (PHY
functionality is not embedded within the controller).

The intention of creating this framework is to bring the phy drivers spread
all over the Linux kernel to drivers/phy to increase code re-use and to
increase code maintainability.

Comments to make PHY as bus wasn't done because PHY devices can be part of
other bus and making a same device attached to multiple bus leads to bad
design.

If the PHY driver has to send notification on connect/disconnect, the PHY
driver should make use of the extcon framework. Using this susbsystem 
to use extcon framwork will have to be analysed.

Making omap-usb2 and twl4030 to use this framework is provided as a sample.

This patch series is developed on linux mainline tree.

Changes from v6
* corrected few typos in Documentation
* Changed PHY Subsystem to *bool* in Kconfig (to avoid compilation errors when
  PHY Subsystem is kept as module and the dependent modules are built-in)
* Added if pm_runtime_enabled check before runtime pm calls.

Changes from v5:
* removed the new sysfs entries as it dint have any new information other than
  what is already there in /sys/devices/...
* removed a bunch of APIs added to get the PHY and now only phy_get and
  devm_phy_get are used.
* Added new APIs to register/unregister the PHY provider. This is needed for
  dt boot case.
* Enabled pm runtime and incorporated the comments given by Alan Stern in a
  different patch series by Gautam.
* Removed the *phy_bind* API. Now the phy binding information should be passed
  using the platform data to the controller devices.
* Fixed a few typos.

Changes from v4:
* removed of_phy_get_with_args/devm_of_phy_get_with_args. Now the *phy providers*
  should use their custom implementation of of_xlate or use of_phy_xlate to get
  *phy instance* from *phy providers*.
* Added of_phy_xlate to be used by *phy providers* if it provides only one PHY.
* changed phy_core from having subsys_initcall to module_init.
* other minor fixes.

Changes from v3:
* Changed the return value of PHY APIs to ENOSYS
* Added APIs of_phy_get_with_args/devm_of_phy_get_with_args to support getting
  PHYs if the same IP implements multiple PHYs.
* modified phy_bind API so that the binding information can now be _updated_.
  In effect of this removed the binding information added in board files and
  added only in usb-musb.c. If a particular board uses a different phy binding,
  it can update it in board file after usb_musb_init().
* Added Documentation/devicetree/bindings/phy/phy-bindings.txt for dt binding
  information.

Changes from v2:
* removed phy_descriptor structure completely so changed the APIs which were
  taking phy_descriptor as parameters
* Added 2 more APIs *of_phy_get_byname* and *devm_of_phy_get_byname* to be used
  by PHY user drivers which has *phy* and *phy-names* binding in the dt data
* Fixed a few typos
* Removed phy_list and we now use class_dev_iter_init, class_dev_iter_next and
  class_dev_iter_exit for traversing through the phy list. (Note we still need
  phy_bind list and phy_bind_mutex).
* Changed the sysfs entry name from *bind* to *phy_bind*.

Changes from v1:
* Added Documentation for the PHY framework
* Added few more APIs mostly w.r.t devres
* Modified omap-usb2 and twl4030 to make use of the new framework

Did USB enumeration testing in panda and beagle.

Kishon Vijay Abraham I (9):
  drivers: phy: add generic PHY framework
  usb: phy: omap-usb2: use the new generic PHY framework
  usb: phy: twl4030: use the new generic PHY framework
  usb: phy: twl4030: twl4030 shouldn't be subsys_initcall
  ARM: OMAP: USB: Add phy binding information
  ARM: dts: omap: update usb_otg_hs data
  usb: musb: omap2430: use the new generic PHY framework
  usb: phy: omap-usb2: remove *set_suspend* callback from omap-usb2
  usb: phy: twl4030-usb: remove *set_suspend* and *phy_init* ops

 .../devicetree/bindings/phy/phy-bindings.txt       |   66 +++
 Documentation/devicetree/bindings/usb/omap-usb.txt |    5 +
 Documentation/devicetree/bindings/usb/usb-phy.txt  |    6 +
 Documentation/phy.txt                              |  124 +++++
 MAINTAINERS                                        |    7 +
 arch/arm/boot/dts/omap3-beagle-xm.dts              |    2 +
 arch/arm/boot/dts/omap3-evm.dts                    |    2 +
 arch/arm/boot/dts/omap3-overo.dtsi                 |    2 +
 arch/arm/boot/dts/omap4.dtsi                       |    3 +
 arch/arm/boot/dts/twl4030.dtsi                     |    1 +
 arch/arm/mach-omap2/usb-musb.c                     |    6 +-
 drivers/Kconfig                                    |    2 +
 drivers/Makefile                                   |    2 +
 drivers/phy/Kconfig                                |   13 +
 drivers/phy/Makefile                               |    5 +
 drivers/phy/phy-core.c                             |  543 ++++++++++++++++++++
 drivers/usb/musb/musb_core.c                       |    1 +
 drivers/usb/musb/musb_core.h                       |    3 +
 drivers/usb/musb/omap2430.c                        |   29 +-
 drivers/usb/phy/phy-omap-usb2.c                    |   54 +-
 drivers/usb/phy/phy-twl4030-usb.c                  |   73 +--
 include/linux/phy/phy.h                            |  284 ++++++++++
 include/linux/usb/musb.h                           |    3 +
 23 files changed, 1172 insertions(+), 64 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-bindings.txt
 create mode 100644 Documentation/phy.txt
 create mode 100644 drivers/phy/Kconfig
 create mode 100644 drivers/phy/Makefile
 create mode 100644 drivers/phy/phy-core.c
 create mode 100644 include/linux/phy/phy.h

-- 
1.7.10.4

[PATCH v7 1/9] drivers: phy: add generic PHY framework

[Kishon Vijay Abraham I]

The PHY framework provides a set of APIs for the PHY drivers to
create/destroy a PHY and APIs for the PHY users to obtain a reference to the
PHY with or without using phandle. For dt-boot, the PHY drivers should
also register *PHY provider* with the framework.

PHY drivers should create the PHY by passing id and ops like init, exit,
power_on and power_off. This framework is also pm runtime enabled.

The documentation for the generic PHY framework is added in
Documentation/phy.txt and the documentation for dt binding can be found at
Documentation/devicetree/bindings/phy/phy-bindings.txt

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 .../devicetree/bindings/phy/phy-bindings.txt       |   66 +++
 Documentation/phy.txt                              |  124 +++++
 MAINTAINERS                                        |    7 +
 drivers/Kconfig                                    |    2 +
 drivers/Makefile                                   |    2 +
 drivers/phy/Kconfig                                |   13 +
 drivers/phy/Makefile                               |    5 +
 drivers/phy/phy-core.c                             |  543 ++++++++++++++++++++
 include/linux/phy/phy.h                            |  284 ++++++++++
 9 files changed, 1046 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-bindings.txt
 create mode 100644 Documentation/phy.txt
 create mode 100644 drivers/phy/Kconfig
 create mode 100644 drivers/phy/Makefile
 create mode 100644 drivers/phy/phy-core.c
 create mode 100644 include/linux/phy/phy.h

diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt b/Documentation/devicetree/bindings/phy/phy-bindings.txt
new file mode 100644
index 0000000..8ae844f
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
@@ -0,0 +1,66 @@
+This document explains only the device tree data binding. For general
+information about PHY subsystem refer to Documentation/phy.txt
+
+PHY device node
+===============
+
+Required Properties:
+#phy-cells:	Number of cells in a PHY specifier;  The meaning of all those
+		cells is defined by the binding for the phy node. The PHY
+		provider can use the values in cells to find the appropriate
+		PHY.
+
+For example:
+
+phys: phy {
+    compatible = "xxx";
+    reg = <...>;
+    .
+    .
+    #phy-cells = <1>;
+    .
+    .
+};
+
+That node describes an IP block (PHY provider) that implements 2 different PHYs.
+In order to differentiate between these 2 PHYs, an additonal specifier should be
+given while trying to get a reference to it.
+
+PHY user node
+=============
+
+Required Properties:
+phys : the phandle for the PHY device (used by the PHY subsystem)
+phy-names : the names of the PHY corresponding to the PHYs present in the
+	    *phys* phandle
+
+Example 1:
+usb1: usb_otg_ss@xxx {
+    compatible = "xxx";
+    reg = <xxx>;
+    .
+    .
+    phys = <&usb2_phy>, <&usb3_phy>;
+    phy-names = "usb2phy", "usb3phy";
+    .
+    .
+};
+
+This node represents a controller that uses two PHYs, one for usb2 and one for
+usb3.
+
+Example 2:
+usb2: usb_otg_ss@xxx {
+    compatible = "xxx";
+    reg = <xxx>;
+    .
+    .
+    phys = <&phys 1>;
+    phy-names = "usbphy";
+    .
+    .
+};
+
+This node represents a controller that uses one of the PHYs of the PHY provider
+device defined previously. Note that the phy handle has an additional specifier
+"1" to differentiate between the two PHYs.
diff --git a/Documentation/phy.txt b/Documentation/phy.txt
new file mode 100644
index 0000000..059ef0e
--- /dev/null
+++ b/Documentation/phy.txt
@@ -0,0 +1,124 @@
+			    PHY SUBSYSTEM
+		  Kishon Vijay Abraham I <kishon@ti.com>
+
+This document explains the Generic PHY Framework along with the APIs provided,
+and how-to-use.
+
+1. Introduction
+
+*PHY* is the abbreviation for physical layer. It is used to connect a device
+to the physical medium e.g., the USB controller has a PHY to provide functions
+such as serialization, de-serialization, encoding, decoding and is responsible
+for obtaining the required data transmission rate. Note that some USB
+controllers have PHY functionality embedded into it and others use an external
+PHY. Other peripherals that use PHY include Wireless LAN, Ethernet,
+SATA etc.
+
+The intention of creating this framework is to bring the PHY drivers spread
+all over the Linux kernel to drivers/phy to increase code re-use and for
+better code maintainability.
+
+This framework will be of use only to devices that use external PHY (PHY
+functionality is not embedded within the controller).
+
+2. Registering/Unregistering the PHY provider
+
+PHY provider refers to an entity that implements one or more PHY instances.
+For the simple case where the PHY provider implements only a single instance of
+the PHY, the framework provides its own implementation of of_xlate in
+of_phy_simple_xlate. If the PHY provider implements multiple instances, it
+should provide its own implementation of of_xlate. of_xlate is used only for
+dt boot case.
+
+struct phy_provider *of_phy_provider_register(struct device *dev,
+	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+	struct of_phandle_args *args));
+struct phy_provider *devm_of_phy_provider_register(struct device *dev,
+	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+	struct of_phandle_args *args))
+
+of_phy_provider_register and devm_of_phy_provider_register can be used to
+register the phy_provider and it takes device, owner and of_xlate as
+arguments. For the dt boot case, all PHY providers should use one of the above
+2 APIs to register the PHY provider.
+
+void devm_of_phy_provider_unregister(struct device *dev,
+	struct phy_provider *phy_provider);
+void of_phy_provider_unregister(struct phy_provider *phy_provider);
+
+devm_of_phy_provider_unregister and of_phy_provider_unregister can be used to
+unregister the PHY.
+
+3. Creating the PHY
+
+The PHY driver should create the PHY in order for other peripheral controllers
+to make use of it. The PHY framework provides 2 APIs to create the PHY.
+
+struct phy *phy_create(struct device *dev, int id, const struct phy_ops *ops,
+	void *priv);
+struct phy *devm_phy_create(struct device *dev, int id,
+	const struct phy_ops *ops, void *priv);
+
+The PHY drivers can use one of the above 2 APIs to create the PHY by passing
+the device pointer, id, phy ops and a driver data.
+phy_ops is a set of function pointers for performing PHY operations such as
+init, exit, power_on and power_off.
+
+4. Getting a reference to the PHY
+
+Before the controller can make use of the PHY, it has to get a reference to
+it. This framework provides the following APIs to get a reference to the PHY.
+
+struct phy *phy_get(struct device *dev, const char *string);
+struct phy *devm_phy_get(struct device *dev, const char *string);
+
+phy_get and devm_phy_get can be used to get the PHY. In the case of dt boot,
+the string arguments should contain the phy name as given in the dt data and
+in the case of non-dt boot, it should contain the label of the PHY.
+The only difference between the two APIs is that devm_phy_get associates the
+device with the PHY using devres on successful PHY get. On driver detach,
+release function is invoked on the the devres data and devres data is freed.
+
+5. Releasing a reference to the PHY
+
+When the controller no longer needs the PHY, it has to release the reference
+to the PHY it has obtained using the APIs mentioned in the above section. The
+PHY framework provides 2 APIs to release a reference to the PHY.
+
+void phy_put(struct phy *phy);
+void devm_phy_put(struct device *dev, struct phy *phy);
+
+Both these APIs are used to release a reference to the PHY and devm_phy_put
+destroys the devres associated with this PHY.
+
+6. Destroying the PHY
+
+When the driver that created the PHY is unloaded, it should destroy the PHY it
+created using one of the following 2 APIs.
+
+void phy_destroy(struct phy *phy);
+void devm_phy_destroy(struct device *dev, struct phy *phy);
+
+Both these APIs destroy the PHY and devm_phy_destroy destroys the devres
+associated with this PHY.
+
+7. PM Runtime
+
+This subsystem is pm runtime enabled. So while creating the PHY,
+pm_runtime_enable of the phy device created by this subsystem is called and
+while destroying the PHY, pm_runtime_disable is called. Note that the phy
+device created by this subsystem will be a child of the device that calls
+phy_create (PHY provider device).
+
+During phy_init, the clocks are enabled by calling pm_runtime_get_sync and
+the clocks are disabled by calling pm_runtime_put_sync during phy_exit.
+pm_runtime_get_sync of the phy_device created by this subsystem will invoke
+pm_runtime_get_sync of PHY provider device because of parent-child relationship.
+For all other pm operations, 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.
+
+8. DeviceTree Binding
+
+The documentation for PHY dt binding can be found @
+Documentation/devicetree/bindings/phy/phy-bindings.txt
diff --git a/MAINTAINERS b/MAINTAINERS
index 0c9dc71..c234461 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3536,6 +3536,13 @@ S:	Maintained
 F:	include/asm-generic
 F:	include/uapi/asm-generic
 
+GENERIC PHY FRAMEWORK
+M:	Kishon Vijay Abraham I <kishon@ti.com>
+L:	linux-kernel@vger.kernel.org
+S:	Supported
+F:	drivers/phy/
+F:	include/linux/phy/
+
 GENERIC UIO DRIVER FOR PCI DEVICES
 M:	"Michael S. Tsirkin" <mst@redhat.com>
 L:	kvm@vger.kernel.org
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 9953a42..2943fb6 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -166,4 +166,6 @@ source "drivers/ipack/Kconfig"
 
 source "drivers/reset/Kconfig"
 
+source "drivers/phy/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 130abc1..ce9f0d2 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -8,6 +8,8 @@
 obj-y				+= irqchip/
 obj-y				+= bus/
 
+obj-y				+= phy/
+
 # GPIO must come after pinctrl as gpios may need to mux pins etc
 obj-y				+= pinctrl/
 obj-y				+= gpio/
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
new file mode 100644
index 0000000..0764a54
--- /dev/null
+++ b/drivers/phy/Kconfig
@@ -0,0 +1,13 @@
+#
+# PHY
+#
+
+menuconfig GENERIC_PHY
+	bool "PHY Subsystem"
+	help
+	  Generic PHY support.
+
+	  This framework is designed to provide a generic interface for PHY
+	  devices present in the kernel. This layer will have the generic
+	  API by which phy drivers can create PHY using the phy framework and
+	  phy users can obtain reference to the PHY.
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
new file mode 100644
index 0000000..9e9560f
--- /dev/null
+++ b/drivers/phy/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for the phy drivers.
+#
+
+obj-$(CONFIG_GENERIC_PHY)	+= phy-core.o
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
new file mode 100644
index 0000000..b599d0f
--- /dev/null
+++ b/drivers/phy/phy-core.c
@@ -0,0 +1,543 @@
+/*
+ * phy-core.c  --  Generic Phy framework.
+ *
+ * Copyright (C) 2013 Texas Instruments
+ *
+ * Author: Kishon Vijay Abraham I <kishon@ti.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/export.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/pm_runtime.h>
+
+static struct class *phy_class;
+static DEFINE_MUTEX(phy_provider_mutex);
+static LIST_HEAD(phy_provider_list);
+
+static void devm_phy_release(struct device *dev, void *res)
+{
+	struct phy *phy = *(struct phy **)res;
+
+	phy_put(phy);
+}
+
+static void devm_phy_provider_release(struct device *dev, void *res)
+{
+	struct phy_provider *phy_provider = *(struct phy_provider **)res;
+
+	of_phy_provider_unregister(phy_provider);
+}
+
+static void devm_phy_consume(struct device *dev, void *res)
+{
+	struct phy *phy = *(struct phy **)res;
+
+	phy_destroy(phy);
+}
+
+static int devm_phy_match(struct device *dev, void *res, void *match_data)
+{
+	return res == match_data;
+}
+
+static struct phy *phy_lookup(const char *phy_name)
+{
+	struct phy *phy;
+	struct device *dev;
+	struct class_dev_iter iter;
+
+	class_dev_iter_init(&iter, phy_class, NULL, NULL);
+	while ((dev = class_dev_iter_next(&iter))) {
+		phy = container_of(dev, struct phy, dev);
+		if (strcmp(phy->label, phy_name))
+			continue;
+
+		class_dev_iter_exit(&iter);
+		return phy;
+	}
+
+	class_dev_iter_exit(&iter);
+	return ERR_PTR(-ENODEV);
+}
+
+static struct phy_provider *of_phy_provider_lookup(struct device_node *node)
+{
+	struct phy_provider *phy_provider;
+
+	list_for_each_entry(phy_provider, &phy_provider_list, list) {
+		if (phy_provider->dev->of_node == node)
+			return phy_provider;
+	}
+
+	return ERR_PTR(-EPROBE_DEFER);
+}
+
+/**
+ * of_phy_get() - lookup and obtain a reference to a phy by phandle
+ * @dev: device that requests this phy
+ * @index: the index of the phy
+ *
+ * Returns the phy associated with the given phandle value,
+ * after getting a refcount to it or -ENODEV if there is no such phy or
+ * -EPROBE_DEFER if there is a phandle to the phy, but the device is
+ * not yet loaded. This function uses of_xlate call back function provided
+ * while registering the phy_provider to find the phy instance.
+ */
+static struct phy *of_phy_get(struct device *dev, int index)
+{
+	int ret;
+	struct phy_provider *phy_provider;
+	struct phy *phy = NULL;
+	struct of_phandle_args args;
+
+	ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
+		index, &args);
+	if (ret) {
+		dev_dbg(dev, "failed to get phy in %s node\n",
+			dev->of_node->full_name);
+		return ERR_PTR(-ENODEV);
+	}
+
+	mutex_lock(&phy_provider_mutex);
+	phy_provider = of_phy_provider_lookup(args.np);
+	if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner)) {
+		phy = ERR_PTR(-EPROBE_DEFER);
+		goto err0;
+	}
+
+	phy = phy_provider->of_xlate(phy_provider->dev, &args);
+	module_put(phy_provider->owner);
+
+err0:
+	mutex_unlock(&phy_provider_mutex);
+	of_node_put(args.np);
+
+	return phy;
+}
+
+/**
+ * phy_put() - release the PHY
+ * @phy: the phy returned by phy_get()
+ *
+ * Releases a refcount the caller received from phy_get().
+ */
+void phy_put(struct phy *phy)
+{
+	if (IS_ERR(phy))
+		return;
+
+	module_put(phy->ops->owner);
+	put_device(&phy->dev);
+}
+EXPORT_SYMBOL_GPL(phy_put);
+
+/**
+ * devm_phy_put() - release the PHY
+ * @dev: device that wants to release this phy
+ * @phy: the phy returned by devm_phy_get()
+ *
+ * destroys the devres associated with this phy and invokes phy_put
+ * to release the phy.
+ */
+void devm_phy_put(struct device *dev, struct phy *phy)
+{
+	int r;
+
+	r = devres_destroy(dev, devm_phy_release, devm_phy_match, phy);
+	dev_WARN_ONCE(dev, r, "couldn't find PHY resource\n");
+}
+EXPORT_SYMBOL_GPL(devm_phy_put);
+
+/**
+ * of_phy_simple_xlate() - returns the phy instance from phy provider
+ * @dev: the PHY provider device
+ * @args: of_phandle_args (not used here)
+ *
+ * Intended to be used by phy provider for the common case where #phy-cells is
+ * 0. For other cases where #phy-cells is greater than '0', the phy provider
+ * should provide a custom of_xlate function that reads the *args* and returns
+ * the appropriate phy.
+ */
+struct phy *of_phy_simple_xlate(struct device *dev, struct of_phandle_args
+	*args)
+{
+	struct phy *phy;
+	struct class_dev_iter iter;
+	struct device_node *node = dev->of_node;
+
+	class_dev_iter_init(&iter, phy_class, NULL, NULL);
+	while ((dev = class_dev_iter_next(&iter))) {
+		phy = container_of(dev, struct phy, dev);
+		if (node != phy->dev.of_node)
+			continue;
+
+		class_dev_iter_exit(&iter);
+		return phy;
+	}
+
+	class_dev_iter_exit(&iter);
+	return ERR_PTR(-ENODEV);
+}
+EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
+
+/**
+ * phy_get() - lookup and obtain a reference to a phy.
+ * @dev: device that requests this phy
+ * @string: the phy name as given in the dt data or phy device name
+ * for non-dt case
+ *
+ * Returns the phy driver, after getting a refcount to it; or
+ * -ENODEV if there is no such phy.  The caller is responsible for
+ * calling phy_put() to release that count.
+ */
+struct phy *phy_get(struct device *dev, const char *string)
+{
+	int index = 0;
+	struct phy *phy = NULL;
+
+	if (string == NULL) {
+		dev_WARN(dev, "missing string\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (dev->of_node) {
+		index = of_property_match_string(dev->of_node, "phy-names",
+			string);
+		phy = of_phy_get(dev, index);
+		if (IS_ERR(phy)) {
+			dev_WARN(dev, "unable to find phy\n");
+			return phy;
+		}
+	} else {
+		phy = phy_lookup(string);
+		if (IS_ERR(phy)) {
+			dev_WARN(dev, "unable to find phy\n");
+			return phy;
+		}
+	}
+
+	if (!try_module_get(phy->ops->owner))
+		return ERR_PTR(-EPROBE_DEFER);
+
+	get_device(&phy->dev);
+
+	return phy;
+}
+EXPORT_SYMBOL_GPL(phy_get);
+
+/**
+ * devm_phy_get() - lookup and obtain a reference to a phy.
+ * @dev: device that requests this phy
+ * @string: the phy name as given in the dt data or phy device name
+ * for non-dt case
+ *
+ * Gets the phy using phy_get(), and associates a device with it using
+ * devres. On driver detach, release function is invoked on the devres data,
+ * then, devres data is freed.
+ */
+struct phy *devm_phy_get(struct device *dev, const char *string)
+{
+	struct phy **ptr, *phy;
+
+	ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	phy = phy_get(dev, string);
+	if (!IS_ERR(phy)) {
+		*ptr = phy;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return phy;
+}
+EXPORT_SYMBOL_GPL(devm_phy_get);
+
+/**
+ * phy_create() - create a new phy
+ * @dev: device that is creating the new phy
+ * @id: id of the phy
+ * @ops: function pointers for performing phy operations
+ * @label: label given to the phy
+ * @priv: private data from phy driver
+ *
+ * Called to create a phy using phy framework.
+ */
+struct phy *phy_create(struct device *dev, u8 id, const struct phy_ops *ops,
+	const char *label, void *priv)
+{
+	int ret;
+	struct phy *phy;
+
+	if (!dev) {
+		dev_WARN(dev, "no device provided for PHY\n");
+		ret = -EINVAL;
+		goto err0;
+	}
+
+	phy = kzalloc(sizeof(*phy), GFP_KERNEL);
+	if (!phy) {
+		ret = -ENOMEM;
+		goto err0;
+	}
+
+	device_initialize(&phy->dev);
+
+	phy->dev.class = phy_class;
+	phy->dev.parent = dev;
+	phy->dev.of_node = dev->of_node;
+	phy->id = id;
+	phy->ops = ops;
+	phy->label = label;
+
+	dev_set_drvdata(&phy->dev, priv);
+
+	ret = dev_set_name(&phy->dev, "%s.%d", dev_name(dev), id);
+	if (ret)
+		goto err1;
+
+	ret = device_add(&phy->dev);
+	if (ret)
+		goto err1;
+
+	if (pm_runtime_enabled(dev))
+		pm_runtime_enable(&phy->dev);
+
+	return phy;
+
+err1:
+	put_device(&phy->dev);
+	kfree(phy);
+
+err0:
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(phy_create);
+
+/**
+ * devm_phy_create() - create a new phy
+ * @dev: device that is creating the new phy
+ * @id: id of the phy
+ * @ops: function pointers for performing phy operations
+ * @label: label given to the phy
+ * @priv: private data from phy driver
+ *
+ * Creates a new PHY device adding it to the PHY class.
+ * While at that, it also associates the device with the phy using devres.
+ * On driver detach, release function is invoked on the devres data,
+ * then, devres data is freed.
+ */
+struct phy *devm_phy_create(struct device *dev, u8 id,
+	const struct phy_ops *ops, const char *label, void *priv)
+{
+	struct phy **ptr, *phy;
+
+	ptr = devres_alloc(devm_phy_consume, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	phy = phy_create(dev, id, ops, label, priv);
+	if (!IS_ERR(phy)) {
+		*ptr = phy;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return phy;
+}
+EXPORT_SYMBOL_GPL(devm_phy_create);
+
+/**
+ * phy_destroy() - destroy the phy
+ * @phy: the phy to be destroyed
+ *
+ * Called to destroy the phy.
+ */
+void phy_destroy(struct phy *phy)
+{
+	pm_runtime_disable(&phy->dev);
+	device_unregister(&phy->dev);
+}
+EXPORT_SYMBOL_GPL(phy_destroy);
+
+/**
+ * devm_phy_destroy() - destroy the PHY
+ * @dev: device that wants to release this phy
+ * @phy: the phy returned by devm_phy_get()
+ *
+ * destroys the devres associated with this phy and invokes phy_destroy
+ * to destroy the phy.
+ */
+void devm_phy_destroy(struct device *dev, struct phy *phy)
+{
+	int r;
+
+	r = devres_destroy(dev, devm_phy_consume, devm_phy_match, phy);
+	dev_WARN_ONCE(dev, r, "couldn't find PHY resource\n");
+}
+EXPORT_SYMBOL_GPL(devm_phy_destroy);
+
+/**
+ * of_phy_provider_register() - create/register phy provider with the framework
+ * @dev: struct device of the phy provider
+ * @owner: the module owner containing of_xlate
+ * @of_xlate: function pointer to obtain phy instance from phy provider
+ *
+ * Creates struct phy_provider from dev and of_xlate function pointer.
+ * This is used in the case of dt boot for finding the phy instance from
+ * phy provider.
+ */
+struct phy_provider *of_phy_provider_register(struct device *dev,
+	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+	struct of_phandle_args *args))
+{
+	struct phy_provider *phy_provider;
+
+	phy_provider = kzalloc(sizeof(*phy_provider), GFP_KERNEL);
+	if (!phy_provider)
+		return ERR_PTR(-ENOMEM);
+
+	phy_provider->dev = dev;
+	phy_provider->owner = owner;
+	phy_provider->of_xlate = of_xlate;
+
+	mutex_lock(&phy_provider_mutex);
+	list_add_tail(&phy_provider->list, &phy_provider_list);
+	mutex_unlock(&phy_provider_mutex);
+
+	return phy_provider;
+}
+EXPORT_SYMBOL_GPL(of_phy_provider_register);
+
+/**
+ * devm_of_phy_provider_register() - create/register phy provider with the
+ * framework
+ * @dev: struct device of the phy provider
+ * @owner: the module owner containing of_xlate
+ * @of_xlate: function pointer to obtain phy instance from phy provider
+ *
+ * Creates struct phy_provider from dev and of_xlate function pointer.
+ * This is used in the case of dt boot for finding the phy instance from
+ * phy provider. While at that, it also associates the device with the
+ * phy provider using devres. On driver detach, release function is invoked
+ * on the devres data, then, devres data is freed.
+ */
+struct phy_provider *devm_of_phy_provider_register(struct device *dev,
+	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+	struct of_phandle_args *args))
+{
+	struct phy_provider **ptr, *phy_provider;
+
+	ptr = devres_alloc(devm_phy_provider_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	phy_provider = of_phy_provider_register(dev, owner, of_xlate);
+	if (!IS_ERR(phy_provider)) {
+		*ptr = phy_provider;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return phy_provider;
+}
+EXPORT_SYMBOL_GPL(devm_of_phy_provider_register);
+
+/**
+ * of_phy_provider_unregister() - unregister phy provider from the framework
+ * @phy_provider: phy provider returned by of_phy_provider_register()
+ *
+ * Removes the phy_provider created using of_phy_provider_register().
+ */
+void of_phy_provider_unregister(struct phy_provider *phy_provider)
+{
+	if (IS_ERR(phy_provider))
+		return;
+
+	mutex_lock(&phy_provider_mutex);
+	list_del(&phy_provider->list);
+	kfree(phy_provider);
+	mutex_unlock(&phy_provider_mutex);
+}
+EXPORT_SYMBOL_GPL(of_phy_provider_unregister);
+
+/**
+ * devm_of_phy_provider_unregister() - remove phy provider from the framework
+ * @dev: struct device of the phy provider
+ *
+ * destroys the devres associated with this phy provider and invokes
+ * of_phy_provider_unregister to unregister the phy provider.
+ */
+void devm_of_phy_provider_unregister(struct device *dev,
+	struct phy_provider *phy_provider) {
+	int r;
+
+	r = devres_destroy(dev, devm_phy_provider_release, devm_phy_match,
+		phy_provider);
+	dev_WARN_ONCE(dev, r, "couldn't find PHY provider device resource\n");
+}
+EXPORT_SYMBOL_GPL(devm_of_phy_provider_unregister);
+
+/**
+ * phy_release() - release the phy
+ * @dev: the dev member within phy
+ *
+ * When the last reference to the device is removed, it is called
+ * from the embedded kobject as release method.
+ */
+static void phy_release(struct device *dev)
+{
+	struct phy *phy;
+
+	phy = container_of(dev, struct phy, dev);
+	dev_dbg(dev, "releasing '%s'\n", dev_name(dev));
+	kfree(phy);
+}
+
+static int __init phy_core_init(void)
+{
+	phy_class = class_create(THIS_MODULE, "phy");
+	if (IS_ERR(phy_class)) {
+		pr_err("failed to create phy class --> %ld\n",
+			PTR_ERR(phy_class));
+		return PTR_ERR(phy_class);
+	}
+
+	phy_class->dev_release = phy_release;
+
+	return 0;
+}
+module_init(phy_core_init);
+
+static void __exit phy_core_exit(void)
+{
+	class_destroy(phy_class);
+}
+module_exit(phy_core_exit);
+
+MODULE_DESCRIPTION("Generic PHY Framework");
+MODULE_AUTHOR("Kishon Vijay Abraham I <kishon@ti.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
new file mode 100644
index 0000000..df0c98a
--- /dev/null
+++ b/include/linux/phy/phy.h
@@ -0,0 +1,284 @@
+/*
+ * phy.h -- generic phy header file
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Author: Kishon Vijay Abraham I <kishon@ti.com>
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef __DRIVERS_PHY_H
+#define __DRIVERS_PHY_H
+
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/device.h>
+#include <linux/pm_runtime.h>
+
+struct phy;
+
+/**
+ * struct phy_ops - set of function pointers for performing phy operations
+ * @init: operation to be performed for initializing phy
+ * @exit: operation to be performed while exiting
+ * @power_on: powering on the phy
+ * @power_off: powering off the phy
+ * @owner: the module owner containing the ops
+ */
+struct phy_ops {
+	int	(*init)(struct phy *phy);
+	int	(*exit)(struct phy *phy);
+	int	(*power_on)(struct phy *phy);
+	int	(*power_off)(struct phy *phy);
+	struct module *owner;
+};
+
+/**
+ * struct phy - represents the phy device
+ * @dev: phy device
+ * @id: id of the phy
+ * @ops: function pointers for performing phy operations
+ * @label: label given to the phy
+ */
+struct phy {
+	struct device		dev;
+	int			id;
+	const struct phy_ops	*ops;
+	const char		*label;
+};
+
+/**
+ * struct phy_provider - represents the phy provider
+ * @dev: phy provider device
+ * @owner: the module owner having of_xlate
+ * @of_xlate: function pointer to obtain phy instance from phy pointer
+ * @list: to maintain a linked list of PHY providers
+ */
+struct phy_provider {
+	struct device		*dev;
+	struct module		*owner;
+	struct list_head	list;
+	struct phy * (*of_xlate)(struct device *dev,
+		struct of_phandle_args *args);
+};
+
+#if IS_ENABLED(CONFIG_GENERIC_PHY)
+extern struct phy *phy_get(struct device *dev, const char *string);
+extern struct phy *devm_phy_get(struct device *dev, const char *string);
+extern void phy_put(struct phy *phy);
+extern void devm_phy_put(struct device *dev, struct phy *phy);
+extern struct phy *of_phy_simple_xlate(struct device *dev,
+	struct of_phandle_args *args);
+extern struct phy *phy_create(struct device *dev, u8 id,
+	const struct phy_ops *ops, const char *label, void *priv);
+extern struct phy *devm_phy_create(struct device *dev, u8 id,
+	const struct phy_ops *ops, const char *label, void *priv);
+extern void phy_destroy(struct phy *phy);
+extern void devm_phy_destroy(struct device *dev, struct phy *phy);
+extern struct phy_provider *of_phy_provider_register(struct device *dev,
+	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+	struct of_phandle_args *args));
+extern struct phy_provider *devm_of_phy_provider_register(struct device *dev,
+	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+	struct of_phandle_args *args));
+extern void of_phy_provider_unregister(struct phy_provider *phy_provider);
+extern void devm_of_phy_provider_unregister(struct device *dev,
+	struct phy_provider *phy_provider);
+#else
+static inline struct phy *phy_get(struct device *dev, const char *string)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
+static inline struct phy *devm_phy_get(struct device *dev, const char *string)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
+static inline void phy_put(struct phy *phy)
+{
+}
+
+static inline void devm_phy_put(struct device *dev, struct phy *phy)
+{
+}
+
+static inline struct phy *of_phy_simple_xlate(struct device *dev,
+	struct of_phandle_args *args)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
+static inline struct phy *phy_create(struct device *dev, u8 id,
+	const struct phy_ops *ops, const char *label, void *priv)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
+static inline struct phy *devm_phy_create(struct device *dev, u8 id,
+	const struct phy_ops *ops, const char *label, void *priv)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
+static inline void phy_destroy(struct phy *phy)
+{
+}
+
+static inline void devm_phy_destroy(struct device *dev, struct phy *phy)
+{
+}
+
+static inline struct phy_provider *of_phy_provider_register(struct device *dev,
+	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+	struct of_phandle_args *args))
+{
+	return ERR_PTR(-ENOSYS);
+}
+
+static inline struct phy_provider *devm_of_phy_provider_register(struct device
+	*dev, struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+	struct of_phandle_args *args))
+{
+	return ERR_PTR(-ENOSYS);
+}
+
+static inline void of_phy_provider_unregister(struct phy_provider *phy_provider)
+{
+}
+
+static inline void devm_of_phy_provider_unregister(struct device *dev,
+	struct phy_provider *phy_provider)
+{
+}
+#endif
+
+static inline int phy_init(struct phy *phy)
+{
+	int ret = -ENOTSUPP;
+
+	if (!pm_runtime_enabled(&phy->dev))
+		goto no_pm_runtime;
+
+	ret = pm_runtime_get_sync(&phy->dev);
+	if (ret < 0) {
+		dev_err(&phy->dev, "get sync failed: %d\n", ret);
+		return ret;
+	}
+
+no_pm_runtime:
+	if (phy->ops->init)
+		ret = phy->ops->init(phy);
+
+	return ret;
+}
+
+static inline int phy_exit(struct phy *phy)
+{
+	int ret = -ENOTSUPP;
+
+	if (phy->ops->exit)
+		ret = phy->ops->exit(phy);
+
+	if (!pm_runtime_enabled(&phy->dev))
+		goto no_pm_runtime;
+
+	ret = pm_runtime_put_sync(&phy->dev);
+	if (ret < 0)
+		dev_err(&phy->dev, "put sync failed: %d\n", ret);
+
+no_pm_runtime:
+	return ret;
+}
+
+static inline int phy_power_on(struct phy *phy)
+{
+	if (phy->ops->power_on)
+		return phy->ops->power_on(phy);
+
+	return -ENOTSUPP;
+}
+
+static inline int phy_power_off(struct phy *phy)
+{
+	if (phy->ops->power_off)
+		return phy->ops->power_off(phy);
+
+	return -ENOTSUPP;
+}
+
+static inline int phy_pm_runtime_get(struct phy *phy)
+{
+	if (WARN(IS_ERR(phy), "Invalid PHY reference\n"))
+		return -EINVAL;
+
+	if (!pm_runtime_enabled(&phy->dev))
+		return -ENOTSUPP;
+
+	return pm_runtime_get(&phy->dev);
+}
+
+static inline int phy_pm_runtime_get_sync(struct phy *phy)
+{
+	if (WARN(IS_ERR(phy), "Invalid PHY reference\n"))
+		return -EINVAL;
+
+	if (!pm_runtime_enabled(&phy->dev))
+		return -ENOTSUPP;
+
+	return pm_runtime_get_sync(&phy->dev);
+}
+
+static inline int phy_pm_runtime_put(struct phy *phy)
+{
+	if (WARN(IS_ERR(phy), "Invalid PHY reference\n"))
+		return -EINVAL;
+
+	if (!pm_runtime_enabled(&phy->dev))
+		return -ENOTSUPP;
+
+	return pm_runtime_put(&phy->dev);
+}
+
+static inline int phy_pm_runtime_put_sync(struct phy *phy)
+{
+	if (WARN(IS_ERR(phy), "Invalid PHY reference\n"))
+		return -EINVAL;
+
+	if (!pm_runtime_enabled(&phy->dev))
+		return -ENOTSUPP;
+
+	return pm_runtime_put_sync(&phy->dev);
+}
+
+static inline void phy_pm_runtime_allow(struct phy *phy)
+{
+	if (WARN(IS_ERR(phy), "Invalid PHY reference\n"))
+		return;
+
+	if (!pm_runtime_enabled(&phy->dev))
+		return;
+
+	pm_runtime_allow(&phy->dev);
+}
+
+static inline void phy_pm_runtime_forbid(struct phy *phy)
+{
+	if (WARN(IS_ERR(phy), "Invalid PHY reference\n"))
+		return;
+
+	if (!pm_runtime_enabled(&phy->dev))
+		return;
+
+	pm_runtime_forbid(&phy->dev);
+}
+#endif /* __DRIVERS_PHY_H */
-- 
1.7.10.4

[PATCH v7 2/9] usb: phy: omap-usb2: use the new generic PHY framework

[Kishon Vijay Abraham I]

Used the generic PHY framework API to create the PHY. Now the power off and
power on are done in omap_usb_power_off and omap_usb_power_on respectively.

However using the old USB PHY library cannot be completely removed
because OTG is intertwined with PHY and moving to the new framework
will break OTG. Once we have a separate OTG state machine, we
can get rid of the USB PHY library.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/usb/phy/phy-omap-usb2.c |   40 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
index 844ab68..3c275e7 100644
--- a/drivers/usb/phy/phy-omap-usb2.c
+++ b/drivers/usb/phy/phy-omap-usb2.c
@@ -28,6 +28,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/delay.h>
 #include <linux/usb/omap_control_usb.h>
+#include <linux/phy/phy.h>
 
 /**
  * omap_usb2_set_comparator - links the comparator present in the sytem with
@@ -119,10 +120,36 @@ static int omap_usb2_suspend(struct usb_phy *x, int suspend)
 	return 0;
 }
 
+static int omap_usb_power_off(struct phy *x)
+{
+	struct omap_usb *phy = dev_get_drvdata(&x->dev);
+
+	omap_control_usb_phy_power(phy->control_dev, 0);
+
+	return 0;
+}
+
+static int omap_usb_power_on(struct phy *x)
+{
+	struct omap_usb *phy = dev_get_drvdata(&x->dev);
+
+	omap_control_usb_phy_power(phy->control_dev, 1);
+
+	return 0;
+}
+
+static struct phy_ops ops = {
+	.power_on	= omap_usb_power_on,
+	.power_off	= omap_usb_power_off,
+	.owner		= THIS_MODULE,
+};
+
 static int omap_usb2_probe(struct platform_device *pdev)
 {
 	struct omap_usb			*phy;
+	struct phy			*generic_phy;
 	struct usb_otg			*otg;
+	struct phy_provider		*phy_provider;
 
 	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
 	if (!phy) {
@@ -144,6 +171,11 @@ static int omap_usb2_probe(struct platform_device *pdev)
 	phy->phy.otg		= otg;
 	phy->phy.type		= USB_PHY_TYPE_USB2;
 
+	phy_provider = devm_of_phy_provider_register(phy->dev, THIS_MODULE,
+		of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
 	phy->control_dev = omap_get_control_dev();
 	if (IS_ERR(phy->control_dev)) {
 		dev_dbg(&pdev->dev, "Failed to get control device\n");
@@ -159,6 +191,12 @@ static int omap_usb2_probe(struct platform_device *pdev)
 	otg->start_srp		= omap_usb_start_srp;
 	otg->phy		= &phy->phy;
 
+	pm_runtime_enable(phy->dev);
+
+	generic_phy = devm_phy_create(phy->dev, 0, &ops, "omap-usb2", phy);
+	if (IS_ERR(generic_phy))
+		return PTR_ERR(generic_phy);
+
 	phy->wkupclk = devm_clk_get(phy->dev, "usb_phy_cm_clk32k");
 	if (IS_ERR(phy->wkupclk)) {
 		dev_err(&pdev->dev, "unable to get usb_phy_cm_clk32k\n");
@@ -176,8 +214,6 @@ static int omap_usb2_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, phy);
 
-	pm_runtime_enable(phy->dev);
-
 	return 0;
 }
 
-- 
1.7.10.4

[PATCH v7 3/9] usb: phy: twl4030: use the new generic PHY framework

[Kishon Vijay Abraham I]

Used the generic PHY framework API to create the PHY. For powering on
and powering off the PHY, power_on and power_off ops are used. Once the
MUSB OMAP glue is adapted to the new framework, the suspend and resume
ops of usb phy library will be removed.

However using the old usb phy library cannot be completely removed
because otg is intertwined with phy and moving to the new
framework completely will break otg. Once we have a separate otg state machine,
we can get rid of the usb phy library.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/usb/phy/phy-twl4030-usb.c |   48 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-twl4030-usb.c b/drivers/usb/phy/phy-twl4030-usb.c
index 8f78d2d..65a5e43 100644
--- a/drivers/usb/phy/phy-twl4030-usb.c
+++ b/drivers/usb/phy/phy-twl4030-usb.c
@@ -33,6 +33,7 @@
 #include <linux/io.h>
 #include <linux/delay.h>
 #include <linux/usb/otg.h>
+#include <linux/phy/phy.h>
 #include <linux/usb/musb-omap.h>
 #include <linux/usb/ulpi.h>
 #include <linux/i2c/twl.h>
@@ -431,6 +432,14 @@ static void twl4030_phy_suspend(struct twl4030_usb *twl, int controller_off)
 	dev_dbg(twl->dev, "%s\n", __func__);
 }
 
+static int twl4030_phy_power_off(struct phy *phy)
+{
+	struct twl4030_usb *twl = dev_get_drvdata(&phy->dev);
+
+	twl4030_phy_suspend(twl, 0);
+	return 0;
+}
+
 static void __twl4030_phy_resume(struct twl4030_usb *twl)
 {
 	twl4030_phy_power(twl, 1);
@@ -459,6 +468,14 @@ static void twl4030_phy_resume(struct twl4030_usb *twl)
 	}
 }
 
+static int twl4030_phy_power_on(struct phy *phy)
+{
+	struct twl4030_usb *twl = dev_get_drvdata(&phy->dev);
+
+	twl4030_phy_resume(twl);
+	return 0;
+}
+
 static int twl4030_usb_ldo_init(struct twl4030_usb *twl)
 {
 	/* Enable writing to power configuration registers */
@@ -602,13 +619,22 @@ static int twl4030_usb_phy_init(struct usb_phy *phy)
 	status = twl4030_usb_linkstat(twl);
 	twl->linkstat = status;
 
-	if (status == OMAP_MUSB_ID_GROUND || status == OMAP_MUSB_VBUS_VALID)
+	if (status == OMAP_MUSB_ID_GROUND || status == OMAP_MUSB_VBUS_VALID) {
 		omap_musb_mailbox(twl->linkstat);
+		twl4030_phy_power_on(phy);
+	}
 
 	sysfs_notify(&twl->dev->kobj, NULL, "vbus");
 	return 0;
 }
 
+static int twl4030_phy_init(struct phy *phy)
+{
+	struct twl4030_usb *twl = dev_get_drvdata(&phy->dev);
+
+	return twl4030_usb_phy_init(&twl->phy);
+}
+
 static int twl4030_set_suspend(struct usb_phy *x, int suspend)
 {
 	struct twl4030_usb *twl = phy_to_twl(x);
@@ -646,13 +672,22 @@ static int twl4030_set_host(struct usb_otg *otg, struct usb_bus *host)
 	return 0;
 }
 
+static const struct phy_ops ops = {
+	.init		= twl4030_phy_init,
+	.power_on	= twl4030_phy_power_on,
+	.power_off	= twl4030_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
 static int twl4030_usb_probe(struct platform_device *pdev)
 {
 	struct twl4030_usb_data *pdata = pdev->dev.platform_data;
 	struct twl4030_usb	*twl;
+	struct phy		*phy;
 	int			status, err;
 	struct usb_otg		*otg;
 	struct device_node	*np = pdev->dev.of_node;
+	struct phy_provider	*phy_provider;
 
 	twl = devm_kzalloc(&pdev->dev, sizeof *twl, GFP_KERNEL);
 	if (!twl)
@@ -689,6 +724,17 @@ static int twl4030_usb_probe(struct platform_device *pdev)
 	otg->set_host		= twl4030_set_host;
 	otg->set_peripheral	= twl4030_set_peripheral;
 
+	phy_provider = devm_of_phy_provider_register(twl->dev, THIS_MODULE,
+		of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	phy = devm_phy_create(twl->dev, 0, &ops, "twl4030", twl);
+	if (IS_ERR(phy)) {
+		dev_dbg(&pdev->dev, "Failed to create PHY\n");
+		return PTR_ERR(phy);
+	}
+
 	/* init spinlock for workqueue */
 	spin_lock_init(&twl->lock);
 
-- 
1.7.10.4

[PATCH v7 4/9] usb: phy: twl4030: twl4030 shouldn't be subsys_initcall

[Kishon Vijay Abraham I]

Changed the inticall from subsys_initcall to module_init for
twl4030-usb.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/usb/phy/phy-twl4030-usb.c |   12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/usb/phy/phy-twl4030-usb.c b/drivers/usb/phy/phy-twl4030-usb.c
index 65a5e43..fba9697 100644
--- a/drivers/usb/phy/phy-twl4030-usb.c
+++ b/drivers/usb/phy/phy-twl4030-usb.c
@@ -822,17 +822,7 @@ static struct platform_driver twl4030_usb_driver = {
 	},
 };
 
-static int __init twl4030_usb_init(void)
-{
-	return platform_driver_register(&twl4030_usb_driver);
-}
-subsys_initcall(twl4030_usb_init);
-
-static void __exit twl4030_usb_exit(void)
-{
-	platform_driver_unregister(&twl4030_usb_driver);
-}
-module_exit(twl4030_usb_exit);
+module_platform_driver(twl4030_usb_driver);
 
 MODULE_ALIAS("platform:twl4030_usb");
 MODULE_AUTHOR("Texas Instruments, Inc, Nokia Corporation");
-- 
1.7.10.4

[PATCH v7 5/9] ARM: OMAP: USB: Add phy binding information

[Kishon Vijay Abraham I]

In order for controllers to get PHY in case of non dt boot, the phy
binding information (phy device name) should be added in the platform
data of the controller.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 arch/arm/mach-omap2/usb-musb.c |    6 +++++-
 include/linux/usb/musb.h       |    3 +++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
index 3242a55..284ba51 100644
--- a/arch/arm/mach-omap2/usb-musb.c
+++ b/arch/arm/mach-omap2/usb-musb.c
@@ -85,8 +85,12 @@ void __init usb_musb_init(struct omap_musb_board_data *musb_board_data)
 	musb_plat.mode = board_data->mode;
 	musb_plat.extvbus = board_data->extvbus;
 
-	if (cpu_is_omap44xx())
+	if (cpu_is_omap44xx()) {
 		musb_plat.has_mailbox = true;
+		musb_plat.phy_label = "omap-usb2";
+	} else if (cpu_is_omap34xx()) {
+		musb_plat.phy_label = "twl4030";
+	}
 
 	if (soc_is_am35xx()) {
 		oh_name = "am35x_otg_hs";
diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h
index 053c268..596f8c8 100644
--- a/include/linux/usb/musb.h
+++ b/include/linux/usb/musb.h
@@ -104,6 +104,9 @@ struct musb_hdrc_platform_data {
 	/* for clk_get() */
 	const char	*clock;
 
+	/* phy label */
+	const char	*phy_label;
+
 	/* (HOST or OTG) switch VBUS on/off */
 	int		(*set_vbus)(struct device *dev, int is_on);
 
-- 
1.7.10.4

[PATCH v7 6/9] ARM: dts: omap: update usb_otg_hs data

[Kishon Vijay Abraham I]

Updated the usb_otg_hs dt data to include the *phy* and *phy-names*
binding in order for the driver to use the new generic PHY framework.
Also updated the Documentation to include the binding information.
The PHY binding information can be found at
Documentation/devicetree/bindings/phy/phy-bindings.txt

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 Documentation/devicetree/bindings/usb/omap-usb.txt |    5 +++++
 Documentation/devicetree/bindings/usb/usb-phy.txt  |    6 ++++++
 arch/arm/boot/dts/omap3-beagle-xm.dts              |    2 ++
 arch/arm/boot/dts/omap3-evm.dts                    |    2 ++
 arch/arm/boot/dts/omap3-overo.dtsi                 |    2 ++
 arch/arm/boot/dts/omap4.dtsi                       |    3 +++
 arch/arm/boot/dts/twl4030.dtsi                     |    1 +
 7 files changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt
index d4769f3..c0871a7 100644
--- a/Documentation/devicetree/bindings/usb/omap-usb.txt
+++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
@@ -19,6 +19,9 @@ OMAP MUSB GLUE
  - power : Should be "50". This signifies the controller can supply upto
    100mA when operating in host mode.
  - usb-phy : the phandle for the PHY device
+ - phys : the phandle for the PHY device (used by generic PHY framework)
+ - phy-names : the names of the PHY corresponding to the PHYs present in the
+   *phy* phandle.
 
 Optional properties:
  - ctrl-module : phandle of the control module this glue uses to write to
@@ -33,6 +36,8 @@ usb_otg_hs: usb_otg_hs@4a0ab000 {
 	num-eps = <16>;
 	ram-bits = <12>;
 	ctrl-module = <&omap_control_usb>;
+	phys = <&usb2_phy>;
+	phy-names = "usb2-phy";
 };
 
 Board specific device node entry
diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
index 61496f5..c0245c8 100644
--- a/Documentation/devicetree/bindings/usb/usb-phy.txt
+++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
@@ -5,6 +5,8 @@ OMAP USB2 PHY
 Required properties:
  - compatible: Should be "ti,omap-usb2"
  - reg : Address and length of the register set for the device.
+ - #phy-cells: determine the number of cells that should be given in the
+   phandle while referencing this phy.
 
 Optional properties:
  - ctrl-module : phandle of the control module used by PHY driver to power on
@@ -16,6 +18,7 @@ usb2phy@4a0ad080 {
 	compatible = "ti,omap-usb2";
 	reg = <0x4a0ad080 0x58>;
 	ctrl-module = <&omap_control_usb>;
+	#phy-cells = <0>;
 };
 
 OMAP USB3 PHY
@@ -25,6 +28,8 @@ Required properties:
  - reg : Address and length of the register set for the device.
  - reg-names: The names of the register addresses corresponding to the registers
    filled in "reg".
+ - #phy-cells: determine the number of cells that should be given in the
+   phandle while referencing this phy.
 
 Optional properties:
  - ctrl-module : phandle of the control module used by PHY driver to power on
@@ -39,4 +44,5 @@ usb3phy@4a084400 {
 	      <0x4a084c00 0x40>;
 	reg-names = "phy_rx", "phy_tx", "pll_ctrl";
 	ctrl-module = <&omap_control_usb>;
+	#phy-cells = <0>;
 };
diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts b/arch/arm/boot/dts/omap3-beagle-xm.dts
index 3046d1f..023596e 100644
--- a/arch/arm/boot/dts/omap3-beagle-xm.dts
+++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
@@ -123,6 +123,8 @@
 &usb_otg_hs {
 	interface-type = <0>;
 	usb-phy = <&usb2_phy>;
+	phys = <&usb2_phy>;
+	phy-names = "usb2-phy";
 	mode = <3>;
 	power = <50>;
 };
diff --git a/arch/arm/boot/dts/omap3-evm.dts b/arch/arm/boot/dts/omap3-evm.dts
index 96d1c20..f2b8314 100644
--- a/arch/arm/boot/dts/omap3-evm.dts
+++ b/arch/arm/boot/dts/omap3-evm.dts
@@ -69,6 +69,8 @@
 &usb_otg_hs {
 	interface-type = <0>;
 	usb-phy = <&usb2_phy>;
+	phys = <&usb2_phy>;
+	phy-names = "usb2-phy";
 	mode = <3>;
 	power = <50>;
 };
diff --git a/arch/arm/boot/dts/omap3-overo.dtsi b/arch/arm/boot/dts/omap3-overo.dtsi
index a626c50..b65916e 100644
--- a/arch/arm/boot/dts/omap3-overo.dtsi
+++ b/arch/arm/boot/dts/omap3-overo.dtsi
@@ -74,6 +74,8 @@
 &usb_otg_hs {
 	interface-type = <0>;
 	usb-phy = <&usb2_phy>;
+	phys = <&usb2_phy>;
+	phy-names = "usb2-phy";
 	mode = <3>;
 	power = <50>;
 };
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 2a56428..dae620b 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -517,6 +517,7 @@
 				compatible = "ti,omap-usb2";
 				reg = <0x4a0ad080 0x58>;
 				ctrl-module = <&omap_control_usb>;
+				#phy-cells = <0>;
 			};
 		};
 
@@ -655,6 +656,8 @@
 			interrupt-names = "mc", "dma";
 			ti,hwmods = "usb_otg_hs";
 			usb-phy = <&usb2_phy>;
+			phys = <&usb2_phy>;
+			phy-names = "usb2-phy";
 			multipoint = <1>;
 			num-eps = <16>;
 			ram-bits = <12>;
diff --git a/arch/arm/boot/dts/twl4030.dtsi b/arch/arm/boot/dts/twl4030.dtsi
index b3034da..ce4cd6f 100644
--- a/arch/arm/boot/dts/twl4030.dtsi
+++ b/arch/arm/boot/dts/twl4030.dtsi
@@ -80,6 +80,7 @@
 		usb1v8-supply = <&vusb1v8>;
 		usb3v1-supply = <&vusb3v1>;
 		usb_mode = <1>;
+		#phy-cells = <0>;
 	};
 
 	twl_pwm: pwm {
-- 
1.7.10.4

[PATCH v7 7/9] usb: musb: omap2430: use the new generic PHY framework

[Kishon Vijay Abraham I]

Use the generic PHY framework API to get the PHY. The usb_phy_set_resume
and usb_phy_set_suspend is replaced with power_on/get_sync and
power_off/put_sync to align with the new PHY framework.

musb->xceiv can't be removed as of now because musb core uses xceiv.state and
xceiv.otg. Once there is a separate state machine to handle otg, these can be
moved out of xceiv and then we can start using the generic PHY framework.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/usb/musb/musb_core.c |    1 +
 drivers/usb/musb/musb_core.h |    3 +++
 drivers/usb/musb/omap2430.c  |   29 +++++++++++++++++++++--------
 3 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 37a261a..f732bcc 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1864,6 +1864,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 	musb->board_set_power = plat->set_power;
 	musb->min_power = plat->min_power;
 	musb->ops = plat->platform_ops;
+	musb->phy_label = plat->phy_label;
 
 	/* The musb_platform_init() call:
 	 *   - adjusts musb->mregs
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 7fb4819..498ae21 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -46,6 +46,7 @@
 #include <linux/usb.h>
 #include <linux/usb/otg.h>
 #include <linux/usb/musb.h>
+#include <linux/phy/phy.h>
 
 struct musb;
 struct musb_hw_ep;
@@ -357,6 +358,7 @@ struct musb {
 	u16			int_tx;
 
 	struct usb_phy		*xceiv;
+	struct phy		*phy;
 
 	int nIrq;
 	unsigned		irq_wake:1;
@@ -434,6 +436,7 @@ struct musb {
 	unsigned                double_buffer_not_ok:1;
 
 	struct musb_hdrc_config	*config;
+	const char		*phy_label;
 
 #ifdef MUSB_CONFIG_PROC_FS
 	struct proc_dir_entry *proc_entry;
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 628b93f..c62a004 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -348,14 +348,24 @@ static int omap2430_musb_init(struct musb *musb)
 	 * up through ULPI.  TWL4030-family PMICs include one,
 	 * which needs a driver, drivers aren't always needed.
 	 */
-	if (dev->parent->of_node)
+	if (dev->parent->of_node) {
+		musb->phy = devm_phy_get(dev->parent, "usb2-phy");
+
+		/* We can't totally remove musb->xceiv as of now because
+		 * musb core uses xceiv.state and xceiv.otg. Once we have
+		 * a separate state machine to handle otg, these can be moved
+		 * out of xceiv and then we can start using the generic PHY
+		 * framework
+		 */
 		musb->xceiv = devm_usb_get_phy_by_phandle(dev->parent,
 		    "usb-phy", 0);
-	else
+	} else {
 		musb->xceiv = devm_usb_get_phy_dev(dev, 0);
+		musb->phy = devm_phy_get(dev, musb->phy_label);
+	}
 
-	if (IS_ERR(musb->xceiv)) {
-		status = PTR_ERR(musb->xceiv);
+	if (IS_ERR(musb->xceiv) || IS_ERR(musb->phy)) {
+		status = PTR_ERR(musb->xceiv) | PTR_ERR(musb->phy);
 
 		if (status == -ENXIO)
 			return status;
@@ -397,9 +407,10 @@ static int omap2430_musb_init(struct musb *musb)
 	if (glue->status != OMAP_MUSB_UNKNOWN)
 		omap_musb_set_mailbox(glue);
 
-	usb_phy_init(musb->xceiv);
+	phy_init(musb->phy);
 
 	pm_runtime_put_noidle(musb->controller);
+	phy_pm_runtime_put(musb->phy);
 	return 0;
 
 err1:
@@ -460,6 +471,7 @@ static int omap2430_musb_exit(struct musb *musb)
 	del_timer_sync(&musb_idle_timer);
 
 	omap2430_low_level_exit(musb);
+	phy_exit(musb->phy);
 
 	return 0;
 }
@@ -619,7 +631,8 @@ static int omap2430_runtime_suspend(struct device *dev)
 				OTG_INTERFSEL);
 
 		omap2430_low_level_exit(musb);
-		usb_phy_set_suspend(musb->xceiv, 1);
+		phy_power_off(musb->phy);
+		phy_pm_runtime_put(musb->phy);
 	}
 
 	return 0;
@@ -634,8 +647,8 @@ static int omap2430_runtime_resume(struct device *dev)
 		omap2430_low_level_init(musb);
 		musb_writel(musb->mregs, OTG_INTERFSEL,
 				musb->context.otg_interfsel);
-
-		usb_phy_set_suspend(musb->xceiv, 0);
+		phy_pm_runtime_get_sync(musb->phy);
+		phy_power_on(musb->phy);
 	}
 
 	return 0;
-- 
1.7.10.4

[PATCH v7 8/9] usb: phy: omap-usb2: remove *set_suspend* callback from omap-usb2

[Kishon Vijay Abraham I]

Now that omap-usb2 is adapted to the new generic PHY framework,
*set_suspend* ops can be removed from omap-usb2 driver.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/usb/phy/phy-omap-usb2.c |   24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
index 3c275e7..bd2e4ca 100644
--- a/drivers/usb/phy/phy-omap-usb2.c
+++ b/drivers/usb/phy/phy-omap-usb2.c
@@ -97,29 +97,6 @@ static int omap_usb_set_peripheral(struct usb_otg *otg,
 	return 0;
 }
 
-static int omap_usb2_suspend(struct usb_phy *x, int suspend)
-{
-	u32 ret;
-	struct omap_usb *phy = phy_to_omapusb(x);
-
-	if (suspend && !phy->is_suspended) {
-		omap_control_usb_phy_power(phy->control_dev, 0);
-		pm_runtime_put_sync(phy->dev);
-		phy->is_suspended = 1;
-	} else if (!suspend && phy->is_suspended) {
-		ret = pm_runtime_get_sync(phy->dev);
-		if (ret < 0) {
-			dev_err(phy->dev, "get_sync failed with err %d\n",
-									ret);
-			return ret;
-		}
-		omap_control_usb_phy_power(phy->control_dev, 1);
-		phy->is_suspended = 0;
-	}
-
-	return 0;
-}
-
 static int omap_usb_power_off(struct phy *x)
 {
 	struct omap_usb *phy = dev_get_drvdata(&x->dev);
@@ -167,7 +144,6 @@ static int omap_usb2_probe(struct platform_device *pdev)
 
 	phy->phy.dev		= phy->dev;
 	phy->phy.label		= "omap-usb2";
-	phy->phy.set_suspend	= omap_usb2_suspend;
 	phy->phy.otg		= otg;
 	phy->phy.type		= USB_PHY_TYPE_USB2;
 
-- 
1.7.10.4

[PATCH v7 9/9] usb: phy: twl4030-usb: remove *set_suspend* and *phy_init* ops

[Kishon Vijay Abraham I]

Now that twl4030-usb is adapted to the new generic PHY framework,
*set_suspend* and *phy_init* ops can be removed from twl4030-usb driver.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/usb/phy/phy-twl4030-usb.c |   55 ++++++++-----------------------------
 1 file changed, 12 insertions(+), 43 deletions(-)

diff --git a/drivers/usb/phy/phy-twl4030-usb.c b/drivers/usb/phy/phy-twl4030-usb.c
index fba9697..5245b86 100644
--- a/drivers/usb/phy/phy-twl4030-usb.c
+++ b/drivers/usb/phy/phy-twl4030-usb.c
@@ -422,25 +422,20 @@ static void twl4030_phy_power(struct twl4030_usb *twl, int on)
 	}
 }
 
-static void twl4030_phy_suspend(struct twl4030_usb *twl, int controller_off)
+static int twl4030_phy_power_off(struct phy *phy)
 {
+	struct twl4030_usb *twl = dev_get_drvdata(&phy->dev);
+
 	if (twl->asleep)
-		return;
+		return 0;
 
 	twl4030_phy_power(twl, 0);
 	twl->asleep = 1;
 	dev_dbg(twl->dev, "%s\n", __func__);
-}
-
-static int twl4030_phy_power_off(struct phy *phy)
-{
-	struct twl4030_usb *twl = dev_get_drvdata(&phy->dev);
-
-	twl4030_phy_suspend(twl, 0);
 	return 0;
 }
 
-static void __twl4030_phy_resume(struct twl4030_usb *twl)
+static void __twl4030_phy_power_on(struct twl4030_usb *twl)
 {
 	twl4030_phy_power(twl, 1);
 	twl4030_i2c_access(twl, 1);
@@ -449,11 +444,13 @@ static void __twl4030_phy_resume(struct twl4030_usb *twl)
 		twl4030_i2c_access(twl, 0);
 }
 
-static void twl4030_phy_resume(struct twl4030_usb *twl)
+static int twl4030_phy_power_on(struct phy *phy)
 {
+	struct twl4030_usb *twl = dev_get_drvdata(&phy->dev);
+
 	if (!twl->asleep)
-		return;
-	__twl4030_phy_resume(twl);
+		return 0;
+	__twl4030_phy_power_on(twl);
 	twl->asleep = 0;
 	dev_dbg(twl->dev, "%s\n", __func__);
 
@@ -466,13 +463,6 @@ static void twl4030_phy_resume(struct twl4030_usb *twl)
 		cancel_delayed_work(&twl->id_workaround_work);
 		schedule_delayed_work(&twl->id_workaround_work, HZ);
 	}
-}
-
-static int twl4030_phy_power_on(struct phy *phy)
-{
-	struct twl4030_usb *twl = dev_get_drvdata(&phy->dev);
-
-	twl4030_phy_resume(twl);
 	return 0;
 }
 
@@ -604,9 +594,9 @@ static void twl4030_id_workaround_work(struct work_struct *work)
 	}
 }
 
-static int twl4030_usb_phy_init(struct usb_phy *phy)
+static int twl4030_phy_init(struct phy *phy)
 {
-	struct twl4030_usb *twl = phy_to_twl(phy);
+	struct twl4030_usb *twl = dev_get_drvdata(&phy->dev);
 	enum omap_musb_vbus_id_status status;
 
 	/*
@@ -628,25 +618,6 @@ static int twl4030_usb_phy_init(struct usb_phy *phy)
 	return 0;
 }
 
-static int twl4030_phy_init(struct phy *phy)
-{
-	struct twl4030_usb *twl = dev_get_drvdata(&phy->dev);
-
-	return twl4030_usb_phy_init(&twl->phy);
-}
-
-static int twl4030_set_suspend(struct usb_phy *x, int suspend)
-{
-	struct twl4030_usb *twl = phy_to_twl(x);
-
-	if (suspend)
-		twl4030_phy_suspend(twl, 1);
-	else
-		twl4030_phy_resume(twl);
-
-	return 0;
-}
-
 static int twl4030_set_peripheral(struct usb_otg *otg,
 					struct usb_gadget *gadget)
 {
@@ -717,8 +688,6 @@ static int twl4030_usb_probe(struct platform_device *pdev)
 	twl->phy.label		= "twl4030";
 	twl->phy.otg		= otg;
 	twl->phy.type		= USB_PHY_TYPE_USB2;
-	twl->phy.set_suspend	= twl4030_set_suspend;
-	twl->phy.init		= twl4030_usb_phy_init;
 
 	otg->phy		= &twl->phy;
 	otg->set_host		= twl4030_set_host;
-- 
1.7.10.4

Re: [PATCH v7 0/9] Generic PHY Framework

[Sylwester Nawrocki]

Hi,

On 06/13/2013 10:43 AM, Kishon Vijay Abraham I wrote:
> Added a generic PHY framework that provides a set of APIs for the PHY drivers
> to create/destroy a PHY and APIs for the PHY users to obtain a reference to
> the PHY with or without using phandle.
> 
> This framework will be of use only to devices that uses external PHY (PHY
> functionality is not embedded within the controller).
> 
> The intention of creating this framework is to bring the phy drivers spread
> all over the Linux kernel to drivers/phy to increase code re-use and to
> increase code maintainability.
> 
> Comments to make PHY as bus wasn't done because PHY devices can be part of
> other bus and making a same device attached to multiple bus leads to bad
> design.
> 
> If the PHY driver has to send notification on connect/disconnect, the PHY
> driver should make use of the extcon framework. Using this susbsystem 
> to use extcon framwork will have to be analysed.
> 
> Making omap-usb2 and twl4030 to use this framework is provided as a sample.
> 
> This patch series is developed on linux mainline tree.
> 
> Changes from v6
> * corrected few typos in Documentation
> * Changed PHY Subsystem to *bool* in Kconfig (to avoid compilation errors when
>   PHY Subsystem is kept as module and the dependent modules are built-in)
> * Added if pm_runtime_enabled check before runtime pm calls.

Looks good, feel free to add:

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com> for patches
1...3/9, 5...7/9

and Tested-by: Sylwester Nawrocki <s.nawrocki@samsung.com> for patch 1/9

Hopefully we can gather more Acks and merge it for 3.11.

I have already used this API for our MIPI CSI-2/DSIM DPHYs driver,
the RFC patch series can be found at [1].

Thanks,
Sylwester

[1] http://www.spinics.net/lists/arm-kernel/msg251666.html

Re: [PATCH v7 1/9] drivers: phy: add generic PHY framework

[Felipe Balbi]

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

Hi,

On Thu, Jun 13, 2013 at 02:13:51PM +0530, Kishon Vijay Abraham I wrote:
> +struct phy_provider *of_phy_provider_register(struct device *dev,
> +	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
> +	struct of_phandle_args *args))

I would rename this to __of_phy_provider_register() and in a header
have:

#define of_phy_provider_register(dev, xlate)	\
	__of_phy_provider_register((dev), THIS_MODULE, (xlate))

then your users don't need to remember always passing THIS_MODULE
argument.

> +struct phy_provider *devm_of_phy_provider_register(struct device *dev,
> +	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
> +	struct of_phandle_args *args))

likewise.

> +/**
> + * phy_release() - release the phy
> + * @dev: the dev member within phy
> + *
> + * When the last reference to the device is removed, it is called
> + * from the embedded kobject as release method.
> + */
> +static void phy_release(struct device *dev)
> +{
> +	struct phy *phy;
> +
> +	phy = container_of(dev, struct phy, dev);

I would have a:

#define to_phy(dev)	(container_of((dev), struct phy dev))

somewhere in a header, just for the sake of brevity :-p

> +	dev_dbg(dev, "releasing '%s'\n", dev_name(dev));

make it dev_vdbg() instead.

> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> new file mode 100644
> index 0000000..df0c98a
> --- /dev/null
> +++ b/include/linux/phy/phy.h

[ ... ]

> +static inline int phy_init(struct phy *phy)
> +{
> +	int ret = -ENOTSUPP;
> +
> +	if (!pm_runtime_enabled(&phy->dev))
> +		goto no_pm_runtime;
> +
> +	ret = pm_runtime_get_sync(&phy->dev);
> +	if (ret < 0) {
> +		dev_err(&phy->dev, "get sync failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +no_pm_runtime:

you can avoid this goto if you code as below, note that I'm also solving
a possible unbalanced PM runtime call which would prevent actual
*runtime* suspend of the PHY:

	ret = phy_pm_runtime_get_sync(phy);
	if (ret < 0 && ret != -ENOTSUPP)
		return ret;

	if (phy->ops->init) {
		ret = phy->ops->init(phy);
		if (ret < 0) {
			dev_err(&phy->dev, "failed to initialize --> %d\n", ret);
			goto out;
		}
	}

out:
	/*
	 * In order to avoid unbalanced PM runtime calls, let's make
	 * sure to disable what we might have enabled when entering this
	 * function.
	 */
	 phy_pm_runtime_put(phy);

	return ret;


> +}
> +
> +static inline int phy_exit(struct phy *phy)
> +{
> +	int ret = -ENOTSUPP;
> +
> +	if (phy->ops->exit)
> +		ret = phy->ops->exit(phy);
> +
> +	if (!pm_runtime_enabled(&phy->dev))
> +		goto no_pm_runtime;
> +
> +	ret = pm_runtime_put_sync(&phy->dev);

move your pm runtime wrappers before these calls and you can use them.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 2/9] usb: phy: omap-usb2: use the new generic PHY framework

[Felipe Balbi]

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

Hi,

On Thu, Jun 13, 2013 at 02:13:52PM +0530, Kishon Vijay Abraham I wrote:
> @@ -159,6 +191,12 @@ static int omap_usb2_probe(struct platform_device *pdev)
>  	otg->start_srp		= omap_usb_start_srp;
>  	otg->phy		= &phy->phy;
>  
> +	pm_runtime_enable(phy->dev);

enabling pm_runtime here has the potential to cause a real big problem.
How have you tested this patch ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 3/9] usb: phy: twl4030: use the new generic PHY framework

[Felipe Balbi]

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

On Thu, Jun 13, 2013 at 02:13:53PM +0530, Kishon Vijay Abraham I wrote:
> Used the generic PHY framework API to create the PHY. For powering on
> and powering off the PHY, power_on and power_off ops are used. Once the
> MUSB OMAP glue is adapted to the new framework, the suspend and resume
> ops of usb phy library will be removed.
> 
> However using the old usb phy library cannot be completely removed
> because otg is intertwined with phy and moving to the new
> framework completely will break otg. Once we have a separate otg state machine,
> we can get rid of the usb phy library.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

Reviewed-by: Felipe Balbi <balbi@ti.com>

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 4/9] usb: phy: twl4030: twl4030 shouldn't be subsys_initcall

[Felipe Balbi]

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

On Thu, Jun 13, 2013 at 02:13:54PM +0530, Kishon Vijay Abraham I wrote:
> Changed the inticall from subsys_initcall to module_init for
> twl4030-usb.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

not part of the series, should be sent separately. I'll queue this one
for v3.12 once v3.11-rc1 is tagged.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 5/9] ARM: OMAP: USB: Add phy binding information

[Felipe Balbi]

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

On Thu, Jun 13, 2013 at 02:13:55PM +0530, Kishon Vijay Abraham I wrote:
> In order for controllers to get PHY in case of non dt boot, the phy
> binding information (phy device name) should be added in the platform
> data of the controller.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

I would rather not pass strings around, any other way to handle this ?
Why do you need to pass this string ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 6/9] ARM: dts: omap: update usb_otg_hs data

[Felipe Balbi]

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

On Thu, Jun 13, 2013 at 02:13:56PM +0530, Kishon Vijay Abraham I wrote:
> Updated the usb_otg_hs dt data to include the *phy* and *phy-names*
> binding in order for the driver to use the new generic PHY framework.
> Also updated the Documentation to include the binding information.
> The PHY binding information can be found at
> Documentation/devicetree/bindings/phy/phy-bindings.txt
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

Reviewed-by: Felipe Balbi <balbi@ti.com>

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 7/9] usb: musb: omap2430: use the new generic PHY framework

[Felipe Balbi]

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

Hi,

On Thu, Jun 13, 2013 at 02:13:57PM +0530, Kishon Vijay Abraham I wrote:
> Use the generic PHY framework API to get the PHY. The usb_phy_set_resume
> and usb_phy_set_suspend is replaced with power_on/get_sync and
> power_off/put_sync to align with the new PHY framework.
> 
> musb->xceiv can't be removed as of now because musb core uses xceiv.state and
> xceiv.otg. Once there is a separate state machine to handle otg, these can be
> moved out of xceiv and then we can start using the generic PHY framework.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/usb/musb/musb_core.c |    1 +
>  drivers/usb/musb/musb_core.h |    3 +++
>  drivers/usb/musb/omap2430.c  |   29 +++++++++++++++++++++--------
>  3 files changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 37a261a..f732bcc 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -1864,6 +1864,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
>  	musb->board_set_power = plat->set_power;
>  	musb->min_power = plat->min_power;
>  	musb->ops = plat->platform_ops;
> +	musb->phy_label = plat->phy_label;
>  
>  	/* The musb_platform_init() call:
>  	 *   - adjusts musb->mregs
> diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
> index 7fb4819..498ae21 100644
> --- a/drivers/usb/musb/musb_core.h
> +++ b/drivers/usb/musb/musb_core.h
> @@ -46,6 +46,7 @@
>  #include <linux/usb.h>
>  #include <linux/usb/otg.h>
>  #include <linux/usb/musb.h>
> +#include <linux/phy/phy.h>
>  
>  struct musb;
>  struct musb_hw_ep;
> @@ -357,6 +358,7 @@ struct musb {
>  	u16			int_tx;
>  
>  	struct usb_phy		*xceiv;
> +	struct phy		*phy;
>  
>  	int nIrq;
>  	unsigned		irq_wake:1;
> @@ -434,6 +436,7 @@ struct musb {
>  	unsigned                double_buffer_not_ok:1;
>  
>  	struct musb_hdrc_config	*config;
> +	const char		*phy_label;
>  
>  #ifdef MUSB_CONFIG_PROC_FS
>  	struct proc_dir_entry *proc_entry;
> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> index 628b93f..c62a004 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -348,14 +348,24 @@ static int omap2430_musb_init(struct musb *musb)
>  	 * up through ULPI.  TWL4030-family PMICs include one,
>  	 * which needs a driver, drivers aren't always needed.
>  	 */
> -	if (dev->parent->of_node)
> +	if (dev->parent->of_node) {
> +		musb->phy = devm_phy_get(dev->parent, "usb2-phy");
> +
> +		/* We can't totally remove musb->xceiv as of now because
> +		 * musb core uses xceiv.state and xceiv.otg. Once we have
> +		 * a separate state machine to handle otg, these can be moved
> +		 * out of xceiv and then we can start using the generic PHY
> +		 * framework
> +		 */
>  		musb->xceiv = devm_usb_get_phy_by_phandle(dev->parent,
>  		    "usb-phy", 0);
> -	else
> +	} else {
>  		musb->xceiv = devm_usb_get_phy_dev(dev, 0);
> +		musb->phy = devm_phy_get(dev, musb->phy_label);
> +	}
>  
> -	if (IS_ERR(musb->xceiv)) {
> -		status = PTR_ERR(musb->xceiv);
> +	if (IS_ERR(musb->xceiv) || IS_ERR(musb->phy)) {
> +		status = PTR_ERR(musb->xceiv) | PTR_ERR(musb->phy);
>  
>  		if (status == -ENXIO)
>  			return status;
> @@ -397,9 +407,10 @@ static int omap2430_musb_init(struct musb *musb)
>  	if (glue->status != OMAP_MUSB_UNKNOWN)
>  		omap_musb_set_mailbox(glue);
>  
> -	usb_phy_init(musb->xceiv);
> +	phy_init(musb->phy);
>  
>  	pm_runtime_put_noidle(musb->controller);
> +	phy_pm_runtime_put(musb->phy);

see, weird unbalanced calls :-)

Make it all explicit:

phy_pm_runtime_get_sync(phy);
phy_init(phy);
phy_pm_runtime_put(phy);

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 8/9] usb: phy: omap-usb2: remove *set_suspend* callback from omap-usb2

[Felipe Balbi]

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

On Thu, Jun 13, 2013 at 02:13:58PM +0530, Kishon Vijay Abraham I wrote:
> Now that omap-usb2 is adapted to the new generic PHY framework,
> *set_suspend* ops can be removed from omap-usb2 driver.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

Reviewed-by: Felipe Balbi <balbi@ti.com>

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 9/9] usb: phy: twl4030-usb: remove *set_suspend* and *phy_init* ops

[Felipe Balbi]

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

On Thu, Jun 13, 2013 at 02:13:59PM +0530, Kishon Vijay Abraham I wrote:
> Now that twl4030-usb is adapted to the new generic PHY framework,
> *set_suspend* and *phy_init* ops can be removed from twl4030-usb driver.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

Reviewed-by: Felipe Balbi <balbi@ti.com>

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 2/9] usb: phy: omap-usb2: use the new generic PHY framework

[Kishon Vijay Abraham I]

Hi,

On Tuesday 18 June 2013 03:10 PM, Felipe Balbi wrote:
> Hi,
>
> On Thu, Jun 13, 2013 at 02:13:52PM +0530, Kishon Vijay Abraham I wrote:
>> @@ -159,6 +191,12 @@ static int omap_usb2_probe(struct platform_device *pdev)
>>   	otg->start_srp		= omap_usb_start_srp;
>>   	otg->phy		= &phy->phy;
>>
>> +	pm_runtime_enable(phy->dev);
>
> enabling pm_runtime here has the potential to cause a real big problem.
> How have you tested this patch ?

performed boot and enumeration testing. What issue do you expect here?

Thanks
Kishon

Re: [PATCH v7 0/9] Generic PHY Framework

[Felipe Balbi]

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

Hi,

On Mon, Jun 17, 2013 at 12:16:35PM +0200, Sylwester Nawrocki wrote:
> I have already used this API for our MIPI CSI-2/DSIM DPHYs driver,
> the RFC patch series can be found at [1].
> 
> Thanks,
> Sylwester
> 
> [1] http://www.spinics.net/lists/arm-kernel/msg251666.html

one comment to that series:

let's make the phy names standardized, how about phy-exynos-video-mipi.c
or phy-mipi-csi-dsim.c ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 2/9] usb: phy: omap-usb2: use the new generic PHY framework

[Felipe Balbi]

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

Hi,

On Tue, Jun 18, 2013 at 03:19:03PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Tuesday 18 June 2013 03:10 PM, Felipe Balbi wrote:
> >Hi,
> >
> >On Thu, Jun 13, 2013 at 02:13:52PM +0530, Kishon Vijay Abraham I wrote:
> >>@@ -159,6 +191,12 @@ static int omap_usb2_probe(struct platform_device *pdev)
> >>  	otg->start_srp		= omap_usb_start_srp;
> >>  	otg->phy		= &phy->phy;
> >>
> >>+	pm_runtime_enable(phy->dev);
> >
> >enabling pm_runtime here has the potential to cause a real big problem.
> >How have you tested this patch ?
> 
> performed boot and enumeration testing. What issue do you expect here?

hint: look at drvdata usage around the driver. Where is it initialized ?
Where is it used ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 2/9] usb: phy: omap-usb2: use the new generic PHY framework

[Kishon Vijay Abraham I]

Hi,

On Tuesday 18 June 2013 03:20 PM, Felipe Balbi wrote:
> Hi,
>
> On Tue, Jun 18, 2013 at 03:19:03PM +0530, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Tuesday 18 June 2013 03:10 PM, Felipe Balbi wrote:
>>> Hi,
>>>
>>> On Thu, Jun 13, 2013 at 02:13:52PM +0530, Kishon Vijay Abraham I wrote:
>>>> @@ -159,6 +191,12 @@ static int omap_usb2_probe(struct platform_device *pdev)
>>>>   	otg->start_srp		= omap_usb_start_srp;
>>>>   	otg->phy		= &phy->phy;
>>>>
>>>> +	pm_runtime_enable(phy->dev);
>>>
>>> enabling pm_runtime here has the potential to cause a real big problem.
>>> How have you tested this patch ?
>>
>> performed boot and enumeration testing. What issue do you expect here?
>
> hint: look at drvdata usage around the driver. Where is it initialized ?
> Where is it used ?

hmm.. since runtime_get calls weren't made very early, I think I dint 
see any issues. Thanks for pointing this.

Thanks
Kishon

Re: [PATCH v7 2/9] usb: phy: omap-usb2: use the new generic PHY framework

[Felipe Balbi]

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

Hi,

On Tue, Jun 18, 2013 at 03:26:23PM +0530, Kishon Vijay Abraham I wrote:
> >>>On Thu, Jun 13, 2013 at 02:13:52PM +0530, Kishon Vijay Abraham I wrote:
> >>>>@@ -159,6 +191,12 @@ static int omap_usb2_probe(struct platform_device *pdev)
> >>>>  	otg->start_srp		= omap_usb_start_srp;
> >>>>  	otg->phy		= &phy->phy;
> >>>>
> >>>>+	pm_runtime_enable(phy->dev);
> >>>
> >>>enabling pm_runtime here has the potential to cause a real big problem.
> >>>How have you tested this patch ?
> >>
> >>performed boot and enumeration testing. What issue do you expect here?
> >
> >hint: look at drvdata usage around the driver. Where is it initialized ?
> >Where is it used ?
> 
> hmm.. since runtime_get calls weren't made very early, I think I dint
> see any issues. Thanks for pointing this.

right, no problem. BTW, you don't get to_platform_device() ->
platform_get_drvdata(). A simple dev_get_drvdata() is sufficient :-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 5/9] ARM: OMAP: USB: Add phy binding information

[Kishon Vijay Abraham I]

Hi,

On Tuesday 18 June 2013 03:14 PM, Felipe Balbi wrote:
> On Thu, Jun 13, 2013 at 02:13:55PM +0530, Kishon Vijay Abraham I wrote:
>> In order for controllers to get PHY in case of non dt boot, the phy
>> binding information (phy device name) should be added in the platform
>> data of the controller.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>
> I would rather not pass strings around, any other way to handle this ?
> Why do you need to pass this string ?

Our old way of binding the controller and the phy using device name 
started creating problems after the devices are created using 
PLATFORM_DEVID_AUTO. Infact non-dt boot is broken in mainline for OMAP3 
platforms for which I have posted a RFC series 
http://www.serverphorums.com/read.php?12,708632 which also uses strings.
I'm not sure of any other way to deal with this.

Thanks
Kishon

Re: [PATCH v7 5/9] ARM: OMAP: USB: Add phy binding information

[Felipe Balbi]

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

Hi,

On Tue, Jun 18, 2013 at 03:34:36PM +0530, Kishon Vijay Abraham I wrote:
> On Tuesday 18 June 2013 03:14 PM, Felipe Balbi wrote:
> >On Thu, Jun 13, 2013 at 02:13:55PM +0530, Kishon Vijay Abraham I wrote:
> >>In order for controllers to get PHY in case of non dt boot, the phy
> >>binding information (phy device name) should be added in the platform
> >>data of the controller.
> >>
> >>Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> >
> >I would rather not pass strings around, any other way to handle this ?
> >Why do you need to pass this string ?
> 
> Our old way of binding the controller and the phy using device name
> started creating problems after the devices are created using
> PLATFORM_DEVID_AUTO. Infact non-dt boot is broken in mainline for
> OMAP3 platforms for which I have posted a RFC series
> http://www.serverphorums.com/read.php?12,708632 which also uses
> strings.
> I'm not sure of any other way to deal with this.

have you checked how other frameworks handle it ? Regulator has some
sort of binding in board-files, but I guess it passes the regulator
name?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 5/9] ARM: OMAP: USB: Add phy binding information

[Kishon Vijay Abraham I]

Hi,

On Tuesday 18 June 2013 03:57 PM, Felipe Balbi wrote:
> Hi,
>
> On Tue, Jun 18, 2013 at 03:34:36PM +0530, Kishon Vijay Abraham I wrote:
>> On Tuesday 18 June 2013 03:14 PM, Felipe Balbi wrote:
>>> On Thu, Jun 13, 2013 at 02:13:55PM +0530, Kishon Vijay Abraham I wrote:
>>>> In order for controllers to get PHY in case of non dt boot, the phy
>>>> binding information (phy device name) should be added in the platform
>>>> data of the controller.
>>>>
>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>>
>>> I would rather not pass strings around, any other way to handle this ?
>>> Why do you need to pass this string ?
>>
>> Our old way of binding the controller and the phy using device name
>> started creating problems after the devices are created using
>> PLATFORM_DEVID_AUTO. Infact non-dt boot is broken in mainline for
>> OMAP3 platforms for which I have posted a RFC series
>> http://www.serverphorums.com/read.php?12,708632 which also uses
>> strings.
>> I'm not sure of any other way to deal with this.
>
> have you checked how other frameworks handle it ? Regulator has some
> sort of binding in board-files, but I guess it passes the regulator
> name?

 From whatever I could make of, regulator has 3 ways to get the 
regulator one of which is using the binding in board-files (but it also 
uses device name which could create the same problem that we are facing).

1.) from dt data
2.) from _supply_ name
3.) from binding in board file

(referred regulator_dev_lookup() in regulator/core.c)

Thanks
Kishon

Re: [PATCH v7 5/9] ARM: OMAP: USB: Add phy binding information

[Felipe Balbi]

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

Hi,

On Tue, Jun 18, 2013 at 04:43:44PM +0530, Kishon Vijay Abraham I wrote:
> >On Tue, Jun 18, 2013 at 03:34:36PM +0530, Kishon Vijay Abraham I wrote:
> >>On Tuesday 18 June 2013 03:14 PM, Felipe Balbi wrote:
> >>>On Thu, Jun 13, 2013 at 02:13:55PM +0530, Kishon Vijay Abraham I wrote:
> >>>>In order for controllers to get PHY in case of non dt boot, the phy
> >>>>binding information (phy device name) should be added in the platform
> >>>>data of the controller.
> >>>>
> >>>>Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> >>>
> >>>I would rather not pass strings around, any other way to handle this ?
> >>>Why do you need to pass this string ?
> >>
> >>Our old way of binding the controller and the phy using device name
> >>started creating problems after the devices are created using
> >>PLATFORM_DEVID_AUTO. Infact non-dt boot is broken in mainline for
> >>OMAP3 platforms for which I have posted a RFC series
> >>http://www.serverphorums.com/read.php?12,708632 which also uses
> >>strings.
> >>I'm not sure of any other way to deal with this.
> >
> >have you checked how other frameworks handle it ? Regulator has some
> >sort of binding in board-files, but I guess it passes the regulator
> >name?
> 
> From whatever I could make of, regulator has 3 ways to get the
> regulator one of which is using the binding in board-files (but it
> also uses device name which could create the same problem that we are
> facing).
> 
> 1.) from dt data
> 2.) from _supply_ name
> 3.) from binding in board file
> 
> (referred regulator_dev_lookup() in regulator/core.c)

right, spot on. Which means we don't have a better, more elegant
solution now. Let's go ahead with this.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 5/9] ARM: OMAP: USB: Add phy binding information

[Kishon Vijay Abraham I]

Hi,

On Tuesday 18 June 2013 06:05 PM, Felipe Balbi wrote:
> Hi,
>
> On Tue, Jun 18, 2013 at 04:43:44PM +0530, Kishon Vijay Abraham I wrote:
>>> On Tue, Jun 18, 2013 at 03:34:36PM +0530, Kishon Vijay Abraham I wrote:
>>>> On Tuesday 18 June 2013 03:14 PM, Felipe Balbi wrote:
>>>>> On Thu, Jun 13, 2013 at 02:13:55PM +0530, Kishon Vijay Abraham I wrote:
>>>>>> In order for controllers to get PHY in case of non dt boot, the phy
>>>>>> binding information (phy device name) should be added in the platform
>>>>>> data of the controller.
>>>>>>
>>>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>>>>
>>>>> I would rather not pass strings around, any other way to handle this ?
>>>>> Why do you need to pass this string ?
>>>>
>>>> Our old way of binding the controller and the phy using device name
>>>> started creating problems after the devices are created using
>>>> PLATFORM_DEVID_AUTO. Infact non-dt boot is broken in mainline for
>>>> OMAP3 platforms for which I have posted a RFC series
>>>> http://www.serverphorums.com/read.php?12,708632 which also uses
>>>> strings.
>>>> I'm not sure of any other way to deal with this.
>>>
>>> have you checked how other frameworks handle it ? Regulator has some
>>> sort of binding in board-files, but I guess it passes the regulator
>>> name?
>>
>>  From whatever I could make of, regulator has 3 ways to get the
>> regulator one of which is using the binding in board-files (but it
>> also uses device name which could create the same problem that we are
>> facing).
>>
>> 1.) from dt data
>> 2.) from _supply_ name
>> 3.) from binding in board file
>>
>> (referred regulator_dev_lookup() in regulator/core.c)
>
> right, spot on. Which means we don't have a better, more elegant
> solution now. Let's go ahead with this.

Ok. So I'll drop RFC and resend this patch series
http://www.serverphorums.com/read.php?12,708632

Thanks
Kishon

Re: [PATCH v7 5/9] ARM: OMAP: USB: Add phy binding information

[Felipe Balbi]

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

Hi,

On Tue, Jun 18, 2013 at 08:25:00PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Tuesday 18 June 2013 06:05 PM, Felipe Balbi wrote:
> >Hi,
> >
> >On Tue, Jun 18, 2013 at 04:43:44PM +0530, Kishon Vijay Abraham I wrote:
> >>>On Tue, Jun 18, 2013 at 03:34:36PM +0530, Kishon Vijay Abraham I wrote:
> >>>>On Tuesday 18 June 2013 03:14 PM, Felipe Balbi wrote:
> >>>>>On Thu, Jun 13, 2013 at 02:13:55PM +0530, Kishon Vijay Abraham I wrote:
> >>>>>>In order for controllers to get PHY in case of non dt boot, the phy
> >>>>>>binding information (phy device name) should be added in the platform
> >>>>>>data of the controller.
> >>>>>>
> >>>>>>Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> >>>>>
> >>>>>I would rather not pass strings around, any other way to handle this ?
> >>>>>Why do you need to pass this string ?
> >>>>
> >>>>Our old way of binding the controller and the phy using device name
> >>>>started creating problems after the devices are created using
> >>>>PLATFORM_DEVID_AUTO. Infact non-dt boot is broken in mainline for
> >>>>OMAP3 platforms for which I have posted a RFC series
> >>>>http://www.serverphorums.com/read.php?12,708632 which also uses
> >>>>strings.
> >>>>I'm not sure of any other way to deal with this.
> >>>
> >>>have you checked how other frameworks handle it ? Regulator has some
> >>>sort of binding in board-files, but I guess it passes the regulator
> >>>name?
> >>
> >> From whatever I could make of, regulator has 3 ways to get the
> >>regulator one of which is using the binding in board-files (but it
> >>also uses device name which could create the same problem that we are
> >>facing).
> >>
> >>1.) from dt data
> >>2.) from _supply_ name
> >>3.) from binding in board file
> >>
> >>(referred regulator_dev_lookup() in regulator/core.c)
> >
> >right, spot on. Which means we don't have a better, more elegant
> >solution now. Let's go ahead with this.
> 
> Ok. So I'll drop RFC and resend this patch series
> http://www.serverphorums.com/read.php?12,708632

please do :-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 1/9] drivers: phy: add generic PHY framework

[Sylwester Nawrocki]

Hi Kishon,

I've noticed there is a little inconsistency between the code and 
documentation.

On 06/13/2013 10:43 AM, Kishon Vijay Abraham I wrote:
> +3. Creating the PHY
> +
> +The PHY driver should create the PHY in order for other peripheral controllers
> +to make use of it. The PHY framework provides 2 APIs to create the PHY.
> +
> +struct phy *phy_create(struct device *dev, int id, const struct phy_ops *ops,
> +	void *priv);
> +struct phy *devm_phy_create(struct device *dev, int id,
> +	const struct phy_ops *ops, void *priv);

The 'label' argument is missing in those function prototypes. It seems the
labels must be unique, I guess this needs to be documented. And do we allow
NULL labels ? If so, then there is probably a check for NULL label needed
in phy_lookup() and if not, then phy_create() should fail when NULL label
is passed ?

> +The PHY drivers can use one of the above 2 APIs to create the PHY by passing
> +the device pointer, id, phy ops and a driver data.
> +phy_ops is a set of function pointers for performing PHY operations such as
> +init, exit, power_on and power_off.

> +/**
> + * phy_create() - create a new phy
> + * @dev: device that is creating the new phy
> + * @id: id of the phy
> + * @ops: function pointers for performing phy operations
> + * @label: label given to the phy
> + * @priv: private data from phy driver
> + *
> + * Called to create a phy using phy framework.
> + */
> +struct phy *phy_create(struct device *dev, u8 id, const struct phy_ops *ops,
> +	const char *label, void *priv)
> +{
> +	int ret;
> +	struct phy *phy;
> +
> +	if (!dev) {
> +		dev_WARN(dev, "no device provided for PHY\n");
> +		ret = -EINVAL;
> +		goto err0;
> +	}
> +
> +	phy = kzalloc(sizeof(*phy), GFP_KERNEL);
> +	if (!phy) {
> +		ret = -ENOMEM;
> +		goto err0;
> +	}
> +
> +	device_initialize(&phy->dev);
> +
> +	phy->dev.class = phy_class;
> +	phy->dev.parent = dev;
> +	phy->dev.of_node = dev->of_node;
> +	phy->id = id;
> +	phy->ops = ops;
> +	phy->label = label;
> +
> +	dev_set_drvdata(&phy->dev, priv);
> +
> +	ret = dev_set_name(&phy->dev, "%s.%d", dev_name(dev), id);
> +	if (ret)
> +		goto err1;
> +
> +	ret = device_add(&phy->dev);
> +	if (ret)
> +		goto err1;
> +
> +	if (pm_runtime_enabled(dev))
> +		pm_runtime_enable(&phy->dev);
> +
> +	return phy;
> +
> +err1:
> +	put_device(&phy->dev);
> +	kfree(phy);
> +
> +err0:
> +	return ERR_PTR(ret);
> +}

Thanks,
Sylwester

Re: [PATCH v7 1/9] drivers: phy: add generic PHY framework

[Sylwester Nawrocki]

Hi,

On 06/13/2013 10:43 AM, Kishon Vijay Abraham I wrote:
> +/**
> + * phy_create() - create a new phy
> + * @dev: device that is creating the new phy
> + * @id: id of the phy
> + * @ops: function pointers for performing phy operations
> + * @label: label given to the phy
> + * @priv: private data from phy driver
> + *
> + * Called to create a phy using phy framework.
> + */
> +struct phy *phy_create(struct device *dev, u8 id, const struct phy_ops *ops,
> +	const char *label, void *priv)
> +{
> +	int ret;
> +	struct phy *phy;
> +
> +	if (!dev) {
> +		dev_WARN(dev, "no device provided for PHY\n");
> +		ret = -EINVAL;
> +		goto err0;
> +	}
> +
> +	phy = kzalloc(sizeof(*phy), GFP_KERNEL);
> +	if (!phy) {
> +		ret = -ENOMEM;
> +		goto err0;
> +	}
> +
> +	device_initialize(&phy->dev);
> +
> +	phy->dev.class = phy_class;
> +	phy->dev.parent = dev;
> +	phy->dev.of_node = dev->of_node;
> +	phy->id = id;
> +	phy->ops = ops;
> +	phy->label = label;

Perhaps we could make it:

	phy->label = kstrdup(label, GFP_KERNEL);

and free the label in phy_destroy() ?

That way the users would't need to keep the label around. It might
be useful when PHY provider generates the labels dynamically. I guess
it is OK for PHY providers to hard code the labels and having the PHY
user drivers passed labels in platform data ?

> +	dev_set_drvdata(&phy->dev, priv);
> +
> +	ret = dev_set_name(&phy->dev, "%s.%d", dev_name(dev), id);
> +	if (ret)
> +		goto err1;
> +
> +	ret = device_add(&phy->dev);
> +	if (ret)
> +		goto err1;
> +
> +	if (pm_runtime_enabled(dev))
> +		pm_runtime_enable(&phy->dev);
> +
> +	return phy;
> +
> +err1:
> +	put_device(&phy->dev);
> +	kfree(phy);
> +
> +err0:
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(phy_create);

> +/**
> + * phy_destroy() - destroy the phy
> + * @phy: the phy to be destroyed
> + *
> + * Called to destroy the phy.
> + */
> +void phy_destroy(struct phy *phy)
> +{
> +	pm_runtime_disable(&phy->dev);
> +	device_unregister(&phy->dev);

Isn't kfree(phy); missing here ?

> +}
> +EXPORT_SYMBOL_GPL(phy_destroy);

Thanks,
Sylwester

Re: [PATCH v7 1/9] drivers: phy: add generic PHY framework

[Kishon Vijay Abraham I]

Hi,

On Wednesday 19 June 2013 09:19 PM, Sylwester Nawrocki wrote:
> Hi,
>
> On 06/13/2013 10:43 AM, Kishon Vijay Abraham I wrote:
>> +/**
>> + * phy_create() - create a new phy
>> + * @dev: device that is creating the new phy
>> + * @id: id of the phy
>> + * @ops: function pointers for performing phy operations
>> + * @label: label given to the phy
>> + * @priv: private data from phy driver
>> + *
>> + * Called to create a phy using phy framework.
>> + */
>> +struct phy *phy_create(struct device *dev, u8 id, const struct phy_ops *ops,
>> +	const char *label, void *priv)
>> +{
>> +	int ret;
>> +	struct phy *phy;
>> +
>> +	if (!dev) {
>> +		dev_WARN(dev, "no device provided for PHY\n");
>> +		ret = -EINVAL;
>> +		goto err0;
>> +	}
>> +
>> +	phy = kzalloc(sizeof(*phy), GFP_KERNEL);
>> +	if (!phy) {
>> +		ret = -ENOMEM;
>> +		goto err0;
>> +	}
>> +
>> +	device_initialize(&phy->dev);
>> +
>> +	phy->dev.class = phy_class;
>> +	phy->dev.parent = dev;
>> +	phy->dev.of_node = dev->of_node;
>> +	phy->id = id;
>> +	phy->ops = ops;
>> +	phy->label = label;
>
> Perhaps we could make it:
>
> 	phy->label = kstrdup(label, GFP_KERNEL);
>
> and free the label in phy_destroy() ?

yeah.. Fixed.
>
> That way the users would't need to keep the label around. It might
> be useful when PHY provider generates the labels dynamically. I guess
> it is OK for PHY providers to hard code the labels and having the PHY
> user drivers passed labels in platform data ?

yeah. But the only concern here is there is no way to enforce the
labels are passed in platform data. The PHY user driver can also
hard code the labels while getting the reference to the PHY and we can
catch such cases only by review.

>
>> +	dev_set_drvdata(&phy->dev, priv);
>> +
>> +	ret = dev_set_name(&phy->dev, "%s.%d", dev_name(dev), id);
>> +	if (ret)
>> +		goto err1;
>> +
>> +	ret = device_add(&phy->dev);
>> +	if (ret)
>> +		goto err1;
>> +
>> +	if (pm_runtime_enabled(dev))
>> +		pm_runtime_enable(&phy->dev);
>> +
>> +	return phy;
>> +
>> +err1:
>> +	put_device(&phy->dev);
>> +	kfree(phy);
>> +
>> +err0:
>> +	return ERR_PTR(ret);
>> +}
>> +EXPORT_SYMBOL_GPL(phy_create);
>
>> +/**
>> + * phy_destroy() - destroy the phy
>> + * @phy: the phy to be destroyed
>> + *
>> + * Called to destroy the phy.
>> + */
>> +void phy_destroy(struct phy *phy)
>> +{
>> +	pm_runtime_disable(&phy->dev);
>> +	device_unregister(&phy->dev);
>
> Isn't kfree(phy); missing here ?

Not actually. Its done in phy_release (class's dev_release method)

Thanks
Kishon

Re: [PATCH v7 7/9] usb: musb: omap2430: use the new generic PHY framework

[Kishon Vijay Abraham I]

Hi,

On Tuesday 18 June 2013 03:17 PM, Felipe Balbi wrote:
> Hi,
>
> On Thu, Jun 13, 2013 at 02:13:57PM +0530, Kishon Vijay Abraham I wrote:
>> Use the generic PHY framework API to get the PHY. The usb_phy_set_resume
>> and usb_phy_set_suspend is replaced with power_on/get_sync and
>> power_off/put_sync to align with the new PHY framework.
>>
>> musb->xceiv can't be removed as of now because musb core uses xceiv.state and
>> xceiv.otg. Once there is a separate state machine to handle otg, these can be
>> moved out of xceiv and then we can start using the generic PHY framework.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>   drivers/usb/musb/musb_core.c |    1 +
>>   drivers/usb/musb/musb_core.h |    3 +++
>>   drivers/usb/musb/omap2430.c  |   29 +++++++++++++++++++++--------
>>   3 files changed, 25 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
>> index 37a261a..f732bcc 100644
>> --- a/drivers/usb/musb/musb_core.c
>> +++ b/drivers/usb/musb/musb_core.c
>> @@ -1864,6 +1864,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
>>   	musb->board_set_power = plat->set_power;
>>   	musb->min_power = plat->min_power;
>>   	musb->ops = plat->platform_ops;
>> +	musb->phy_label = plat->phy_label;
>>
>>   	/* The musb_platform_init() call:
>>   	 *   - adjusts musb->mregs
>> diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
>> index 7fb4819..498ae21 100644
>> --- a/drivers/usb/musb/musb_core.h
>> +++ b/drivers/usb/musb/musb_core.h
>> @@ -46,6 +46,7 @@
>>   #include <linux/usb.h>
>>   #include <linux/usb/otg.h>
>>   #include <linux/usb/musb.h>
>> +#include <linux/phy/phy.h>
>>
>>   struct musb;
>>   struct musb_hw_ep;
>> @@ -357,6 +358,7 @@ struct musb {
>>   	u16			int_tx;
>>
>>   	struct usb_phy		*xceiv;
>> +	struct phy		*phy;
>>
>>   	int nIrq;
>>   	unsigned		irq_wake:1;
>> @@ -434,6 +436,7 @@ struct musb {
>>   	unsigned                double_buffer_not_ok:1;
>>
>>   	struct musb_hdrc_config	*config;
>> +	const char		*phy_label;
>>
>>   #ifdef MUSB_CONFIG_PROC_FS
>>   	struct proc_dir_entry *proc_entry;
>> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
>> index 628b93f..c62a004 100644
>> --- a/drivers/usb/musb/omap2430.c
>> +++ b/drivers/usb/musb/omap2430.c
>> @@ -348,14 +348,24 @@ static int omap2430_musb_init(struct musb *musb)
>>   	 * up through ULPI.  TWL4030-family PMICs include one,
>>   	 * which needs a driver, drivers aren't always needed.
>>   	 */
>> -	if (dev->parent->of_node)
>> +	if (dev->parent->of_node) {
>> +		musb->phy = devm_phy_get(dev->parent, "usb2-phy");
>> +
>> +		/* We can't totally remove musb->xceiv as of now because
>> +		 * musb core uses xceiv.state and xceiv.otg. Once we have
>> +		 * a separate state machine to handle otg, these can be moved
>> +		 * out of xceiv and then we can start using the generic PHY
>> +		 * framework
>> +		 */
>>   		musb->xceiv = devm_usb_get_phy_by_phandle(dev->parent,
>>   		    "usb-phy", 0);
>> -	else
>> +	} else {
>>   		musb->xceiv = devm_usb_get_phy_dev(dev, 0);
>> +		musb->phy = devm_phy_get(dev, musb->phy_label);
>> +	}
>>
>> -	if (IS_ERR(musb->xceiv)) {
>> -		status = PTR_ERR(musb->xceiv);
>> +	if (IS_ERR(musb->xceiv) || IS_ERR(musb->phy)) {
>> +		status = PTR_ERR(musb->xceiv) | PTR_ERR(musb->phy);
>>
>>   		if (status == -ENXIO)
>>   			return status;
>> @@ -397,9 +407,10 @@ static int omap2430_musb_init(struct musb *musb)
>>   	if (glue->status != OMAP_MUSB_UNKNOWN)
>>   		omap_musb_set_mailbox(glue);
>>
>> -	usb_phy_init(musb->xceiv);
>> +	phy_init(musb->phy);
>>
>>   	pm_runtime_put_noidle(musb->controller);
>> +	phy_pm_runtime_put(musb->phy);
>
> see, weird unbalanced calls :-)
>
> Make it all explicit:
>
> phy_pm_runtime_get_sync(phy);
> phy_init(phy);
> phy_pm_runtime_put(phy);

I think then it makes sense to drop get_sync from phy_init()?

Thanks
Kishon

Re: [PATCH v7 1/9] drivers: phy: add generic PHY framework

[Kishon Vijay Abraham I]

On Wednesday 19 June 2013 02:52 AM, Sylwester Nawrocki wrote:
> Hi Kishon,
>
> I've noticed there is a little inconsistency between the code and documentation.
>
> On 06/13/2013 10:43 AM, Kishon Vijay Abraham I wrote:
>> +3. Creating the PHY
>> +
>> +The PHY driver should create the PHY in order for other peripheral controllers
>> +to make use of it. The PHY framework provides 2 APIs to create the PHY.
>> +
>> +struct phy *phy_create(struct device *dev, int id, const struct phy_ops *ops,
>> +    void *priv);
>> +struct phy *devm_phy_create(struct device *dev, int id,
>> +    const struct phy_ops *ops, void *priv);
>
> The 'label' argument is missing in those function prototypes. It seems the
> labels must be unique, I guess this needs to be documented. And do we allow
> NULL labels ? If so, then there is probably a check for NULL label needed
> in phy_lookup() and if not, then phy_create() should fail when NULL label
> is passed ?

Yeah. Label is used only for non-dt case so NULL should be allowed while 
phy_create().

Thanks
Kishon

Re: [PATCH v7 7/9] usb: musb: omap2430: use the new generic PHY framework

[Felipe Balbi]

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

Hi,

On Mon, Jun 24, 2013 at 11:01:56AM +0530, Kishon Vijay Abraham I wrote:
> >>@@ -397,9 +407,10 @@ static int omap2430_musb_init(struct musb *musb)
> >>  	if (glue->status != OMAP_MUSB_UNKNOWN)
> >>  		omap_musb_set_mailbox(glue);
> >>
> >>-	usb_phy_init(musb->xceiv);
> >>+	phy_init(musb->phy);
> >>
> >>  	pm_runtime_put_noidle(musb->controller);
> >>+	phy_pm_runtime_put(musb->phy);
> >
> >see, weird unbalanced calls :-)
> >
> >Make it all explicit:
> >
> >phy_pm_runtime_get_sync(phy);
> >phy_init(phy);
> >phy_pm_runtime_put(phy);
> 
> I think then it makes sense to drop get_sync from phy_init()?

maybe not, you don't know if the phy has already autosuspended or not.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 0/9] Generic PHY Framework

[Sylwester Nawrocki]

Hi,

On 06/18/2013 11:49 AM, Felipe Balbi wrote:
> On Mon, Jun 17, 2013 at 12:16:35PM +0200, Sylwester Nawrocki wrote:
>> I have already used this API for our MIPI CSI-2/DSIM DPHYs driver,
>> the RFC patch series can be found at [1].
>>
>> Thanks,
>> Sylwester
>>
>> [1] http://www.spinics.net/lists/arm-kernel/msg251666.html
> 
> one comment to that series:
> 
> let's make the phy names standardized, how about phy-exynos-video-mipi.c
> or phy-mipi-csi-dsim.c ?

Yes, I have been thinking about that, wasn't sure exactly what pattern to
chose. I would make it phy-exynos-mipi-video.c, phy-exynos-dsim-csis.c feels
a bit odd. phy-mipi-csis-dsim.c might be to generic as MIPI CSIS stands for
MIPI CSI Slave and MIPI DSIM - MIPI DSI Master. And there might be (actually
are) IP blocks in an SoC that use the other MIPI Aliance standards.
For the HDMI PHY I guess just phy-exynos-hdmi.c could be used.

Thanks,
Sylwester

Re: [PATCH v7 0/9] Generic PHY Framework

[Felipe Balbi]

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

On Mon, Jun 24, 2013 at 09:27:20PM +0200, Sylwester Nawrocki wrote:
> Hi,
> 
> On 06/18/2013 11:49 AM, Felipe Balbi wrote:
> > On Mon, Jun 17, 2013 at 12:16:35PM +0200, Sylwester Nawrocki wrote:
> >> I have already used this API for our MIPI CSI-2/DSIM DPHYs driver,
> >> the RFC patch series can be found at [1].
> >>
> >> Thanks,
> >> Sylwester
> >>
> >> [1] http://www.spinics.net/lists/arm-kernel/msg251666.html
> > 
> > one comment to that series:
> > 
> > let's make the phy names standardized, how about phy-exynos-video-mipi.c
> > or phy-mipi-csi-dsim.c ?
> 
> Yes, I have been thinking about that, wasn't sure exactly what pattern to
> chose. I would make it phy-exynos-mipi-video.c, phy-exynos-dsim-csis.c feels
> a bit odd. phy-mipi-csis-dsim.c might be to generic as MIPI CSIS stands for
> MIPI CSI Slave and MIPI DSIM - MIPI DSI Master. And there might be (actually
> are) IP blocks in an SoC that use the other MIPI Aliance standards.
> For the HDMI PHY I guess just phy-exynos-hdmi.c could be used.

sounds good to me :-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]