[net-next PATCH 0/3] Adding phy register fixup in DT

[net-next PATCH 0/3] Adding phy register fixup in DT

[Mugunthan V N]

In earlier days phy fixup was added to phy frame work in board files.
As there won't be any board files here after the same has to be done in DT
This patch series adds the following features
* support for adding phy resigter fixup via DT
* adds phy id for EVMsk n DTS file
* adds phy fixup for AM335x EVM and EVMsk

Mugunthan V N (3):
  drivers: of: add phy fixup support in DT
  ARM: dts: AM33XX: Add CPSW phy_id device tree data to am335x-evmsk
  ARM: dts: AM33XX: add phy fixup for evm and evmsk boards

 .../devicetree/bindings/net/phy-fixup.txt          |   26 ++++++
 arch/arm/boot/dts/am335x-evm.dts                   |   10 +++
 arch/arm/boot/dts/am335x-evmsk.dts                 |   18 ++++
 drivers/of/of_net.c                                |   92 ++++++++++++++++++++
 include/linux/of_net.h                             |    6 ++
 5 files changed, 152 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/phy-fixup.txt

-- 
1.7.9.5

[net-next PATCH 1/3] drivers: of: add phy fixup support in DT

[Mugunthan V N]

In earlier case phy fixup are added in board file as this is no more the case
so adding support for phy register fixup in Device Tree

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 .../devicetree/bindings/net/phy-fixup.txt          |   26 ++++++
 drivers/of/of_net.c                                |   92 ++++++++++++++++++++
 include/linux/of_net.h                             |    6 ++
 3 files changed, 124 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/phy-fixup.txt

diff --git a/Documentation/devicetree/bindings/net/phy-fixup.txt b/Documentation/devicetree/bindings/net/phy-fixup.txt
new file mode 100644
index 0000000..460f76d
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/phy-fixup.txt
@@ -0,0 +1,26 @@
+Ethernet Phy fixup Device Tree Bindings
+---------------------------------------
+
+The following DT fields can be added to MDIO DT notes and can be used to
+add phy fix up needed
+
+Required properties:
+- phy-fixup-registers	: Will contain a array of register fix nodes which has
+			  the following node parameters
+- phy-id		: Specifies the phy id for which the fix belongs to
+- phy-mask		: Specifies the phy mask for which the fix belongs to
+- fixup-registers	: Specifies the fix up registers and values in array
+			  of offset value pair
+Optional properties:
+
+Examples:
+
+&davinci_mdio {
+	phy-fixup-registers = <&atheros_txclk_delay_fixup>;
+
+	atheros_txclk_delay_fixup: atheros_txclk_delay_fixup {
+		phy-id = <0x4dd074>;
+		phy-mask = <0xfffffffe>;
+		fixup-registers = <0x1d 0x5 0x1e 0x100>;
+	};
+};
diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index ffab033..50ad671 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c
@@ -10,6 +10,7 @@
 #include <linux/of_net.h>
 #include <linux/phy.h>
 #include <linux/export.h>
+#include <linux/of_net.h>
 
 /**
  * It maps 'enum phy_interface_t' found in include/linux/phy.h
@@ -92,3 +93,94 @@ const void *of_get_mac_address(struct device_node *np)
 	return NULL;
 }
 EXPORT_SYMBOL(of_get_mac_address);
+
+static int __of_phy_fixup_cb(struct phy_device *phydev)
+{
+	struct device_node *node = phydev->bus->parent->of_node;
+	struct device_node *phy_fixup_node;
+	const __be32 *parp;
+	int lenp;
+	int i, j;
+
+	parp = of_get_property(node, "phy-fixup-registers", &lenp);
+	if (parp == NULL)
+		return 0;
+	lenp /= sizeof(void *);
+
+	for (i = 0; i < lenp; i++) {
+		u32 phy_id;
+		const __be32 *fixups;
+		int fixup_len;
+
+		phy_fixup_node = of_find_node_by_phandle(be32_to_cpup(parp+i));
+		if (of_property_read_u32(phy_fixup_node, "phy-id", &phy_id)) {
+			pr_err("Missing PHY id in Phy fixup\n");
+			return -EINVAL;
+		}
+		if (phy_id != phydev->phy_id)
+			continue;
+
+		fixups = of_get_property(phy_fixup_node, "fixup-registers",
+					 &fixup_len);
+		if (fixups == NULL) {
+			pr_err("Missing fixup registers in Phy fixup\n");
+			return -EINVAL;
+		}
+		fixup_len /= sizeof(void *);
+		for (j = 0; j < fixup_len; j += 2) {
+			u16 regnum = be32_to_cpup(fixups + j);
+			u16 val = be32_to_cpup(fixups + j + 1);
+			phy_write(phydev, regnum, val);
+		}
+	}
+
+	return 0;
+}
+
+int of_add_phy_fixup_register(struct device_node *node)
+{
+	struct device_node *phy_fixup_node;
+	const __be32 *parp;
+	int lenp;
+	int i;
+
+	parp = of_get_property(node, "phy-fixup-registers", &lenp);
+	if (parp == NULL)
+		return 0;
+	lenp /= sizeof(void *);
+
+	for (i = 0; i < lenp; i++) {
+		u32 phy_id;
+		u32 phy_mask;
+		const __be32 *fixups;
+		int fixup_len;
+
+		phy_fixup_node = of_find_node_by_phandle(be32_to_cpup(parp+i));
+		if (of_property_read_u32(phy_fixup_node, "phy-id", &phy_id)) {
+			pr_err("Missing PHY id in Phy fixup\n");
+			continue;
+		}
+
+		if (of_property_read_u32(phy_fixup_node, "phy-mask",
+					 &phy_mask)) {
+			pr_err("Missing PHY mask in Phy fixup\n");
+			continue;
+		}
+
+		fixups = of_get_property(phy_fixup_node, "fixup-registers",
+					 &fixup_len);
+		if (fixups == NULL) {
+			pr_err("Missing fixup registers in Phy fixup\n");
+			continue;
+		}
+		fixup_len /= sizeof(void *);
+		if (fixup_len % 2) {
+			pr_err("Fixup registers length is invalid in Phy fixup\n");
+			continue;
+		}
+		phy_register_fixup_for_uid(phy_id, phy_mask, __of_phy_fixup_cb);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_add_phy_fixup_register);
diff --git a/include/linux/of_net.h b/include/linux/of_net.h
index 61bf53b..c690e38 100644
--- a/include/linux/of_net.h
+++ b/include/linux/of_net.h
@@ -11,6 +11,7 @@
 #include <linux/of.h>
 extern const int of_get_phy_mode(struct device_node *np);
 extern const void *of_get_mac_address(struct device_node *np);
+extern int of_add_phy_fixup_register(struct device_node *node);
 #else
 static inline const int of_get_phy_mode(struct device_node *np)
 {
@@ -21,6 +22,11 @@ static inline const void *of_get_mac_address(struct device_node *np)
 {
 	return NULL;
 }
+
+static int of_add_phy_fixup_register(struct device_node *node)
+{
+	return -ENODEV;
+}
 #endif
 
 #endif /* __LINUX_OF_NET_H */
-- 
1.7.9.5

[net-next PATCH 2/3] ARM: dts: AM33XX: Add CPSW phy_id device tree data to am335x-evmsk

[Mugunthan V N]

Add phy_id device tree data to am335x-evmsk device to bring up CPSW
ethernet present on am335x starter kit.

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 arch/arm/boot/dts/am335x-evmsk.dts |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/am335x-evmsk.dts b/arch/arm/boot/dts/am335x-evmsk.dts
index f5a6162..f297b85 100644
--- a/arch/arm/boot/dts/am335x-evmsk.dts
+++ b/arch/arm/boot/dts/am335x-evmsk.dts
@@ -248,3 +248,11 @@
 		};
 	};
 };
+
+&cpsw_emac0 {
+	phy_id = <&davinci_mdio>, <0>;
+};
+
+&cpsw_emac1 {
+	phy_id = <&davinci_mdio>, <1>;
+};
-- 
1.7.9.5

[net-next PATCH 3/3] ARM: dts: AM33XX: add phy fixup for evm and evmsk boards

[Mugunthan V N]

As RGMII tx clock internal delay is not supported in AM335x, the same has
to be enabled in phy. This patch adds support for enabling tx clock internal
delay via phy debug registers

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 arch/arm/boot/dts/am335x-evm.dts   |   10 ++++++++++
 arch/arm/boot/dts/am335x-evmsk.dts |   10 ++++++++++
 2 files changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index d649644..72805c5 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -244,3 +244,13 @@
 &cpsw_emac1 {
 	phy_id = <&davinci_mdio>, <1>;
 };
+
+&davinci_mdio {
+	phy-fixup-registers = <&atheros_txclk_delay_fixup>;
+
+	atheros_txclk_delay_fixup: atheros_txclk_delay_fixup {
+		phy-id = <0x4dd074>;
+		phy-mask = <0xfffffffe>;
+		fixup-registers = <0x1d 0x5 0x1e 0x100>;
+	};
+};
diff --git a/arch/arm/boot/dts/am335x-evmsk.dts b/arch/arm/boot/dts/am335x-evmsk.dts
index f297b85..f398cb3 100644
--- a/arch/arm/boot/dts/am335x-evmsk.dts
+++ b/arch/arm/boot/dts/am335x-evmsk.dts
@@ -256,3 +256,13 @@
 &cpsw_emac1 {
 	phy_id = <&davinci_mdio>, <1>;
 };
+
+&davinci_mdio {
+	phy-fixup-registers = <&atheros_txclk_delay_fixup>;
+
+	atheros_txclk_delay_fixup: atheros_txclk_delay_fixup {
+		phy-id = <0x4dd074>;
+		phy-mask = <0xfffffffe>;
+		fixup-registers = <0x1d 0x5 0x1e 0x100>;
+	};
+};
-- 
1.7.9.5

Re: [net-next PATCH 0/3] Adding phy register fixup in DT

[Sascha Hauer]

On Mon, Apr 22, 2013 at 11:50:35PM +0530, Mugunthan V N wrote:
> In earlier days phy fixup was added to phy frame work in board files.
> As there won't be any board files here after the same has to be done in DT
> This patch series adds the following features
> * support for adding phy resigter fixup via DT
> * adds phy id for EVMsk n DTS file
> * adds phy fixup for AM335x EVM and EVMsk
> 
> Mugunthan V N (3):
>   drivers: of: add phy fixup support in DT
>   ARM: dts: AM33XX: Add CPSW phy_id device tree data to am335x-evmsk
>   ARM: dts: AM33XX: add phy fixup for evm and evmsk boards

I generally do not offend to phy fixups from the devicetree. I see
though that becomes more and more common that we have to configure
the tx delays in phys.

The current way seems to be to hardcode register values for each board
which seems not very flexible and forces us to read phy datasheets
each time for a new board.

Wouldn't it make more sense to configure the actual delays (in ns) and
let the phy drivers figure out how to turn this into register values?

Not that I volunteer to write such things... :-/

Sascha

-- 
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: [net-next PATCH 0/3] Adding phy register fixup in DT

[Mugunthan V N]

On 4/23/2013 1:32 PM, Sascha Hauer wrote:
> On Mon, Apr 22, 2013 at 11:50:35PM +0530, Mugunthan V N wrote:
>> In earlier days phy fixup was added to phy frame work in board files.
>> As there won't be any board files here after the same has to be done in DT
>> This patch series adds the following features
>> * support for adding phy resigter fixup via DT
>> * adds phy id for EVMsk n DTS file
>> * adds phy fixup for AM335x EVM and EVMsk
>>
>> Mugunthan V N (3):
>>    drivers: of: add phy fixup support in DT
>>    ARM: dts: AM33XX: Add CPSW phy_id device tree data to am335x-evmsk
>>    ARM: dts: AM33XX: add phy fixup for evm and evmsk boards
> I generally do not offend to phy fixups from the devicetree. I see
> though that becomes more and more common that we have to configure
> the tx delays in phys.
>
> The current way seems to be to hardcode register values for each board
> which seems not very flexible and forces us to read phy datasheets
> each time for a new board.
>
> Wouldn't it make more sense to configure the actual delays (in ns) and
> let the phy drivers figure out how to turn this into register values?
>
> Not that I volunteer to write such things... :-/
>
There is no calculation done for enabling RGMII Tx internal delay. Its all
pre-calculated in Hardware (Either in Phy or EMAC)
In this patch as CPSW IP in AM335x doesn't support internal delay inside
SoC, so enabling the same in Phy and it is just a bit set in the phy debug
registers

Regards
Mugunthan V N

Re: [net-next PATCH 1/3] drivers: of: add phy fixup support in DT

[David Miller]

From: Mugunthan V N <mugunthanvnm@ti.com>
Date: Mon, 22 Apr 2013 23:50:36 +0530

> In earlier case phy fixup are added in board file as this is no more the case
> so adding support for phy register fixup in Device Tree
> 
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>

When people put a series of undocumented PHY register writes using
constants, we tell them it's firmware.

If these PHY registers are actually documented in the driver, write a
function in that driver which does the programming sequence, then add
a property that the driver looks for in order to determine whether to
call that sequence or not.

I don't want people putting random PHY raw programming sequences and
other crap like that into the OF device nodes.  It's extremely
inelegant and inviting abuse.

Re: [net-next PATCH 1/3] drivers: of: add phy fixup support in DT

[Mugunthan V N]

On 4/25/2013 1:26 PM, David Miller wrote:
> From: Mugunthan V N <mugunthanvnm@ti.com>
> Date: Mon, 22 Apr 2013 23:50:36 +0530
>
>> In earlier case phy fixup are added in board file as this is no more the case
>> so adding support for phy register fixup in Device Tree
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> When people put a series of undocumented PHY register writes using
> constants, we tell them it's firmware.
>
> If these PHY registers are actually documented in the driver, write a
> function in that driver which does the programming sequence, then add
> a property that the driver looks for in order to determine whether to
> call that sequence or not.
>
> I don't want people putting random PHY raw programming sequences and
> other crap like that into the OF device nodes.  It's extremely
> inelegant and inviting abuse.
>
Will modify the source as per your comments and will submit v2 patch set.

Regards
Mugunthan V N

Re: [net-next PATCH 1/3] drivers: of: add phy fixup support in DT

[Ben Hutchings]

On Thu, 2013-04-25 at 03:56 -0400, David Miller wrote:
> From: Mugunthan V N <mugunthanvnm@ti.com>
> Date: Mon, 22 Apr 2013 23:50:36 +0530
> 
> > In earlier case phy fixup are added in board file as this is no more the case
> > so adding support for phy register fixup in Device Tree
> > 
> > Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> 
> When people put a series of undocumented PHY register writes using
> constants, we tell them it's firmware.
> 
> If these PHY registers are actually documented in the driver, write a
> function in that driver which does the programming sequence, then add
> a property that the driver looks for in order to determine whether to
> call that sequence or not.
> 
> I don't want people putting random PHY raw programming sequences and
> other crap like that into the OF device nodes.  It's extremely
> inelegant and inviting abuse.

I don't think booleans will be enough in general; a PHY may well require
link tuning for a specific board.  But the set of register addresses to
program is likely to be known in advance and then only the values would
belong in DT.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.