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

From: Sylwester Nawrocki
Date: Tue May 28 2013 - 18:37:50 EST


Hi Kishon,

On 04/29/2013 12:03 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. For dt-boot, the PHY drivers should
also register *PHY provider* with the framework.

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

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

Signed-off-by: Kishon Vijay Abraham I<kishon@xxxxxx>

Thanks for working on this. For the record, I plan to give this a try
in the end of this week, with my simple MIPI CSI/DSI PHY driver. I might
have some more comments then. For now just couple of remarks after
reading the documentation.

---
.../devicetree/bindings/phy/phy-bindings.txt | 66 +++
Documentation/phy.txt | 123 +++++
MAINTAINERS | 7 +
drivers/Kconfig | 2 +
drivers/Makefile | 2 +
drivers/phy/Kconfig | 13 +
drivers/phy/Makefile | 5 +
drivers/phy/phy-core.c | 539 ++++++++++++++++++++
include/linux/phy/phy.h | 248 +++++++++
9 files changed, 1005 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/phy-bindings.txt
create mode 100644 Documentation/phy.txt
create mode 100644 drivers/phy/Kconfig
create mode 100644 drivers/phy/Makefile
create mode 100644 drivers/phy/phy-core.c
create mode 100644 include/linux/phy/phy.h

diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt b/Documentation/devicetree/bindings/phy/phy-bindings.txt
new file mode 100644
index 0000000..8ae844f
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
@@ -0,0 +1,66 @@
+This document explains only the device tree data binding. For general
+information about PHY subsystem refer to Documentation/phy.txt
+
+PHY device node
+===============
+
+Required Properties:
+#phy-cells: Number of cells in a PHY specifier; The meaning of all those
+ cells is defined by the binding for the phy node. The PHY
+ provider can use the values in cells to find the appropriate
+ PHY.
+
+For example:
+
+phys: phy {
+ compatible = "xxx";
+ reg =<...>;
+ .
+ .
+ #phy-cells =<1>;
+ .
+ .
+};
+
+That node describes an IP block (PHY provider) that implements 2 different PHYs.
+In order to differentiate between these 2 PHYs, an additonal specifier should be
+given while trying to get a reference to it.
+
+PHY user node
+=============
+
+Required Properties:
+phys : the phandle for the PHY device (used by the PHY subsystem)
+phy-names : the names of the PHY corresponding to the PHYs present in the
+ *phys* phandle
+
+Example 1:
+usb1: usb_otg_ss@xxx {
+ compatible = "xxx";
+ reg =<xxx>;
+ .
+ .
+ phys =<&usb2_phy>,<&usb3_phy>;
+ phy-names = "usb2phy", "usb3phy";
+ .
+ .
+};
+
+This node represents a controller that uses two PHYs, one for usb2 and one for
+usb3.
+
+Example 2:
+usb2: usb_otg_ss@xxx {
+ compatible = "xxx";
+ reg =<xxx>;
+ .
+ .
+ phys =<&phys 1>;
+ phy-names = "usbphy";
+ .
+ .
+};
+
+This node represents a controller that uses one of the PHYs of the PHY provider
+device defined previously. Note that the phy handle has an additional specifier
+"1" to differentiate between the two PHYs.
diff --git a/Documentation/phy.txt b/Documentation/phy.txt
new file mode 100644
index 0000000..408d25f
--- /dev/null
+++ b/Documentation/phy.txt
@@ -0,0 +1,123 @@
+ PHY SUBSYSTEM
+ Kishon Vijay Abraham I<kishon@xxxxxx>
+
+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

"Note that some USB
controllers have PHY functionality embedded into them..." ?

+PHY. Other peripherals that uses a PHY include Wireless LAN, Ethernet,

s/uses/use ?

+SATA etc.
+
+The intention of creating this framework is to bring the phy drivers spread

s/phy/PHY ?

+all over the Linux kernel to drivers/phy to increase code re-use and for
+better code maintainability.
+
+This framework will be of use only to devices that uses external PHY (PHY

s/that uses/that use ?

+functionality is not embedded within the controller).
+
+2. Registering/UnRegistering the PHY provider

s/UnRegistering/Unregistering ?

+
+PHY provider refers to an entity that implements one or more PHY instances.
+For the simple case where the PHY provider implements only a single instance of
+the PHY, the framework provides its own implementation of of_xlate in
+of_phy_simple_xlate. If the PHY provider implements multiple instances, it
+should provide it's own implementation of of_xlate. of_xlate is used only for

s/it's/its

+dt boot case.
+
+struct phy_provider *of_phy_provider_register(struct device *dev,
+ struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+ struct of_phandle_args *args));
+struct phy_provider *devm_of_phy_provider_register(struct device *dev,
+ struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+ struct of_phandle_args *args))
+
+of_phy_provider_register and devm_of_phy_provider_register can be used to
+register the phy_provider and it takes device, owner and of_xlate as
+arguments. For the dt boot case, all PHY providers should use one of the above
+2 APIs to register the PHY provider.
+
+void devm_of_phy_provider_unregister(struct device *dev,
+ struct phy_provider *phy_provider);
+void of_phy_provider_unregister(struct phy_provider *phy_provider);
+
+devm_of_phy_provider_unregister and of_phy_provider_unregister can be used to
+unregister the PHY.
+
+3. Creating the PHY
+
+The PHY driver should create the PHY in order for other peripheral controllers
+to make use of it. The PHY framework provides 2 APIs to create the PHY.
+
+struct phy *phy_create(struct device *dev, int id, const struct phy_ops *ops,
+ void *priv);
+struct phy *devm_phy_create(struct device *dev, int id,
+ const struct phy_ops *ops, void *priv);
+
+The PHY drivers can use one of the above 2 APIs to create the PHY by passing
+the device pointer, id, phy ops and a driver data.
+phy_ops is a set of function pointers for performing PHY operations such as
+init, exit, power_on and power_off.
+
+4. Getting a reference to the PHY
+
+Before the controller can make use of the PHY, it has to get a reference to
+it. This framework provides the following APIs to get a reference to the PHY.
+
+struct phy *phy_get(struct device *dev, const char *string);
+struct phy *devm_phy_get(struct device *dev, const char *string);
+
+phy_get and devm_phy_get can be used to get the PHY. In the case of dt boot,
+the string arguments should contain the phy name as given in the dt data and
+in the case of non-dt boot, it should contain the device name of the PHY.

How about allowing null strings ? At least it seems not difficult for dt boot.
But there would have been some asymmetry, for non-dt an error would have to
be returned when the passed string is null.

+The only difference between the two APIs is that devm_phy_get associates the
+device with the PHY using devres on successful PHY get. On driver detach,
+release function is invoked on the the devres data and devres data is freed.
+
+5. Releasing a reference to the PHY
+
+When the controller no longer needs the PHY, it has to release the reference
+to the PHY it has obtained using the APIs mentioned in the above section. The
+PHY framework provides 2 APIS to release a reference to the PHY.

APIs ?

+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

s/APIs destroys/APIs destroy ?

+associated with this PHY.
+
+7. PM Runtime
+
+This subsystem is pm runtime enabled. So while creating the PHY,
+pm_runtime_enable of the phy device created by this subsystem is called and
+while destroying the PHY, pm_runtime_disable is called. Note that the phy
+device created by this subsystem will be a child of the device that calls
+phy_create (PHY provider device).
+
+During phy_init, the clocks are enabled by calling get_sync and the clocks are

I think, if you mean pm_runtime_get_sync(), it should be said precisely here.

+disable by calling put_sync during phy_exit. get_sync of the phy_device

Ditto.

s/disable/disabled

+created by this susbsystem will invoke get_sync of PHY provider device because

s/susbsystem/subsystem

+of parent-child relationship. For all other pm operations, there are exported
+APIs like phy_pm_runtime_get, phy_pm_runtime_get_sync, phy_pm_runtime_put,
+phy_pm_runtime_put_sync, phy_pm_runtime_allow and phy_pm_runtime_forbid.
+
+8. DeviceTree Binding
+
+The documentation for PHY dt binding can be found @
+Documentation/devicetree/bindings/phy/phy-bindings.txt

+/**
+ * phy_release() - release the phy
+ * @dev: the dev member within phy
+ *
+ * when the last reference to the device is removed; it is called

s/when/When
s/removed;/removed ?

+ * from the embedded kobject as release method.
+ */

+/**
+ * struct phy - represent the phy device

s/represent/represents ?

+ * @dev: phy device
+ * @id: id of the phy
+ * @ops: function pointers for performing phy operations
+ */
+struct phy {
+ struct device dev;
+ int id;
+ const struct phy_ops *ops;
+};
+
+/**
+ * struct phy_provider - represent the phy provider

s/represent/represents ?

+ * @dev: phy provider device
+ * @owner: the module owner having of_xlate
+ * @of_xlate: function pointer to obtain phy instance from phy pointer
+ * @list: to maintain a linked list of PHY provider

s/provider/providers ?

+ */
+struct phy_provider {
+ struct device *dev;
+ struct module *owner;
+ struct list_head list;
+ struct phy * (*of_xlate)(struct device *dev,
+ struct of_phandle_args *args);
+};

Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/