linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Generic PHY Framework
@ 2013-02-19  5:53 Kishon Vijay Abraham I
  2013-02-19  5:53 ` [PATCH v2 1/5] drivers: phy: add generic PHY framework Kishon Vijay Abraham I
                   ` (5 more replies)
  0 siblings, 6 replies; 24+ messages in thread
From: Kishon Vijay Abraham I @ 2013-02-19  5:53 UTC (permalink / raw)
  To: rob, tony, linux, eballetbo, javier, kishon, balbi, gregkh, akpm,
	mchehab, cesarb, davem, arnd, santosh.shilimkar, broonie,
	swarren, linux-doc, linux-kernel, linux-arm-kernel, linux-omap,
	linux-usb, netdev

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. To obtain a reference to the PHY
without using phandle, the platform specfic intialization code (say from board
file) should have already called phy_bind with the binding information. The
binding information consists of phy's device name, phy user device name and an
index. The index is used when the same phy user binds to mulitple phys.

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.

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 (5):
  drivers: phy: add generic PHY framework
  usb: phy: omap-usb2: use the new generic PHY framework
  usb: otg: twl4030: use the new generic PHY framework
  ARM: OMAP: USB: Add phy binding information
  usb: musb: omap2430: use the new generic PHY framework

 Documentation/ABI/testing/sysfs-class-phy    |   15 +
 Documentation/phy.txt                        |  113 ++++++
 MAINTAINERS                                  |    7 +
 arch/arm/mach-omap2/board-2430sdp.c          |    2 +
 arch/arm/mach-omap2/board-3430sdp.c          |    2 +
 arch/arm/mach-omap2/board-4430sdp.c          |    2 +
 arch/arm/mach-omap2/board-cm-t35.c           |    2 +
 arch/arm/mach-omap2/board-devkit8000.c       |    2 +
 arch/arm/mach-omap2/board-igep0020.c         |    2 +
 arch/arm/mach-omap2/board-ldp.c              |    2 +
 arch/arm/mach-omap2/board-omap3beagle.c      |    2 +
 arch/arm/mach-omap2/board-omap3evm.c         |    2 +
 arch/arm/mach-omap2/board-omap3logic.c       |    2 +
 arch/arm/mach-omap2/board-omap3pandora.c     |    2 +
 arch/arm/mach-omap2/board-omap3stalker.c     |    2 +
 arch/arm/mach-omap2/board-omap3touchbook.c   |    2 +
 arch/arm/mach-omap2/board-omap4panda.c       |    2 +
 arch/arm/mach-omap2/board-overo.c            |    2 +
 arch/arm/mach-omap2/board-rm680.c            |    2 +
 arch/arm/mach-omap2/board-zoom-peripherals.c |    2 +
 drivers/Kconfig                              |    2 +
 drivers/Makefile                             |    2 +
 drivers/phy/Kconfig                          |   13 +
 drivers/phy/Makefile                         |    5 +
 drivers/phy/phy-core.c                       |  519 ++++++++++++++++++++++++++
 drivers/usb/musb/musb_core.h                 |    2 +
 drivers/usb/musb/omap2430.c                  |   22 +-
 drivers/usb/otg/twl4030-usb.c                |   41 ++
 drivers/usb/phy/omap-usb2.c                  |   49 +++
 include/linux/phy/phy.h                      |  198 ++++++++++
 include/linux/usb/omap_usb.h                 |    3 +
 31 files changed, 1019 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-phy
 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


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

* [PATCH v2 1/5] drivers: phy: add generic PHY framework
  2013-02-19  5:53 [PATCH v2 0/5] Generic PHY Framework Kishon Vijay Abraham I
@ 2013-02-19  5:53 ` Kishon Vijay Abraham I
  2013-02-19  8:01   ` Felipe Balbi
                     ` (2 more replies)
  2013-02-19  5:53 ` [PATCH v2 2/5] usb: phy: omap-usb2: use the new " Kishon Vijay Abraham I
                   ` (4 subsequent siblings)
  5 siblings, 3 replies; 24+ messages in thread
From: Kishon Vijay Abraham I @ 2013-02-19  5:53 UTC (permalink / raw)
  To: rob, tony, linux, eballetbo, javier, kishon, balbi, gregkh, akpm,
	mchehab, cesarb, davem, arnd, santosh.shilimkar, broonie,
	swarren, linux-doc, linux-kernel, linux-arm-kernel, linux-omap,
	linux-usb, netdev

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. To obtain a reference to the PHY without
using phandle, the platform specfic intialization code (say from board file)
should have already called phy_bind with the binding information. The binding
information consists of phy's device name, phy user device name and an index.
The index is used when the same phy user binds to mulitple phys.

PHY drivers should create the PHY by passing phy_descriptor that has
describes the PHY (label, type etc..) and ops like init, exit, suspend, resume,
poweron, shutdown.

The documentation for the generic PHY framework is added in
Documentation/phy.txt and the documentation for the sysfs entry is added
in Documentation/ABI/testing/sysfs-class-phy.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 Documentation/ABI/testing/sysfs-class-phy |   15 +
 Documentation/phy.txt                     |  113 +++++++
 MAINTAINERS                               |    7 +
 drivers/Kconfig                           |    2 +
 drivers/Makefile                          |    2 +
 drivers/phy/Kconfig                       |   13 +
 drivers/phy/Makefile                      |    5 +
 drivers/phy/phy-core.c                    |  519 +++++++++++++++++++++++++++++
 include/linux/phy/phy.h                   |  198 +++++++++++
 9 files changed, 874 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-phy
 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/ABI/testing/sysfs-class-phy b/Documentation/ABI/testing/sysfs-class-phy
new file mode 100644
index 0000000..ffd9930
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-phy
@@ -0,0 +1,15 @@
+What:		/sys/class/phy/<phy>/label
+Date:		Feb 2013
+KernelVersion:	3.10
+Contact:	Kishon Vijay Abraham I <kishon@ti.com>
+Description:
+		This is a read-only file for getting the label of the phy.
+
+What:		/sys/class/phy/<phy>/bind
+Date:		Feb 2013
+KernelVersion:	3.10
+Contact:	Kishon Vijay Abraham I <kishon@ti.com>
+Description:
+		This is a read-only file for reading the phy binding
+		information. It contains the device name of the controller,
+		the index and the device name of the PHY in that order.
diff --git a/Documentation/phy.txt b/Documentation/phy.txt
new file mode 100644
index 0000000..a850aa0
--- /dev/null
+++ b/Documentation/phy.txt
@@ -0,0 +1,113 @@
+			The Generic PHY Framework
+		  Kishon Vijay Abraham I <kishon@ti.com>
+
+This document explains the Generic PHY Framework along with the APIs provided,
+how-to-use, current status and the future work list.
+
+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
+controller has PHY functionality embedded into it and others use an external
+PHY. Other peripherals that uses a 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 to
+increase code maintainability.
+
+This framework will be of use only to devices that uses external PHY (PHY
+functionality is not embedded within the controller).
+
+2. 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, struct phy_descriptor *desc);
+struct phy *devm_phy_create(struct device *dev, struct phy_descriptor *desc);
+
+The PHY drivers can use one of the above 2 APIs to create the PHY by passing
+the device pointer and the phy_descriptor. phy_descriptor is a structure that
+contains information about the PHY such as label, type, bus_type and phy_ops.
+phy_ops is a set of function pointers for performing PHY operations such as
+init, exit, suspend, resume, poweron and shutdown.
+
+3. Binding the PHY to the controller
+
+The framework provides an API for binding the controller to the PHY in the
+case of non dt boot.
+
+struct phy_bind *phy_bind(const char *dev_name, u8 index,
+				const char *phy_dev_name);
+
+The API fills the phy_bind structure with the dev_name (device name of the
+controller), index and phy_dev_name (device name of the PHY). This will
+be used when the controller requests this phy. This API should be used by
+platform specific initialization code (board file).
+
+In the case of dt boot, the binding information should be added in the dt
+data of the controller.
+
+4. Getting a reference to the PHY
+
+Before the controller can make use of the PHY, it has to get a reference to
+the PHY. The PHY framework provides 4 APIs to get a reference to the PHY.
+
+struct phy *phy_get(struct device *dev, u8 index);
+struct phy *devm_phy_get(struct device *dev, u8 index);
+struct phy *of_phy_get(struct device *dev, const char *phandle, u8 index);
+struct phy *devm_of_phy_get(struct device *dev, const char *phandle, u8 index);
+
+phy_get and devm_phy_get can be used to get the PHY in non-dt boot. This API
+uses the binding information added using the phy_bind API to find and return
+the appropriate 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.
+
+of_phy_get and devm_of_phy_get can be used to get the PHY in dt boot. These
+APIs take the phandle and index to get a reference to the PHY. The only
+difference between the two APIs is that devm_of_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 destroys the PHY and devm_phy_destroy destroys the devres
+associated with this PHY.
+
+7. Current Status
+
+Currently only USB in OMAP is made to use this framework. However using the
+USB PHY library cannot be completely removed because it is intertwined with
+OTG. Once we move OTG out of PHY completely, using the old PHY library can be
+completely removed. SATA in OMAP will also more likely use this new framework
+and we should have a patch for it soon.
+
+8. Future Work list
+
+The intention of writing this framework is to have all the phy drivers spread
+all over the kernel to use this framework and have it in drivers/phy/. So
+immediate action items include make USB and Ethernet use this Generic PHY
+Framework.
diff --git a/MAINTAINERS b/MAINTAINERS
index c5b37de..cba2a92 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3381,6 +3381,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 f5fb072..5230abe 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -158,4 +158,6 @@ source "drivers/irqchip/Kconfig"
 
 source "drivers/ipack/Kconfig"
 
+source "drivers/phy/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 7863b9f..4d5c9cb 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -45,6 +45,8 @@ obj-y				+= char/
 # gpu/ comes after char for AGP vs DRM startup
 obj-y				+= gpu/
 
+obj-y				+= phy/
+
 obj-$(CONFIG_CONNECTOR)		+= connector/
 
 # i810fb and intelfb depend on char/agp/
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
new file mode 100644
index 0000000..34f7077
--- /dev/null
+++ b/drivers/phy/Kconfig
@@ -0,0 +1,13 @@
+#
+# PHY
+#
+
+menuconfig GENERIC_PHY
+	tristate "Generic PHY Support"
+	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..c52aaf8
--- /dev/null
+++ b/drivers/phy/phy-core.c
@@ -0,0 +1,519 @@
+/*
+ * 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>
+
+static struct class *phy_class;
+static LIST_HEAD(phy_list);
+static DEFINE_MUTEX(phy_list_mutex);
+static LIST_HEAD(phy_bind_list);
+
+static void devm_phy_release(struct device *dev, void *res)
+{
+	struct phy *phy = *(struct phy **)res;
+
+	phy_put(phy);
+}
+
+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(struct device *dev, u8 index)
+{
+	struct phy_bind *phy_bind = NULL;
+
+	list_for_each_entry(phy_bind, &phy_bind_list, list) {
+		if (!(strcmp(phy_bind->dev_name, dev_name(dev)) &&
+				phy_bind->index == index)) {
+			if (phy_bind->phy)
+				return phy_bind->phy;
+			else
+				return ERR_PTR(-EPROBE_DEFER);
+		}
+	}
+
+	return ERR_PTR(-ENODEV);
+}
+
+static struct phy *of_phy_lookup(struct device *dev, struct device_node *node)
+{
+	struct phy *phy;
+
+	list_for_each_entry(phy, &phy_list, head) {
+		if (node != phy->desc->of_node)
+			continue;
+
+		return phy;
+	}
+
+	return ERR_PTR(-EPROBE_DEFER);
+}
+
+/**
+ * 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 (phy) {
+		module_put(phy->desc->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_get - lookup and obtain a reference to a phy by phandle
+ * @dev: device that requests this phy
+ * @phandle: name of the property holding the phy phandle value
+ * @index - the index of the phy
+ *
+ * Returns the phy 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.
+ */
+struct phy *of_phy_get(struct device *dev, const char *phandle, u8 index)
+{
+	struct phy *phy = NULL;
+	struct phy_bind *phy_map = NULL;
+	struct device_node *node;
+
+	if (!dev->of_node) {
+		dev_dbg(dev, "device does not have a device node entry\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	node = of_parse_phandle(dev->of_node, phandle, index);
+	if (!node) {
+		dev_dbg(dev, "failed to get %s phandle in %s node\n", phandle,
+			dev->of_node->full_name);
+		return ERR_PTR(-ENODEV);
+	}
+
+	mutex_lock(&phy_list_mutex);
+
+	phy = of_phy_lookup(dev, node);
+	if (IS_ERR(phy) || !try_module_get(phy->desc->ops->owner)) {
+		phy = ERR_PTR(-EPROBE_DEFER);
+		goto err0;
+	}
+
+	phy_map = phy_bind(dev_name(dev), index, dev_name(&phy->dev));
+	if (!IS_ERR(phy_map)) {
+		phy_map->phy = phy;
+		phy_map->auto_bind = true;
+	}
+
+	get_device(&phy->dev);
+
+err0:
+	mutex_unlock(&phy_list_mutex);
+	of_node_put(node);
+
+	return phy;
+}
+EXPORT_SYMBOL_GPL(of_phy_get);
+
+/**
+ * devm_of_phy_get - lookup and obtain a reference to a phy by phandle
+ * @dev: device that requests this phy
+ * @phandle: name of the property holding the phy phandle value
+ * @index - the index of the phy
+ *
+ * Calls of_phy_get to get a reference to the PHY and passes on the return
+ * value of of_phy_get. While at that, it also 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.
+ */
+struct phy *devm_of_phy_get(struct device *dev, const char *phandle, u8 index)
+{
+	struct phy *phy = NULL, **ptr;
+
+	ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	phy = of_phy_get(dev, phandle, index);
+	if (!IS_ERR(phy)) {
+		*ptr = phy;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return phy;
+}
+EXPORT_SYMBOL_GPL(devm_of_phy_get);
+
+/**
+ * phy_get - lookup and obtain a reference to a phy.
+ * @dev: device that requests this phy
+ * @index: the index of the phy
+ *
+ * Returns the phy driver, after getting a refcount to it; or
+ * -ENODEV if there is no such phy.  The caller is responsible for
+ * calling phy_put() to release that count.
+ */
+struct phy *phy_get(struct device *dev, u8 index)
+{
+	struct phy *phy = NULL;
+
+	mutex_lock(&phy_list_mutex);
+
+	phy = phy_lookup(dev, index);
+	if (IS_ERR(phy)) {
+		dev_err(dev, "unable to find phy\n");
+		goto err0;
+	}
+
+	if (!try_module_get(phy->desc->ops->owner)) {
+		phy = ERR_PTR(-EPROBE_DEFER);
+		goto err0;
+	}
+
+	get_device(&phy->dev);
+
+err0:
+	mutex_unlock(&phy_list_mutex);
+
+	return phy;
+}
+EXPORT_SYMBOL_GPL(phy_get);
+
+/**
+ * devm_phy_get - lookup and obtain a reference to a phy.
+ * @dev: device that requests this phy
+ * @index: the index of the phy
+ *
+ * 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, u8 index)
+{
+	struct phy **ptr, *phy;
+
+	ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	phy = phy_get(dev, index);
+	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
+ * @desc: descriptor of the phy
+ *
+ * Called to create a phy using phy framework.
+ */
+struct phy *phy_create(struct device *dev, struct phy_descriptor *desc)
+{
+	int ret;
+	struct phy *phy;
+	struct phy_bind *phy_bind;
+	const char *devname = NULL;
+
+	if (!dev || !desc) {
+		dev_err(dev, "no descriptor/device provided for PHY\n");
+		ret = -EINVAL;
+		goto err0;
+	}
+
+	if (!(desc->ops)) {
+		dev_err(dev, "no PHY ops/PHY owner provided\n");
+		ret = -EINVAL;
+		goto err0;
+	}
+
+	phy = kzalloc(sizeof(*phy), GFP_KERNEL);
+	if (!phy) {
+		ret = -ENOMEM;
+		goto err0;
+	}
+
+	devname = dev_name(dev);
+	device_initialize(&phy->dev);
+	phy->desc = desc;
+	phy->dev.class = phy_class;
+	phy->dev.parent = dev;
+	phy->dev.bus = desc->bus;
+	ret = dev_set_name(&phy->dev, "%s", devname);
+	if (ret)
+		goto err1;
+
+	mutex_lock(&phy_list_mutex);
+	list_for_each_entry(phy_bind, &phy_bind_list, list)
+		if (!(strcmp(phy_bind->phy_dev_name, devname)))
+			phy_bind->phy = phy;
+
+	list_add_tail(&phy->head, &phy_list);
+
+	ret = device_add(&phy->dev);
+	if (ret)
+		goto err2;
+
+	mutex_unlock(&phy_list_mutex);
+
+	return phy;
+
+err2:
+	phy_bind->phy = NULL;
+	list_del(&phy->head);
+	mutex_unlock(&phy_list_mutex);
+
+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
+ * @desc: descriptor of the phy
+ *
+ * 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, struct phy_descriptor *desc)
+{
+	struct phy **ptr, *phy;
+
+	ptr = devres_alloc(devm_phy_consume, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	phy = phy_create(dev, desc);
+	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)
+{
+	struct phy_bind *phy_bind;
+
+	mutex_lock(&phy_list_mutex);
+	list_for_each_entry(phy_bind, &phy_bind_list, list) {
+		if (phy_bind->phy == phy)
+			phy_bind->phy = NULL;
+
+		if (phy_bind->auto_bind) {
+			list_del(&phy_bind->list);
+			kfree(phy_bind);
+		}
+	}
+
+	list_del(&phy->head);
+	mutex_unlock(&phy_list_mutex);
+
+	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);
+
+/**
+ * phy_bind - bind the phy and the controller that uses the phy
+ * @dev_name: the device name of the device that will bind to the phy
+ * @index: index to specify the port number
+ * @phy_dev_name: the device name of the phy
+ *
+ * Fills the phy_bind structure with the dev_name and phy_dev_name. This will
+ * be used when the phy driver registers the phy and when the controller
+ * requests this phy.
+ *
+ * To be used by platform specific initialization code.
+ */
+struct phy_bind *phy_bind(const char *dev_name, u8 index,
+				const char *phy_dev_name)
+{
+	struct phy_bind *phy_bind;
+
+	phy_bind = kzalloc(sizeof(*phy_bind), GFP_KERNEL);
+	if (!phy_bind)
+		return ERR_PTR(-ENOMEM);
+
+	phy_bind->dev_name = dev_name;
+	phy_bind->phy_dev_name = phy_dev_name;
+	phy_bind->index = index;
+	phy_bind->auto_bind = false;
+
+	list_add_tail(&phy_bind->list, &phy_bind_list);
+
+	return phy_bind;
+}
+EXPORT_SYMBOL_GPL(phy_bind);
+
+static ssize_t phy_name_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct phy *phy;
+
+	phy = container_of(dev, struct phy, dev);
+
+	return sprintf(buf, "%s\n", phy->desc->label);
+}
+
+static ssize_t phy_bind_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct phy_bind *phy_bind;
+	struct phy *phy;
+	char *p = buf;
+	int len;
+
+	phy = container_of(dev, struct phy, dev);
+
+	list_for_each_entry(phy_bind, &phy_bind_list, list)
+		if (phy_bind->phy == phy)
+			p += sprintf(p, "%s %d %s\n", phy_bind->dev_name,
+				phy_bind->index, phy_bind->phy_dev_name);
+	len = (p - buf);
+
+	return len;
+}
+
+static struct device_attribute phy_dev_attrs[] = {
+	__ATTR(label, 0444, phy_name_show, NULL),
+	__ATTR(bind, 0444, phy_bind_show, NULL),
+	__ATTR_NULL,
+};
+
+/**
+ * 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;
+	phy_class->dev_attrs = phy_dev_attrs;
+
+	return 0;
+}
+subsys_initcall(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..4e9076d
--- /dev/null
+++ b/include/linux/phy/phy.h
@@ -0,0 +1,198 @@
+/*
+ * 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>
+
+struct phy;
+
+/**
+ * struct phy_descriptor - structure that describes the PHY
+ * @label: label given to phy
+ * @type: specifies the phy type
+ * @of_node: device node of the phy
+ * @ops: function pointers for performing phy operations
+ */
+struct phy_descriptor {
+	const char		*label;
+	int			type;
+	struct bus_type		*bus;
+	struct device_node	*of_node;
+	struct phy_ops		*ops;
+};
+
+/**
+ * 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
+ * @suspend: suspending the phy
+ * @resume: resuming the phy
+ * @poweron: powering on the phy
+ * @shutdown: shutting down the phy
+ * @owner: the module owner containing the ops
+ */
+struct phy_ops {
+	int	(*init)(struct phy_descriptor *desc);
+	int	(*exit)(struct phy_descriptor *desc);
+	int	(*suspend)(struct phy_descriptor *desc);
+	int	(*resume)(struct phy_descriptor *desc);
+	int	(*poweron)(struct phy_descriptor *desc);
+	int	(*shutdown)(struct phy_descriptor *desc);
+	struct module *owner;
+};
+
+/**
+ * struct phy - represent the phy device
+ * @desc: descriptor for the phy
+ * @head: to support multiple transceivers
+ */
+struct phy {
+	struct device		dev;
+	struct phy_descriptor	*desc;
+	struct list_head	head;
+};
+
+/**
+ * struct phy_bind - represent the binding for the phy
+ * @dev_name: the device name of the device that will bind to the phy
+ * @phy_dev_name: the device name of the phy
+ * @index: used if a single controller uses multiple phys
+ * @auto_bind: tells if the binding is done explicitly from board file or not
+ * @phy: reference to the phy
+ * @list: to maintain a linked list of the binding information
+ */
+struct phy_bind {
+	const char	*dev_name;
+	const char	*phy_dev_name;
+	u8		index;
+	int		auto_bind:1;
+	struct phy	*phy;
+	struct list_head list;
+};
+
+#if defined(CONFIG_GENERIC_PHY) || defined(CONFIG_GENERIC_PHY_MODULE)
+extern struct phy *devm_phy_get(struct device *dev, u8 index);
+extern void devm_phy_put(struct device *dev, struct phy *phy);
+extern struct phy *devm_of_phy_get(struct device *dev, const char *phandle,
+	u8 index);
+extern struct phy *phy_get(struct device *dev, u8 index);
+extern void phy_put(struct phy *phy);
+extern struct phy *phy_create(struct device *dev, struct phy_descriptor *desc);
+extern struct phy *devm_phy_create(struct device *dev,
+	struct phy_descriptor *desc);
+extern void phy_destroy(struct phy *phy);
+extern struct phy_bind *phy_bind(const char *dev_name, u8 index,
+				const char *phy_dev_name);
+#else
+static inline struct phy *devm_phy_get(struct device *dev, u8 index)
+{
+	return ERR_PTR(-EINVAL);
+}
+
+static inline void devm_phy_put(struct device *dev, struct phy *phy)
+{
+}
+
+static inline struct phy *devm_of_phy_get(struct device *dev,
+		const char *phandlei, u8 index)
+{
+	return ERR_PTR(-EINVAL);
+}
+
+static inline struct phy *phy_get(struct device *dev, u8 index)
+{
+	return ERR_PTR(-EINVAL);
+}
+
+static inline void phy_put(struct phy *phy)
+{
+}
+
+static inline struct phy *phy_create(struct device *dev,
+	struct phy_descriptor *desc)
+{
+	return ERR_PTR(-EINVAL);
+}
+
+static inline struct phy *devm_phy_create(struct device *dev,
+	struct phy_descriptor *desc)
+{
+	return ERR_PTR(-EINVAL);
+}
+
+static inline void phy_destroy(struct phy *phy)
+{
+}
+
+static inline struct phy_bind *phy_bind(const char *dev_name, u8 index,
+				const char *phy_dev_name)
+{
+	return ERR_PTR(-EINVAL);
+}
+#endif
+
+static inline int phy_init(struct phy *phy)
+{
+	if (phy->desc->ops->init)
+		return phy->desc->ops->init(phy->desc);
+
+	return -EINVAL;
+}
+
+static inline int phy_exit(struct phy *phy)
+{
+	if (phy->desc->ops->exit)
+		return phy->desc->ops->exit(phy->desc);
+
+	return -EINVAL;
+}
+
+static inline int phy_suspend(struct phy *phy)
+{
+	if (phy->desc->ops->suspend)
+		return phy->desc->ops->suspend(phy->desc);
+
+	return -EINVAL;
+}
+
+static inline int phy_resume(struct phy *phy)
+{
+	if (phy->desc->ops->resume)
+		return phy->desc->ops->resume(phy->desc);
+
+	return -EINVAL;
+}
+
+static inline int phy_poweron(struct phy *phy)
+{
+	if (phy->desc->ops->poweron)
+		return phy->desc->ops->poweron(phy->desc);
+
+	return -EINVAL;
+}
+
+static inline int phy_shutdown(struct phy *phy)
+{
+	if (phy->desc->ops->shutdown)
+		return phy->desc->ops->shutdown(phy->desc);
+
+	return -EINVAL;
+}
+#endif /* __DRIVERS_PHY_H */
-- 
1.7.10.4


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

* [PATCH v2 2/5] usb: phy: omap-usb2: use the new generic PHY framework
  2013-02-19  5:53 [PATCH v2 0/5] Generic PHY Framework Kishon Vijay Abraham I
  2013-02-19  5:53 ` [PATCH v2 1/5] drivers: phy: add generic PHY framework Kishon Vijay Abraham I
@ 2013-02-19  5:53 ` Kishon Vijay Abraham I
  2013-02-19  8:11   ` Felipe Balbi
  2013-02-19  5:53 ` [PATCH v2 3/5] usb: otg: twl4030: " Kishon Vijay Abraham I
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 24+ messages in thread
From: Kishon Vijay Abraham I @ 2013-02-19  5:53 UTC (permalink / raw)
  To: rob, tony, linux, eballetbo, javier, kishon, balbi, gregkh, akpm,
	mchehab, cesarb, davem, arnd, santosh.shilimkar, broonie,
	swarren, linux-doc, linux-kernel, linux-arm-kernel, linux-omap,
	linux-usb, netdev

Used the generic PHY framework API to create the PHY. omap_usb2_suspend
is split into omap_usb_suspend and omap_usb_resume in order to align
with the new framework.

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/omap-usb2.c  |   49 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/omap_usb.h |    3 +++
 2 files changed, 52 insertions(+)

diff --git a/drivers/usb/phy/omap-usb2.c b/drivers/usb/phy/omap-usb2.c
index 844ab68..924ae59 100644
--- a/drivers/usb/phy/omap-usb2.c
+++ b/drivers/usb/phy/omap-usb2.c
@@ -119,9 +119,48 @@ static int omap_usb2_suspend(struct usb_phy *x, int suspend)
 	return 0;
 }
 
+static int omap_usb_suspend(struct phy_descriptor *desc)
+{
+	struct omap_usb *phy = desc_to_omapusb(desc);
+
+	if (!phy->is_suspended) {
+		omap_control_usb_phy_power(phy->control_dev, 0);
+		pm_runtime_put_sync(phy->dev);
+		phy->is_suspended = 1;
+	}
+
+	return 0;
+}
+
+static int omap_usb_resume(struct phy_descriptor *desc)
+{
+	u32 ret;
+	struct omap_usb *phy = desc_to_omapusb(desc);
+
+	if (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 struct phy_ops ops = {
+	.suspend	= omap_usb_suspend,
+	.resume		= omap_usb_resume,
+	.owner		= THIS_MODULE,
+};
+
 static int omap_usb2_probe(struct platform_device *pdev)
 {
 	struct omap_usb			*phy;
+	struct phy			*generic_phy;
 	struct usb_otg			*otg;
 
 	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
@@ -144,6 +183,16 @@ static int omap_usb2_probe(struct platform_device *pdev)
 	phy->phy.otg		= otg;
 	phy->phy.type		= USB_PHY_TYPE_USB2;
 
+	phy->desc.ops		= &ops;
+	phy->desc.label		= "omap-usb2";
+	phy->desc.of_node	= pdev->dev.of_node;
+
+	generic_phy = devm_phy_create(phy->dev, &phy->desc);
+	if (IS_ERR(generic_phy)) {
+		dev_dbg(&pdev->dev, "Failed to create PHY\n");
+		return PTR_ERR(generic_phy);
+	}
+
 	phy->control_dev = omap_get_control_dev();
 	if (IS_ERR(phy->control_dev)) {
 		dev_dbg(&pdev->dev, "Failed to get control device\n");
diff --git a/include/linux/usb/omap_usb.h b/include/linux/usb/omap_usb.h
index 6ae2936..4c6878e 100644
--- a/include/linux/usb/omap_usb.h
+++ b/include/linux/usb/omap_usb.h
@@ -20,6 +20,7 @@
 #define __DRIVERS_OMAP_USB2_H
 
 #include <linux/io.h>
+#include <linux/phy/phy.h>
 #include <linux/usb/otg.h>
 
 struct usb_dpll_params {
@@ -32,6 +33,7 @@ struct usb_dpll_params {
 
 struct omap_usb {
 	struct usb_phy		phy;
+	struct phy_descriptor	desc;
 	struct phy_companion	*comparator;
 	void __iomem		*pll_ctrl_base;
 	struct device		*dev;
@@ -43,6 +45,7 @@ struct omap_usb {
 };
 
 #define	phy_to_omapusb(x)	container_of((x), struct omap_usb, phy)
+#define	desc_to_omapusb(x)	container_of((x), struct omap_usb, desc)
 
 #if defined(CONFIG_OMAP_USB2) || defined(CONFIG_OMAP_USB2_MODULE)
 extern int omap_usb2_set_comparator(struct phy_companion *comparator);
-- 
1.7.10.4


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

* [PATCH v2 3/5] usb: otg: twl4030: use the new generic PHY framework
  2013-02-19  5:53 [PATCH v2 0/5] Generic PHY Framework Kishon Vijay Abraham I
  2013-02-19  5:53 ` [PATCH v2 1/5] drivers: phy: add generic PHY framework Kishon Vijay Abraham I
  2013-02-19  5:53 ` [PATCH v2 2/5] usb: phy: omap-usb2: use the new " Kishon Vijay Abraham I
@ 2013-02-19  5:53 ` Kishon Vijay Abraham I
  2013-02-19  5:53 ` [PATCH v2 4/5] ARM: OMAP: USB: Add phy binding information Kishon Vijay Abraham I
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 24+ messages in thread
From: Kishon Vijay Abraham I @ 2013-02-19  5:53 UTC (permalink / raw)
  To: rob, tony, linux, eballetbo, javier, kishon, balbi, gregkh, akpm,
	mchehab, cesarb, davem, arnd, santosh.shilimkar, broonie,
	swarren, linux-doc, linux-kernel, linux-arm-kernel, linux-omap,
	linux-usb, netdev

Used the generic PHY framework API to create the PHY. twl4030_usb_suspend
and twl4030_usb_resume is added to phy_ops in order to align
with the new framework.

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/otg/twl4030-usb.c |   41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/drivers/usb/otg/twl4030-usb.c b/drivers/usb/otg/twl4030-usb.c
index a994715..b79c319 100644
--- a/drivers/usb/otg/twl4030-usb.c
+++ b/drivers/usb/otg/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>
@@ -145,6 +146,7 @@
 
 struct twl4030_usb {
 	struct usb_phy		phy;
+	struct phy_descriptor	desc;
 	struct device		*dev;
 
 	/* TWL4030 internal USB regulator supplies */
@@ -167,6 +169,7 @@ struct twl4030_usb {
 
 /* internal define on top of container_of */
 #define phy_to_twl(x)		container_of((x), struct twl4030_usb, phy)
+#define desc_to_twl(x)		container_of((x), struct twl4030_usb, desc)
 
 /*-------------------------------------------------------------------------*/
 
@@ -575,10 +578,38 @@ static int twl4030_set_host(struct usb_otg *otg, struct usb_bus *host)
 	return 0;
 }
 
+static int twl4030_usb_suspend(struct phy_descriptor *desc)
+{
+	struct twl4030_usb *twl = desc_to_twl(desc);
+
+	twl4030_phy_suspend(twl, 1);
+
+	return 0;
+}
+
+static int twl4030_usb_resume(struct phy_descriptor *desc)
+{
+	struct twl4030_usb *twl = desc_to_twl(desc);
+
+	if (!twl->asleep)
+		return -EBUSY;
+	__twl4030_phy_resume(twl);
+	twl->asleep = 0;
+
+	return 0;
+}
+
+static struct phy_ops ops = {
+	.suspend	= twl4030_usb_suspend,
+	.resume		= twl4030_usb_resume,
+	.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;
@@ -617,6 +648,16 @@ static int twl4030_usb_probe(struct platform_device *pdev)
 	otg->set_host		= twl4030_set_host;
 	otg->set_peripheral	= twl4030_set_peripheral;
 
+	twl->desc.ops		= &ops;
+	twl->desc.label		= "twl4030";
+	twl->desc.of_node	= pdev->dev.of_node;
+
+	phy = devm_phy_create(twl->dev, &twl->desc);
+	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


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

* [PATCH v2 4/5] ARM: OMAP: USB: Add phy binding information
  2013-02-19  5:53 [PATCH v2 0/5] Generic PHY Framework Kishon Vijay Abraham I
                   ` (2 preceding siblings ...)
  2013-02-19  5:53 ` [PATCH v2 3/5] usb: otg: twl4030: " Kishon Vijay Abraham I
@ 2013-02-19  5:53 ` Kishon Vijay Abraham I
  2013-02-19  5:53 ` [PATCH v2 5/5] usb: musb: omap2430: use the new generic PHY framework Kishon Vijay Abraham I
  2013-02-19 10:44 ` [PATCH v2 0/5] Generic PHY Framework Arnd Bergmann
  5 siblings, 0 replies; 24+ messages in thread
From: Kishon Vijay Abraham I @ 2013-02-19  5:53 UTC (permalink / raw)
  To: rob, tony, linux, eballetbo, javier, kishon, balbi, gregkh, akpm,
	mchehab, cesarb, davem, arnd, santosh.shilimkar, broonie,
	swarren, linux-doc, linux-kernel, linux-arm-kernel, linux-omap,
	linux-usb, netdev

In order for controllers to get PHY in case of non dt boot, the phy
binding information should be added in the platform specific
initialization code using phy_bind. The previously added usb_bind_phy
can't be removed yet because the musb controller continues to use the
old PHY library which has OTG in it (struct usb_phy has struct usb_otg
as member). Until we have a separate OTG state machine to handle all of
that, the new generic PHY framework and the old phy library will co-exist.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 arch/arm/mach-omap2/board-2430sdp.c          |    2 ++
 arch/arm/mach-omap2/board-3430sdp.c          |    2 ++
 arch/arm/mach-omap2/board-4430sdp.c          |    2 ++
 arch/arm/mach-omap2/board-cm-t35.c           |    2 ++
 arch/arm/mach-omap2/board-devkit8000.c       |    2 ++
 arch/arm/mach-omap2/board-igep0020.c         |    2 ++
 arch/arm/mach-omap2/board-ldp.c              |    2 ++
 arch/arm/mach-omap2/board-omap3beagle.c      |    2 ++
 arch/arm/mach-omap2/board-omap3evm.c         |    2 ++
 arch/arm/mach-omap2/board-omap3logic.c       |    2 ++
 arch/arm/mach-omap2/board-omap3pandora.c     |    2 ++
 arch/arm/mach-omap2/board-omap3stalker.c     |    2 ++
 arch/arm/mach-omap2/board-omap3touchbook.c   |    2 ++
 arch/arm/mach-omap2/board-omap4panda.c       |    2 ++
 arch/arm/mach-omap2/board-overo.c            |    2 ++
 arch/arm/mach-omap2/board-rm680.c            |    2 ++
 arch/arm/mach-omap2/board-zoom-peripherals.c |    2 ++
 17 files changed, 34 insertions(+)

diff --git a/arch/arm/mach-omap2/board-2430sdp.c b/arch/arm/mach-omap2/board-2430sdp.c
index 1337f2c..a28cc5b 100644
--- a/arch/arm/mach-omap2/board-2430sdp.c
+++ b/arch/arm/mach-omap2/board-2430sdp.c
@@ -28,6 +28,7 @@
 #include <linux/io.h>
 #include <linux/gpio.h>
 #include <linux/usb/phy.h>
+#include <linux/phy/phy.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -265,6 +266,7 @@ static void __init omap_2430sdp_init(void)
 
 	omap_mux_init_signal("usb0hs_stp", OMAP_PULL_ENA | OMAP_PULL_UP);
 	usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
+	phy_bind("musb-hdrc.0.auto", 0, "twl4030_usb");
 	usb_musb_init(NULL);
 
 	board_smc91x_init();
diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c
index 8a2e242..a4f4b8e 100644
--- a/arch/arm/mach-omap2/board-3430sdp.c
+++ b/arch/arm/mach-omap2/board-3430sdp.c
@@ -26,6 +26,7 @@
 #include <linux/mmc/host.h>
 #include <linux/platform_data/spi-omap2-mcspi.h>
 #include <linux/usb/phy.h>
+#include <linux/phy/phy.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -581,6 +582,7 @@ static void __init omap_3430sdp_init(void)
 	omap_serial_init();
 	omap_sdrc_init(hyb18m512160af6_sdrc_params, NULL);
 	usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
+	phy_bind("musb-hdrc.0.auto", 0, "twl4030_usb");
 	usb_musb_init(NULL);
 	board_smc91x_init();
 	board_flash_init(sdp_flash_partitions, chip_sel_3430, 0);
diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
index 8e8efcc..d5bc353 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -29,6 +29,7 @@
 #include <linux/platform_data/omap4-keypad.h>
 #include <linux/usb/musb.h>
 #include <linux/usb/phy.h>
+#include <linux/phy/phy.h>
 
 #include <asm/hardware/gic.h>
 #include <asm/mach-types.h>
@@ -698,6 +699,7 @@ static void __init omap_4430sdp_init(void)
 	omap4_twl6030_hsmmc_init(mmc);
 
 	usb_bind_phy("musb-hdrc.0.auto", 0, "omap-usb2.1.auto");
+	phy_bind("musb-hdrc.0.auto", 0, "omap-usb2.1.auto");
 	usb_musb_init(&musb_board_data);
 
 	status = omap_ethernet_init();
diff --git a/arch/arm/mach-omap2/board-cm-t35.c b/arch/arm/mach-omap2/board-cm-t35.c
index f1172f2..6860dc9 100644
--- a/arch/arm/mach-omap2/board-cm-t35.c
+++ b/arch/arm/mach-omap2/board-cm-t35.c
@@ -31,6 +31,7 @@
 #include <linux/regulator/machine.h>
 #include <linux/mmc/host.h>
 #include <linux/usb/phy.h>
+#include <linux/phy/phy.h>
 
 #include <linux/spi/spi.h>
 #include <linux/spi/tdo24m.h>
@@ -726,6 +727,7 @@ static void __init cm_t3x_common_init(void)
 	omap_twl4030_audio_init("cm-t3x");
 
 	usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
+	phy_bind("musb-hdrc.0.auto", 0, "twl4030_usb");
 	usb_musb_init(NULL);
 	cm_t35_init_usbh();
 	cm_t35_init_camera();
diff --git a/arch/arm/mach-omap2/board-devkit8000.c b/arch/arm/mach-omap2/board-devkit8000.c
index 77cade52..89c3e64 100644
--- a/arch/arm/mach-omap2/board-devkit8000.c
+++ b/arch/arm/mach-omap2/board-devkit8000.c
@@ -30,6 +30,7 @@
 #include <linux/mtd/nand.h>
 #include <linux/mmc/host.h>
 #include <linux/usb/phy.h>
+#include <linux/phy/phy.h>
 
 #include <linux/regulator/machine.h>
 #include <linux/i2c/twl.h>
@@ -624,6 +625,7 @@ static void __init devkit8000_init(void)
 	omap_ads7846_init(2, OMAP3_DEVKIT_TS_GPIO, 0, NULL);
 
 	usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
+	phy_bind("musb-hdrc.0.auto", 0, "twl4030_usb");
 	usb_musb_init(NULL);
 	usbhs_init(&usbhs_bdata);
 	board_nand_init(devkit8000_nand_partitions,
diff --git a/arch/arm/mach-omap2/board-igep0020.c b/arch/arm/mach-omap2/board-igep0020.c
index 15e5881..2ef8b31 100644
--- a/arch/arm/mach-omap2/board-igep0020.c
+++ b/arch/arm/mach-omap2/board-igep0020.c
@@ -19,6 +19,7 @@
 #include <linux/interrupt.h>
 #include <linux/input.h>
 #include <linux/usb/phy.h>
+#include <linux/phy/phy.h>
 
 #include <linux/regulator/machine.h>
 #include <linux/regulator/fixed.h>
@@ -627,6 +628,7 @@ static void __init igep_init(void)
 	omap_sdrc_init(m65kxxxxam_sdrc_params,
 				  m65kxxxxam_sdrc_params);
 	usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
+	phy_bind("musb-hdrc.0.auto", 0, "twl4030_usb");
 	usb_musb_init(NULL);
 
 	igep_flash_init();
diff --git a/arch/arm/mach-omap2/board-ldp.c b/arch/arm/mach-omap2/board-ldp.c
index 3b5510a..c2d8cbd 100644
--- a/arch/arm/mach-omap2/board-ldp.c
+++ b/arch/arm/mach-omap2/board-ldp.c
@@ -29,6 +29,7 @@
 #include <linux/smsc911x.h>
 #include <linux/mmc/host.h>
 #include <linux/usb/phy.h>
+#include <linux/phy/phy.h>
 #include <linux/platform_data/spi-omap2-mcspi.h>
 
 #include <asm/mach-types.h>
@@ -420,6 +421,7 @@ static void __init omap_ldp_init(void)
 	omap_serial_init();
 	omap_sdrc_init(NULL, NULL);
 	usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
+	phy_bind("musb-hdrc.0.auto", 0, "twl4030_usb");
 	usb_musb_init(NULL);
 	board_nand_init(ldp_nand_partitions, ARRAY_SIZE(ldp_nand_partitions),
 			ZOOM_NAND_CS, 0, nand_default_timings);
diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
index 4616f92..6f17343 100644
--- a/arch/arm/mach-omap2/board-omap3beagle.c
+++ b/arch/arm/mach-omap2/board-omap3beagle.c
@@ -31,6 +31,7 @@
 #include <linux/mtd/nand.h>
 #include <linux/mmc/host.h>
 #include <linux/usb/phy.h>
+#include <linux/phy/phy.h>
 
 #include <linux/regulator/machine.h>
 #include <linux/i2c/twl.h>
@@ -521,6 +522,7 @@ static void __init omap3_beagle_init(void)
 				  mt46h32m32lf6_sdrc_params);
 
 	usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
+	phy_bind("musb-hdrc.0.auto", 0, "twl4030_usb");
 	usb_musb_init(NULL);
 	usbhs_init(&usbhs_bdata);
 	board_nand_init(omap3beagle_nand_partitions,
diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
index a198b61..bfd3927 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -42,6 +42,7 @@
 #include <linux/mmc/host.h>
 #include <linux/export.h>
 #include <linux/usb/phy.h>
+#include <linux/phy/phy.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -736,6 +737,7 @@ static void __init omap3_evm_init(void)
 		usbhs_bdata.reset_gpio_port[1] = 135;
 	}
 	usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
+	phy_bind("musb-hdrc.0.auto", 0, "twl4030_usb");
 	usb_musb_init(&musb_board_data);
 	usbhs_init(&usbhs_bdata);
 	board_nand_init(omap3evm_nand_partitions,
diff --git a/arch/arm/mach-omap2/board-omap3logic.c b/arch/arm/mach-omap2/board-omap3logic.c
index 9409eb8..64d4daf 100644
--- a/arch/arm/mach-omap2/board-omap3logic.c
+++ b/arch/arm/mach-omap2/board-omap3logic.c
@@ -30,6 +30,7 @@
 #include <linux/i2c/twl.h>
 #include <linux/mmc/host.h>
 #include <linux/usb/phy.h>
+#include <linux/phy/phy.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -217,6 +218,7 @@ static void __init omap3logic_init(void)
 	board_smsc911x_init();
 
 	usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
+	phy_bind("musb-hdrc.0.auto", 0, "twl4030_usb");
 	usb_musb_init(NULL);
 
 	/* Ensure SDRC pins are mux'd for self-refresh */
diff --git a/arch/arm/mach-omap2/board-omap3pandora.c b/arch/arm/mach-omap2/board-omap3pandora.c
index 1ac3e81..b391107 100644
--- a/arch/arm/mach-omap2/board-omap3pandora.c
+++ b/arch/arm/mach-omap2/board-omap3pandora.c
@@ -36,6 +36,7 @@
 #include <linux/mmc/card.h>
 #include <linux/regulator/fixed.h>
 #include <linux/usb/phy.h>
+#include <linux/phy/phy.h>
 #include <linux/platform_data/spi-omap2-mcspi.h>
 
 #include <asm/mach-types.h>
@@ -603,6 +604,7 @@ static void __init omap3pandora_init(void)
 	omap_ads7846_init(1, OMAP3_PANDORA_TS_GPIO, 0, NULL);
 	usbhs_init(&usbhs_bdata);
 	usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
+	phy_bind("musb-hdrc.0.auto", 0, "twl4030_usb");
 	usb_musb_init(NULL);
 	gpmc_nand_init(&pandora_nand_data, NULL);
 
diff --git a/arch/arm/mach-omap2/board-omap3stalker.c b/arch/arm/mach-omap2/board-omap3stalker.c
index 63cb204..8f22977 100644
--- a/arch/arm/mach-omap2/board-omap3stalker.c
+++ b/arch/arm/mach-omap2/board-omap3stalker.c
@@ -34,6 +34,7 @@
 #include <linux/smsc911x.h>
 #include <linux/i2c/at24.h>
 #include <linux/usb/phy.h>
+#include <linux/phy/phy.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -406,6 +407,7 @@ static void __init omap3_stalker_init(void)
 	omap_serial_init();
 	omap_sdrc_init(mt46h32m32lf6_sdrc_params, NULL);
 	usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
+	phy_bind("musb-hdrc.0.auto", 0, "twl4030_usb");
 	usb_musb_init(NULL);
 	usbhs_init(&usbhs_bdata);
 	omap_ads7846_init(1, OMAP3_STALKER_TS_GPIO, 310, NULL);
diff --git a/arch/arm/mach-omap2/board-omap3touchbook.c b/arch/arm/mach-omap2/board-omap3touchbook.c
index 6b22ce3..acd82e4 100644
--- a/arch/arm/mach-omap2/board-omap3touchbook.c
+++ b/arch/arm/mach-omap2/board-omap3touchbook.c
@@ -29,6 +29,7 @@
 #include <linux/mtd/nand.h>
 #include <linux/mmc/host.h>
 #include <linux/usb/phy.h>
+#include <linux/phy/phy.h>
 
 #include <linux/platform_data/spi-omap2-mcspi.h>
 #include <linux/spi/spi.h>
@@ -367,6 +368,7 @@ static void __init omap3_touchbook_init(void)
 	/* Touchscreen and accelerometer */
 	omap_ads7846_init(4, OMAP3_TS_GPIO, 310, &ads7846_pdata);
 	usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
+	phy_bind("musb-hdrc.0.auto", 0, "twl4030_usb");
 	usb_musb_init(NULL);
 	usbhs_init(&usbhs_bdata);
 	board_nand_init(omap3touchbook_nand_partitions,
diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c
index 40184cc..7fe4363 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -31,6 +31,7 @@
 #include <linux/ti_wilink_st.h>
 #include <linux/usb/musb.h>
 #include <linux/usb/phy.h>
+#include <linux/phy/phy.h>
 #include <linux/wl12xx.h>
 #include <linux/platform_data/omap-abe-twl6040.h>
 
@@ -449,6 +450,7 @@ static void __init omap4_panda_init(void)
 	omap4_twl6030_hsmmc_init(mmc);
 	omap4_ehci_init();
 	usb_bind_phy("musb-hdrc.0.auto", 0, "omap-usb2.1.auto");
+	phy_bind("musb-hdrc.0.auto", 0, "omap-usb2.1.auto");
 	usb_musb_init(&musb_board_data);
 	omap4_panda_display_init();
 }
diff --git a/arch/arm/mach-omap2/board-overo.c b/arch/arm/mach-omap2/board-overo.c
index 7e43ff3..92835cc 100644
--- a/arch/arm/mach-omap2/board-overo.c
+++ b/arch/arm/mach-omap2/board-overo.c
@@ -37,6 +37,7 @@
 #include <linux/mtd/partitions.h>
 #include <linux/mmc/host.h>
 #include <linux/usb/phy.h>
+#include <linux/phy/phy.h>
 
 #include <linux/platform_data/mtd-nand-omap2.h>
 #include <linux/platform_data/spi-omap2-mcspi.h>
@@ -501,6 +502,7 @@ static void __init overo_init(void)
 	board_nand_init(overo_nand_partitions,
 			ARRAY_SIZE(overo_nand_partitions), NAND_CS, 0, NULL);
 	usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
+	phy_bind("musb-hdrc.0.auto", 0, "twl4030_usb");
 	usb_musb_init(NULL);
 	usbhs_init(&usbhs_bdata);
 	overo_spi_init();
diff --git a/arch/arm/mach-omap2/board-rm680.c b/arch/arm/mach-omap2/board-rm680.c
index f8a272c..ed854ca 100644
--- a/arch/arm/mach-omap2/board-rm680.c
+++ b/arch/arm/mach-omap2/board-rm680.c
@@ -19,6 +19,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/platform_data/mtd-onenand-omap2.h>
 #include <linux/usb/phy.h>
+#include <linux/phy/phy.h>
 
 #include <asm/mach/arch.h>
 #include <asm/mach-types.h>
@@ -136,6 +137,7 @@ static void __init rm680_init(void)
 	omap_sdrc_init(sdrc_params, sdrc_params);
 
 	usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
+	phy_bind("musb-hdrc.0.auto", 0, "twl4030_usb");
 	usb_musb_init(NULL);
 	rm680_peripherals_init();
 }
diff --git a/arch/arm/mach-omap2/board-zoom-peripherals.c b/arch/arm/mach-omap2/board-zoom-peripherals.c
index dc5498b..2093e25 100644
--- a/arch/arm/mach-omap2/board-zoom-peripherals.c
+++ b/arch/arm/mach-omap2/board-zoom-peripherals.c
@@ -21,6 +21,7 @@
 #include <linux/mmc/host.h>
 #include <linux/platform_data/gpio-omap.h>
 #include <linux/usb/phy.h>
+#include <linux/phy/phy.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -300,6 +301,7 @@ void __init zoom_peripherals_init(void)
 	omap_i2c_init();
 	platform_device_register(&omap_vwlan_device);
 	usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
+	phy_bind("musb-hdrc.0.auto", 0, "twl4030_usb");
 	usb_musb_init(NULL);
 	enable_board_wakeup_source();
 	omap_serial_init();
-- 
1.7.10.4


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

* [PATCH v2 5/5] usb: musb: omap2430: use the new generic PHY framework
  2013-02-19  5:53 [PATCH v2 0/5] Generic PHY Framework Kishon Vijay Abraham I
                   ` (3 preceding siblings ...)
  2013-02-19  5:53 ` [PATCH v2 4/5] ARM: OMAP: USB: Add phy binding information Kishon Vijay Abraham I
@ 2013-02-19  5:53 ` Kishon Vijay Abraham I
  2013-02-19 10:44 ` [PATCH v2 0/5] Generic PHY Framework Arnd Bergmann
  5 siblings, 0 replies; 24+ messages in thread
From: Kishon Vijay Abraham I @ 2013-02-19  5:53 UTC (permalink / raw)
  To: rob, tony, linux, eballetbo, javier, kishon, balbi, gregkh, akpm,
	mchehab, cesarb, davem, arnd, santosh.shilimkar, broonie,
	swarren, linux-doc, linux-kernel, linux-arm-kernel, linux-omap,
	linux-usb, netdev

Use the generic PHY framework API to get the PHY. The usb_phy_set_suspend
and usb_phy_set_resume is replaced with phy_suspend and phy_resume 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.h |    2 ++
 drivers/usb/musb/omap2430.c  |   22 ++++++++++++++++------
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 7fb4819..78251fd 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;
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 1762354..99378af 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -345,14 +345,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_of_phy_get(dev->parent, "usb-phy", 0);
+
+		/* 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, 0);
+	}
 
-	if (IS_ERR_OR_NULL(musb->xceiv)) {
-		pr_err("HS USB OTG: no transceiver configured\n");
+	if (IS_ERR_OR_NULL(musb->xceiv) || IS_ERR_OR_NULL(musb->phy)) {
+		dev_err(dev, "no transceiver configured\n");
 		return -EPROBE_DEFER;
 	}
 
@@ -608,7 +618,7 @@ static int omap2430_runtime_suspend(struct device *dev)
 				OTG_INTERFSEL);
 
 		omap2430_low_level_exit(musb);
-		usb_phy_set_suspend(musb->xceiv, 1);
+		phy_suspend(musb->phy);
 	}
 
 	return 0;
@@ -624,7 +634,7 @@ static int omap2430_runtime_resume(struct device *dev)
 		musb_writel(musb->mregs, OTG_INTERFSEL,
 				musb->context.otg_interfsel);
 
-		usb_phy_set_suspend(musb->xceiv, 0);
+		phy_resume(musb->phy);
 	}
 
 	return 0;
-- 
1.7.10.4


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

* Re: [PATCH v2 1/5] drivers: phy: add generic PHY framework
  2013-02-19  5:53 ` [PATCH v2 1/5] drivers: phy: add generic PHY framework Kishon Vijay Abraham I
@ 2013-02-19  8:01   ` Felipe Balbi
  2013-02-19 12:56   ` Arnd Bergmann
  2013-02-23 22:44   ` Rob Landley
  2 siblings, 0 replies; 24+ messages in thread
From: Felipe Balbi @ 2013-02-19  8:01 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: rob, tony, linux, eballetbo, javier, balbi, gregkh, akpm,
	mchehab, cesarb, davem, arnd, santosh.shilimkar, broonie,
	swarren, linux-doc, linux-kernel, linux-arm-kernel, linux-omap,
	linux-usb, netdev

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

Hi,

On Tue, Feb 19, 2013 at 11:23:14AM +0530, Kishon Vijay Abraham I wrote:
> 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. To obtain a reference to the PHY without
> using phandle, the platform specfic intialization code (say from board file)
> should have already called phy_bind with the binding information. The binding
> information consists of phy's device name, phy user device name and an index.
> The index is used when the same phy user binds to mulitple phys.
> 
> PHY drivers should create the PHY by passing phy_descriptor that has
> describes the PHY (label, type etc..) and ops like init, exit, suspend, resume,

s/has describes/describes

> poweron, shutdown.
> 
> The documentation for the generic PHY framework is added in
> Documentation/phy.txt and the documentation for the sysfs entry is added
> in Documentation/ABI/testing/sysfs-class-phy.

you forgot to mention here that one case of re-use is OMAP5 where the
same PHY IP (different instances of it) is used for SATA and USB3
functionality, which means that we would have to maintain 2 drivers for
the same thing.

> diff --git a/Documentation/ABI/testing/sysfs-class-phy b/Documentation/ABI/testing/sysfs-class-phy
> new file mode 100644
> index 0000000..ffd9930
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-phy
> @@ -0,0 +1,15 @@
> +What:		/sys/class/phy/<phy>/label
> +Date:		Feb 2013
> +KernelVersion:	3.10
> +Contact:	Kishon Vijay Abraham I <kishon@ti.com>
> +Description:
> +		This is a read-only file for getting the label of the phy.
> +
> +What:		/sys/class/phy/<phy>/bind

this will cause problems with the generic bind/unbind sysfs files which
are write-only. You should probably rename it to something else.

> +4. Getting a reference to the PHY
> +
> +Before the controller can make use of the PHY, it has to get a reference to
> +the PHY. The PHY framework provides 4 APIs to get a reference to the PHY.

'it has to get a reference to it. This framework', decreases the
duplication of 'PHY'.

> +struct phy *phy_get(struct device *dev, u8 index);
> +struct phy *devm_phy_get(struct device *dev, u8 index);
> +struct phy *of_phy_get(struct device *dev, const char *phandle, u8 index);
> +struct phy *devm_of_phy_get(struct device *dev, const char *phandle, u8 index);
> +
> +phy_get and devm_phy_get can be used to get the PHY in non-dt boot. This API
> +uses the binding information added using the phy_bind API to find and return
> +the appropriate 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

s/the the/the/

> +devres data is freed.

s/devres data/it/

> +7. Current Status
> +
> +Currently only USB in OMAP is made to use this framework. However using the
> +USB PHY library cannot be completely removed because it is intertwined with
> +OTG. Once we move OTG out of PHY completely, using the old PHY library can be
> +completely removed. SATA in OMAP will also more likely use this new framework
> +and we should have a patch for it soon.

not sure if this should be in the documentation, but fair enough.

> +static DEFINE_MUTEX(phy_list_mutex);

not sure if a mutex is enough to protect list traversal...

> +struct phy_ops {
> +	int	(*init)(struct phy_descriptor *desc);
> +	int	(*exit)(struct phy_descriptor *desc);
> +	int	(*suspend)(struct phy_descriptor *desc);
> +	int	(*resume)(struct phy_descriptor *desc);

should you really have these two pointers ? I wonder if it wouldn't be
better to force runtime_pm down on the users and have phy_suspend() and
phy_resume() just be wrappers to pm_runtime_get() and pm_runtime_put().

Up to discussion, I guess.

> +#if defined(CONFIG_GENERIC_PHY) || defined(CONFIG_GENERIC_PHY_MODULE)

#if IS_ENABLED(CONFIG_GENERIC_PHY)

> +static inline struct phy *devm_phy_get(struct device *dev, u8 index)
> +{
> +	return ERR_PTR(-EINVAL);

would -EOPNOTSUPP fit better here (and all others)


-- 
balbi

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

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

* Re: [PATCH v2 2/5] usb: phy: omap-usb2: use the new generic PHY framework
  2013-02-19  5:53 ` [PATCH v2 2/5] usb: phy: omap-usb2: use the new " Kishon Vijay Abraham I
@ 2013-02-19  8:11   ` Felipe Balbi
  0 siblings, 0 replies; 24+ messages in thread
From: Felipe Balbi @ 2013-02-19  8:11 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: rob, tony, linux, eballetbo, javier, balbi, gregkh, akpm,
	mchehab, cesarb, davem, arnd, santosh.shilimkar, broonie,
	swarren, linux-doc, linux-kernel, linux-arm-kernel, linux-omap,
	linux-usb, netdev

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

Hi,

On Tue, Feb 19, 2013 at 11:23:15AM +0530, Kishon Vijay Abraham I wrote:
> Used the generic PHY framework API to create the PHY. omap_usb2_suspend
> is split into omap_usb_suspend and omap_usb_resume in order to align
> with the new framework.
> 
> 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/omap-usb2.c  |   49 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/usb/omap_usb.h |    3 +++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/drivers/usb/phy/omap-usb2.c b/drivers/usb/phy/omap-usb2.c
> index 844ab68..924ae59 100644
> --- a/drivers/usb/phy/omap-usb2.c
> +++ b/drivers/usb/phy/omap-usb2.c
> @@ -119,9 +119,48 @@ static int omap_usb2_suspend(struct usb_phy *x, int suspend)
>  	return 0;
>  }
>  
> +static int omap_usb_suspend(struct phy_descriptor *desc)
> +{
> +	struct omap_usb *phy = desc_to_omapusb(desc);
> +
> +	if (!phy->is_suspended) {

I wonder if it wouldn't be better to have some refcounting done by phy
framework itself so that this only gets called when phy->refcount == 0

> +		omap_control_usb_phy_power(phy->control_dev, 0);

coudln't you move this to ->runtime_suspend() ?

> +		pm_runtime_put_sync(phy->dev);
> +		phy->is_suspended = 1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int omap_usb_resume(struct phy_descriptor *desc)
> +{
> +	u32 ret;
> +	struct omap_usb *phy = desc_to_omapusb(desc);
> +
> +	if (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);

likewise, this could go to ->runtime_resume().

-- 
balbi

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

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

* Re: [PATCH v2 0/5] Generic PHY Framework
  2013-02-19  5:53 [PATCH v2 0/5] Generic PHY Framework Kishon Vijay Abraham I
                   ` (4 preceding siblings ...)
  2013-02-19  5:53 ` [PATCH v2 5/5] usb: musb: omap2430: use the new generic PHY framework Kishon Vijay Abraham I
@ 2013-02-19 10:44 ` Arnd Bergmann
  2013-02-19 11:28   ` kishon
  5 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2013-02-19 10:44 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: rob, tony, linux, eballetbo, javier, balbi, gregkh, akpm,
	mchehab, cesarb, davem, santosh.shilimkar, broonie, swarren,
	linux-doc, linux-kernel, linux-arm-kernel, linux-omap, linux-usb,
	netdev

On Tuesday 19 February 2013, 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. To obtain a reference to the PHY
> without using phandle, the platform specfic intialization code (say from board
> file) should have already called phy_bind with the binding information. The
> binding information consists of phy's device name, phy user device name and an
> index. The index is used when the same phy user binds to mulitple phys.
> 
> 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.

How does this relate to the generic PHY interfaces in drivers/net/phy?

Do you expect that to get merged into drivers/phy in the long run, or
do you want to keep the generic phy only for everything but ethernet?

I think it would be problematic to have two alternative interfaces for
ethernet PHYs because then an ethernet driver still needs to decide
which subsystem to interface with.

	Arnd

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

* Re: [PATCH v2 0/5] Generic PHY Framework
  2013-02-19 10:44 ` [PATCH v2 0/5] Generic PHY Framework Arnd Bergmann
@ 2013-02-19 11:28   ` kishon
  2013-02-19 12:33     ` Arnd Bergmann
  0 siblings, 1 reply; 24+ messages in thread
From: kishon @ 2013-02-19 11:28 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: rob, tony, linux, eballetbo, javier, balbi, gregkh, akpm,
	mchehab, cesarb, davem, santosh.shilimkar, broonie, swarren,
	linux-doc, linux-kernel, linux-arm-kernel, linux-omap, linux-usb,
	netdev

Hi,

On Tuesday 19 February 2013 04:14 PM, Arnd Bergmann wrote:
> On Tuesday 19 February 2013, 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. To obtain a reference to the PHY
>> without using phandle, the platform specfic intialization code (say from board
>> file) should have already called phy_bind with the binding information. The
>> binding information consists of phy's device name, phy user device name and an
>> index. The index is used when the same phy user binds to mulitple phys.
>>
>> 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.
>
> How does this relate to the generic PHY interfaces in drivers/net/phy?

Currently drivers/phy and drivers/net/phy are independent and are not 
related to each other. There are some fundamental differences on how 
these frameworks work. IIUC, the *net* uses bus layer (MDIO bus) to 
match a PHY device with a PHY driver and the Ethernet device uses the 
bus layer to get the PHY.
The Generic PHY Framework however doesn't have any bus layer. The PHY 
should be like any other Platform Devices and Drivers and the framework 
will provide some APIs to register with the framework. And there are 
other APIs which any controller can use to get the PHY (for e.g., in the 
case of dt boot, it can use phandle to get a reference to the PHY).
>
> Do you expect that to get merged into drivers/phy in the long run, or
> do you want to keep the generic phy only for everything but ethernet?

We'd like the PHY drivers spread all over the kernel to get merged to 
drivers/phy including Ethernet. Having said that, Ethernet itself has a 
huge PHY framework and it's going to be little challenging to adapt to 
the new PHY framework (of course there'll be changes in this PHY 
framework when we want to have network PHY added here). But adapting USB 
PHYs to the new framework will be simpler and will be taken first. Also 
when we add SATA PHY (OMAP5), it will make use of this framework as both 
SATA and USB3 uses the same PHY IP.
>
> I think it would be problematic to have two alternative interfaces for
> ethernet PHYs because then an ethernet driver still needs to decide
> which subsystem to interface with.

Right. For now Ethernet should continue to use their own PHY abstraction 
layer till we are a bit more clear on how to move it to drivers/phy.

Thanks
Kishon

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

* Re: [PATCH v2 0/5] Generic PHY Framework
  2013-02-19 11:28   ` kishon
@ 2013-02-19 12:33     ` Arnd Bergmann
  2013-02-19 13:12       ` Felipe Balbi
  0 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2013-02-19 12:33 UTC (permalink / raw)
  To: kishon
  Cc: rob, tony, linux, eballetbo, javier, balbi, gregkh, akpm,
	mchehab, cesarb, davem, santosh.shilimkar, broonie, swarren,
	linux-doc, linux-kernel, linux-arm-kernel, linux-omap, linux-usb,
	netdev

On Tuesday 19 February 2013, kishon wrote:
> On Tuesday 19 February 2013 04:14 PM, Arnd Bergmann wrote:
> > On Tuesday 19 February 2013, 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. To obtain a reference to the PHY
> >> without using phandle, the platform specfic intialization code (say from board
> >> file) should have already called phy_bind with the binding information. The
> >> binding information consists of phy's device name, phy user device name and an
> >> index. The index is used when the same phy user binds to mulitple phys.
> >>
> >> 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.
> >
> > How does this relate to the generic PHY interfaces in drivers/net/phy?
> 
> Currently drivers/phy and drivers/net/phy are independent and are not 
> related to each other. There are some fundamental differences on how 
> these frameworks work. IIUC, the *net* uses bus layer (MDIO bus) to 
> match a PHY device with a PHY driver and the Ethernet device uses the 
> bus layer to get the PHY.
> The Generic PHY Framework however doesn't have any bus layer. The PHY 
> should be like any other Platform Devices and Drivers and the framework 
> will provide some APIs to register with the framework. And there are 
> other APIs which any controller can use to get the PHY (for e.g., in the 
> case of dt boot, it can use phandle to get a reference to the PHY).

Hmm, I think the use of a bus_type for a PHY actually sounds quite
appropriate. The actual PHY device would then be a child of the
platform_device (or something else) that gets probed by its parent
bus or the DT. The operations that you define for the PHY
actually mirror some of the things that we have for a 'struct device',
so I think it would be quite logical to do it the same way.

Note that MDIO has both a 'bus' and a 'class', and what we want here is more
like what the 'class' was meant for, except that for new classes, we
should actually use a 'bus', since the long-term plan is to kill off
the concept of a 'class'. I hope that was not too confusing. :)

> > Do you expect that to get merged into drivers/phy in the long run, or
> > do you want to keep the generic phy only for everything but ethernet?
> 
> We'd like the PHY drivers spread all over the kernel to get merged to 
> drivers/phy including Ethernet. Having said that, Ethernet itself has a 
> huge PHY framework and it's going to be little challenging to adapt to 
> the new PHY framework (of course there'll be changes in this PHY 
> framework when we want to have network PHY added here). But adapting USB 
> PHYs to the new framework will be simpler and will be taken first. Also 
> when we add SATA PHY (OMAP5), it will make use of this framework as both 
> SATA and USB3 uses the same PHY IP.

I think you need to have at least a concept of where you want to end up.
It's totally fine to introduce a new subsystem that does only the
minimum of what you need here, and add things as you go, but if you
already plan to merge the existing mdio bus into this, you should really
have an idea of what that will mean in the long run.

> > I think it would be problematic to have two alternative interfaces for
> > ethernet PHYs because then an ethernet driver still needs to decide
> > which subsystem to interface with.
> 
> Right. For now Ethernet should continue to use their own PHY abstraction 
> layer till we are a bit more clear on how to move it to drivers/phy.

Ok.

	Arnd

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

* Re: [PATCH v2 1/5] drivers: phy: add generic PHY framework
  2013-02-19  5:53 ` [PATCH v2 1/5] drivers: phy: add generic PHY framework Kishon Vijay Abraham I
  2013-02-19  8:01   ` Felipe Balbi
@ 2013-02-19 12:56   ` Arnd Bergmann
  2013-02-19 13:56     ` kishon
  2013-02-23 22:44   ` Rob Landley
  2 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2013-02-19 12:56 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: rob, tony, linux, eballetbo, javier, balbi, gregkh, akpm,
	mchehab, cesarb, davem, santosh.shilimkar, broonie, swarren,
	linux-doc, linux-kernel, linux-arm-kernel, linux-omap, linux-usb,
	netdev

On Tuesday 19 February 2013, Kishon Vijay Abraham I wrote:
> +static struct class *phy_class;
> +static LIST_HEAD(phy_list);
> +static DEFINE_MUTEX(phy_list_mutex);
> +static LIST_HEAD(phy_bind_list);

Hmm, so you actually do have a 'class'. There is a GregKH mandated ban on
new classes, meaning that one should be converted to a bus_type instead.

Also, you really should not need the global phy_list, phy_list_mutex
and phy_bind_list variables, since the driver core already provides
you with ways to iterate through devices on a class or bus.

> +/**
> + * of_phy_get - lookup and obtain a reference to a phy by phandle
> + * @dev: device that requests this phy
> + * @phandle: name of the property holding the phy phandle value
> + * @index - the index of the phy
> + *
> + * Returns the phy 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.
> + */
> +struct phy *of_phy_get(struct device *dev, const char *phandle, u8 index)
> +{
> +	struct phy *phy = NULL;
> +	struct phy_bind *phy_map = NULL;
> +	struct device_node *node;
> +
> +	if (!dev->of_node) {
> +		dev_dbg(dev, "device does not have a device node entry\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	node = of_parse_phandle(dev->of_node, phandle, index);
> +	if (!node) {
> +		dev_dbg(dev, "failed to get %s phandle in %s node\n", phandle,
> +			dev->of_node->full_name);
> +		return ERR_PTR(-ENODEV);
> +	}

I wonder whether this one should be of_parse_phandle_with_args() instead,
so you can have client-specific configuration in the property. Basically
instead of 

	phy = <&usbphy0 &usbphy1>;

you can pass an arbitrary number of arguments along with this, as
determined by some property in the phy node:

	usbphy0: phy@10000 {
		#phy-cells = <1>;
	};

	ehci@20000 {
		phy = <&usbphy0 23>;
	};

Which in turn leads to the argument (23) being passed into a phy_bind().

I also wonder if you should allow arbitrary names for the property.
Can't this always be called 'phy'? If you allow named phys, it would
more more consistent with other bindings to have a list of phy handles
in a property called "phy", and a second property called "phy-names"
that contains the named strings.


> +/**
> + * phy_create - create a new phy
> + * @dev: device that is creating the new phy
> + * @desc: descriptor of the phy
> + *
> + * Called to create a phy using phy framework.
> + */
> +struct phy *phy_create(struct device *dev, struct phy_descriptor *desc)
> +{
> +	int ret;
> +	struct phy *phy;
> +	struct phy_bind *phy_bind;
> +	const char *devname = NULL;
> ...
> +
> +	devname = dev_name(dev);
> +	device_initialize(&phy->dev);
> +	phy->desc = desc;
> +	phy->dev.class = phy_class;
> +	phy->dev.parent = dev;
> +	phy->dev.bus = desc->bus;
> +	ret = dev_set_name(&phy->dev, "%s", devname);


Passing a bus_type through the descriptor seems misplaced. What is this for?

Why is this function not just:

struct phy *phy_create(struct device *dev, const char *label, int type,
			struct phy_ops *ops);

Passing a structure like you do here seems dangerous because when someone
decides to add members to the structure, existing code will not give a
build error but silently break.

> +/**
> + * 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
> + * @suspend: suspending the phy
> + * @resume: resuming the phy
> + * @poweron: powering on the phy
> + * @shutdown: shutting down the phy
> + * @owner: the module owner containing the ops
> + */
> +struct phy_ops {
> +	int	(*init)(struct phy_descriptor *desc);
> +	int	(*exit)(struct phy_descriptor *desc);
> +	int	(*suspend)(struct phy_descriptor *desc);
> +	int	(*resume)(struct phy_descriptor *desc);
> +	int	(*poweron)(struct phy_descriptor *desc);
> +	int	(*shutdown)(struct phy_descriptor *desc);
> +	struct module *owner;
> +};

Shouldn't these take the 'struct phy' as an argument? struct phy_descriptor is
not even based on a 'struct device'.

> +struct phy {
> +	struct device		dev;
> +	struct phy_descriptor	*desc;
> +	struct list_head	head;
> +};

Please kill off the 'head' member here and use the infrastructure we
already have. The concept of the phy_descriptor seems even more foreign
here, so best just collapse that into 'struct phy'. Maybe also rename
the structure to 'phy_device'.

	Arnd

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

* Re: [PATCH v2 0/5] Generic PHY Framework
  2013-02-19 12:33     ` Arnd Bergmann
@ 2013-02-19 13:12       ` Felipe Balbi
  2013-02-19 14:34         ` Arnd Bergmann
  0 siblings, 1 reply; 24+ messages in thread
From: Felipe Balbi @ 2013-02-19 13:12 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: kishon, rob, tony, linux, eballetbo, javier, balbi, gregkh, akpm,
	mchehab, cesarb, davem, santosh.shilimkar, broonie, swarren,
	linux-doc, linux-kernel, linux-arm-kernel, linux-omap, linux-usb,
	netdev

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

Hi,

On Tue, Feb 19, 2013 at 12:33:54PM +0000, Arnd Bergmann wrote:
> On Tuesday 19 February 2013, kishon wrote:
> > On Tuesday 19 February 2013 04:14 PM, Arnd Bergmann wrote:
> > > On Tuesday 19 February 2013, 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. To obtain a reference to the PHY
> > >> without using phandle, the platform specfic intialization code (say from board
> > >> file) should have already called phy_bind with the binding information. The
> > >> binding information consists of phy's device name, phy user device name and an
> > >> index. The index is used when the same phy user binds to mulitple phys.
> > >>
> > >> 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.
> > >
> > > How does this relate to the generic PHY interfaces in drivers/net/phy?
> > 
> > Currently drivers/phy and drivers/net/phy are independent and are not 
> > related to each other. There are some fundamental differences on how 
> > these frameworks work. IIUC, the *net* uses bus layer (MDIO bus) to 
> > match a PHY device with a PHY driver and the Ethernet device uses the 
> > bus layer to get the PHY.
> > The Generic PHY Framework however doesn't have any bus layer. The PHY 
> > should be like any other Platform Devices and Drivers and the framework 
> > will provide some APIs to register with the framework. And there are 
> > other APIs which any controller can use to get the PHY (for e.g., in the 
> > case of dt boot, it can use phandle to get a reference to the PHY).
> 
> Hmm, I think the use of a bus_type for a PHY actually sounds quite
> appropriate. The actual PHY device would then be a child of the

really ? I'm not so sure, the *bus* used by the PHY is ULPI, UTMI,
UTMI+, PIP3, I2C, etc... adding another 'fake' bus representation is a
bit overkill IMO.

Imagine an I2C-controlled PHY driver like isp1301, that driver will have
to register i2c_driver and phy_driver, which looks weird to me. If the
only substitute for class is a bus, we can't drop classes just yet, I'm
afraid.

Imagine a regulator bus, a pwm bus, an LED bus etc. They don't make
sense IMHO.

> platform_device (or something else) that gets probed by its parent
> bus or the DT. The operations that you define for the PHY
> actually mirror some of the things that we have for a 'struct device',
> so I think it would be quite logical to do it the same way.
> 
> Note that MDIO has both a 'bus' and a 'class', and what we want here is more
> like what the 'class' was meant for, except that for new classes, we
> should actually use a 'bus', since the long-term plan is to kill off
> the concept of a 'class'. I hope that was not too confusing. :)

:)

-- 
balbi

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

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

* Re: [PATCH v2 1/5] drivers: phy: add generic PHY framework
  2013-02-19 12:56   ` Arnd Bergmann
@ 2013-02-19 13:56     ` kishon
  2013-02-19 14:28       ` Arnd Bergmann
  0 siblings, 1 reply; 24+ messages in thread
From: kishon @ 2013-02-19 13:56 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: rob, tony, linux, eballetbo, javier, balbi, gregkh, akpm,
	mchehab, cesarb, davem, santosh.shilimkar, broonie, swarren,
	linux-doc, linux-kernel, linux-arm-kernel, linux-omap, linux-usb,
	netdev

Hi,

On Tuesday 19 February 2013 06:26 PM, Arnd Bergmann wrote:
> On Tuesday 19 February 2013, Kishon Vijay Abraham I wrote:
>> +static struct class *phy_class;
>> +static LIST_HEAD(phy_list);
>> +static DEFINE_MUTEX(phy_list_mutex);
>> +static LIST_HEAD(phy_bind_list);
>
> Hmm, so you actually do have a 'class'. There is a GregKH mandated ban on
> new classes, meaning that one should be converted to a bus_type instead.
>
> Also, you really should not need the global phy_list, phy_list_mutex
> and phy_bind_list variables, since the driver core already provides
> you with ways to iterate through devices on a class or bus.

ok.

>
>> +/**
>> + * of_phy_get - lookup and obtain a reference to a phy by phandle
>> + * @dev: device that requests this phy
>> + * @phandle: name of the property holding the phy phandle value
>> + * @index - the index of the phy
>> + *
>> + * Returns the phy 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.
>> + */
>> +struct phy *of_phy_get(struct device *dev, const char *phandle, u8 index)
>> +{
>> +	struct phy *phy = NULL;
>> +	struct phy_bind *phy_map = NULL;
>> +	struct device_node *node;
>> +
>> +	if (!dev->of_node) {
>> +		dev_dbg(dev, "device does not have a device node entry\n");
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> +
>> +	node = of_parse_phandle(dev->of_node, phandle, index);
>> +	if (!node) {
>> +		dev_dbg(dev, "failed to get %s phandle in %s node\n", phandle,
>> +			dev->of_node->full_name);
>> +		return ERR_PTR(-ENODEV);
>> +	}
>
> I wonder whether this one should be of_parse_phandle_with_args() instead,
> so you can have client-specific configuration in the property. Basically
> instead of
>
> 	phy = <&usbphy0 &usbphy1>;
>
> you can pass an arbitrary number of arguments along with this, as
> determined by some property in the phy node:
>
> 	usbphy0: phy@10000 {
> 		#phy-cells = <1>;
> 	};
>
> 	ehci@20000 {
> 		phy = <&usbphy0 23>;
> 	};
>
> Which in turn leads to the argument (23) being passed into a phy_bind().
>
> I also wonder if you should allow arbitrary names for the property.
> Can't this always be called 'phy'? If you allow named phys, it would
> more more consistent with other bindings to have a list of phy handles
> in a property called "phy", and a second property called "phy-names"
> that contains the named strings.

Ok. Makes sense. We should make both *phy* and *phy-cells* standard here.
>
>
>> +/**
>> + * phy_create - create a new phy
>> + * @dev: device that is creating the new phy
>> + * @desc: descriptor of the phy
>> + *
>> + * Called to create a phy using phy framework.
>> + */
>> +struct phy *phy_create(struct device *dev, struct phy_descriptor *desc)
>> +{
>> +	int ret;
>> +	struct phy *phy;
>> +	struct phy_bind *phy_bind;
>> +	const char *devname = NULL;
>> ...
>> +
>> +	devname = dev_name(dev);
>> +	device_initialize(&phy->dev);
>> +	phy->desc = desc;
>> +	phy->dev.class = phy_class;
>> +	phy->dev.parent = dev;
>> +	phy->dev.bus = desc->bus;
>> +	ret = dev_set_name(&phy->dev, "%s", devname);
>
>
> Passing a bus_type through the descriptor seems misplaced. What is this for?

I thought if we are adding ethernet phys here (say drivers/phy/net), we 
can make phy_device_create() (currently in drivers/net/phy/phy_device.c) 
call phy_create with bus_type set to mdio_bus_type. Then we can have all 
the PHYs showing up in /sys/class/phy/ and ethernet can continue to use 
its own phy abstraction layer.
>
> Why is this function not just:
>
> struct phy *phy_create(struct device *dev, const char *label, int type,
> 			struct phy_ops *ops);

since while calling the callback functions using ops, there wont be 
anyway to get back the device specific structure pointer.

struct phy_dev {
  	.
         .
	struct phy_descriptor	desc;
	void __iomem		*base;
	.
	.
};

static int phy_resume(struct phy_descriptor *desc)
{

//if we dont pass a member of phy_dev while *phy_create* we can't get 
back phy_dev from callback functions as used below.
	struct phy_dev *phy = desc_to_omapusb(desc);

	return 0;
}

static struct phy_ops ops = {
	.resume		= phy_resume,
	.owner		= THIS_MODULE,
};

>
> Passing a structure like you do here seems dangerous because when someone
> decides to add members to the structure, existing code will not give a
> build error but silently break.

Not sure I understood this point. Care to explain?
>
>> +/**
>> + * 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
>> + * @suspend: suspending the phy
>> + * @resume: resuming the phy
>> + * @poweron: powering on the phy
>> + * @shutdown: shutting down the phy
>> + * @owner: the module owner containing the ops
>> + */
>> +struct phy_ops {
>> +	int	(*init)(struct phy_descriptor *desc);
>> +	int	(*exit)(struct phy_descriptor *desc);
>> +	int	(*suspend)(struct phy_descriptor *desc);
>> +	int	(*resume)(struct phy_descriptor *desc);
>> +	int	(*poweron)(struct phy_descriptor *desc);
>> +	int	(*shutdown)(struct phy_descriptor *desc);
>> +	struct module *owner;
>> +};
>
> Shouldn't these take the 'struct phy' as an argument? struct phy_descriptor is
> not even based on a 'struct device'.

I actually used struct phy_descriptor for the reason mentioned above.

Thanks a lot for reviewing.

Regards
Kishon

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

* Re: [PATCH v2 1/5] drivers: phy: add generic PHY framework
  2013-02-19 13:56     ` kishon
@ 2013-02-19 14:28       ` Arnd Bergmann
  0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2013-02-19 14:28 UTC (permalink / raw)
  To: kishon
  Cc: rob, tony, linux, eballetbo, javier, balbi, gregkh, akpm,
	mchehab, cesarb, davem, santosh.shilimkar, broonie, swarren,
	linux-doc, linux-kernel, linux-arm-kernel, linux-omap, linux-usb,
	netdev

On Tuesday 19 February 2013, kishon wrote:
> >> +
> >> +	devname = dev_name(dev);
> >> +	device_initialize(&phy->dev);
> >> +	phy->desc = desc;
> >> +	phy->dev.class = phy_class;
> >> +	phy->dev.parent = dev;
> >> +	phy->dev.bus = desc->bus;
> >> +	ret = dev_set_name(&phy->dev, "%s", devname);
> >
> >
> > Passing a bus_type through the descriptor seems misplaced. What is this for?
> 
> I thought if we are adding ethernet phys here (say drivers/phy/net), we 
> can make phy_device_create() (currently in drivers/net/phy/phy_device.c) 
> call phy_create with bus_type set to mdio_bus_type. Then we can have all 
> the PHYs showing up in /sys/class/phy/ and ethernet can continue to use 
> its own phy abstraction layer.

Hmm, that relies on the fact that mdio uses a 'bus_type' while the new phy
support uses a 'class', and it will break if we ever get to the point
where those two concepts are merged. I would rather not plan ahead here.

> > Why is this function not just:
> >
> > struct phy *phy_create(struct device *dev, const char *label, int type,
> > 			struct phy_ops *ops);
> 
> since while calling the callback functions using ops, there wont be 
> anyway to get back the device specific structure pointer.
> 
> struct phy_dev {
>   	.
>          .
> 	struct phy_descriptor	desc;
> 	void __iomem		*base;
> 	.
> 	.
> };
> 
> static int phy_resume(struct phy_descriptor *desc)
> {
> 
> //if we dont pass a member of phy_dev while *phy_create* we can't get 
> back phy_dev from callback functions as used below.
> 	struct phy_dev *phy = desc_to_omapusb(desc);
> 
> 	return 0;
> }
> 
> static struct phy_ops ops = {
> 	.resume		= phy_resume,
> 	.owner		= THIS_MODULE,
> };


In other subsystems, that is what the device->private_data pointer is used
for, which you could also pass to phy_create, or set after calling that
function.

> > Passing a structure like you do here seems dangerous because when someone
> > decides to add members to the structure, existing code will not give a
> > build error but silently break.
> 
> Not sure I understood this point. Care to explain?

Nevermind, when I wrote that sentence, I had not yet noticed that the
phy_descriptor is kept around. I was thinking that the structure was
only used to pass more arguments into phy_create.

	Arnd

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

* Re: [PATCH v2 0/5] Generic PHY Framework
  2013-02-19 13:12       ` Felipe Balbi
@ 2013-02-19 14:34         ` Arnd Bergmann
  2013-02-19 15:05           ` Felipe Balbi
  0 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2013-02-19 14:34 UTC (permalink / raw)
  To: balbi
  Cc: kishon, rob, tony, linux, eballetbo, javier, gregkh, akpm,
	mchehab, cesarb, davem, santosh.shilimkar, broonie, swarren,
	linux-doc, linux-kernel, linux-arm-kernel, linux-omap, linux-usb,
	netdev

On Tuesday 19 February 2013, Felipe Balbi wrote:
> On Tue, Feb 19, 2013 at 12:33:54PM +0000, Arnd Bergmann wrote:
> > > Currently drivers/phy and drivers/net/phy are independent and are not 
> > > related to each other. There are some fundamental differences on how 
> > > these frameworks work. IIUC, the *net* uses bus layer (MDIO bus) to 
> > > match a PHY device with a PHY driver and the Ethernet device uses the 
> > > bus layer to get the PHY.
> > > The Generic PHY Framework however doesn't have any bus layer. The PHY 
> > > should be like any other Platform Devices and Drivers and the framework 
> > > will provide some APIs to register with the framework. And there are 
> > > other APIs which any controller can use to get the PHY (for e.g., in the 
> > > case of dt boot, it can use phandle to get a reference to the PHY).
> > 
> > Hmm, I think the use of a bus_type for a PHY actually sounds quite
> > appropriate. The actual PHY device would then be a child of the
> 
> really ? I'm not so sure, the *bus* used by the PHY is ULPI, UTMI,
> UTMI+, PIP3, I2C, etc... adding another 'fake' bus representation is a
> bit overkill IMO.
> 
> Imagine an I2C-controlled PHY driver like isp1301, that driver will have
> to register i2c_driver and phy_driver, which looks weird to me. If the
> only substitute for class is a bus, we can't drop classes just yet, I'm
> afraid.
> 
> Imagine a regulator bus, a pwm bus, an LED bus etc. They don't make
> sense IMHO.

It's a fine line, but I think a phy is something that resembles a device
more than an LED does. When I read patch 1, I also noticed and commented
on the fact that it does use a 'class'. Now, according to Greg, we should
use 'bus_type' instead of 'class' in new code. I originally disagreed with
that concept, but he's the boss here and it's good if he has a vision
how things should be lined out.

In practice, there is little difference between a 'bus_type' and a 'class',
so just replace any instance of the former with the latter in your head
when reading the code ;-)
I understand that there is not a real common bus here, and the bus_type
infrastructure would basically be used as a way to represent each PHY
in sysfs and provide a way to enumerate and look them up inside of the
kernel.

	Arnd

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

* Re: [PATCH v2 0/5] Generic PHY Framework
  2013-02-19 14:34         ` Arnd Bergmann
@ 2013-02-19 15:05           ` Felipe Balbi
  2013-02-19 15:28             ` Arnd Bergmann
                               ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Felipe Balbi @ 2013-02-19 15:05 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: balbi, kishon, rob, tony, linux, eballetbo, javier, gregkh, akpm,
	mchehab, cesarb, davem, santosh.shilimkar, broonie, swarren,
	linux-doc, linux-kernel, linux-arm-kernel, linux-omap, linux-usb,
	netdev

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

Hi,

On Tue, Feb 19, 2013 at 02:34:40PM +0000, Arnd Bergmann wrote:
> On Tuesday 19 February 2013, Felipe Balbi wrote:
> > On Tue, Feb 19, 2013 at 12:33:54PM +0000, Arnd Bergmann wrote:
> > > > Currently drivers/phy and drivers/net/phy are independent and are not 
> > > > related to each other. There are some fundamental differences on how 
> > > > these frameworks work. IIUC, the *net* uses bus layer (MDIO bus) to 
> > > > match a PHY device with a PHY driver and the Ethernet device uses the 
> > > > bus layer to get the PHY.
> > > > The Generic PHY Framework however doesn't have any bus layer. The PHY 
> > > > should be like any other Platform Devices and Drivers and the framework 
> > > > will provide some APIs to register with the framework. And there are 
> > > > other APIs which any controller can use to get the PHY (for e.g., in the 
> > > > case of dt boot, it can use phandle to get a reference to the PHY).
> > > 
> > > Hmm, I think the use of a bus_type for a PHY actually sounds quite
> > > appropriate. The actual PHY device would then be a child of the
> > 
> > really ? I'm not so sure, the *bus* used by the PHY is ULPI, UTMI,
> > UTMI+, PIP3, I2C, etc... adding another 'fake' bus representation is a
> > bit overkill IMO.
> > 
> > Imagine an I2C-controlled PHY driver like isp1301, that driver will have
> > to register i2c_driver and phy_driver, which looks weird to me. If the
> > only substitute for class is a bus, we can't drop classes just yet, I'm
> > afraid.
> > 
> > Imagine a regulator bus, a pwm bus, an LED bus etc. They don't make
> > sense IMHO.
> 
> It's a fine line, but I think a phy is something that resembles a device
> more than an LED does. When I read patch 1, I also noticed and commented
> on the fact that it does use a 'class'. Now, according to Greg, we should
> use 'bus_type' instead of 'class' in new code. I originally disagreed with
> that concept, but he's the boss here and it's good if he has a vision
> how things should be lined out.
> 
> In practice, there is little difference between a 'bus_type' and a 'class',
> so just replace any instance of the former with the latter in your head
> when reading the code ;-)

it's not so simple :-) if we must use bus_type we need to introduce all
the device/driver matching mechanism which isn't necessary with a class.

> I understand that there is not a real common bus here, and the bus_type
> infrastructure would basically be used as a way to represent each PHY
> in sysfs and provide a way to enumerate and look them up inside of the
> kernel.

right, but maybe we need another mechanism. If, in the long run we must
use bus_type, then eventually pwm, led, regulators, etc will all be
converted to bus_type. It will look quite weird IMHO.

Greg, can you pitch your suggestion here ? It would be great to hear
your rationale behind dropping class infrastructure, couldn't find
anything through Google and since feature-removal-schedule.txt has been
removed (without adding it to feature-removal-schedule.txt, I must add
:-) I don't know what's the idea behind removing classes.

cheers

-- 
balbi

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

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

* Re: [PATCH v2 0/5] Generic PHY Framework
  2013-02-19 15:05           ` Felipe Balbi
@ 2013-02-19 15:28             ` Arnd Bergmann
  2013-02-19 15:47               ` Felipe Balbi
  2013-02-19 16:07             ` Marc Kleine-Budde
  2013-02-23 20:05             ` Rob Landley
  2 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2013-02-19 15:28 UTC (permalink / raw)
  To: balbi
  Cc: kishon, rob, tony, linux, eballetbo, javier, gregkh, akpm,
	mchehab, cesarb, davem, santosh.shilimkar, broonie, swarren,
	linux-doc, linux-kernel, linux-arm-kernel, linux-omap, linux-usb,
	netdev

On Tuesday 19 February 2013, Felipe Balbi wrote:
> On Tue, Feb 19, 2013 at 02:34:40PM +0000, Arnd Bergmann wrote:
> > On Tuesday 19 February 2013, Felipe Balbi wrote:
> > > On Tue, Feb 19, 2013 at 12:33:54PM +0000, Arnd Bergmann wrote:
> > It's a fine line, but I think a phy is something that resembles a device
> > more than an LED does. When I read patch 1, I also noticed and commented
> > on the fact that it does use a 'class'. Now, according to Greg, we should
> > use 'bus_type' instead of 'class' in new code. I originally disagreed with
> > that concept, but he's the boss here and it's good if he has a vision
> > how things should be lined out.
> > 
> > In practice, there is little difference between a 'bus_type' and a 'class',
> > so just replace any instance of the former with the latter in your head
> > when reading the code ;-)
> 
> it's not so simple :-) if we must use bus_type we need to introduce all
> the device/driver matching mechanism which isn't necessary with a class.

I think the idea is to use a bus_type that has devices but no drivers
associated with it, but I might be misremembering things.

> > I understand that there is not a real common bus here, and the bus_type
> > infrastructure would basically be used as a way to represent each PHY
> > in sysfs and provide a way to enumerate and look them up inside of the
> > kernel.
> 
> right, but maybe we need another mechanism. If, in the long run we must
> use bus_type, then eventually pwm, led, regulators, etc will all be
> converted to bus_type. It will look quite weird IMHO.

Yes, it would be a bit unusual, I agree.

> Greg, can you pitch your suggestion here ? It would be great to hear
> your rationale behind dropping class infrastructure, couldn't find
> anything through Google and since feature-removal-schedule.txt has been
> removed (without adding it to feature-removal-schedule.txt, I must add
> :-) I don't know what's the idea behind removing classes.

I believe for now, the idea is to not add any new classes, but keep
the existing ones for compatibility. 'struct class_device' however
was already removed and got turned into 'struct device'.

	Arnd

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

* Re: [PATCH v2 0/5] Generic PHY Framework
  2013-02-19 15:28             ` Arnd Bergmann
@ 2013-02-19 15:47               ` Felipe Balbi
  0 siblings, 0 replies; 24+ messages in thread
From: Felipe Balbi @ 2013-02-19 15:47 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: balbi, kishon, rob, tony, linux, eballetbo, javier, gregkh, akpm,
	mchehab, cesarb, davem, santosh.shilimkar, broonie, swarren,
	linux-doc, linux-kernel, linux-arm-kernel, linux-omap, linux-usb,
	netdev

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

On Tue, Feb 19, 2013 at 03:28:17PM +0000, Arnd Bergmann wrote:
> On Tuesday 19 February 2013, Felipe Balbi wrote:
> > On Tue, Feb 19, 2013 at 02:34:40PM +0000, Arnd Bergmann wrote:
> > > On Tuesday 19 February 2013, Felipe Balbi wrote:
> > > > On Tue, Feb 19, 2013 at 12:33:54PM +0000, Arnd Bergmann wrote:
> > > It's a fine line, but I think a phy is something that resembles a device
> > > more than an LED does. When I read patch 1, I also noticed and commented
> > > on the fact that it does use a 'class'. Now, according to Greg, we should
> > > use 'bus_type' instead of 'class' in new code. I originally disagreed with
> > > that concept, but he's the boss here and it's good if he has a vision
> > > how things should be lined out.
> > > 
> > > In practice, there is little difference between a 'bus_type' and a 'class',
> > > so just replace any instance of the former with the latter in your head
> > > when reading the code ;-)
> > 
> > it's not so simple :-) if we must use bus_type we need to introduce all
> > the device/driver matching mechanism which isn't necessary with a class.
> 
> I think the idea is to use a bus_type that has devices but no drivers
> associated with it, but I might be misremembering things.

but then drivers wouldn't probe ever, although devices would get
created, so maybe it'll work...

> > Greg, can you pitch your suggestion here ? It would be great to hear
> > your rationale behind dropping class infrastructure, couldn't find
> > anything through Google and since feature-removal-schedule.txt has been
> > removed (without adding it to feature-removal-schedule.txt, I must add
> > :-) I don't know what's the idea behind removing classes.
> 
> I believe for now, the idea is to not add any new classes, but keep
> the existing ones for compatibility. 'struct class_device' however
> was already removed and got turned into 'struct device'.

was there ever a "struct class_device".

What about struct class ?

<linux/device.h> ::

 334 struct class {
 335         const char              *name;
 336         struct module           *owner;
 337 
 338         struct class_attribute          *class_attrs;
 339         struct device_attribute         *dev_attrs;
 340         struct bin_attribute            *dev_bin_attrs;
 341         struct kobject                  *dev_kobj;
 342 
 343         int (*dev_uevent)(struct device *dev, struct kobj_uevent_env *env);
 344         char *(*devnode)(struct device *dev, umode_t *mode);
 345 
 346         void (*class_release)(struct class *class);
 347         void (*dev_release)(struct device *dev);
 348 
 349         int (*suspend)(struct device *dev, pm_message_t state);
 350         int (*resume)(struct device *dev);
 351 
 352         const struct kobj_ns_type_operations *ns_type;
 353         const void *(*namespace)(struct device *dev);
 354 
 355         const struct dev_pm_ops *pm;
 356 
 357         struct subsys_private *p;
 358 };

-- 
balbi

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

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

* Re: [PATCH v2 0/5] Generic PHY Framework
  2013-02-19 15:05           ` Felipe Balbi
  2013-02-19 15:28             ` Arnd Bergmann
@ 2013-02-19 16:07             ` Marc Kleine-Budde
  2013-02-19 16:17               ` Felipe Balbi
  2013-02-23 20:05             ` Rob Landley
  2 siblings, 1 reply; 24+ messages in thread
From: Marc Kleine-Budde @ 2013-02-19 16:07 UTC (permalink / raw)
  To: balbi
  Cc: Arnd Bergmann, linux-usb, linux, mchehab, davem, tony, gregkh,
	broonie, linux-doc, linux-kernel, kishon, eballetbo,
	santosh.shilimkar, linux-arm-kernel, rob, netdev, swarren,
	javier, linux-omap, akpm, cesarb

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

On 02/19/2013 04:05 PM, Felipe Balbi wrote:
> Hi,
> 
> On Tue, Feb 19, 2013 at 02:34:40PM +0000, Arnd Bergmann wrote:
>> On Tuesday 19 February 2013, Felipe Balbi wrote:
>>> On Tue, Feb 19, 2013 at 12:33:54PM +0000, Arnd Bergmann wrote:
>>>>> Currently drivers/phy and drivers/net/phy are independent and are not 
>>>>> related to each other. There are some fundamental differences on how 
>>>>> these frameworks work. IIUC, the *net* uses bus layer (MDIO bus) to 
>>>>> match a PHY device with a PHY driver and the Ethernet device uses the 
>>>>> bus layer to get the PHY.
>>>>> The Generic PHY Framework however doesn't have any bus layer. The PHY 
>>>>> should be like any other Platform Devices and Drivers and the framework 
>>>>> will provide some APIs to register with the framework. And there are 
>>>>> other APIs which any controller can use to get the PHY (for e.g., in the 
>>>>> case of dt boot, it can use phandle to get a reference to the PHY).
>>>>
>>>> Hmm, I think the use of a bus_type for a PHY actually sounds quite
>>>> appropriate. The actual PHY device would then be a child of the
>>>
>>> really ? I'm not so sure, the *bus* used by the PHY is ULPI, UTMI,
>>> UTMI+, PIP3, I2C, etc... adding another 'fake' bus representation is a
>>> bit overkill IMO.
>>>
>>> Imagine an I2C-controlled PHY driver like isp1301, that driver will have
>>> to register i2c_driver and phy_driver, which looks weird to me. If the
>>> only substitute for class is a bus, we can't drop classes just yet, I'm
>>> afraid.
>>>
>>> Imagine a regulator bus, a pwm bus, an LED bus etc. They don't make
>>> sense IMHO.
>>
>> It's a fine line, but I think a phy is something that resembles a device
>> more than an LED does. When I read patch 1, I also noticed and commented
>> on the fact that it does use a 'class'. Now, according to Greg, we should
>> use 'bus_type' instead of 'class' in new code. I originally disagreed with
>> that concept, but he's the boss here and it's good if he has a vision
>> how things should be lined out.
>>
>> In practice, there is little difference between a 'bus_type' and a 'class',
>> so just replace any instance of the former with the latter in your head
>> when reading the code ;-)
> 
> it's not so simple :-) if we must use bus_type we need to introduce all
> the device/driver matching mechanism which isn't necessary with a class.

You have the code for phy <-> device matching in your framework anyway.
Using the bus infrastructure brings changes the open coded matching into
bus callbacks.

regards,
Marc

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

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

* Re: [PATCH v2 0/5] Generic PHY Framework
  2013-02-19 16:07             ` Marc Kleine-Budde
@ 2013-02-19 16:17               ` Felipe Balbi
  0 siblings, 0 replies; 24+ messages in thread
From: Felipe Balbi @ 2013-02-19 16:17 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: balbi, Arnd Bergmann, linux-usb, linux, mchehab, davem, tony,
	gregkh, broonie, linux-doc, linux-kernel, kishon, eballetbo,
	santosh.shilimkar, linux-arm-kernel, rob, netdev, swarren,
	javier, linux-omap, akpm, cesarb

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

Hi,

On Tue, Feb 19, 2013 at 05:07:09PM +0100, Marc Kleine-Budde wrote:
> On 02/19/2013 04:05 PM, Felipe Balbi wrote:
> > Hi,
> > 
> > On Tue, Feb 19, 2013 at 02:34:40PM +0000, Arnd Bergmann wrote:
> >> On Tuesday 19 February 2013, Felipe Balbi wrote:
> >>> On Tue, Feb 19, 2013 at 12:33:54PM +0000, Arnd Bergmann wrote:
> >>>>> Currently drivers/phy and drivers/net/phy are independent and are not 
> >>>>> related to each other. There are some fundamental differences on how 
> >>>>> these frameworks work. IIUC, the *net* uses bus layer (MDIO bus) to 
> >>>>> match a PHY device with a PHY driver and the Ethernet device uses the 
> >>>>> bus layer to get the PHY.
> >>>>> The Generic PHY Framework however doesn't have any bus layer. The PHY 
> >>>>> should be like any other Platform Devices and Drivers and the framework 
> >>>>> will provide some APIs to register with the framework. And there are 
> >>>>> other APIs which any controller can use to get the PHY (for e.g., in the 
> >>>>> case of dt boot, it can use phandle to get a reference to the PHY).
> >>>>
> >>>> Hmm, I think the use of a bus_type for a PHY actually sounds quite
> >>>> appropriate. The actual PHY device would then be a child of the
> >>>
> >>> really ? I'm not so sure, the *bus* used by the PHY is ULPI, UTMI,
> >>> UTMI+, PIP3, I2C, etc... adding another 'fake' bus representation is a
> >>> bit overkill IMO.
> >>>
> >>> Imagine an I2C-controlled PHY driver like isp1301, that driver will have
> >>> to register i2c_driver and phy_driver, which looks weird to me. If the
> >>> only substitute for class is a bus, we can't drop classes just yet, I'm
> >>> afraid.
> >>>
> >>> Imagine a regulator bus, a pwm bus, an LED bus etc. They don't make
> >>> sense IMHO.
> >>
> >> It's a fine line, but I think a phy is something that resembles a device
> >> more than an LED does. When I read patch 1, I also noticed and commented
> >> on the fact that it does use a 'class'. Now, according to Greg, we should
> >> use 'bus_type' instead of 'class' in new code. I originally disagreed with
> >> that concept, but he's the boss here and it's good if he has a vision
> >> how things should be lined out.
> >>
> >> In practice, there is little difference between a 'bus_type' and a 'class',
> >> so just replace any instance of the former with the latter in your head
> >> when reading the code ;-)
> > 
> > it's not so simple :-) if we must use bus_type we need to introduce all
> > the device/driver matching mechanism which isn't necessary with a class.
> 
> You have the code for phy <-> device matching in your framework anyway.
> Using the bus infrastructure brings changes the open coded matching into
> bus callbacks.

it's not the same thing. Current matching is just to figure out which
phy belongs to which user. The bus matching rules are to bind a device
with its driver, but that part has been taken care of by the underlying
control bus used by the phy, be it i2c, spi, or whatever else.

-- 
balbi

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

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

* Re: [PATCH v2 0/5] Generic PHY Framework
  2013-02-19 15:05           ` Felipe Balbi
  2013-02-19 15:28             ` Arnd Bergmann
  2013-02-19 16:07             ` Marc Kleine-Budde
@ 2013-02-23 20:05             ` Rob Landley
  2 siblings, 0 replies; 24+ messages in thread
From: Rob Landley @ 2013-02-23 20:05 UTC (permalink / raw)
  To: balbi
  Cc: Arnd Bergmann, balbi, kishon, tony, linux, eballetbo, javier,
	gregkh, akpm, mchehab, cesarb, davem, santosh.shilimkar, broonie,
	swarren, linux-doc, linux-kernel, linux-arm-kernel, linux-omap,
	linux-usb, netdev

On 02/19/2013 09:05:00 AM, Felipe Balbi wrote:
> Greg, can you pitch your suggestion here ? It would be great to hear
> your rationale behind dropping class infrastructure, couldn't find
> anything through Google and since feature-removal-schedule.txt has  
> been
> removed (without adding it to feature-removal-schedule.txt, I must add
> :-) I don't know what's the idea behind removing classes.

I actually went through and poked a couple of people about old entries  
in feature-removal-shedule.txt last year, but I haven't been very  
active since the kernel.org breakin because my account got disabled,  
and I needed to meet kernel developers in person to get keys signed to  
get it switched back on (or set up a separate git tree with signed  
commits -next could pull from).

I don't get out much; as a consultant I have to take time off from work  
and pay for my own travel and lodging. So I've been to exactly two  
conferences in the past 3 years: last year's Texas Linux Fest (my house  
got broken into and a netbook with the key on it stolen the following  
wednesday), and CELF (which I'm on the plane back from now, Greg KH  
signed my key! Woo!).

If I can use that to get my account back, set up a tree feeding into  
linux-next, and maybe even recover the ability to update  
http://kernel.org/doc, I'd happily field some sort of  
feature-removal-schedule list and make sure it stays current. (Linus  
didn't ask me about removing the old one, I found out about it from the  
git log. But I can't blame him, I haven't exactly been tearing through  
the bureaucracy to get my access back. "Volunteer work" and "painful"  
tend not to combine well on my todo list in terms of scheduling  
priority...)

Rob

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

* Re: [PATCH v2 1/5] drivers: phy: add generic PHY framework
  2013-02-19  5:53 ` [PATCH v2 1/5] drivers: phy: add generic PHY framework Kishon Vijay Abraham I
  2013-02-19  8:01   ` Felipe Balbi
  2013-02-19 12:56   ` Arnd Bergmann
@ 2013-02-23 22:44   ` Rob Landley
  2013-02-25  6:41     ` kishon
  2 siblings, 1 reply; 24+ messages in thread
From: Rob Landley @ 2013-02-23 22:44 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: tony, linux, eballetbo, javier, kishon, balbi, gregkh, akpm,
	mchehab, cesarb, davem, arnd, santosh.shilimkar, broonie,
	swarren, linux-doc, linux-kernel, linux-arm-kernel, linux-omap,
	linux-usb, netdev

On 02/18/2013 11:53:14 PM, Kishon Vijay Abraham I wrote:
> 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. To obtain a reference to the PHY  
> without
> using phandle, the platform specfic intialization code (say from  
> board file)
> should have already called phy_bind with the binding information. The  
> binding
> information consists of phy's device name, phy user device name and  
> an index.
> The index is used when the same phy user binds to mulitple phys.

Given that this has a separately selectable config option, I'm guessing  
that it's useful all by itself even in the absence of a driver using  
this phy? (Or it gives user visibility to the phy buried in an E1000 or  
SATA drive or some such?)

> +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
> +controller has PHY functionality embedded into it and others use an  
> external
> +PHY. Other peripherals that uses a PHY include Wireless LAN,  
> Ethernet,
> +SATA etc.

I've usually heard the word "transciever" used to describe these.

> +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.
> +
> +This framework will be of use only to devices that uses external PHY  
> (PHY
> +functionality is not embedded within the controller).
> +
> +2. 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.

Given that a PHY is a chip (random example  
http://ark.intel.com/products/47620/Intel-82579LM-Gigabit-Ethernet-PHY),  
you seem to be saying that software should manifest a piece of hardware  
out of thin air through sheer willpower. I'm pretty sure I've  
misunderstood this phrasing.

> +struct phy *phy_create(struct device *dev, struct phy_descriptor  
> *desc);
> +struct phy *devm_phy_create(struct device *dev, struct  
> phy_descriptor *desc);
> +
> +The PHY drivers can use one of the above 2 APIs to create the PHY by  
> passing

Um, the driver should _bind_ to the phy, maybe? Allocate? Initialize?

> +6. Destroying the PHY

I've run drivers like that. I try not to, though.

> +7. Current Status
> +
> +Currently only USB in OMAP is made to use this framework. However  
> using the
> +USB PHY library cannot be completely removed because it is  
> intertwined with
> +OTG. Once we move OTG out of PHY completely, using the old PHY  
> library can be
> +completely removed. SATA in OMAP will also more likely use this new  
> framework
> +and we should have a patch for it soon.

Does this paragraph belong in the documentation? (Git commit, sure, but  
I've seen a lot of stale paragraphs like these hang around a  
surprisingly long time.)

Rob

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

* Re: [PATCH v2 1/5] drivers: phy: add generic PHY framework
  2013-02-23 22:44   ` Rob Landley
@ 2013-02-25  6:41     ` kishon
  0 siblings, 0 replies; 24+ messages in thread
From: kishon @ 2013-02-25  6:41 UTC (permalink / raw)
  To: Rob Landley
  Cc: tony, linux, eballetbo, javier, balbi, gregkh, akpm, mchehab,
	cesarb, davem, arnd, santosh.shilimkar, broonie, swarren,
	linux-doc, linux-kernel, linux-arm-kernel, linux-omap, linux-usb,
	netdev

Hi,

On Sunday 24 February 2013 04:14 AM, Rob Landley wrote:
> On 02/18/2013 11:53:14 PM, Kishon Vijay Abraham I wrote:
>> 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. To obtain a reference to the PHY
>> without
>> using phandle, the platform specfic intialization code (say from board
>> file)
>> should have already called phy_bind with the binding information. The
>> binding
>> information consists of phy's device name, phy user device name and an
>> index.
>> The index is used when the same phy user binds to mulitple phys.
>
> Given that this has a separately selectable config option, I'm guessing
> that it's useful all by itself even in the absence of a driver using

Not really. It has to be thought of like a bridge that connects the 
device (can be USB/SATA/..) and the PHY. The PHY driver will register 
the PHY in this framework and the device controller driver can obtain a 
reference to this PHY from this framework.
> this phy? (Or it gives user visibility to the phy buried in an E1000 or
> SATA drive or some such?)

The PHY and the device are generally independent entities and there has 
to be a way to bind them in-order for the device controller to use the 
PHY. For e.g., MUSB in OMAP4 uses a PHY which is different from PHY in 
OMAP3 and it's going to be different from a PHY used in other SoCs. So 
in-order for MUSB to use the correct PHY, there has to be a framework 
that maintains the list of PHY and returns the correct PHY when a device 
controller driver requests for it. So whenever the PHY driver gets 
probed, it registers itself with this framework and then the MUSB can 
get the reference to the PHY from this framework. The use of this 
framework is more prevalent when there are multiple MUSB controllers 
each using a different PHY in a single SoC.

This holds true for other device controllers as well.
>
>> +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
>> +controller has PHY functionality embedded into it and others use an
>> external
>> +PHY. Other peripherals that uses a PHY include Wireless LAN, Ethernet,
>> +SATA etc.
>
> I've usually heard the word "transciever" used to describe these.

hmm.. IMO there is a thin line differentiating "transceiver" and "PHY" 
and can be used interchangeably. Since it's been referred as "PHY" in 
data sheets and TRMs, I preferred to call it PHY.
>
>> +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.
>> +
>> +This framework will be of use only to devices that uses external PHY
>> (PHY
>> +functionality is not embedded within the controller).
>> +
>> +2. 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.
>
> Given that a PHY is a chip (random example
> http://ark.intel.com/products/47620/Intel-82579LM-Gigabit-Ethernet-PHY),
> you seem to be saying that software should manifest a piece of hardware
> out of thin air through sheer willpower. I'm pretty sure I've
> misunderstood this phrasing.
>
>> +struct phy *phy_create(struct device *dev, struct phy_descriptor *desc);
>> +struct phy *devm_phy_create(struct device *dev, struct phy_descriptor
>> *desc);
>> +
>> +The PHY drivers can use one of the above 2 APIs to create the PHY by
>> passing
>
> Um, the driver should _bind_ to the phy, maybe? Allocate? Initialize?

There is actually difference between allocating/initializing and 
binding. Binding is to bind the device controller with the PHY. My 
previous explanation might help to clarify it.
>
>> +6. Destroying the PHY
>
> I've run drivers like that. I try not to, though.
>
>> +7. Current Status
>> +
>> +Currently only USB in OMAP is made to use this framework. However
>> using the
>> +USB PHY library cannot be completely removed because it is
>> intertwined with
>> +OTG. Once we move OTG out of PHY completely, using the old PHY
>> library can be
>> +completely removed. SATA in OMAP will also more likely use this new
>> framework
>> +and we should have a patch for it soon.
>
> Does this paragraph belong in the documentation? (Git commit, sure, but
> I've seen a lot of stale paragraphs like these hang around a
> surprisingly long time.)

hmm.. there are a few others who raised concern on having this 
paragraph. I've planned to remove it in my next version.

Thanks
Kishon

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

end of thread, other threads:[~2013-02-25  6:42 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-19  5:53 [PATCH v2 0/5] Generic PHY Framework Kishon Vijay Abraham I
2013-02-19  5:53 ` [PATCH v2 1/5] drivers: phy: add generic PHY framework Kishon Vijay Abraham I
2013-02-19  8:01   ` Felipe Balbi
2013-02-19 12:56   ` Arnd Bergmann
2013-02-19 13:56     ` kishon
2013-02-19 14:28       ` Arnd Bergmann
2013-02-23 22:44   ` Rob Landley
2013-02-25  6:41     ` kishon
2013-02-19  5:53 ` [PATCH v2 2/5] usb: phy: omap-usb2: use the new " Kishon Vijay Abraham I
2013-02-19  8:11   ` Felipe Balbi
2013-02-19  5:53 ` [PATCH v2 3/5] usb: otg: twl4030: " Kishon Vijay Abraham I
2013-02-19  5:53 ` [PATCH v2 4/5] ARM: OMAP: USB: Add phy binding information Kishon Vijay Abraham I
2013-02-19  5:53 ` [PATCH v2 5/5] usb: musb: omap2430: use the new generic PHY framework Kishon Vijay Abraham I
2013-02-19 10:44 ` [PATCH v2 0/5] Generic PHY Framework Arnd Bergmann
2013-02-19 11:28   ` kishon
2013-02-19 12:33     ` Arnd Bergmann
2013-02-19 13:12       ` Felipe Balbi
2013-02-19 14:34         ` Arnd Bergmann
2013-02-19 15:05           ` Felipe Balbi
2013-02-19 15:28             ` Arnd Bergmann
2013-02-19 15:47               ` Felipe Balbi
2013-02-19 16:07             ` Marc Kleine-Budde
2013-02-19 16:17               ` Felipe Balbi
2013-02-23 20:05             ` Rob Landley

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