diff mbox

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

Message ID 1366654838-26479-2-git-send-email-mugunthanvnm@ti.com
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Mugunthan V N April 22, 2013, 6:20 p.m. UTC
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

Comments

David Miller April 25, 2013, 7:56 a.m. UTC | #1
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.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mugunthan V N April 25, 2013, 10:04 a.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings April 25, 2013, 4:09 p.m. UTC | #3
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.
diff mbox

Patch

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 */