[PATCH v5 1/6] drivers: phy: add generic PHY framework
Kishon Vijay Abraham I
kishon at ti.com
Wed Apr 3 10:18:42 EDT 2013
Hi,
On Wednesday 03 April 2013 07:12 PM, Felipe Balbi wrote:
> On Wed, Apr 03, 2013 at 06:23:49PM +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,
>> power_on, power_off.
>>
>> 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 and the documentation for
>> dt binding is can be found at
>> Documentation/devicetree/bindings/phy/phy-bindings.txt
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon at ti.com>
>> ---
>> Documentation/ABI/testing/sysfs-class-phy | 15 +
>> .../devicetree/bindings/phy/phy-bindings.txt | 67 +++
>> 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 | 616 ++++++++++++++++++++
>> include/linux/phy/phy.h | 228 ++++++++
>> 10 files changed, 1068 insertions(+)
>> create mode 100644 Documentation/ABI/testing/sysfs-class-phy
>> create mode 100644 Documentation/devicetree/bindings/phy/phy-bindings.txt
>> create mode 100644 Documentation/phy.txt
>> create mode 100644 drivers/phy/Kconfig
>> create mode 100644 drivers/phy/Makefile
>> create mode 100644 drivers/phy/phy-core.c
>> create mode 100644 include/linux/phy/phy.h
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-phy b/Documentation/ABI/testing/sysfs-class-phy
>> new file mode 100644
>> index 0000000..b735467
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-class-phy
>> @@ -0,0 +1,15 @@
>> +What: /sys/class/phy/<phy>/label
>> +Date: Apr 2013
>> +KernelVersion: 3.10
>> +Contact: Kishon Vijay Abraham I <kishon at ti.com>
>> +Description:
>> + This is a read-only file for getting the label of the phy.
>> +
>> +What: /sys/class/phy/<phy>/phy_bind
>> +Date: Apr 2013
>> +KernelVersion: 3.10
>> +Contact: Kishon Vijay Abraham I <kishon at 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/devicetree/bindings/phy/phy-bindings.txt b/Documentation/devicetree/bindings/phy/phy-bindings.txt
>> new file mode 100644
>> index 0000000..e7b246a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
>> @@ -0,0 +1,67 @@
>> +This document explains only the dt data binding. For general information about
>> +PHY subsystem refer Documentation/phy.txt
>> +
>> +PHY device node
>> +===============
>> +
>> +Required Properties:
>> +#phy-cells: Number of cells in a PHY specifier; The meaning of all those
>> + cells is defined by the binding for the phy node. The PHY
>> + provider can use the values in cells to find the appropriate
>> + PHY.
>> +
>> +For example:
>> +
>> +phys: phy {
>> + compatible = "xxx";
>> + reg = <...>;
>> + .
>> + .
>> + #phy-cells = <1>;
>> + .
>> + .
>> +};
>> +
>> +That node describes an IP block that implements 2 different PHYs. In order to
>> +differentiate between these 2 PHYs, an additonal specifier should be given
>> +while trying to get a reference to it.
>> +
>> +PHY user node
>> +=============
>> +
>> +Required Properties:
>> +phys : the phandle for the PHY device (used by the PHY subsystem)
>> +
>> +Optional properties:
>> +phy-names : the names of the PHY corresponding to the PHYs present in the
>> + *phys* phandle
>> +
>> +Example 1:
>> +usb1: usb_otg_ss at xxx {
>> + compatible = "xxx";
>> + reg = <xxx>;
>> + .
>> + .
>> + phys = <&usb2_phy>, <&usb3_phy>;
>> + phy-names = "usb2phy", "usb3phy";
>> + .
>> + .
>> +};
>> +
>> +This node represents a controller that uses two PHYs one for usb2 and one for
>> +usb3.
>> +
>> +Example 2:
>> +usb2: usb_otg_ss at xxx {
>> + compatible = "xxx";
>> + reg = <xxx>;
>> + .
>> + .
>> + phys = <&phys 1>;
>> + .
>> + .
>> +};
>> +
>> +This node represents a controller that uses one of the PHYs which is defined
>> +previously. Note that the phy handle has an additional specifier "1" to
>> +differentiate between the two PHYs.
>> diff --git a/Documentation/phy.txt b/Documentation/phy.txt
>> new file mode 100644
>> index 0000000..7785ec0
>> --- /dev/null
>> +++ b/Documentation/phy.txt
>> @@ -0,0 +1,113 @@
>> + PHY SUBSYSTEM
>> + Kishon Vijay Abraham I <kishon at ti.com>
>> +
>> +This document explains the Generic PHY Framework along with the APIs provided,
>> +and how-to-use.
>> +
>> +1. Introduction
>> +
>> +*PHY* is the abbreviation for physical layer. It is used to connect a device
>> +to the physical medium e.g., the USB controller has a PHY to provide functions
>> +such as serialization, de-serialization, encoding, decoding and is responsible
>> +for obtaining the required data transmission rate. Note that some USB
>> +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, const char *label,
>> + struct device_node *of_node, int type, struct phy_ops *ops,
>> + void *priv);
>> +struct phy *devm_phy_create(struct device *dev, const char *label,
>> + struct device_node *of_node, int type, struct phy_ops *ops,
>> + void *priv);
>> +
>> +The PHY drivers can use one of the above 2 APIs to create the PHY by passing
>> +the device pointer, label, device node, type, phy ops and a driver data.
>> +phy_ops is a set of function pointers for performing PHY operations such as
>> +init, exit, suspend, resume, power_on and power_off.
>> +
>> +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, int 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
>> +it. This framework provides 6 APIs to get a reference to the PHY.
>> +
>> +struct phy *phy_get(struct device *dev, int index);
>> +struct phy *devm_phy_get(struct device *dev, int index);
>> +struct phy *of_phy_get(struct device *dev, const char *phandle, int index);
>> +struct phy *devm_of_phy_get(struct device *dev, const char *phandle, int index);
>> +struct phy *of_phy_get_byname(struct device *dev, const char *string);
>> +struct phy *devm_of_phy_get_byname(struct device *dev, const char *string);
>> +
>> +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 devres data and it is freed.
>> +
>> +of_phy_get_byname and devm_of_phy_get_byname can also be used to get the PHY
>> +in dt boot. It is same as the above API except that the user has to pass the
>> +phy name as filled in "phy-names" phandle in dt data and the framework will
>> +find the index and get the PHY.
>> +
>> +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. DeviceTree Binding
>> +
>> +The documentation for PHY dt binding can be found @
>> +Documentation/devicetree/bindings/phy/phy-bindings.txt
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 72b0843..f2674e7 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -3474,6 +3474,13 @@ S: Maintained
>> F: include/asm-generic
>> F: include/uapi/asm-generic
>>
>> +GENERIC PHY FRAMEWORK
>> +M: Kishon Vijay Abraham I <kishon at ti.com>
>> +L: linux-kernel at vger.kernel.org
>> +S: Supported
>> +F: drivers/phy/
>> +F: include/linux/phy/
>> +
>> GENERIC UIO DRIVER FOR PCI DEVICES
>> M: "Michael S. Tsirkin" <mst at redhat.com>
>> L: kvm at vger.kernel.org
>> diff --git a/drivers/Kconfig b/drivers/Kconfig
>> index 202fa6d..ad2c374a 100644
>> --- a/drivers/Kconfig
>> +++ b/drivers/Kconfig
>> @@ -162,4 +162,6 @@ source "drivers/irqchip/Kconfig"
>>
>> source "drivers/ipack/Kconfig"
>>
>> +source "drivers/phy/Kconfig"
>> +
>> endmenu
>> diff --git a/drivers/Makefile b/drivers/Makefile
>> index dce39a9..9da8321 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..5f85909
>> --- /dev/null
>> +++ b/drivers/phy/Kconfig
>> @@ -0,0 +1,13 @@
>> +#
>> +# PHY
>> +#
>> +
>> +menuconfig GENERIC_PHY
>> + tristate "PHY Subsystem"
>> + help
>> + Generic PHY support.
>> +
>> + This framework is designed to provide a generic interface for PHY
>> + devices present in the kernel. This layer will have the generic
>> + API by which phy drivers can create PHY using the phy framework and
>> + phy users can obtain reference to the PHY.
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> new file mode 100644
>> index 0000000..9e9560f
>> --- /dev/null
>> +++ b/drivers/phy/Makefile
>> @@ -0,0 +1,5 @@
>> +#
>> +# Makefile for the phy drivers.
>> +#
>> +
>> +obj-$(CONFIG_GENERIC_PHY) += phy-core.o
>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> new file mode 100644
>> index 0000000..1d753f2
>> --- /dev/null
>> +++ b/drivers/phy/phy-core.c
>> @@ -0,0 +1,616 @@
>> +/*
>> + * phy-core.c -- Generic Phy framework.
>> + *
>> + * Copyright (C) 2013 Texas Instruments
>> + *
>> + * Author: Kishon Vijay Abraham I <kishon at 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 DEFINE_MUTEX(phy_bind_mutex);
>> +static LIST_HEAD(phy_bind_list);
>> +static int phy_core_init(void);
>> +
>> +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, int 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_node *node)
>> +{
>> + struct phy *phy;
>> + struct device *dev;
>> + struct class_dev_iter iter;
>> +
>> + class_dev_iter_init(&iter, phy_class, NULL, NULL);
>> + while ((dev = class_dev_iter_next(&iter))) {
>> + phy = container_of(dev, struct phy, dev);
>
> it would look a bit better if you provided a to_phy() macro. Specially
> since this container_of() repeats multiple times in this file.
hmm.. ok.
>
>> +/**
>> + * 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)
>> +{
>
> I would rather:
>
> if (WARN(IS_ERR(phy), "invalid parameter\n"))
> return;
>
> module_put(phy->ops->owner);
> put_device(&phy->dev);
>
> that way we can catch users passing bogus pointers here. When PHY layer
> is disabled, you want to make this is no-op with a static inline in a
> header anyway.
yeah. Have that no-op in header file.
>
>> +struct phy *of_phy_xlate(struct phy *phy, struct of_phandle_args *args)
>> +{
>> + return phy;
>> +}
>> +EXPORT_SYMBOL_GPL(of_phy_xlate);
>
> so you get a PHY and just return it ? What gives ?? (maybe I skipped
> some of the discussion...)
hmm.. this is for the common case where the PHY provider implements only
one PHY. And both phy provider and phy_instance is represented by struct
phy *.
For the case where PHY provider implements multiple PHYs (here it will
have a single dt node), the PHY provider will implement it's own version
of of_xlate that takes *of_phandle_args* as argument and finds the
appropriate PHY.
>
>> +struct phy *of_phy_get(struct device *dev, int index)
>> +{
>> + int ret;
>> + struct phy *phy = NULL;
>> + struct phy_bind *phy_map = NULL;
>> + struct of_phandle_args args;
>> + struct device_node *node;
>> +
>> + if (!dev->of_node) {
>> + dev_dbg(dev, "device does not have a device node entry\n");
>> + return ERR_PTR(-EINVAL);
>> + }
>> +
>> + ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
>> + index, &args);
>> + if (ret) {
>> + dev_dbg(dev, "failed to get phy in %s node\n",
>> + dev->of_node->full_name);
>> + return ERR_PTR(-ENODEV);
>> + }
>> +
>> + phy = of_phy_lookup(args.np);
>> + if (IS_ERR(phy) || !try_module_get(phy->ops->owner)) {
>> + phy = ERR_PTR(-EPROBE_DEFER);
>> + goto err0;
>> + }
>> +
>> + phy = phy->ops->of_xlate(phy, &args);
>
> alright, so of_xlate() is optional, am I right ? How about not
Not really. of_xlate is mandatory (it's even checked in phy_create).
Either the PHY provider can implement it's own version or use the
implementation above (by filling the function pointer).
> implementing the above and have a check for of_xlate() being a valid
> pointer here ?
Having the way it is actually mandates the PHY providers to always
provide of_xlate which IMO is better since some PHY providers wont
accidentally be using the default implementation.
>
>> +struct phy *phy_create(struct device *dev, const char *label,
>> + struct device_node *of_node, int type, struct phy_ops *ops,
>> + void *priv)
>> +{
>> + int ret;
>> + struct phy *phy;
>> + struct phy_bind *phy_bind;
>> + const char *devname = NULL;
>> +
>> + if (!dev) {
>> + dev_err(dev, "no device provided for PHY\n");
>
> I'd call this a WARN() or am I too pedantic? :-p
>
>> + if (!ops || !ops->of_xlate || !priv) {
>> + dev_err(dev, "no PHY ops/PHY data provided\n");
>
> likewise here.
hmm.. ok.
>
>> + ret = -EINVAL;
>> + goto err0;
>> + }
>> +
>> + if (!phy_class)
>> + phy_core_init();
>
> why don't you setup the class on module_init ? Then this would be a
> terrible error condition here :-)
This is for the case where the PHY driver gets loaded before the PHY
framework. I could have returned EPROBE_DEFER here instead I thought
will have it this way.
>
>> +static struct device_attribute phy_dev_attrs[] = {
>> + __ATTR(label, 0444, phy_name_show, NULL),
>> + __ATTR(phy_bind, 0444, phy_bind_show, NULL),
>
> you could expose a human-readable 'type' string. BTW, how are you using
> type ? USB2/USB3/etc ? Have you considered our OMAP5 SerDes pair which
Actually not using *type* anywhere. Just used as an additional info
about the PHY. It's actually not even enum. Maybe I can remove it
completely.
> are currently for USB3 and SATA (and could just as easily be used for
> PCIe)
Yeah. Me and Balaji were planning to work on it for having a single
driver to be used for all the above.
>
>> +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));
>
> how about dev_vdbg() ? I doubt anyone will be waiting for this
> message... Just a thought
>
>> +static int phy_core_init(void)
>> +{
>> + if (phy_class)
>> + return 0;
>
> Weird.. if you initialize the class here, why do you need to initialize
> it during phy_create() ?
>
> What's going on ? Also, module_init() will only be called once, why this
> if (phy_class) check ?
er.. for the case where phy driver is loaded before this PHY framework,
phy_create would have already called phy_core_init to create the
phy_class. So module_init() is not needed at all since we have already
created the phy_class. I think this looks a bit hacky.
Either we can have EPROBE_DEFER in phy_create or have this module as
subsys_initcall() like I had it before. I would actually prefer it to be
subsys_initcall().
Thanks
Kishon
More information about the linux-arm-kernel
mailing list