Skip to contentSkip to conversation

Add OF bindings to Micrel ksz9021 PHY

PATCH Superseded Sean Cross 4 participants 10 messages Aug 5, 2013 – Aug 20, 2013

Subsystems (4)
Networking Network drivers Device tree bindings Ethernet PHY

v5 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Sean Cross

author
#0 lore raw

PATCH v5 1/1 Add OF bindings to Micrel ksz9021 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 v4:
- Add an example of parameters on an mdio bus
- Specify values in picoseconds rather than as register values

v4 was accidentally unchanged from v3

Changes since v2:
- limited the scope to just ksz9021 rather than all PHYs

Changes since v1:
- removed redefinition of registers and addresses

Sean Cross (1):
net/phy: micrel: Add OF configuration support for ksz9021

 .../devicetree/bindings/net/micrel-ksz9021.txt     |   49 ++++++++++
 drivers/net/phy/micrel.c                           |  101 +++++++++++++++++++-
 2 files changed, 149 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/net/micrel-ksz9021.txt
Signature · 1 line
1.7.9.5
author
#1 lore raw

PATCH v5 1/1 net/phy: micrel: Add OF configuration support for ksz9021

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-ksz9021.txt     |   49 ++++++++++
 drivers/net/phy/micrel.c                           |  101 +++++++++++++++++++-
 2 files changed, 149 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/net/micrel-ksz9021.txt
Documentation/devicetree/bindings/net/micrel-ksz9021.txt new file +49 -0
new file mode 100644
index 0000000..338a7e2
@@ -0,0 +1,49 @@
+Micrel KSZ9021 Gigabit Ethernet PHY
+
+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.
+
+All skew control options are specified in picoseconds.  The minimum
+value is 0, and the maximum value is 3000.
+
+Optional properties:
+ - rxc-skew : Skew control of RXC pad
+ - rxdv-skew : Skew control of RX CTL pad
+ - txc-skew : Skew control of TXC pad
+ - txen-skew : Skew control of TX_CTL pad
+ - rxd0-skew : Skew control of RX data 0 pad
+ - rxd1-skew : Skew control of RX data 1 pad
+ - rxd2-skew : Skew control of RX data 2 pad
+ - rxd3-skew : Skew control of RX data 3 pad
+ - txd0-skew : Skew control of TX data 0 pad
+ - txd1-skew : Skew control of TX data 1 pad
+ - txd2-skew : Skew control of TX data 2 pad
+ - txd3-skew : Skew control of TX data 3 pad
+
+Examples:
+
+	/* Attach to an Ethernet device with autodetected PHY */
+	&enet {
+		rxc-skew = <3000>; // picoseconds
+		rxdv-skew = <0>;   // picoseconds
+		txc-skew = <3000>; // picoseconds
+		txen-skew = <0>;   // picoseconds
+		status = "okay";
+	};
+
+	/* Attach to an explicitly-specified PHY */
+	mdio {
+		phy0: ethernet-phy@0 {
+			rxc-skew = <3000>; // picoseconds
+			rxdv-skew = <0>;   // picoseconds
+			txc-skew = <3000>; // picoseconds
+			txen-skew = <0>;   // picoseconds
+			reg = <0>;
+		};
+	};
+	ethernet@70000 {
+		status = "okay";
+		phy = <&phy0>;
+		phy-mode = "rgmii-id";
+	};
drivers/net/phy/micrel.c +100 -1

131 lines not on this page · raw message

Signature · 1 line
1.7.9.5
#2 lore raw

RE: [PATCH v5] net/phy: micrel: Add OF configuration support for ksz9021

From: Sean Cross [mailto:[email protected]]
Data: Monday, August 05, 2013 3:04 PM

Quoted 43 lines

To: Sascha Hauer; Duan Fugang-B38611; [email protected]; [email protected]
Cc: David Miller; [email protected]; Steven Rostedt; Sean Cross
Subject: [PATCH v5] net/phy: micrel: Add OF configuration support for ksz9021

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]>

---

drivers/net/phy/micrel.c +27 -0
+static int ksz9021_load_values_from_of(struct phy_device *phydev,
+				       struct device_node *of_node, u16 reg,
+				       char *field1, char *field2,
+				       char *field3, char *field4)
+{
+	int val1 = -1;
+	int val2 = -2;
+	int val3 = -3;
+	int val4 = -4;
+	int newval;
+	int matches = 0;
+
+	if (!of_property_read_u32(of_node, field1, &val1))
+		matches++;
+
+	if (!of_property_read_u32(of_node, field2, &val2))
+		matches++;
+
+	if (!of_property_read_u32(of_node, field3, &val3))
+		matches++;
+
+	if (!of_property_read_u32(of_node, field4, &val4))
+		matches++;
+
+	if (!matches)
+		return 0;
+

drivers/net/phy/micrel.c static int ks8051_config_init(struct phy_device *phydev) · line 207

Quoted 10 lines
drivers/net/phy/micrel.c +10 -0
+	if (matches < 4)
+		newval = kszphy_extended_read(phydev, reg);
+	else
+		newval = 0;
+
+	if (val1 != -1)
+		newval = ((newval & 0xfff0) | ((val1/200)&0xf) << 0);
+
+	if (val2 != -1)
+		newval = ((newval & 0xff0f) | ((val2/200)&0xf) << 4);

Must be : Newval |= ...

drivers/net/phy/micrel.c static int ks8051_config_init(struct phy_device *phydev) · line 210

Quoted 3 lines
drivers/net/phy/micrel.c +3 -0
+
+	if (val3 != -1)
+		newval = ((newval & 0xf0ff) | ((val3/200)&0xf) << 8);

Must be : Newval |= ...

drivers/net/phy/micrel.c static int ks8051_config_init(struct phy_device *phydev) · line 213

Quoted 3 lines
drivers/net/phy/micrel.c +3 -0
+
+	if (val4 != -1)
+		newval = ((newval & 0x0fff) | ((val4/200)&0xf) << 12);

Must be : Newval |= ...

Quoted 2 lines
(quoted diff) +2 -0
+
+	return kszphy_extended_write(phydev, reg, newval); }
author
#3 lore raw

Re: [PATCH v5] net/phy: micrel: Add OF configuration support for ksz9021

Signature · 2 lines
-- 
Sean Cross
Quoted 63 lines · On Monday, August 5, 2013 at 3:41 PM, Duan Fugang-B38611 wrote:

From: Sean Cross [mailto:[email protected]]
Data: Monday, August 05, 2013 3:04 PM

Quoted 43 lines

To: Sascha Hauer; Duan Fugang-B38611; [email protected] (mailto:[email protected]); [email protected] (mailto:[email protected])
Cc: David Miller; [email protected] (mailto:[email protected]); Steven Rostedt; Sean Cross
Subject: [PATCH v5] net/phy: micrel: Add OF configuration support for
ksz9021

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])>

---

drivers/net/phy/micrel.c +27 -0
+static int ksz9021_load_values_from_of(struct phy_device *phydev,
+ struct device_node *of_node, u16 reg,
+ char *field1, char *field2,
+ char *field3, char *field4)
+{
+ int val1 = -1;
+ int val2 = -2;
+ int val3 = -3;
+ int val4 = -4;
+ int newval;
+ int matches = 0;
+
+ if (!of_property_read_u32(of_node, field1, &val1))
+ matches++;
+
+ if (!of_property_read_u32(of_node, field2, &val2))
+ matches++;
+
+ if (!of_property_read_u32(of_node, field3, &val3))
+ matches++;
+
+ if (!of_property_read_u32(of_node, field4, &val4))
+ matches++;
+
+ if (!matches)
+ return 0;
+

drivers/net/phy/micrel.c static int ks8051_config_init(struct phy_device *phydev) · line 207

Quoted 10 lines
drivers/net/phy/micrel.c +10 -0
+ if (matches < 4)
+ newval = kszphy_extended_read(phydev, reg);
+ else
+ newval = 0;
+
+ if (val1 != -1)
+ newval = ((newval & 0xfff0) | ((val1/200)&0xf) << 0);
+
+ if (val2 != -1)
+ newval = ((newval & 0xff0f) | ((val2/200)&0xf) << 4);

Must be : Newval |= ...

Quoted 3 lines

+
+ if (val3 != -1)
+ newval = ((newval & 0xf0ff) | ((val3/200)&0xf) << 8);

Quoted 10 lines

Must be : Newval |= ...

Quoted 3 lines

+
+ if (val4 != -1)
+ newval = ((newval & 0x0fff) | ((val4/200)&0xf) << 12);

Must be : Newval |= ...

Quoted 2 lines

+
+ return kszphy_extended_write(phydev, reg, newval); }

If all four fields are updated, then newval will start out as 0. If only a few fields are updated, then newval will start out with the previous value.

If a given field is mentioned in the device tree file, then its value will change to something other than -1. If the value remains at -1, then that particular field could not be found, and it should not be modified.

#4 lore raw

Re: [PATCH v5] net/phy: micrel: Add OF configuration support for ksz9021

In reply to Sean Cross
Quoted 147 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-ksz9021.txt     |   49 ++++++++++
 drivers/net/phy/micrel.c                           |  101 +++++++++++++++++++-
 2 files changed, 149 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/net/micrel-ksz9021.txt
Documentation/devicetree/bindings/net/micrel-ksz9021.txt new file +49 -0
new file mode 100644
index 0000000..338a7e2
@@ -0,0 +1,49 @@
+Micrel KSZ9021 Gigabit Ethernet PHY
+
+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.
+
+All skew control options are specified in picoseconds.  The minimum
+value is 0, and the maximum value is 3000.
+
+Optional properties:
+ - rxc-skew : Skew control of RXC pad
+ - rxdv-skew : Skew control of RX CTL pad
+ - txc-skew : Skew control of TXC pad
+ - txen-skew : Skew control of TX_CTL pad
+ - rxd0-skew : Skew control of RX data 0 pad
+ - rxd1-skew : Skew control of RX data 1 pad
+ - rxd2-skew : Skew control of RX data 2 pad
+ - rxd3-skew : Skew control of RX data 3 pad
+ - txd0-skew : Skew control of TX data 0 pad
+ - txd1-skew : Skew control of TX data 1 pad
+ - txd2-skew : Skew control of TX data 2 pad
+ - txd3-skew : Skew control of TX data 3 pad
+
+Examples:
+
+	/* Attach to an Ethernet device with autodetected PHY */
+	&enet {
+		rxc-skew = <3000>; // picoseconds
+		rxdv-skew = <0>;   // picoseconds
+		txc-skew = <3000>; // picoseconds
+		txen-skew = <0>;   // picoseconds
+		status = "okay";
+	};
+
+	/* Attach to an explicitly-specified PHY */
+	mdio {
+		phy0: ethernet-phy@0 {
+			rxc-skew = <3000>; // picoseconds
+			rxdv-skew = <0>;   // picoseconds
+			txc-skew = <3000>; // picoseconds
+			txen-skew = <0>;   // picoseconds
+			reg = <0>;
+		};
+	};
+	ethernet@70000 {
+		status = "okay";
+		phy = <&phy0>;
+		phy-mode = "rgmii-id";
+	};
drivers/net/phy/micrel.c +48 -0
index 2510435..3e60ed0 100644
@@ -25,6 +25,7 @@
 #include <linux/module.h>
 #include <linux/phy.h>
 #include <linux/micrel_phy.h>
+#include <linux/of.h>
 
 /* Operation Mode Strap Override */
 #define MII_KSZPHY_OMSO				0x16

@@ -53,6 +54,18 @@
 #define KS8737_CTRL_INT_ACTIVE_HIGH		(1 << 14)
 #define KSZ8051_RMII_50MHZ_CLK			(1 << 7)
 
+/* Write/read to/from extended registers */
+#define MII_KSZPHY_EXTREG                       0x0b
+#define KSZPHY_EXTREG_WRITE                     0x8000
+
+#define MII_KSZPHY_EXTREG_WRITE                 0x0c
+#define MII_KSZPHY_EXTREG_READ                  0x0d
+
+/* Extended registers */
+#define MII_KSZPHY_CLK_CONTROL_PAD_SKEW         0x104
+#define MII_KSZPHY_RX_DATA_PAD_SKEW             0x105
+#define MII_KSZPHY_TX_DATA_PAD_SKEW             0x106
+
 static int ksz_config_flags(struct phy_device *phydev)
 {
 	int regval;

@@ -65,6 +78,20 @@ static int ksz_config_flags(struct phy_device *phydev)
 	return 0;
 }
 
+static int kszphy_extended_write(struct phy_device *phydev,
+                                 u32 regnum, u16 val)
+{
+	phy_write(phydev, MII_KSZPHY_EXTREG, KSZPHY_EXTREG_WRITE | regnum);
+	return phy_write(phydev, MII_KSZPHY_EXTREG_WRITE, val);
+}
+
+static int kszphy_extended_read(struct phy_device *phydev,
+                                 u32 regnum)
+{
+	phy_write(phydev, MII_KSZPHY_EXTREG, regnum);
+	return phy_read(phydev, MII_KSZPHY_EXTREG_READ);
+}
+
 static int kszphy_ack_interrupt(struct phy_device *phydev)
 {
 	/* bit[7..0] int status, which is a read and clear register. */

@@ -141,6 +168,78 @@ static int ks8051_config_init(struct phy_device *phydev)
 	return rc < 0 ? rc : 0;
 }
 
+static int ksz9021_load_values_from_of(struct phy_device *phydev,
+				       struct device_node *of_node, u16 reg,
+				       char *field1, char *field2,
+				       char *field3, char *field4)
+{
+	int val1 = -1;
+	int val2 = -2;
+	int val3 = -3;
+	int val4 = -4;
+	int newval;
+	int matches = 0;
+
+	if (!of_property_read_u32(of_node, field1, &val1))
+		matches++;
+
+	if (!of_property_read_u32(of_node, field2, &val2))
+		matches++;
+
+	if (!of_property_read_u32(of_node, field3, &val3))
+		matches++;
+

drivers/net/phy/micrel.c static int ks8051_config_init(struct phy_device *phydev) · line 201

Quoted 10 lines
drivers/net/phy/micrel.c +10 -0
+	if (!of_property_read_u32(of_node, field4, &val4))
+		matches++;
+
+	if (!matches)
+		return 0;
+
+	if (matches < 4)
+		newval = kszphy_extended_read(phydev, reg);
+	else
+		newval = 0;

Just initialize newval with the reset default of this register. It will make this function easier. Also this two step read from dt and evaluate afterwards seems unnecessary.

Quoted 27 lines
drivers/net/phy/micrel.c +27 -0
+
+	if (val1 != -1)
+		newval = ((newval & 0xfff0) | ((val1/200)&0xf) << 0);
+
+	if (val2 != -1)
+		newval = ((newval & 0xff0f) | ((val2/200)&0xf) << 4);
+
+	if (val3 != -1)
+		newval = ((newval & 0xf0ff) | ((val3/200)&0xf) << 8);
+
+	if (val4 != -1)
+		newval = ((newval & 0x0fff) | ((val4/200)&0xf) << 12);
+
+	return kszphy_extended_write(phydev, reg, newval);
+}
+
+static int ksz9021_config_init(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->dev;
+	struct device_node *of_node = dev->of_node;
+
+	if (!of_node && dev->parent->of_node)
+		of_node = dev->parent->of_node;
+
+	if (of_node) {
+		ksz9021_load_values_from_of(phydev, of_node,
+				    MII_KSZPHY_CLK_CONTROL_PAD_SKEW,

drivers/net/phy/micrel.c static int ks8051_config_init(struct phy_device *phydev) · line 238

Quoted 10 lines
drivers/net/phy/micrel.c +10 -0
+				    "txen-skew", "txc-skew",
+				    "rxdv-skew", "rxc-skew");
+		ksz9021_load_values_from_of(phydev, of_node,
+				    MII_KSZPHY_RX_DATA_PAD_SKEW,
+				    "rxd0-skew", "rxd1-skew",
+				    "rxd2-skew", "rxd3-skew");
+		ksz9021_load_values_from_of(phydev, of_node,
+				    MII_KSZPHY_TX_DATA_PAD_SKEW,
+				    "txd0-skew", "txd1-skew",
+				    "txd2-skew", "txd3-skew");

Are you sure this register exists? It's not mentioned in my datasheet, only the first two are.

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 |
author
#5 lore raw

Re: [PATCH v5] net/phy: micrel: Add OF configuration support for ksz9021

Signature · 2 lines
-- 
Sean Cross
Quoted 156 lines · Sascha Hauer
Quoted 155 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-ksz9021.txt | 49 ++++++++++ drivers/net/phy/micrel.c | 101 +++++++++++++++++++-

2 files changed, 149 insertions(+), 1 deletion(-)

create mode 100644 Documentation/devicetree/bindings/net/micrel-ksz9021.txt

Documentation/devicetree/bindings/net/micrel-ksz9021.txt new file +49 -0
new file mode 100644
index 0000000..338a7e2
@@ -0,0 +1,49 @@
+Micrel KSZ9021 Gigabit Ethernet PHY
+
+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.
+
+All skew control options are specified in picoseconds. The minimum
+value is 0, and the maximum value is 3000.
+
+Optional properties:
+ - rxc-skew : Skew control of RXC pad
+ - rxdv-skew : Skew control of RX CTL pad
+ - txc-skew : Skew control of TXC pad
+ - txen-skew : Skew control of TX_CTL pad
+ - rxd0-skew : Skew control of RX data 0 pad
+ - rxd1-skew : Skew control of RX data 1 pad
+ - rxd2-skew : Skew control of RX data 2 pad
+ - rxd3-skew : Skew control of RX data 3 pad
+ - txd0-skew : Skew control of TX data 0 pad
+ - txd1-skew : Skew control of TX data 1 pad
+ - txd2-skew : Skew control of TX data 2 pad
+ - txd3-skew : Skew control of TX data 3 pad
+
+Examples:
+
+ /* Attach to an Ethernet device with autodetected PHY */
+ &enet {
+ rxc-skew = <3000>; // picoseconds
+ rxdv-skew = <0>; // picoseconds
+ txc-skew = <3000>; // picoseconds
+ txen-skew = <0>; // picoseconds
+ status = "okay";
+ };
+
+ /* Attach to an explicitly-specified PHY */
+ mdio {
+ phy0: ethernet-phy@0 {
+ rxc-skew = <3000>; // picoseconds
+ rxdv-skew = <0>; // picoseconds
+ txc-skew = <3000>; // picoseconds
+ txen-skew = <0>; // picoseconds
+ reg = <0>;
+ };
+ };
+ ethernet@70000 {
+ status = "okay";
+ phy = <&phy0>;
+ phy-mode = "rgmii-id";
+ };
drivers/net/phy/micrel.c +56 -0
index 2510435..3e60ed0 100644
@@ -25,6 +25,7 @@
 #include <linux/module.h>
 #include <linux/phy.h>
 #include <linux/micrel_phy.h>
+#include <linux/of.h>
 
 /* Operation Mode Strap Override */
 #define MII_KSZPHY_OMSO 0x16

@@ -53,6 +54,18 @@
 #define KS8737_CTRL_INT_ACTIVE_HIGH (1 << 14)
 #define KSZ8051_RMII_50MHZ_CLK (1 << 7)
 
+/* Write/read to/from extended registers */
+#define MII_KSZPHY_EXTREG 0x0b
+#define KSZPHY_EXTREG_WRITE 0x8000
+
+#define MII_KSZPHY_EXTREG_WRITE 0x0c
+#define MII_KSZPHY_EXTREG_READ 0x0d
+
+/* Extended registers */
+#define MII_KSZPHY_CLK_CONTROL_PAD_SKEW 0x104
+#define MII_KSZPHY_RX_DATA_PAD_SKEW 0x105
+#define MII_KSZPHY_TX_DATA_PAD_SKEW 0x106
+
 static int ksz_config_flags(struct phy_device *phydev)
 {
 int regval;

@@ -65,6 +78,20 @@ static int ksz_config_flags(struct phy_device *phydev)
 return 0;
 }
 
+static int kszphy_extended_write(struct phy_device *phydev,
+ u32 regnum, u16 val)
+{
+ phy_write(phydev, MII_KSZPHY_EXTREG, KSZPHY_EXTREG_WRITE | regnum);
+ return phy_write(phydev, MII_KSZPHY_EXTREG_WRITE, val);
+}
+
+static int kszphy_extended_read(struct phy_device *phydev,
+ u32 regnum)
+{
+ phy_write(phydev, MII_KSZPHY_EXTREG, regnum);
+ return phy_read(phydev, MII_KSZPHY_EXTREG_READ);
+}
+
 static int kszphy_ack_interrupt(struct phy_device *phydev)
 {
 /* bit[7..0] int status, which is a read and clear register. */

@@ -141,6 +168,78 @@ static int ks8051_config_init(struct phy_device *phydev)
 return rc < 0 ? rc : 0;
 }
 
+static int ksz9021_load_values_from_of(struct phy_device *phydev,
+ struct device_node *of_node, u16 reg,
+ char *field1, char *field2,
+ char *field3, char *field4)
+{
+ int val1 = -1;
+ int val2 = -2;
+ int val3 = -3;
+ int val4 = -4;
+ int newval;
+ int matches = 0;
+
+ if (!of_property_read_u32(of_node, field1, &val1))
+ matches++;
+
+ if (!of_property_read_u32(of_node, field2, &val2))
+ matches++;
+
+ if (!of_property_read_u32(of_node, field3, &val3))
+ matches++;
+
+ if (!of_property_read_u32(of_node, field4, &val4))
+ matches++;
+
+ if (!matches)
+ return 0;
+
+ if (matches < 4)
+ newval = kszphy_extended_read(phydev, reg);
Quoted 8 lines

drivers/net/phy/micrel.c static int ks8051_config_init(struct phy_device *phydev) · line 201

Quoted 2 lines
drivers/net/phy/micrel.c +2 -0
+ else
+ newval = 0;

Just initialize newval with the reset default of this register. It will make this function easier. Also this two step read from dt and evaluate afterwards seems unnecessary.

The documentation seems inaccurate here. My datasheet says it defaults to 0x0111, but when I check it for myself it comes out 0x7777. Which value do you think I should take?

Quoted 34 lines
Quoted 34 lines
drivers/net/phy/micrel.c +34 -0
+
+ if (val1 != -1)
+ newval = ((newval & 0xfff0) | ((val1/200)&0xf) << 0);
+
+ if (val2 != -1)
+ newval = ((newval & 0xff0f) | ((val2/200)&0xf) << 4);
+
+ if (val3 != -1)
+ newval = ((newval & 0xf0ff) | ((val3/200)&0xf) << 8);
+
+ if (val4 != -1)
+ newval = ((newval & 0x0fff) | ((val4/200)&0xf) << 12);
+
+ return kszphy_extended_write(phydev, reg, newval);
+}
+
+static int ksz9021_config_init(struct phy_device *phydev)
+{
+ struct device *dev = &phydev->dev;
+ struct device_node *of_node = dev->of_node;
+
+ if (!of_node && dev->parent->of_node)
+ of_node = dev->parent->of_node;
+
+ if (of_node) {
+ ksz9021_load_values_from_of(phydev, of_node,
+ MII_KSZPHY_CLK_CONTROL_PAD_SKEW,
+ "txen-skew", "txc-skew",
+ "rxdv-skew", "rxc-skew");
+ ksz9021_load_values_from_of(phydev, of_node,
+ MII_KSZPHY_RX_DATA_PAD_SKEW,
+ "rxd0-skew", "rxd1-skew",
+ "rxd2-skew", "rxd3-skew");
+ ksz9021_load_values_from_of(phydev, of_node,
Quoted 9 lines

drivers/net/phy/micrel.c static int ks8051_config_init(struct phy_device *phydev) · line 238

Quoted 3 lines
drivers/net/phy/micrel.c +3 -0
+ MII_KSZPHY_TX_DATA_PAD_SKEW,
+ "txd0-skew", "txd1-skew",
+ "txd2-skew", "txd3-skew");

Are you sure this register exists? It's not mentioned in my datasheet, only the first two are.

I'm very sure the register exists. There's a hole in the documentation where the TX control register should exist. Furthermore, we measured that changing values in this register adjusted the TX skew on the RG-MII wires, and on our board at least, adjusting these values yields increased performance.

author
#6 lore raw

Re: [PATCH v5] net/phy: micrel: Add OF configuration support for ksz9021

Quoted 220 lines · Sean Cross
Quoted 163 lines · Sascha Hauer
Quoted 147 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-ksz9021.txt | 49 ++++++++++ drivers/net/phy/micrel.c | 101 +++++++++++++++++++-

2 files changed, 149 insertions(+), 1 deletion(-)

create mode 100644 Documentation/devicetree/bindings/net/micrel-ksz9021.txt

Documentation/devicetree/bindings/net/micrel-ksz9021.txt new file +49 -0
new file mode 100644
index 0000000..338a7e2
@@ -0,0 +1,49 @@
+Micrel KSZ9021 Gigabit Ethernet PHY
+
+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.
+
+All skew control options are specified in picoseconds. The minimum
+value is 0, and the maximum value is 3000.
+
+Optional properties:
+ - rxc-skew : Skew control of RXC pad
+ - rxdv-skew : Skew control of RX CTL pad
+ - txc-skew : Skew control of TXC pad
+ - txen-skew : Skew control of TX_CTL pad
+ - rxd0-skew : Skew control of RX data 0 pad
+ - rxd1-skew : Skew control of RX data 1 pad
+ - rxd2-skew : Skew control of RX data 2 pad
+ - rxd3-skew : Skew control of RX data 3 pad
+ - txd0-skew : Skew control of TX data 0 pad
+ - txd1-skew : Skew control of TX data 1 pad
+ - txd2-skew : Skew control of TX data 2 pad
+ - txd3-skew : Skew control of TX data 3 pad
+
+Examples:
+
+ /* Attach to an Ethernet device with autodetected PHY */
+ &enet {
+ rxc-skew = <3000>; // picoseconds
+ rxdv-skew = <0>; // picoseconds
+ txc-skew = <3000>; // picoseconds
+ txen-skew = <0>; // picoseconds
+ status = "okay";
+ };
+
+ /* Attach to an explicitly-specified PHY */
+ mdio {
+ phy0: ethernet-phy@0 {
+ rxc-skew = <3000>; // picoseconds
+ rxdv-skew = <0>; // picoseconds
+ txc-skew = <3000>; // picoseconds
+ txen-skew = <0>; // picoseconds
+ reg = <0>;
+ };
+ };
+ ethernet@70000 {
+ status = "okay";
+ phy = <&phy0>;
+ phy-mode = "rgmii-id";
+ };
drivers/net/phy/micrel.c +48 -0
index 2510435..3e60ed0 100644
@@ -25,6 +25,7 @@
 #include <linux/module.h>
 #include <linux/phy.h>
 #include <linux/micrel_phy.h>
+#include <linux/of.h>
 
 /* Operation Mode Strap Override */
 #define MII_KSZPHY_OMSO 0x16

@@ -53,6 +54,18 @@
 #define KS8737_CTRL_INT_ACTIVE_HIGH (1 << 14)
 #define KSZ8051_RMII_50MHZ_CLK (1 << 7)
 
+/* Write/read to/from extended registers */
+#define MII_KSZPHY_EXTREG 0x0b
+#define KSZPHY_EXTREG_WRITE 0x8000
+
+#define MII_KSZPHY_EXTREG_WRITE 0x0c
+#define MII_KSZPHY_EXTREG_READ 0x0d
+
+/* Extended registers */
+#define MII_KSZPHY_CLK_CONTROL_PAD_SKEW 0x104
+#define MII_KSZPHY_RX_DATA_PAD_SKEW 0x105
+#define MII_KSZPHY_TX_DATA_PAD_SKEW 0x106
+
 static int ksz_config_flags(struct phy_device *phydev)
 {
 int regval;

@@ -65,6 +78,20 @@ static int ksz_config_flags(struct phy_device *phydev)
 return 0;
 }
 
+static int kszphy_extended_write(struct phy_device *phydev,
+ u32 regnum, u16 val)
+{
+ phy_write(phydev, MII_KSZPHY_EXTREG, KSZPHY_EXTREG_WRITE | regnum);
+ return phy_write(phydev, MII_KSZPHY_EXTREG_WRITE, val);
+}
+
+static int kszphy_extended_read(struct phy_device *phydev,
+ u32 regnum)
+{
+ phy_write(phydev, MII_KSZPHY_EXTREG, regnum);
+ return phy_read(phydev, MII_KSZPHY_EXTREG_READ);
+}
+
 static int kszphy_ack_interrupt(struct phy_device *phydev)
 {
 /* bit[7..0] int status, which is a read and clear register. */

@@ -141,6 +168,78 @@ static int ks8051_config_init(struct phy_device *phydev)
 return rc < 0 ? rc : 0;
 }
 
+static int ksz9021_load_values_from_of(struct phy_device *phydev,
+ struct device_node *of_node, u16 reg,
+ char *field1, char *field2,
+ char *field3, char *field4)
+{
+ int val1 = -1;
+ int val2 = -2;
+ int val3 = -3;
+ int val4 = -4;
+ int newval;
+ int matches = 0;
+
+ if (!of_property_read_u32(of_node, field1, &val1))
+ matches++;
+
+ if (!of_property_read_u32(of_node, field2, &val2))
+ matches++;
+
+ if (!of_property_read_u32(of_node, field3, &val3))
+ matches++;
+

drivers/net/phy/micrel.c static int ks8051_config_init(struct phy_device *phydev) · line 201

Quoted 10 lines
drivers/net/phy/micrel.c +10 -0
+ if (!of_property_read_u32(of_node, field4, &val4))
+ matches++;
+
+ if (!matches)
+ return 0;
+
+ if (matches < 4)
+ newval = kszphy_extended_read(phydev, reg);
+ else
+ newval = 0;
Quoted 3 lines

Just initialize newval with the reset default of this register. It will make this function easier. Also this two step read from dt and evaluate afterwards seems unnecessary.

The documentation seems inaccurate here. My datasheet says it defaults to 0x0111, but when I check it for myself it comes out 0x7777. Which value do you think I should take?

Quoted 36 lines
Quoted 36 lines
drivers/net/phy/micrel.c +36 -0
+
+ if (val1 != -1)
+ newval = ((newval & 0xfff0) | ((val1/200)&0xf) << 0);
+
+ if (val2 != -1)
+ newval = ((newval & 0xff0f) | ((val2/200)&0xf) << 4);
+
+ if (val3 != -1)
+ newval = ((newval & 0xf0ff) | ((val3/200)&0xf) << 8);
+
+ if (val4 != -1)
+ newval = ((newval & 0x0fff) | ((val4/200)&0xf) << 12);
+
+ return kszphy_extended_write(phydev, reg, newval);
+}
+
+static int ksz9021_config_init(struct phy_device *phydev)
+{
+ struct device *dev = &phydev->dev;
+ struct device_node *of_node = dev->of_node;
+
+ if (!of_node && dev->parent->of_node)
+ of_node = dev->parent->of_node;
+
+ if (of_node) {
+ ksz9021_load_values_from_of(phydev, of_node,
+ MII_KSZPHY_CLK_CONTROL_PAD_SKEW,
+ "txen-skew", "txc-skew",
+ "rxdv-skew", "rxc-skew");
+ ksz9021_load_values_from_of(phydev, of_node,
+ MII_KSZPHY_RX_DATA_PAD_SKEW,
+ "rxd0-skew", "rxd1-skew",
+ "rxd2-skew", "rxd3-skew");
+ ksz9021_load_values_from_of(phydev, of_node,
+ MII_KSZPHY_TX_DATA_PAD_SKEW,
+ "txd0-skew", "txd1-skew",
Quoted 8 lines

drivers/net/phy/micrel.c static int ks8051_config_init(struct phy_device *phydev) · line 238

Quoted 1 line
drivers/net/phy/micrel.c +1 -0
+ "txd2-skew", "txd3-skew");

Are you sure this register exists? It's not mentioned in my datasheet, only the first two are.

Quoted 1 line

I'm very sure the register exists. There's a hole in the documentation where the TX control register should exist. Furthermore, we measured that changing values in this register adjusted the TX skew on the RG-MII wires, and on our board at least, adjusting these values yields increased performance.

There hasn't been any response to this patch in two weeks. Can someone please merge this patch or indicate what's wrong with it?

Reviewed-by
#7 lore raw

RE: [PATCH v5] net/phy: micrel: Add OF configuration support for ksz9021

From: Sean Cross [mailto:[email protected]]
Data: Tuesday, August 20, 2013 4:35 PM

Quoted 9 lines

To: Sascha Hauer
Cc: Duan Fugang-B38611; [email protected]; [email protected]; David Miller; [email protected]; Steven Rostedt
Subject: Re: [PATCH v5] net/phy: micrel: Add OF configuration support for ksz9021

Quoted 16 lines
Quoted 14 lines · Sean Cross
Quoted 11 lines · Sascha Hauer
Quoted 10 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])

Reviewed-by Fugang Duan <[email protected]>

Do somebody have other suggestion for the patch ?

Quoted 186 lines
Quoted 135 lines
Quoted 129 lines
Quoted 114 lines

---
.../devicetree/bindings/net/micrel-ksz9021.txt | 49 ++++++++++ drivers/net/phy/micrel.c | 101 +++++++++++++++++++-

2 files changed, 149 insertions(+), 1 deletion(-) create mode

100644 Documentation/devicetree/bindings/net/micrel-ksz9021.txt

diff --git a/Documentation/devicetree/bindings/net/micrel-ksz9021.txt b/Documentation/devicetree/bindings/net/micrel-ksz9021.txt new file mode 100644
index 0000000..338a7e2

Documentation/devicetree/bindings/net/micrel-ksz9021.txt new file +40 -0
@@ -0,0 +1,49 @@
+Micrel KSZ9021 Gigabit Ethernet PHY
+
+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.
+
+All skew control options are specified in picoseconds. The
+minimum value is 0, and the maximum value is 3000.
+
+Optional properties:
+ - rxc-skew : Skew control of RXC pad
+ - rxdv-skew : Skew control of RX CTL pad
+ - txc-skew : Skew control of TXC pad
+ - txen-skew : Skew control of TX_CTL pad
+ - rxd0-skew : Skew control of RX data 0 pad
+ - rxd1-skew : Skew control of RX data 1 pad
+ - rxd2-skew : Skew control of RX data 2 pad
+ - rxd3-skew : Skew control of RX data 3 pad
+ - txd0-skew : Skew control of TX data 0 pad
+ - txd1-skew : Skew control of TX data 1 pad
+ - txd2-skew : Skew control of TX data 2 pad
+ - txd3-skew : Skew control of TX data 3 pad
+
+Examples:
+
+ /* Attach to an Ethernet device with autodetected PHY */ &enet {
+ rxc-skew = <3000>; // picoseconds rxdv-skew = <0>; //
+ picoseconds txc-skew = <3000>; // picoseconds txen-skew = <0>;
+ // picoseconds status = "okay"; };
+
+ /* Attach to an explicitly-specified PHY */ mdio {
+ phy0: ethernet-phy@0 {
+ rxc-skew = <3000>; // picoseconds rxdv-skew = <0>; //
+ picoseconds txc-skew = <3000>; // picoseconds txen-skew = <0>;
+ // picoseconds reg = <0>; }; };
+ ethernet@70000 {
+ status = "okay";
+ phy = <&phy0>;
+ phy-mode = "rgmii-id";
+ };
drivers/net/phy/micrel.c +12 -0
index 2510435..3e60ed0 100644
@@ -25,6 +25,7 @@
 #include <linux/module.h>
 #include <linux/phy.h>
 #include <linux/micrel_phy.h>
+#include <linux/of.h>
 
 /* Operation Mode Strap Override */ #define MII_KSZPHY_OMSO 0x16

@@ -53,6 +54,18 @@ #define KS8737_CTRL_INT_ACTIVE_HIGH (1 << 14)
 #define KSZ8051_RMII_50MHZ_CLK (1 << 7)
 
+/* Write/read to/from extended registers */ #define
+MII_KSZPHY_EXTREG 0x0b #define KSZPHY_EXTREG_WRITE 0x8000
+
+#define MII_KSZPHY_EXTREG_WRITE 0x0c #define
+MII_KSZPHY_EXTREG_READ 0x0d
+
+/* Extended registers */
+#define MII_KSZPHY_CLK_CONTROL_PAD_SKEW 0x104 #define
+MII_KSZPHY_RX_DATA_PAD_SKEW 0x105 #define
+MII_KSZPHY_TX_DATA_PAD_SKEW 0x106
+

static int ksz_config_flags(struct phy_device *phydev) { int regval; @@ -65,6 +78,20 @@ static int ksz_config_flags(struct phy_device *phydev) return 0; }

drivers/net/phy/micrel.c +13 -0
+static int kszphy_extended_write(struct phy_device *phydev,
+ u32 regnum, u16 val)
+{
+ phy_write(phydev, MII_KSZPHY_EXTREG, KSZPHY_EXTREG_WRITE |
+regnum);  return phy_write(phydev, MII_KSZPHY_EXTREG_WRITE, val);
+}
+
+static int kszphy_extended_read(struct phy_device *phydev,
+ u32 regnum)
+{
+ phy_write(phydev, MII_KSZPHY_EXTREG, regnum);  return
+phy_read(phydev, MII_KSZPHY_EXTREG_READ); }
+

static int kszphy_ack_interrupt(struct phy_device *phydev) { /* bit[7..0] int status, which is a read and clear register. */ @@ -141,6 +168,78 @@ static int ks8051_config_init(struct phy_device *phydev) return rc < 0 ? rc : 0; }

drivers/net/phy/micrel.c +13 -0
+static int ksz9021_load_values_from_of(struct phy_device *phydev,
+struct device_node *of_node, u16 reg,  char *field1, char
+*field2,  char *field3, char *field4) {  int val1 = -1;  int val2
+= -2;  int val3 = -3;  int val4 = -4;  int newval;  int matches =
+0;
+
+ if (!of_property_read_u32(of_node, field1, &val1))
+ matches++;
+
+ if (!of_property_read_u32(of_node, field2, &val2))
+ matches++;
+
+ if (!of_property_read_u32(of_node, field3, &val3))

drivers/net/phy/micrel.c static int ks8051_config_init(struct phy_device *phydev)

Quoted 10 lines
drivers/net/phy/micrel.c +10 -0
+ matches++;
+
+ if (!of_property_read_u32(of_node, field4, &val4))
+ matches++;
+
+ if (!matches)
+ return 0;
+
+ if (matches < 4)
+ newval = kszphy_extended_read(phydev, reg); else newval = 0;
Quoted 3 lines

Just initialize newval with the reset default of this register. It will make this function easier. Also this two step read from dt and evaluate afterwards seems unnecessary.

Quoted 1 line

The documentation seems inaccurate here. My datasheet says it defaults

to 0x0111, but when I check it for myself it comes out 0x7777. Which value do you think I should take?

Quoted 40 lines
Quoted 29 lines
Quoted 29 lines
drivers/net/phy/micrel.c +29 -0
+
+ if (val1 != -1)
+ newval = ((newval & 0xfff0) | ((val1/200)&0xf) << 0);
+
+ if (val2 != -1)
+ newval = ((newval & 0xff0f) | ((val2/200)&0xf) << 4);
+
+ if (val3 != -1)
+ newval = ((newval & 0xf0ff) | ((val3/200)&0xf) << 8);
+
+ if (val4 != -1)
+ newval = ((newval & 0x0fff) | ((val4/200)&0xf) << 12);
+
+ return kszphy_extended_write(phydev, reg, newval); }
+
+static int ksz9021_config_init(struct phy_device *phydev) {
+struct device *dev = &phydev->dev;  struct device_node *of_node =
+dev->of_node;
+
+ if (!of_node && dev->parent->of_node) of_node =
+ dev->parent->of_node;
+
+ if (of_node) {
+ ksz9021_load_values_from_of(phydev, of_node,
+ MII_KSZPHY_CLK_CONTROL_PAD_SKEW, "txen-skew", "txc-skew",
+ "rxdv-skew", "rxc-skew"); ksz9021_load_values_from_of(phydev,
+ of_node, MII_KSZPHY_RX_DATA_PAD_SKEW, "rxd0-skew", "rxd1-skew",
+ "rxd2-skew", "rxd3-skew"); ksz9021_load_values_from_of(phydev,
+ of_node, MII_KSZPHY_TX_DATA_PAD_SKEW, "txd0-skew", "txd1-skew",
Quoted 8 lines
Quoted 1 line

+ "txd2-skew", "txd3-skew");

Are you sure this register exists? It's not mentioned in my datasheet, only the first two are.

Quoted 1 line

I'm very sure the register exists. There's a hole in the documentation

where the TX control register should exist. Furthermore, we measured that changing values in this register adjusted the TX skew on the RG-MII wires, and on our board at least, adjusting these values yields increased performance.
There hasn't been any response to this patch in two weeks. Can someone please merge this patch or indicate what's wrong with it?

maintainer
#8 lore raw

Re: [PATCH v5] net/phy: micrel: Add OF configuration support for ksz9021

In reply to Sean Cross

From: Sean Cross <[email protected]>
Date: Tue, 20 Aug 2013 16:34:59 +0800

Quoted 2 lines

There hasn't been any response to this patch in two weeks. Can someone please merge this patch or indicate what's wrong with it?

At this point you will need to simply resubmit it.

Reviewed-by on v5 1/1
#9 lore raw

Re: [PATCH v5] net/phy: micrel: Add OF configuration support for ksz9021

In reply to Sean Cross
Quoted 42 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-ksz9021.txt     |   49 ++++++++++
 drivers/net/phy/micrel.c                           |  101 +++++++++++++++++++-
 2 files changed, 149 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/net/micrel-ksz9021.txt
Documentation/devicetree/bindings/net/micrel-ksz9021.txt new file +21 -0
new file mode 100644
index 0000000..338a7e2
@@ -0,0 +1,49 @@
+Micrel KSZ9021 Gigabit Ethernet PHY
+
+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.
+
+All skew control options are specified in picoseconds.  The minimum
+value is 0, and the maximum value is 3000.
+
+Optional properties:
+ - rxc-skew : Skew control of RXC pad
+ - rxdv-skew : Skew control of RX CTL pad
+ - txc-skew : Skew control of TXC pad
+ - txen-skew : Skew control of TX_CTL pad
+ - rxd0-skew : Skew control of RX data 0 pad
+ - rxd1-skew : Skew control of RX data 1 pad
+ - rxd2-skew : Skew control of RX data 2 pad
+ - rxd3-skew : Skew control of RX data 3 pad
+ - txd0-skew : Skew control of TX data 0 pad
+ - txd1-skew : Skew control of TX data 1 pad
+ - txd2-skew : Skew control of TX data 2 pad

Documentation/devicetree/bindings/net/micrel-ksz9021.txt line 31

Quoted 10 lines
Documentation/devicetree/bindings/net/micrel-ksz9021.txt new file +10 -0
+ - txd3-skew : Skew control of TX data 3 pad
+
+Examples:
+
+	/* Attach to an Ethernet device with autodetected PHY */
+	&enet {
+		rxc-skew = <3000>; // picoseconds
+		rxdv-skew = <0>;   // picoseconds
+		txc-skew = <3000>; // picoseconds
+		txen-skew = <0>;   // picoseconds

I prefer to add a -ps suffix to the property names. This makes the comments unnecessary and provides the unit information automatically where it is needed.

Documentation/devicetree/bindings/net/micrel-ksz9021.txt line 33

Quoted 2 lines
Documentation/devicetree/bindings/net/micrel-ksz9021.txt new file +2 -0
+	if (val1 != -1)
+		newval = ((newval & 0xfff0) | ((val1/200)&0xf) << 0);

Please add whitespaces at both sides os operators as required in Documentation/CodingStyle.

Otherwise:

Reviewed-by Sascha Hauer <[email protected]>

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 |