Add OF bindings to Micrel PHY
Subsystems (4)
PATCH v2 1/1 Add OF bindings to Micrel PHY
Some boards require custom parameters be passed to the Micrel PHY. Allow these boards to specify custom timing parameters in the device tree node.
Changes since v1:
- removed redefinition of registers and addresses
Sean Cross (1):
net/phy: micrel: Add OF configuration support
.../devicetree/bindings/net/micrel-phy.txt | 20 ++++++++ drivers/net/phy/micrel.c | 50 ++++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/micrel-phy.txt
Signature · 1 line
1.7.9.5
PATCH v2 1/1 net/phy: micrel: Add OF configuration support
Some boards require custom PHY configuration, for example due to trace length differences. Add the ability to configure these registers in order to get the PHY to function on boards that need it.
Because PHYs are auto-detected based on MDIO device IDs, allow PHY configuration to be specified in the parent Ethernet device node if no PHY device node is present.
--- .../devicetree/bindings/net/micrel-phy.txt | 20 ++++++++ drivers/net/phy/micrel.c | 50 ++++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/micrel-phy.txt
Documentation/devicetree/bindings/net/micrel-phy.txt new file +20 -0
@@ -0,0 +1,20 @@ +Micrel KS8737, KSZ8041, KSZ8001, KS8721, KSZ8081, KSZ8091, KSZ8061, KSZ9021, +KSZ9031, Ethernet PHYs, and KSZ8873MLL and KSZ886X Ethernet switches. + +Some boards require special tuning values, particularly when it comes to +clock delays. You can specify clock delay values by adding +micrel-specific properties to an Ethernet OF device node. + +Optional properties: +- micrel,clk-control-pad-skew : Timing offset for the MII clock line +- micrel,rx-data-pad-skew : Timing offset for the RX MII pad +- micrel,tx-data-pad-skew : Timing offset for the TX MII pad + +Example: + &enet { + micrel,clk-control-pad-skew = <0xf0f0>; + micrel,rx-data-pad-skew = <0x0000>; + micrel,tx-data-pad-skew = <0xffff>; + status = "okay"; + }; +
drivers/net/phy/micrel.c +50 -0
74 lines not on this page · raw message
Signature · 1 line
1.7.9.5
RE: [PATCH v2] net/phy: micrel: Add OF configuration support
Quoted 7 lines · Sean Cross
Some boards require custom PHY configuration, for example due to trace length differences. Add the ability to configure these registers in order to get the PHY to function on boards that need it.
Because PHYs are auto-detected based on MDIO device IDs, allow PHY configuration to be specified in the parent Ethernet device node if no PHY device node is present.
Signed-off-by Sean Cross <[email protected]>
Re: [PATCH v2] net/phy: micrel: Add OF configuration support
In reply to Sean CrossQuoted 30 lines · Sean Cross
Some boards require custom PHY configuration, for example due to trace length differences. Add the ability to configure these registers in order to get the PHY to function on boards that need it.
Because PHYs are auto-detected based on MDIO device IDs, allow PHY configuration to be specified in the parent Ethernet device node if no PHY device node is present.
Signed-off-by Sean Cross <[email protected]>--- .../devicetree/bindings/net/micrel-phy.txt | 20 ++++++++ drivers/net/phy/micrel.c | 50 ++++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/micrel-phy.txtDocumentation/devicetree/bindings/net/micrel-phy.txt new file +9
-0@@ -0,0 +1,20 @@ +Micrel KS8737, KSZ8041, KSZ8001, KS8721, KSZ8081, KSZ8091, KSZ8061, KSZ9021, +KSZ9031, Ethernet PHYs, and KSZ8873MLL and KSZ886X Ethernet switches. + +Some boards require special tuning values, particularly when it comes to +clock delays. You can specify clock delay values by adding +micrel-specific properties to an Ethernet OF device node. + +Optional properties: +- micrel,clk-control-pad-skew : Timing offset for the MII clock line
Documentation/devicetree/bindings/net/micrel-phy.txt line 19
Quoted 10 lines
Documentation/devicetree/bindings/net/micrel-phy.txt new file +10
-0+- micrel,rx-data-pad-skew : Timing offset for the RX MII pad +- micrel,tx-data-pad-skew : Timing offset for the TX MII pad + +Example: + &enet { + micrel,clk-control-pad-skew = <0xf0f0>; + micrel,rx-data-pad-skew = <0x0000>; + micrel,tx-data-pad-skew = <0xffff>; + status = "okay"; + };
Given that this patch adds a devicetree binding which I think we now agree that this introduces an ABI I think this needs more thought. Some questions:
- Does binding this also work for MII controllers external to the MAC? Several Marvell SoCs have this situation. MDIO is a bus. With the binding above you assume that all devices on the bus use the same settings
- You directly put the register contents into dt. This assumes that all micrel phys have a compatible register layout. This may be the case now, but what's with future phys?
- The pad skew settings are needed for other phys aswell. It might be worth introducing a binding which could work for say Artheros phys aswell.
Sascha
Signature · 4 lines
Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Re: [PATCH v2] net/phy: micrel: Add OF configuration support
Signature · 2 lines
-- Sean Cross
Quoted 48 lines · Sascha Hauer
Quoted 30 lines · Sean Cross
Some boards require custom PHY configuration, for example due to trace length differences. Add the ability to configure these registers in order to get the PHY to function on boards that need it.
Because PHYs are auto-detected based on MDIO device IDs, allow PHY configuration to be specified in the parent Ethernet device node if no PHY device node is present.
Signed-off-by Sean Cross <[email protected] (mailto:[email protected])>---
.../devicetree/bindings/net/micrel-phy.txt| 20 ++++++++drivers/net/phy/micrel.c| 50 ++++++++++++++++++++2 files changed, 70 insertions(+)create mode 100644
Documentation/devicetree/bindings/net/micrel-phy.txtDocumentation/devicetree/bindings/net/micrel-phy.txt new file +9
-0@@ -0,0 +1,20 @@ +Micrel KS8737, KSZ8041, KSZ8001, KS8721, KSZ8081, KSZ8091, KSZ8061, KSZ9021, +KSZ9031, Ethernet PHYs, and KSZ8873MLL and KSZ886X Ethernet switches. + +Some boards require special tuning values, particularly when it comes to +clock delays. You can specify clock delay values by adding +micrel-specific properties to an Ethernet OF device node. + +Optional properties: +- micrel,clk-control-pad-skew : Timing offset for the MII clock lineDocumentation/devicetree/bindings/net/micrel-phy.txt line 19
Quoted 10 lines
Documentation/devicetree/bindings/net/micrel-phy.txt new file +10
-0+- micrel,rx-data-pad-skew : Timing offset for the RX MII pad +- micrel,tx-data-pad-skew : Timing offset for the TX MII pad + +Example: + &enet { + micrel,clk-control-pad-skew = <0xf0f0>; + micrel,rx-data-pad-skew = <0x0000>; + micrel,tx-data-pad-skew = <0xffff>; + status = "okay"; + };Given that this patch adds a devicetree binding which I think we now agree that this introduces an ABI I think this needs more thought. Some questions:
Quoted 4 lines
- Does binding this also work for MII controllers external to the MAC? Several Marvell SoCs have this situation. MDIO is a bus. With the binding above you assume that all devices on the bus use the same settings
I'm not quite sure what you mean here. The board I'm testing on is based on an i.MX6q, which has a gigabit MAC and uses a Micrel PHY connected via MDIO. I assumed (perhaps inaccurately) that phy_write() would pick the correct PHY. The example binding is for a single Ethernet device, named "enet". If a board had two Ethernet devices, enet1 and enet2, each could have its own micrel definitions. For example:
&enet1 {
micrel,clk-control-pad-skew = <0xf0f0>;
micrel,rx-data-pad-skew = <0x0000>;
micrel,tx-data-pad-skew = <0xffff>;
status = "okay";
};&enet2 {
micrel,clk-control-pad-skew = <0x0000>;
micrel,rx-data-pad-skew = <0x0000>;
micrel,tx-data-pad-skew = <0x0000>;
status = "okay";
};Quoted 3 lines
- You directly put the register contents into dt. This assumes that all micrel phys have a compatible register layout. This may be the case now, but what's with future phys?
This is a very valid point. I assumed that most Micrel PHYs had similar register sets, but upon closer inspection it seems as though the KSZ9021RN is unique in this regard. I should come up with a new function ksz9021_config_init that does this only for that one PHY, and rename the devicetree doc to reflect that.
Quoted 3 lines
- The pad skew settings are needed for other phys aswell. It might be worth introducing a binding which could work for say Artheros phys aswell.
I can't comment on other phys, as I'm not familiar with them. How would such a system work? This certainly seems like the most open-ended of the questions.
I'll limit the scope of the patch to the KSZ9021 family of parts, and submit a v3 patch.
Re: [PATCH v2] net/phy: micrel: Add OF configuration support
Quoted 24 lines · Sean Cross
Quoted 8 lines
Given that this patch adds a devicetree binding which I think we now agree that this introduces an ABI I think this needs more thought. Some questions:
- Does binding this also work for MII controllers external to the MAC? Several Marvell SoCs have this situation. MDIO is a bus. With the binding above you assume that all devices on the bus use the same settings
I'm not quite sure what you mean here. The board I'm testing on is based on an i.MX6q, which has a gigabit MAC and uses a Micrel PHY connected via MDIO. I assumed (perhaps inaccurately) that
phy_write()would pick the correct PHY. The example binding is for a single Ethernet device, named "enet". If a board had two Ethernet devices, enet1 and enet2, each could have its own micrel definitions. For example:&enet1 { micrel,clk-control-pad-skew = <0xf0f0>; micrel,rx-data-pad-skew = <0x0000>; micrel,tx-data-pad-skew = <0xffff>; status = "okay"; };
Quoted 6 lines
&enet2 {
micrel,clk-control-pad-skew = <0x0000>;
micrel,rx-data-pad-skew = <0x0000>;
micrel,tx-data-pad-skew = <0x0000>;
status = "okay";
};Here each enet controller has its own mdio bus, but look for example at Marvell armada-370-db.dts:
mdio {
phy0: ethernet-phy@0 {
reg = <0>;
};
phy1: ethernet-phy@1 {
reg = <1>;
};
};ethernet@70000 {
status = "okay";
phy = <&phy0>;
phy-mode = "rgmii-id";
};ethernet@74000 {
status = "okay";
phy = <&phy1>;
phy-mode = "rgmii-id";
};Here two ethernet controllers share a single mdio bus. This at least requires the binding in the phy node, not the ethernet node.
Quoted 9 lines
Quoted 3 lines
- You directly put the register contents into dt. This assumes that all micrel phys have a compatible register layout. This may be the case now, but what's with future phys?
This is a very valid point. I assumed that most Micrel PHYs had similar register sets, but upon closer inspection it seems as though the KSZ9021RN is unique in this regard. I should come up with a new function
ksz9021_config_initthat does this only for that one PHY, and rename the devicetree doc to reflect that.
Quoted 8 lines
Quoted 4 lines
- The pad skew settings are needed for other phys aswell. It might be
worth introducing a binding which could work for say Artheros phys aswell.I can't comment on other phys, as I'm not familiar with them. How would such a system work? This certainly seems like the most open-ended of the questions.
I think we would need the delay for the different lines in ps, not in hardware register specific values. This would have the additional benefit that we don't have to hardcode the exact phy type.
Sascha
Signature · 4 lines
Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Re: [PATCH v2] net/phy: micrel: Add OF configuration support
Quoted 55 lines · Sascha Hauer
Quoted 26 lines · Sean Cross
Quoted 8 lines
Given that this patch adds a devicetree binding which I think we now agree that this introduces an ABI I think this needs more thought. Some questions:
- Does binding this also work for MII controllers external to the MAC? Several Marvell SoCs have this situation. MDIO is a bus. With the binding above you assume that all devices on the bus use the same settings
I'm not quite sure what you mean here. The board I'm testing on is based on an i.MX6q, which has a gigabit MAC and uses a Micrel PHY connected via MDIO. I assumed (perhaps inaccurately) that
phy_write()would pick the correct PHY. The example binding is for a single Ethernet device, named "enet". If a board had two Ethernet devices, enet1 and enet2, each could have its own micrel definitions. For example:&enet1 {
micrel,clk-control-pad-skew = <0xf0f0>;
micrel,rx-data-pad-skew = <0x0000>;
micrel,tx-data-pad-skew = <0xffff>;
status = "okay";
};Quoted 6 lines
&enet2 {
micrel,clk-control-pad-skew = <0x0000>;
micrel,rx-data-pad-skew = <0x0000>;
micrel,tx-data-pad-skew = <0x0000>;
status = "okay";
};Here each enet controller has its own mdio bus, but look for example at Marvell
armada-370-db.dts:mdio {
phy0: ethernet-phy@0 {
reg = <0>;
};phy1: ethernet-phy@1 {
reg = <1>;
};
};ethernet@70000 {
status = "okay";
phy = <&phy0>;
phy-mode = "rgmii-id";
};
Quoted 8 lines
ethernet@74000 {
status = "okay";
phy = <&phy1>;
phy-mode = "rgmii-id";
};Here two ethernet controllers share a single mdio bus. This at least requires the binding in the phy node, not the ethernet node.
The code handles this case. It will only read from the ethernet node if there is no phy node present:
struct device_node *of_node = dev->of_node;
if (!of_node && dev->parent->of_node)
of_node = dev->parent->of_node;I can add an example in the Documentation to reflect the fact that if a phy has a device tree node, you should put the skew values there.
Quoted 13 lines
Quoted 10 lines
Quoted 4 lines
- The pad skew settings are needed for other phys aswell. It might be
worth introducing a binding which could work for say Artheros phys aswell.I can't comment on other phys, as I'm not familiar with them. How would such a system work? This certainly seems like the most open-ended of the questions.
Quoted 3 lines
I think we would need the delay for the different lines in ps, not in hardware register specific values. This would have the additional benefit that we don't have to hardcode the exact phy type.
Ah, I see. Your proposal would be to break micrel,clk-control-pad-skew into four different values, and rename them to something like:
rxc-pad-skew
rx-ctl-pad-skew
txc-pad-skew
tx-ctl-pad-skew
Then the device tree entries would look something like:
rxc-pad-skew = <3000>; // picoseconds rx-ctl-pad-skew <0>; // picoseconds txc-pad-skew = <3000>; // picoseconds tx-ctl-pad-skew = <0>; // picoseconds
How would this affect the Atheros driver? Are there other names that should be preferred?