Re: [PATCH v4 1/6] drivers: phy: add generic PHY framework
Hi,
On Thursday 28 March 2013 09:15 PM, Stephen Warren wrote:
On 03/27/2013 11:43 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
diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt
b/Documentation/devicetree/bindings/phy/phy-bindings.txt
+This document explains only the dt data binding. For general information about
+PHY subsystem refer Documentation/phy.txt
+
+PHY device node
+===
+
+Optional 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. However
+ in-order to return the correct PHY, the PHY susbsystem
+ requires the first cell always refers to the port.
Why impose that restriction? Other DT bindings do not.
This is typically implemented by having each provider driver implement a
.of_xlate() operation, which parses all of the specifier cells, and
returns the ID of the object it represents. This allows bindings to use
whatever arbitrary representation they want.
Do you mean something like this
struct phy *of_phy_get(struct device *dev, int index)
{
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);
}
//Here we have to get a reference to the phy in order to call of_xlate
which seems a little hacky to me. I'm not sure how else can we call the
provider driver :-(
phy = of_phy_lookup(dev, node);
if (IS_ERR(phy) || !try_module_get(phy-ops-owner)) {
phy = ERR_PTR(-EPROBE_DEFER);
goto err0;
}
//here we are checking if the phy has additional specifiers and if so
call of_xlate using the phy we just obtained. The provider driver should
check the args and return the appropriate *phy in this case.
if (args.args_count 0) {
phy = phy-of_xlate(args);
if (IS_ERR(phy))
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:
of_node_put(node);
return phy;
}
EXPORT_SYMBOL_GPL(of_phy_get);
For the common/simple cases where #phy-cells==0, or #phy-cells==1 and
directly represents the PHY ID, the PHY core can provide an
implementation of that common .of_xlate() function, which PHY provider
drivers can simply plug in as their .of_xlate() function.
+This property is optional because it is needed only for the case where a
+single IP implements multiple PHYs.
The property should always exist so that the DT-parsing code in the PHY
core can always validate exactly how many cells are present in the PHY
specifier.
+
+For example:
+
+phys: phy {
+compatible = xxx;
+reg1 = ...;
+reg2 = ...;
+reg3 = ...;
3 separate reg values should be 3 separate entries in a single reg
property, not 3 separate reg properties, with non-standard names.
+.
+.
+#phy-cells = 1;
+.
+.
+};
+
+That node describes an IP block that implements 3 different PHYs. In order to
+differentiate between these 3 PHYs, an additonal specifier should be given
+while trying to get a reference to it. (The PHY subsystem assumes the
+specifier is port id).
A single DT node would typically represent a single HW IP block, and
hence typically have a single reg property. If there are 3 separate HW
IP blocks, and their register aren't interleaved, and hence can be
represented by 3 separate reg values, then I'd typically expect to see 3
separate DT nodes, one for each independent register range.
The only case where I'd expect a single DT node to provide multipe PHYs,
is when a single HW IP block actually implements multiple PHYs /and/ the
registers for those 3 PHYs are interleaved (or share bits in the same
registers) such that each PHY can't be represented by a separate
non-overlapping reg property.
+example1:
+phys: phy {
How about:
Example 1:
usb1: usb@xxx {
+};
+This node represents a controller that uses two PHYs one for usb2 and one for
Blank line after }?
+usb3. The controller driver can get the appropriate PHY either by using
+devm_of_phy_get/of_phy_get by passing the correct index or by using
+of_phy_get_byname/devm_of_phy_get_byname by passing the names given in
Re: [PATCH v4 1/6] drivers: phy: add generic PHY framework
Hi,
On Tuesday 02 April 2013 01:04 AM, Sylwester Nawrocki wrote:
Just couple minor comments...
On 03/28/2013 06:43 AM, 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,
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 and the documentation for
dt binding is can be found at
Documentation/devicetree/bindings/phy/phy-bindings.txt
Signed-off-by: Kishon Vijay Abraham Ikis...@ti.com
---
[...]
+/**
+ * phy_put - release the PHY
nit: According to kernel-doc documentation function names should have
parentheses appended to the name. So it would need to be
+ * phy_put() - release the PHY
in this case and it applies to multiple places elsewhere in this patch.
Will fix it.
+ * @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-ops-owner);
+put_device(phy-dev);
+}
+}
+EXPORT_SYMBOL_GPL(phy_put);
[...]
+/**
+ * devm_of_phy_get_byname - lookup and obtain a reference to a phy by
name
+ * @dev: device that requests this phy
+ * @string - the phy name as given in the dt data
s/ -/:
Ok.
+ *
+ * Calls devm_of_phy_get (which associates the device with the phy
using devres
+ * on successful phy get) and passes on the return value of
devm_of_phy_get.
+ */
+struct phy *devm_of_phy_get_byname(struct device *dev, const char
*string)
+{
+int index;
+
+if (!dev-of_node) {
+dev_dbg(dev, device does not have a device node entry\n);
+return ERR_PTR(-EINVAL);
+}
+
+index = of_property_match_string(dev-of_node, phy-names, string);
+
+return devm_of_phy_get(dev, index);
+}
+EXPORT_SYMBOL_GPL(devm_of_phy_get_byname);
+
+/**
+ * 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, int index)
+{
+struct phy *phy = NULL;
Unneeded initialization.
+phy = phy_lookup(dev, index);
+if (IS_ERR(phy)) {
+dev_err(dev, unable to find phy\n);
+goto err0;
Wouldn't it be better to just do:
Indeed.
return phy;
+}
+
+if (!try_module_get(phy-ops-owner)) {
+phy = ERR_PTR(-EPROBE_DEFER);
and
return ERR_PTR(-EPROBE_DEFER);
+goto err0;
and to drop this goto and the label below ?
+}
+
+get_device(phy-dev);
+
+err0:
+return phy;
+}
+EXPORT_SYMBOL_GPL(phy_get);
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
new file mode 100644
index 000..0cb2298
--- /dev/null
+++ b/include/linux/phy/phy.h
@@ -0,0 +1,237 @@
+/*
+ * phy.h -- generic phy header file
[...]
+#ifndef __DRIVERS_PHY_H
+#define __DRIVERS_PHY_H
+
+#includelinux/err.h
+#includelinux/of.h
+
+struct phy
I think you also need to add either
#include linux/device.h
or
struct device;
struct device * is used further in this file.
Ok.
+/**
+ * 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
Could these be named power_on/power_off ? Looks a bit more symmetric
to me that way.
Ok. Will have it that way.
Thanks
Kishon
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/6] drivers: phy: add generic PHY framework
On 04/02/2013 02:37 AM, Kishon Vijay Abraham I wrote:
Hi,
On Thursday 28 March 2013 09:15 PM, Stephen Warren wrote:
On 03/27/2013 11:43 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
diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt
+PHY subsystem refer Documentation/phy.txt
+
+PHY device node
+===
+
+Optional 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. However
+in-order to return the correct PHY, the PHY susbsystem
+requires the first cell always refers to the port.
Why impose that restriction? Other DT bindings do not.
This is typically implemented by having each provider driver implement a
.of_xlate() operation, which parses all of the specifier cells, and
returns the ID of the object it represents. This allows bindings to use
whatever arbitrary representation they want.
Do you mean something like this
struct phy *of_phy_get(struct device *dev, int index)
{
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);
}
Looks good.
//Here we have to get a reference to the phy in order to call of_xlate
which seems a little hacky to me. I'm not sure how else can we call the
provider driver :-(
phy = of_phy_lookup(dev, node);
if (IS_ERR(phy) || !try_module_get(phy-ops-owner)) {
phy = ERR_PTR(-EPROBE_DEFER);
goto err0;
}
I think the concept of a PHY provider and a PHY instance are different.
of_xlate should be called on a PHY provider, and return a PHY
instance. Hence, above you want to only look up a PHY provider, so
there's no hackiness involved.
//here we are checking if the phy has additional specifiers and if so
call of_xlate using the phy we just obtained. The provider driver should
check the args and return the appropriate *phy in this case.
if (args.args_count 0) {
It's probably simplest to always call of_xlate; that way, you're always
calling it on a PHY provider and getting back a PHY instance. For
providers that only provide 1 instance, the implementation should be
simple:-)
phy = phy-of_xlate(args);
if (IS_ERR(phy))
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:
of_node_put(node);
return phy;
}
EXPORT_SYMBOL_GPL(of_phy_get);
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/6] drivers: phy: add generic PHY framework
On 04/02/2013 12:38 AM, Stephen Warren wrote:
On 04/01/2013 04:27 PM, Sylwester Nawrocki wrote:
On 03/28/2013 06:43 AM, Kishon Vijay Abraham I wrote:
diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt
+example2:
+phys: phy {
+compatible = xxx;
+reg =...;
+.
+.
+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 three PHYs. For this case, the controller driver
+should use of_phy_get_with_args/devm_of_phy_get_with_args.
This means a PHY user needs to know indexes at the PHY driver ?
The client node's DT has to specify which of the provider's PHYs it
references, yes. Presumably the device driver for the client node knows
absolutely nothing about this.
Ah, right. The device driver for the client node not necessarily need to be
aware about this. I think I got misled by the 'index' argument of
of_phy_get_with_args() which the PHY consumer need to supply. However it is
only an index pointing to a PHY specifier in its 'phys' property, hence it
would be normally 0 if the client needs only one PHY. Hopefully I got it
right this time.
I have been thinking of using this for an IP which has 4 video PHYs: 2 MIPI
CSI-2 and 2 MIPI DSI. The IP has just 2 registers, each of which is shared
between one MIPI CSI-2 DPHY and one MIPI DSI DPHY. So I thought about creating
a single device node for this IP and using 4 indexes for the PHYs, e.g. 0...3.
That sounds right.
Then users of each PHY type would use only indexes 0..1 (to select their
corresponding port).
I don't follow that. If the provider exports PHYs 0..3, then the client
nodes would refer to PHYS 0..3 not 0..1.
Indeed, it seems I just wanted to preserve the CSI/DSI channel information
(0, 1), but that is not really needed anywhere.
However I fail to see how this could now be represented in the bindings.
Exactly like the example you gave below, I would expect.
I assume struct phy::type could be used to differentiate between CSI-2 and DSI.
And struct phy::port could be used to select specific CSI-2 or DSI channel
(0, 1). Ideally the phy users should not care about index of a PHY at the PHY
device tree node. E.g. there are 2 MIPI CSI-2 receivers and each has only
one PHY assigned to it. I'm just wondering how the binding should look like,
so a PHY could be associated with a receiver automatically by the phy-core,
e.g.
Details such as phy::type and phy::port sounds like internal driver
implementation details which would have no effect on the bindings.
Yes, I seem to have mixed the phy-core implementation and the bindings a
bit.
My intention was to have the PHYs registered with phy::port that would
reflect
data channel id, as specified in the SoC's datasheet. However I can't
think of
any use of it at the moment, except for debugging purpose.
/* DPHY IP node */
video-phy {
reg =0x1000 8;
};
/* MIPI DSI nodes */
dsi_0 {
phys =video-phy 0;
};
dsi_1 {
phys =video-phy 1;
};
/* MIPI CSI-2 nodes */
csi_0 {
phys =video-phy 2;
};
csi_1 {
phys =video-phy 3;
};
That looks correct to me.
I'm not sure if it is not an overkill to use this the PHY framework with
a device which has only 2 registers. Perhaps something less heavy could
be designed for it. However, if the PHY framework is commonly used there
should be no issue wrt enabling the whole big infrastructure for a simple
device like this.
I don't think the number of registers should really makes any
difference; the whole point of the PHY framework is to decouple to
providers and consumers, so doing anything custom for special cases
would completely destroy the ability of the PHY framework to do that.
Ok, that's a very good argument. Something I have not been focused on
that much, given the architecture of hardware I used to work with.
I'll git it a try and I'll see if any any further questions jump out.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/6] drivers: phy: add generic PHY framework
On Tuesday 02 April 2013 09:10 PM, Stephen Warren wrote:
On 04/02/2013 02:37 AM, Kishon Vijay Abraham I wrote:
Hi,
On Thursday 28 March 2013 09:15 PM, Stephen Warren wrote:
On 03/27/2013 11:43 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
diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt
+PHY subsystem refer Documentation/phy.txt
+
+PHY device node
+===
+
+Optional 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. However
+in-order to return the correct PHY, the PHY susbsystem
+requires the first cell always refers to the port.
Why impose that restriction? Other DT bindings do not.
This is typically implemented by having each provider driver implement a
.of_xlate() operation, which parses all of the specifier cells, and
returns the ID of the object it represents. This allows bindings to use
whatever arbitrary representation they want.
Do you mean something like this
struct phy *of_phy_get(struct device *dev, int index)
{
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);
}
Looks good.
//Here we have to get a reference to the phy in order to call of_xlate
which seems a little hacky to me. I'm not sure how else can we call the
provider driver :-(
phy = of_phy_lookup(dev, node);
if (IS_ERR(phy) || !try_module_get(phy-ops-owner)) {
phy = ERR_PTR(-EPROBE_DEFER);
goto err0;
}
I think the concept of a PHY provider and a PHY instance are different.
of_xlate should be called on a PHY provider, and return a PHY
instance. Hence, above you want to only look up a PHY provider, so
there's no hackiness involved.
Cool. That makes it a lot clearer.
Thanks
Kishon
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/6] drivers: phy: add generic PHY framework
Just couple minor comments...
On 03/28/2013 06:43 AM, 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,
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 and the documentation for
dt binding is can be found at
Documentation/devicetree/bindings/phy/phy-bindings.txt
Signed-off-by: Kishon Vijay Abraham Ikis...@ti.com
---
[...]
+/**
+ * phy_put - release the PHY
nit: According to kernel-doc documentation function names should have
parentheses appended to the name. So it would need to be
+ * phy_put() - release the PHY
in this case and it applies to multiple places elsewhere in this patch.
+ * @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-ops-owner);
+ put_device(phy-dev);
+ }
+}
+EXPORT_SYMBOL_GPL(phy_put);
[...]
+/**
+ * devm_of_phy_get_byname - lookup and obtain a reference to a phy by name
+ * @dev: device that requests this phy
+ * @string - the phy name as given in the dt data
s/ -/:
+ *
+ * Calls devm_of_phy_get (which associates the device with the phy using devres
+ * on successful phy get) and passes on the return value of devm_of_phy_get.
+ */
+struct phy *devm_of_phy_get_byname(struct device *dev, const char *string)
+{
+ int index;
+
+ if (!dev-of_node) {
+ dev_dbg(dev, device does not have a device node entry\n);
+ return ERR_PTR(-EINVAL);
+ }
+
+ index = of_property_match_string(dev-of_node, phy-names, string);
+
+ return devm_of_phy_get(dev, index);
+}
+EXPORT_SYMBOL_GPL(devm_of_phy_get_byname);
+
+/**
+ * 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, int index)
+{
+ struct phy *phy = NULL;
Unneeded initialization.
+ phy = phy_lookup(dev, index);
+ if (IS_ERR(phy)) {
+ dev_err(dev, unable to find phy\n);
+ goto err0;
Wouldn't it be better to just do:
return phy;
+ }
+
+ if (!try_module_get(phy-ops-owner)) {
+ phy = ERR_PTR(-EPROBE_DEFER);
and
return ERR_PTR(-EPROBE_DEFER);
+ goto err0;
and to drop this goto and the label below ?
+ }
+
+ get_device(phy-dev);
+
+err0:
+ return phy;
+}
+EXPORT_SYMBOL_GPL(phy_get);
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
new file mode 100644
index 000..0cb2298
--- /dev/null
+++ b/include/linux/phy/phy.h
@@ -0,0 +1,237 @@
+/*
+ * phy.h -- generic phy header file
[...]
+#ifndef __DRIVERS_PHY_H
+#define __DRIVERS_PHY_H
+
+#includelinux/err.h
+#includelinux/of.h
+
+struct phy
I think you also need to add either
#include linux/device.h
or
struct device;
struct device * is used further in this file.
+/**
+ * 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
Could these be named power_on/power_off ? Looks a bit more symmetric
to me that way.
+ * @owner: the module owner containing the ops
+ */
+struct phy_ops {
+ int (*init)(struct phy *phy);
+ int (*exit)(struct phy *phy);
+ int (*suspend)(struct phy *phy);
+ int (*resume)(struct phy *phy);
+ int (*poweron)(struct phy *phy);
+ int (*shutdown)(struct phy *phy);
+ struct module *owner;
+};
Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/6] drivers: phy: add generic PHY framework
On 03/28/2013 06:43 AM, Kishon Vijay Abraham I wrote:
diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt
b/Documentation/devicetree/bindings/phy/phy-bindings.txt
new file mode 100644
index 000..35696b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
@@ -0,0 +1,76 @@
+This document explains only the dt data binding. For general information about
+PHY subsystem refer Documentation/phy.txt
+
+PHY device node
+===
+
+Optional 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. However
+ in-order to return the correct PHY, the PHY susbsystem
+ requires the first cell always refers to the port.
+
+This property is optional because it is needed only for the case where a
+single IP implements multiple PHYs.
+
+For example:
+
+phys: phy {
+compatible = xxx;
+reg1 =...;
+reg2 =...;
+reg3 =...;
+.
+.
+#phy-cells =1;
+.
+.
+};
+
+That node describes an IP block that implements 3 different PHYs. In order to
+differentiate between these 3 PHYs, an additonal specifier should be given
+while trying to get a reference to it. (The PHY subsystem assumes the
+specifier is port id).
+
+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
+
+example1:
+phys: phy {
+compatible = xxx;
+reg =...;
+.
+.
+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. The controller driver can get the appropriate PHY either by using
+devm_of_phy_get/of_phy_get by passing the correct index or by using
+of_phy_get_byname/devm_of_phy_get_byname by passing the names given in
+*phy-names*.
+
+example2:
+phys: phy {
+compatible = xxx;
+reg =...;
+.
+.
+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 three PHYs. For this case, the controller driver
+should use of_phy_get_with_args/devm_of_phy_get_with_args.
This means a PHY user needs to know indexes at the PHY driver ?
I have been thinking of using this for an IP which has 4 video PHYs: 2 MIPI
CSI-2 and 2 MIPI DSI. The IP has just 2 registers, each of which is shared
between one MIPI CSI-2 DPHY and one MIPI DSI DPHY. So I thought about
creating
a single device node for this IP and using 4 indexes for the PHYs, e.g.
0...3.
Then users of each PHY type would use only indexes 0..1 (to select their
corresponding port).
However I fail to see how this could now be represented in the bindings.
I assume struct phy::type could be used to differentiate between CSI-2
and DSI.
And struct phy::port could be used to select specific CSI-2 or DSI channel
(0, 1). Ideally the phy users should not care about index of a PHY at
the PHY
device tree node. E.g. there are 2 MIPI CSI-2 receivers and each has only
one PHY assigned to it. I'm just wondering how the binding should look like,
so a PHY could be associated with a receiver automatically by the phy-core,
e.g.
/* DPHY IP node */
video-phy {
reg = 0x1000 8;
};
/* MIPI DSI nodes */
dsi_0 {
phys = video-phy 0;
};
dsi_1 {
phys = video-phy 1;
};
/* MIPI CSI-2 nodes */
csi_0 {
phys = video-phy 2;
};
csi_1 {
phys = video-phy 3;
};
I'm not sure if it is not an overkill to use this the PHY framework with
a device which has only 2 registers. Perhaps something less heavy could
be designed for it. However, if the PHY framework is commonly used there
should be no issue wrt enabling the whole big infrastructure for a simple
device like this.
Thanks,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/6] drivers: phy: add generic PHY framework
On 04/01/2013 04:27 PM, Sylwester Nawrocki wrote:
On 03/28/2013 06:43 AM, Kishon Vijay Abraham I wrote:
diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt
+example2:
+phys: phy {
+compatible = xxx;
+reg =...;
+.
+.
+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 three PHYs. For this case, the controller driver
+should use of_phy_get_with_args/devm_of_phy_get_with_args.
This means a PHY user needs to know indexes at the PHY driver ?
The client node's DT has to specify which of the provider's PHYs it
references, yes. Presumably the device driver for the client node knows
absolutely nothing about this.
I have been thinking of using this for an IP which has 4 video PHYs: 2 MIPI
CSI-2 and 2 MIPI DSI. The IP has just 2 registers, each of which is shared
between one MIPI CSI-2 DPHY and one MIPI DSI DPHY. So I thought about creating
a single device node for this IP and using 4 indexes for the PHYs, e.g. 0...3.
That sounds right.
Then users of each PHY type would use only indexes 0..1 (to select their
corresponding port).
I don't follow that. If the provider exports PHYs 0..3, then the client
nodes would refer to PHYS 0..3 not 0..1.
However I fail to see how this could now be represented in the bindings.
Exactly like the example you gave below, I would expect.
I assume struct phy::type could be used to differentiate between CSI-2 and
DSI.
And struct phy::port could be used to select specific CSI-2 or DSI channel
(0, 1). Ideally the phy users should not care about index of a PHY at the PHY
device tree node. E.g. there are 2 MIPI CSI-2 receivers and each has only
one PHY assigned to it. I'm just wondering how the binding should look like,
so a PHY could be associated with a receiver automatically by the phy-core,
e.g.
Details such as phy::type and phy::port sounds like internal driver
implementation details which would have no effect on the bindings.
/* DPHY IP node */
video-phy {
reg = 0x1000 8;
};
/* MIPI DSI nodes */
dsi_0 {
phys = video-phy 0;
};
dsi_1 {
phys = video-phy 1;
};
/* MIPI CSI-2 nodes */
csi_0 {
phys = video-phy 2;
};
csi_1 {
phys = video-phy 3;
};
That looks correct to me.
I'm not sure if it is not an overkill to use this the PHY framework with
a device which has only 2 registers. Perhaps something less heavy could
be designed for it. However, if the PHY framework is commonly used there
should be no issue wrt enabling the whole big infrastructure for a simple
device like this.
I don't think the number of registers should really makes any
difference; the whole point of the PHY framework is to decouple to
providers and consumers, so doing anything custom for special cases
would completely destroy the ability of the PHY framework to do that.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/6] drivers: phy: add generic PHY framework
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 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 kis...@ti.com
---
Documentation/ABI/testing/sysfs-class-phy | 15 +
.../devicetree/bindings/phy/phy-bindings.txt | 76 +++
Documentation/phy.txt | 119
MAINTAINERS|7 +
drivers/Kconfig|2 +
drivers/Makefile |2 +
drivers/phy/Kconfig| 13 +
drivers/phy/Makefile |5 +
drivers/phy/phy-core.c | 689
include/linux/phy/phy.h| 237 +++
10 files changed, 1165 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 000..47f17c9
--- /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 kis...@ti.com
+Description:
+ This is a read-only file for getting the label of the phy.
+
+What: /sys/class/phy/phy/phy_bind
+Date: Feb 2013
+KernelVersion: 3.10
+Contact: Kishon Vijay Abraham I kis...@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 000..35696b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
@@ -0,0 +1,76 @@
+This document explains only the dt data binding. For general information about
+PHY subsystem refer Documentation/phy.txt
+
+PHY device node
+===
+
+Optional 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. However
+ in-order to return the correct PHY, the PHY susbsystem
+ requires the first cell always refers to the port.
+
+This property is optional because it is needed only for the case where a
+single IP implements multiple PHYs.
+
+For example:
+
+phys: phy {
+compatible = xxx;
+reg1 = ...;
+reg2 = ...;
+reg3 = ...;
+.
+.
+#phy-cells = 1;
+.
+.
+};
+
+That node describes an IP block that implements 3 different PHYs. In order to
+differentiate between these 3 PHYs, an additonal specifier should be given
+while trying to get a reference to it. (The PHY subsystem assumes the
+specifier is port id).
+
+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
+
+example1:
+phys: phy {
+compatible = xxx;
+reg = ...;
+.
+.
+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. The controller driver can get the appropriate PHY either by using
+devm_of_phy_get/of_phy_get by passing the correct index or by using
+of_phy_get_byname/devm_of_phy_get_byname by passing the names given in
+*phy-names*.
+
+example2:
+phys: phy {
+compatible = xxx;
+reg = ...;
+.
+.
+phys = phys 1;
+.
+.
+};
+
+This node
