Fix problems with Holly's DT representation of ethernet PHYs

20 messages, 6 authors, 2007-05-29 · open the first message on its own page

Fix problems with Holly's DT representation of ethernet PHYs

From: David Gibson <hidden>
Date: 2007-05-24 04:16:25

This patch fixes some problems with the way the Ethernet PHYs are
represented in the device tree for the Holly board.  This means
changes to the dts itself, and to the code with instantiates the
tsi108 ethernet platform devices based on the device tree.

	- First, and most importantly, the PHYs are given with an
identical 'reg' property.  This reg currently encodes the accessible
register used to initiate mdio interaction with the PHYs, rather than
a meaningful address on the parent bus (mdio in this case), which is
incorrect.  Instead we give the address of these registers as 'reg' in
the mdio node itself, and encode the ID of each phy in their 'reg'
propertys.  The PHY's unit name addresses are updated to match.
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).

Signed-off-by: David Gibson <redacted>

Index: working-2.6/arch/powerpc/boot/dts/holly.dts
===================================================================
--- working-2.6.orig/arch/powerpc/boot/dts/holly.dts	2007-05-24 13:55:38.000000000 +1000
+++ working-2.6/arch/powerpc/boot/dts/holly.dts	2007-05-24 14:11:56.000000000 +1000
@@ -63,19 +63,20 @@
 		mdio@6000 {
 			device_type = "mdio";
 			compatible = "tsi-ethernet";
+			reg = <6000 50>;
+			#address-cells = <1>;
+			#size-cells = <0>;
 
-			PHY1: ethernet-phy@6000 {
+			PHY1: ethernet-phy@1 {
 				device_type = "ethernet-phy";
-				compatible = "bcm54xx";
-				reg = <6000 50>;
-				phy-id = <1>;
+				compatible = "bcm5461A", "bcm54xx";
+				reg = <1>;
 			};
 
-			PHY2: ethernet-phy@6400 {
+			PHY2: ethernet-phy@2 {
 				device_type = "ethernet-phy";
-				compatible = "bcm54xx";
-				reg = <6000 50>;
-				phy-id = <2>;
+				compatible = "bcm5461A", "bcm54xx";
+				reg = <2>;
 			};
 		};
 
Index: working-2.6/arch/powerpc/sysdev/tsi108_dev.c
===================================================================
--- working-2.6.orig/arch/powerpc/sysdev/tsi108_dev.c	2007-05-24 13:55:38.000000000 +1000
+++ working-2.6/arch/powerpc/sysdev/tsi108_dev.c	2007-05-24 14:12:11.000000000 +1000
@@ -75,9 +75,8 @@ static int __init tsi108_eth_of_init(voi
 	     (np = of_find_compatible_node(np, "network", "tsi-ethernet")) != NULL;
 	     i++) {
 		struct resource r[2];
-		struct device_node *phy;
+		struct device_node *phy, *mdio;
 		hw_info tsi_eth_data;
-		const unsigned int *id;
 		const unsigned int *phy_id;
 		const void *mac_addr;
 		const phandle *ph;
@@ -119,13 +118,13 @@ static int __init tsi108_eth_of_init(voi
 			goto unreg;
 		}
 
-		id = of_get_property(phy, "reg", NULL);
-		phy_id = of_get_property(phy, "phy-id", NULL);
-		ret = of_address_to_resource(phy, 0, &res);
-		if (ret) {
-			of_node_put(phy);
+		phy_id = of_get_property(phy, "reg", NULL);
+
+		mdio = of_get_parent(phy);
+		ret = of_address_to_resource(mdio, 0, &res);
+		of_node_put(mdio);
+		if (ret)
 			goto unreg;
-		}
 		tsi_eth_data.regs = r[0].start;
 		tsi_eth_data.phyregs = res.start;
 		tsi_eth_data.phy = *phy_id;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: Kumar Gala <hidden>
Date: 2007-05-24 04:24:27

On May 23, 2007, at 11:16 PM, David Gibson wrote:
This patch fixes some problems with the way the Ethernet PHYs are
represented in the device tree for the Holly board.  This means
changes to the dts itself, and to the code with instantiates the
tsi108 ethernet platform devices based on the device tree.

	- First, and most importantly, the PHYs are given with an
identical 'reg' property.  This reg currently encodes the accessible
register used to initiate mdio interaction with the PHYs, rather than
a meaningful address on the parent bus (mdio in this case), which is
incorrect.  Instead we give the address of these registers as 'reg' in
the mdio node itself, and encode the ID of each phy in their 'reg'
propertys.  The PHY's unit name addresses are updated to match.
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).
Is the compatible really need here?  We are able to provide and bind  
drivers based on MII_PHYSID1/2.  I don't see what putting the  
compatible proper gets us.
quoted hunk
Signed-off-by: David Gibson <redacted>

Index: working-2.6/arch/powerpc/boot/dts/holly.dts
===================================================================
--- working-2.6.orig/arch/powerpc/boot/dts/holly.dts	2007-05-24  
13:55:38.000000000 +1000
+++ working-2.6/arch/powerpc/boot/dts/holly.dts	2007-05-24  
14:11:56.000000000 +1000
@@ -63,19 +63,20 @@
 		mdio@6000 {
 			device_type = "mdio";
 			compatible = "tsi-ethernet";
+			reg = <6000 50>;
+			#address-cells = <1>;
+			#size-cells = <0>;

-			PHY1: ethernet-phy@6000 {
+			PHY1: ethernet-phy@1 {
 				device_type = "ethernet-phy";
-				compatible = "bcm54xx";
-				reg = <6000 50>;
-				phy-id = <1>;
+				compatible = "bcm5461A", "bcm54xx";
+				reg = <1>;
 			};

-			PHY2: ethernet-phy@6400 {
+			PHY2: ethernet-phy@2 {
 				device_type = "ethernet-phy";
-				compatible = "bcm54xx";
-				reg = <6000 50>;
-				phy-id = <2>;
+				compatible = "bcm5461A", "bcm54xx";
+				reg = <2>;
 			};
 		};
Index: working-2.6/arch/powerpc/sysdev/tsi108_dev.c
===================================================================
--- working-2.6.orig/arch/powerpc/sysdev/tsi108_dev.c	2007-05-24  
13:55:38.000000000 +1000
+++ working-2.6/arch/powerpc/sysdev/tsi108_dev.c	2007-05-24  
14:12:11.000000000 +1000
@@ -75,9 +75,8 @@ static int __init tsi108_eth_of_init(voi
 	     (np = of_find_compatible_node(np, "network", "tsi- 
ethernet")) != NULL;
 	     i++) {
 		struct resource r[2];
-		struct device_node *phy;
+		struct device_node *phy, *mdio;
 		hw_info tsi_eth_data;
-		const unsigned int *id;
 		const unsigned int *phy_id;
 		const void *mac_addr;
 		const phandle *ph;
@@ -119,13 +118,13 @@ static int __init tsi108_eth_of_init(voi
 			goto unreg;
 		}

-		id = of_get_property(phy, "reg", NULL);
-		phy_id = of_get_property(phy, "phy-id", NULL);
-		ret = of_address_to_resource(phy, 0, &res);
-		if (ret) {
-			of_node_put(phy);
+		phy_id = of_get_property(phy, "reg", NULL);
+
+		mdio = of_get_parent(phy);
+		ret = of_address_to_resource(mdio, 0, &res);
+		of_node_put(mdio);
+		if (ret)
 			goto unreg;
-		}
 		tsi_eth_data.regs = r[0].start;
 		tsi_eth_data.phyregs = res.start;
 		tsi_eth_data.phy = *phy_id;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_  
_other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: David Gibson <hidden>
Date: 2007-05-24 05:59:09

On Wed, May 23, 2007 at 11:22:34PM -0500, Kumar Gala wrote:
On May 23, 2007, at 11:16 PM, David Gibson wrote:
quoted
This patch fixes some problems with the way the Ethernet PHYs are
represented in the device tree for the Holly board.  This means
changes to the dts itself, and to the code with instantiates the
tsi108 ethernet platform devices based on the device tree.

	- First, and most importantly, the PHYs are given with an
identical 'reg' property.  This reg currently encodes the accessible
register used to initiate mdio interaction with the PHYs, rather than
a meaningful address on the parent bus (mdio in this case), which is
incorrect.  Instead we give the address of these registers as 'reg' in
the mdio node itself, and encode the ID of each phy in their 'reg'
propertys.  The PHY's unit name addresses are updated to match.
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).
Is the compatible really need here?  We are able to provide and bind  
drivers based on MII_PHYSID1/2.  I don't see what putting the  
compatible proper gets us.
That's a good point (except that having a node without compatible is
kind of odd).  At the moment the code which instantiates the platform
device looks at compatible to enable a workaround.  But actually that
workaround is holly specific, rather than related to the PHY model,
and so should be encoded differently.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: Segher Boessenkool <hidden>
Date: 2007-05-24 09:11:19

Looks okay but...
+				compatible = "bcm5461A", "bcm54xx";
... lowercase please.


Segher

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: Segher Boessenkool <hidden>
Date: 2007-05-24 09:13:54

quoted
Is the compatible really need here?  We are able to provide and bind
drivers based on MII_PHYSID1/2.  I don't see what putting the
compatible proper gets us.
That's a good point (except that having a node without compatible is
kind of odd).
Not really; "compatible" is for the cases where "name"
alone isn't enough.
At the moment the code which instantiates the platform
device looks at compatible to enable a workaround.  But actually that
workaround is holly specific, rather than related to the PHY model,
and so should be encoded differently.
There is no real value in encoding it in the device
tree, even.


Segher

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: Josh Boyer <hidden>
Date: 2007-05-24 13:45:18

On Wed, 2007-05-23 at 23:22 -0500, Kumar Gala wrote:
On May 23, 2007, at 11:16 PM, David Gibson wrote:
quoted
This patch fixes some problems with the way the Ethernet PHYs are
represented in the device tree for the Holly board.  This means
changes to the dts itself, and to the code with instantiates the
tsi108 ethernet platform devices based on the device tree.

	- First, and most importantly, the PHYs are given with an
identical 'reg' property.  This reg currently encodes the accessible
register used to initiate mdio interaction with the PHYs, rather than
a meaningful address on the parent bus (mdio in this case), which is
incorrect.  Instead we give the address of these registers as 'reg' in
the mdio node itself, and encode the ID of each phy in their 'reg'
propertys.  The PHY's unit name addresses are updated to match.
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).
Is the compatible really need here?  We are able to provide and bind  
drivers based on MII_PHYSID1/2.  I don't see what putting the  
compatible proper gets us.
I think it's needed until the TSI driver switches to phylib.  Right now,
it assumes that a Marvell phy is used, which is what is present on the
Taiga MPC7448HPC2 board.  The Holly board uses the Broadcom phy and we
added the compatible code for that.

josh

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: Josh Boyer <hidden>
Date: 2007-05-24 13:46:46

On Thu, 2007-05-24 at 14:16 +1000, David Gibson wrote:
This patch fixes some problems with the way the Ethernet PHYs are
represented in the device tree for the Holly board.  This means
changes to the dts itself, and to the code with instantiates the
tsi108 ethernet platform devices based on the device tree.

	- First, and most importantly, the PHYs are given with an
identical 'reg' property.  This reg currently encodes the accessible
register used to initiate mdio interaction with the PHYs, rather than
a meaningful address on the parent bus (mdio in this case), which is
incorrect.  Instead we give the address of these registers as 'reg' in
the mdio node itself, and encode the ID of each phy in their 'reg'
propertys.  The PHY's unit name addresses are updated to match.
You'll need to fixup the mpc7448hpc2 board's DTS with the same change
then, as it shares the same bridge and ethernet code.
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).
This part is fine with me.

josh

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: David Gibson <hidden>
Date: 2007-05-25 02:48:42

On Thu, May 24, 2007 at 08:45:48AM -0500, Josh Boyer wrote:
On Thu, 2007-05-24 at 14:16 +1000, David Gibson wrote:
quoted
This patch fixes some problems with the way the Ethernet PHYs are
represented in the device tree for the Holly board.  This means
changes to the dts itself, and to the code with instantiates the
tsi108 ethernet platform devices based on the device tree.

	- First, and most importantly, the PHYs are given with an
identical 'reg' property.  This reg currently encodes the accessible
register used to initiate mdio interaction with the PHYs, rather than
a meaningful address on the parent bus (mdio in this case), which is
incorrect.  Instead we give the address of these registers as 'reg' in
the mdio node itself, and encode the ID of each phy in their 'reg'
propertys.  The PHY's unit name addresses are updated to match.
You'll need to fixup the mpc7448hpc2 board's DTS with the same change
then, as it shares the same bridge and ethernet code.
Ah, yes.  Done.
quoted
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).
This part is fine with me.

josh

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: David Gibson <hidden>
Date: 2007-05-25 04:24:20

On Thu, May 24, 2007 at 08:44:20AM -0500, Josh Boyer wrote:
On Wed, 2007-05-23 at 23:22 -0500, Kumar Gala wrote:
quoted
On May 23, 2007, at 11:16 PM, David Gibson wrote:
quoted
This patch fixes some problems with the way the Ethernet PHYs are
represented in the device tree for the Holly board.  This means
changes to the dts itself, and to the code with instantiates the
tsi108 ethernet platform devices based on the device tree.

	- First, and most importantly, the PHYs are given with an
identical 'reg' property.  This reg currently encodes the accessible
register used to initiate mdio interaction with the PHYs, rather than
a meaningful address on the parent bus (mdio in this case), which is
incorrect.  Instead we give the address of these registers as 'reg' in
the mdio node itself, and encode the ID of each phy in their 'reg'
propertys.  The PHY's unit name addresses are updated to match.
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).
Is the compatible really need here?  We are able to provide and bind  
drivers based on MII_PHYSID1/2.  I don't see what putting the  
compatible proper gets us.
I think it's needed until the TSI driver switches to phylib.  Right now,
it assumes that a Marvell phy is used, which is what is present on the
Taiga MPC7448HPC2 board.  The Holly board uses the Broadcom phy and we
added the compatible code for that.
I think Ben's suggested approach of using a special property in the
phy node to indicate that this workaround is necessary is better.
After all, the workaround isn't actually related to the type of PHY,
which is what compatible encodes, but to how it's wired up.  Revised
patch below addressing this and other matters.

Fix problems with Holly's DT representation of ethernet PHYs

This patch fixes some problems with the way the Ethernet PHYs are
represented in the device tree for the Holly board.  This means
changes to the dts itself, and to the code with instantiates the
tsi108 ethernet platform devices based on the device tree.

	- First, and most importantly, the PHYs are given with an
identical 'reg' property.  This reg currently encodes the accessible
register used to initiate mdio interaction with the PHYs, rather than
a meaningful address on the parent bus (mdio in this case), which is
incorrect.  Instead we give the address of these registers as 'reg' in
the mdio node itself, and encode the ID of each phy in their 'reg'
propertys.  The PHY's unit name addresses are updated to match.
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).

Signed-off-by: David Gibson <redacted>

Index: working-2.6/arch/powerpc/boot/dts/holly.dts
===================================================================
--- working-2.6.orig/arch/powerpc/boot/dts/holly.dts	2007-05-25 13:27:01.000000000 +1000
+++ working-2.6/arch/powerpc/boot/dts/holly.dts	2007-05-25 13:53:03.000000000 +1000
@@ -60,22 +60,21 @@
 			reg = <7000 400>;
 		};
 
-		mdio@6000 {
+		MDIO: mdio@6000 {
 			device_type = "mdio";
 			compatible = "tsi-ethernet";
+			reg = <6000 50>;
+			#address-cells = <1>;
+			#size-cells = <0>;
 
-			PHY1: ethernet-phy@6000 {
-				device_type = "ethernet-phy";
-				compatible = "bcm54xx";
-				reg = <6000 50>;
-				phy-id = <1>;
+			PHY1: ethernet-phy@1 {
+				reg = <1>;
+				txc-rxc-delay-disable;
 			};
 
-			PHY2: ethernet-phy@6400 {
-				device_type = "ethernet-phy";
-				compatible = "bcm54xx";
-				reg = <6000 50>;
-				phy-id = <2>;
+			PHY2: ethernet-phy@2 {
+				reg = <2>;
+				txc-rxc-delay-disable;
 			};
 		};
 
@@ -88,6 +87,7 @@
 			local-mac-address = [ 00 00 00 00 00 00 ];
 			interrupt-parent = < &/tsi109@c0000000/pic@7400 >;
 			interrupts = <10 2>;
+			mdio-handle = <&MDIO>;
 			phy-handle = <&PHY1>;
 		};
 
@@ -100,6 +100,7 @@
 			local-mac-address = [ 00 00 00 00 00 00 ];
 			interrupt-parent = < &/tsi109@c0000000/pic@7400 >;
 			interrupts = <11 2>;
+			mdio-handle = <&MDIO>;
 			phy-handle = <&PHY2>;
 		};
 
Index: working-2.6/arch/powerpc/sysdev/tsi108_dev.c
===================================================================
--- working-2.6.orig/arch/powerpc/sysdev/tsi108_dev.c	2007-05-08 14:58:06.000000000 +1000
+++ working-2.6/arch/powerpc/sysdev/tsi108_dev.c	2007-05-25 14:09:16.000000000 +1000
@@ -75,9 +75,9 @@ static int __init tsi108_eth_of_init(voi
 	     (np = of_find_compatible_node(np, "network", "tsi-ethernet")) != NULL;
 	     i++) {
 		struct resource r[2];
-		struct device_node *phy;
+		struct device_node *phy, *mdio;
 		hw_info tsi_eth_data;
-		const unsigned int *id;
+		const void *
 		const unsigned int *phy_id;
 		const void *mac_addr;
 		const phandle *ph;
@@ -111,6 +111,13 @@ static int __init tsi108_eth_of_init(voi
 		if (mac_addr)
 			memcpy(tsi_eth_data.mac_addr, mac_addr, 6);
 
+		ph = of_get_property(np, "mdio-handle", NULL);
+		mdio = of_find_node_by_phandle(*ph);
+		ret = of_address_to_resource(mdio, 0, &res);
+		of_node_put(mdio);
+		if (ret)
+			goto unreg;
+
 		ph = of_get_property(np, "phy-handle", NULL);
 		phy = of_find_node_by_phandle(*ph);
 
@@ -119,20 +126,17 @@ static int __init tsi108_eth_of_init(voi
 			goto unreg;
 		}
 
-		id = of_get_property(phy, "reg", NULL);
-		phy_id = of_get_property(phy, "phy-id", NULL);
-		ret = of_address_to_resource(phy, 0, &res);
-		if (ret) {
-			of_node_put(phy);
-			goto unreg;
-		}
+		phy_id = of_get_property(phy, "reg", NULL);
+
 		tsi_eth_data.regs = r[0].start;
 		tsi_eth_data.phyregs = res.start;
 		tsi_eth_data.phy = *phy_id;
 		tsi_eth_data.irq_num = irq_of_parse_and_map(np, 0);
-		if (of_device_is_compatible(phy, "bcm54xx"))
+
+		if (of_get_property(phy, "txc-rxc-delay-disable"))
 			tsi_eth_data.phy_type = TSI108_PHY_BCM54XX;
 		of_node_put(phy);
+
 		ret =
 		    platform_device_add_data(tsi_eth_dev, &tsi_eth_data,
 					     sizeof(hw_info));
Index: working-2.6/arch/powerpc/boot/dts/mpc7448hpc2.dts
===================================================================
--- working-2.6.orig/arch/powerpc/boot/dts/mpc7448hpc2.dts	2007-05-21 12:47:17.000000000 +1000
+++ working-2.6/arch/powerpc/boot/dts/mpc7448hpc2.dts	2007-05-25 14:03:16.000000000 +1000
@@ -58,24 +58,23 @@
 			compatible  = "tsi-i2c";
 		};
 
-		mdio@6000 {
+		MDIO: mdio@6000 {
 			device_type = "mdio";
 			compatible = "tsi-ethernet";
+			reg = <6000 50>;
+			#address-cells = <1>;
+			#size-cells = <0>;
 
-			phy8: ethernet-phy@6000 {
+			phy8: ethernet-phy@8 {
 				interrupt-parent = <&mpic>;
 				interrupts = <2 1>;
-				reg = <6000 50>;
-				phy-id = <8>;
-				device_type = "ethernet-phy";
+				reg = <8>;
 			};
 
-			phy9: ethernet-phy@6400 {
+			phy9: ethernet-phy@9 {
 				interrupt-parent = <&mpic>;
 				interrupts = <2 1>;
-				reg = <6000 50>;
-				phy-id = <9>;
-				device_type = "ethernet-phy";
+				reg = <9>;
 			};
 
 		};
@@ -89,6 +88,7 @@
 			address = [ 00 06 D2 00 00 01 ];
 			interrupts = <10 2>;
 			interrupt-parent = <&mpic>;
+			mdio-handle = <&MDIO>;
 			phy-handle = <&phy8>;
 		};
 
@@ -102,6 +102,7 @@
 			address = [ 00 06 D2 00 00 02 ];
 			interrupts = <11 2>;
 			interrupt-parent = <&mpic>;
+			mdio-handle = <&MDIO>;
 			phy-handle = <&phy9>;
 		};
 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: David Gibson <hidden>
Date: 2007-05-25 04:37:40

On Fri, May 25, 2007 at 02:23:59PM +1000, David Gibson wrote:
On Thu, May 24, 2007 at 08:44:20AM -0500, Josh Boyer wrote:
quoted
On Wed, 2007-05-23 at 23:22 -0500, Kumar Gala wrote:
quoted
On May 23, 2007, at 11:16 PM, David Gibson wrote:
quoted
This patch fixes some problems with the way the Ethernet PHYs are
represented in the device tree for the Holly board.  This means
changes to the dts itself, and to the code with instantiates the
tsi108 ethernet platform devices based on the device tree.

	- First, and most importantly, the PHYs are given with an
identical 'reg' property.  This reg currently encodes the accessible
register used to initiate mdio interaction with the PHYs, rather than
a meaningful address on the parent bus (mdio in this case), which is
incorrect.  Instead we give the address of these registers as 'reg' in
the mdio node itself, and encode the ID of each phy in their 'reg'
propertys.  The PHY's unit name addresses are updated to match.
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).
Is the compatible really need here?  We are able to provide and bind  
drivers based on MII_PHYSID1/2.  I don't see what putting the  
compatible proper gets us.
I think it's needed until the TSI driver switches to phylib.  Right now,
it assumes that a Marvell phy is used, which is what is present on the
Taiga MPC7448HPC2 board.  The Holly board uses the Broadcom phy and we
added the compatible code for that.
I think Ben's suggested approach of using a special property in the
phy node to indicate that this workaround is necessary is better.
After all, the workaround isn't actually related to the type of PHY,
which is what compatible encodes, but to how it's wired up.  Revised
patch below addressing this and other matters.
Grah! Forgot quilt ref.  Now a copy that might actually work.

Fix problems with Holly's DT representation of ethernet PHYs

This patch fixes some problems with the way the Ethernet PHYs are
represented in the device tree for the Holly board.  This means
changes to the dts itself, and to the code with instantiates the
tsi108 ethernet platform devices based on the device tree.

	- First, and most importantly, the PHYs are given with an
identical 'reg' property.  This reg currently encodes the accessible
register used to initiate mdio interaction with the PHYs, rather than
a meaningful address on the parent bus (mdio in this case), which is
incorrect.  Instead we give the address of these registers as 'reg' in
the mdio node itself, and encode the ID of each phy in their 'reg'
propertys.  The PHY's unit name addresses are updated to match.
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).

Signed-off-by: David Gibson <redacted>

Index: working-2.6/arch/powerpc/boot/dts/holly.dts
===================================================================
--- working-2.6.orig/arch/powerpc/boot/dts/holly.dts	2007-05-25 14:34:00.000000000 +1000
+++ working-2.6/arch/powerpc/boot/dts/holly.dts	2007-05-25 14:34:01.000000000 +1000
@@ -60,22 +60,21 @@
 			reg = <7000 400>;
 		};
 
-		mdio@6000 {
+		MDIO: mdio@6000 {
 			device_type = "mdio";
 			compatible = "tsi-ethernet";
+			reg = <6000 50>;
+			#address-cells = <1>;
+			#size-cells = <0>;
 
-			PHY1: ethernet-phy@6000 {
-				device_type = "ethernet-phy";
-				compatible = "bcm54xx";
-				reg = <6000 50>;
-				phy-id = <1>;
+			PHY1: ethernet-phy@1 {
+				reg = <1>;
+				txc-rxc-delay-disable;
 			};
 
-			PHY2: ethernet-phy@6400 {
-				device_type = "ethernet-phy";
-				compatible = "bcm54xx";
-				reg = <6000 50>;
-				phy-id = <2>;
+			PHY2: ethernet-phy@2 {
+				reg = <2>;
+				txc-rxc-delay-disable;
 			};
 		};
 
@@ -88,6 +87,7 @@
 			local-mac-address = [ 00 00 00 00 00 00 ];
 			interrupt-parent = < &/tsi109@c0000000/pic@7400 >;
 			interrupts = <10 2>;
+			mdio-handle = <&MDIO>;
 			phy-handle = <&PHY1>;
 		};
 
@@ -100,6 +100,7 @@
 			local-mac-address = [ 00 00 00 00 00 00 ];
 			interrupt-parent = < &/tsi109@c0000000/pic@7400 >;
 			interrupts = <11 2>;
+			mdio-handle = <&MDIO>;
 			phy-handle = <&PHY2>;
 		};
 
Index: working-2.6/arch/powerpc/sysdev/tsi108_dev.c
===================================================================
--- working-2.6.orig/arch/powerpc/sysdev/tsi108_dev.c	2007-05-25 14:34:00.000000000 +1000
+++ working-2.6/arch/powerpc/sysdev/tsi108_dev.c	2007-05-25 14:36:48.000000000 +1000
@@ -75,9 +75,8 @@ static int __init tsi108_eth_of_init(voi
 	     (np = of_find_compatible_node(np, "network", "tsi-ethernet")) != NULL;
 	     i++) {
 		struct resource r[2];
-		struct device_node *phy;
+		struct device_node *phy, *mdio;
 		hw_info tsi_eth_data;
-		const unsigned int *id;
 		const unsigned int *phy_id;
 		const void *mac_addr;
 		const phandle *ph;
@@ -111,6 +110,13 @@ static int __init tsi108_eth_of_init(voi
 		if (mac_addr)
 			memcpy(tsi_eth_data.mac_addr, mac_addr, 6);
 
+		ph = of_get_property(np, "mdio-handle", NULL);
+		mdio = of_find_node_by_phandle(*ph);
+		ret = of_address_to_resource(mdio, 0, &res);
+		of_node_put(mdio);
+		if (ret)
+			goto unreg;
+
 		ph = of_get_property(np, "phy-handle", NULL);
 		phy = of_find_node_by_phandle(*ph);
 
@@ -119,20 +125,17 @@ static int __init tsi108_eth_of_init(voi
 			goto unreg;
 		}
 
-		id = of_get_property(phy, "reg", NULL);
-		phy_id = of_get_property(phy, "phy-id", NULL);
-		ret = of_address_to_resource(phy, 0, &res);
-		if (ret) {
-			of_node_put(phy);
-			goto unreg;
-		}
+		phy_id = of_get_property(phy, "reg", NULL);
+
 		tsi_eth_data.regs = r[0].start;
 		tsi_eth_data.phyregs = res.start;
 		tsi_eth_data.phy = *phy_id;
 		tsi_eth_data.irq_num = irq_of_parse_and_map(np, 0);
-		if (of_device_is_compatible(phy, "bcm54xx"))
+
+		if (of_get_property(phy, "txc-rxc-delay-disable"), NULL)
 			tsi_eth_data.phy_type = TSI108_PHY_BCM54XX;
 		of_node_put(phy);
+
 		ret =
 		    platform_device_add_data(tsi_eth_dev, &tsi_eth_data,
 					     sizeof(hw_info));
Index: working-2.6/arch/powerpc/boot/dts/mpc7448hpc2.dts
===================================================================
--- working-2.6.orig/arch/powerpc/boot/dts/mpc7448hpc2.dts	2007-05-25 14:34:00.000000000 +1000
+++ working-2.6/arch/powerpc/boot/dts/mpc7448hpc2.dts	2007-05-25 14:34:01.000000000 +1000
@@ -58,24 +58,23 @@
 			compatible  = "tsi-i2c";
 		};
 
-		mdio@6000 {
+		MDIO: mdio@6000 {
 			device_type = "mdio";
 			compatible = "tsi-ethernet";
+			reg = <6000 50>;
+			#address-cells = <1>;
+			#size-cells = <0>;
 
-			phy8: ethernet-phy@6000 {
+			phy8: ethernet-phy@8 {
 				interrupt-parent = <&mpic>;
 				interrupts = <2 1>;
-				reg = <6000 50>;
-				phy-id = <8>;
-				device_type = "ethernet-phy";
+				reg = <8>;
 			};
 
-			phy9: ethernet-phy@6400 {
+			phy9: ethernet-phy@9 {
 				interrupt-parent = <&mpic>;
 				interrupts = <2 1>;
-				reg = <6000 50>;
-				phy-id = <9>;
-				device_type = "ethernet-phy";
+				reg = <9>;
 			};
 
 		};
@@ -89,6 +88,7 @@
 			address = [ 00 06 D2 00 00 01 ];
 			interrupts = <10 2>;
 			interrupt-parent = <&mpic>;
+			mdio-handle = <&MDIO>;
 			phy-handle = <&phy8>;
 		};
 
@@ -102,6 +102,7 @@
 			address = [ 00 06 D2 00 00 02 ];
 			interrupts = <11 2>;
 			interrupt-parent = <&mpic>;
+			mdio-handle = <&MDIO>;
 			phy-handle = <&phy9>;
 		};
 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: David Gibson <hidden>
Date: 2007-05-25 04:47:49

On Fri, May 25, 2007 at 02:37:28PM +1000, David Gibson wrote:
On Fri, May 25, 2007 at 02:23:59PM +1000, David Gibson wrote:
quoted
On Thu, May 24, 2007 at 08:44:20AM -0500, Josh Boyer wrote:
quoted
On Wed, 2007-05-23 at 23:22 -0500, Kumar Gala wrote:
quoted
On May 23, 2007, at 11:16 PM, David Gibson wrote:
quoted
This patch fixes some problems with the way the Ethernet PHYs are
represented in the device tree for the Holly board.  This means
changes to the dts itself, and to the code with instantiates the
tsi108 ethernet platform devices based on the device tree.

	- First, and most importantly, the PHYs are given with an
identical 'reg' property.  This reg currently encodes the accessible
register used to initiate mdio interaction with the PHYs, rather than
a meaningful address on the parent bus (mdio in this case), which is
incorrect.  Instead we give the address of these registers as 'reg' in
the mdio node itself, and encode the ID of each phy in their 'reg'
propertys.  The PHY's unit name addresses are updated to match.
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).
Is the compatible really need here?  We are able to provide and bind  
drivers based on MII_PHYSID1/2.  I don't see what putting the  
compatible proper gets us.
I think it's needed until the TSI driver switches to phylib.  Right now,
it assumes that a Marvell phy is used, which is what is present on the
Taiga MPC7448HPC2 board.  The Holly board uses the Broadcom phy and we
added the compatible code for that.
I think Ben's suggested approach of using a special property in the
phy node to indicate that this workaround is necessary is better.
After all, the workaround isn't actually related to the type of PHY,
which is what compatible encodes, but to how it's wired up.  Revised
patch below addressing this and other matters.
Grah! Forgot quilt ref.  Now a copy that might actually work.
Waah!  Third time lucky?

Fix problems with Holly's DT representation of ethernet PHYs

This patch fixes some problems with the way the Ethernet PHYs are
represented in the device tree for the Holly board.  This means
changes to the dts itself, and to the code with instantiates the
tsi108 ethernet platform devices based on the device tree.

	- First, and most importantly, the PHYs are given with an
identical 'reg' property.  This reg currently encodes the accessible
register used to initiate mdio interaction with the PHYs, rather than
a meaningful address on the parent bus (mdio in this case), which is
incorrect.  Instead we give the address of these registers as 'reg' in
the mdio node itself, and encode the ID of each phy in their 'reg'
propertys.  The PHY's unit name addresses are updated to match.
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).

Signed-off-by: David Gibson <redacted>

Index: working-2.6/arch/powerpc/boot/dts/holly.dts
===================================================================
--- working-2.6.orig/arch/powerpc/boot/dts/holly.dts	2007-05-25 14:34:00.000000000 +1000
+++ working-2.6/arch/powerpc/boot/dts/holly.dts	2007-05-25 14:34:01.000000000 +1000
@@ -60,22 +60,21 @@
 			reg = <7000 400>;
 		};
 
-		mdio@6000 {
+		MDIO: mdio@6000 {
 			device_type = "mdio";
 			compatible = "tsi-ethernet";
+			reg = <6000 50>;
+			#address-cells = <1>;
+			#size-cells = <0>;
 
-			PHY1: ethernet-phy@6000 {
-				device_type = "ethernet-phy";
-				compatible = "bcm54xx";
-				reg = <6000 50>;
-				phy-id = <1>;
+			PHY1: ethernet-phy@1 {
+				reg = <1>;
+				txc-rxc-delay-disable;
 			};
 
-			PHY2: ethernet-phy@6400 {
-				device_type = "ethernet-phy";
-				compatible = "bcm54xx";
-				reg = <6000 50>;
-				phy-id = <2>;
+			PHY2: ethernet-phy@2 {
+				reg = <2>;
+				txc-rxc-delay-disable;
 			};
 		};
 
@@ -88,6 +87,7 @@
 			local-mac-address = [ 00 00 00 00 00 00 ];
 			interrupt-parent = < &/tsi109@c0000000/pic@7400 >;
 			interrupts = <10 2>;
+			mdio-handle = <&MDIO>;
 			phy-handle = <&PHY1>;
 		};
 
@@ -100,6 +100,7 @@
 			local-mac-address = [ 00 00 00 00 00 00 ];
 			interrupt-parent = < &/tsi109@c0000000/pic@7400 >;
 			interrupts = <11 2>;
+			mdio-handle = <&MDIO>;
 			phy-handle = <&PHY2>;
 		};
 
Index: working-2.6/arch/powerpc/sysdev/tsi108_dev.c
===================================================================
--- working-2.6.orig/arch/powerpc/sysdev/tsi108_dev.c	2007-05-25 14:34:00.000000000 +1000
+++ working-2.6/arch/powerpc/sysdev/tsi108_dev.c	2007-05-25 14:38:32.000000000 +1000
@@ -75,9 +75,8 @@ static int __init tsi108_eth_of_init(voi
 	     (np = of_find_compatible_node(np, "network", "tsi-ethernet")) != NULL;
 	     i++) {
 		struct resource r[2];
-		struct device_node *phy;
+		struct device_node *phy, *mdio;
 		hw_info tsi_eth_data;
-		const unsigned int *id;
 		const unsigned int *phy_id;
 		const void *mac_addr;
 		const phandle *ph;
@@ -111,6 +110,13 @@ static int __init tsi108_eth_of_init(voi
 		if (mac_addr)
 			memcpy(tsi_eth_data.mac_addr, mac_addr, 6);
 
+		ph = of_get_property(np, "mdio-handle", NULL);
+		mdio = of_find_node_by_phandle(*ph);
+		ret = of_address_to_resource(mdio, 0, &res);
+		of_node_put(mdio);
+		if (ret)
+			goto unreg;
+
 		ph = of_get_property(np, "phy-handle", NULL);
 		phy = of_find_node_by_phandle(*ph);
 
@@ -119,20 +125,17 @@ static int __init tsi108_eth_of_init(voi
 			goto unreg;
 		}
 
-		id = of_get_property(phy, "reg", NULL);
-		phy_id = of_get_property(phy, "phy-id", NULL);
-		ret = of_address_to_resource(phy, 0, &res);
-		if (ret) {
-			of_node_put(phy);
-			goto unreg;
-		}
+		phy_id = of_get_property(phy, "reg", NULL);
+
 		tsi_eth_data.regs = r[0].start;
 		tsi_eth_data.phyregs = res.start;
 		tsi_eth_data.phy = *phy_id;
 		tsi_eth_data.irq_num = irq_of_parse_and_map(np, 0);
-		if (of_device_is_compatible(phy, "bcm54xx"))
+
+		if (of_get_property(phy, "txc-rxc-delay-disable", NULL))
 			tsi_eth_data.phy_type = TSI108_PHY_BCM54XX;
 		of_node_put(phy);
+
 		ret =
 		    platform_device_add_data(tsi_eth_dev, &tsi_eth_data,
 					     sizeof(hw_info));
Index: working-2.6/arch/powerpc/boot/dts/mpc7448hpc2.dts
===================================================================
--- working-2.6.orig/arch/powerpc/boot/dts/mpc7448hpc2.dts	2007-05-25 14:34:00.000000000 +1000
+++ working-2.6/arch/powerpc/boot/dts/mpc7448hpc2.dts	2007-05-25 14:34:01.000000000 +1000
@@ -58,24 +58,23 @@
 			compatible  = "tsi-i2c";
 		};
 
-		mdio@6000 {
+		MDIO: mdio@6000 {
 			device_type = "mdio";
 			compatible = "tsi-ethernet";
+			reg = <6000 50>;
+			#address-cells = <1>;
+			#size-cells = <0>;
 
-			phy8: ethernet-phy@6000 {
+			phy8: ethernet-phy@8 {
 				interrupt-parent = <&mpic>;
 				interrupts = <2 1>;
-				reg = <6000 50>;
-				phy-id = <8>;
-				device_type = "ethernet-phy";
+				reg = <8>;
 			};
 
-			phy9: ethernet-phy@6400 {
+			phy9: ethernet-phy@9 {
 				interrupt-parent = <&mpic>;
 				interrupts = <2 1>;
-				reg = <6000 50>;
-				phy-id = <9>;
-				device_type = "ethernet-phy";
+				reg = <9>;
 			};
 
 		};
@@ -89,6 +88,7 @@
 			address = [ 00 06 D2 00 00 01 ];
 			interrupts = <10 2>;
 			interrupt-parent = <&mpic>;
+			mdio-handle = <&MDIO>;
 			phy-handle = <&phy8>;
 		};
 
@@ -102,6 +102,7 @@
 			address = [ 00 06 D2 00 00 02 ];
 			interrupts = <11 2>;
 			interrupt-parent = <&mpic>;
+			mdio-handle = <&MDIO>;
 			phy-handle = <&phy9>;
 		};
 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: Josh Boyer <hidden>
Date: 2007-05-25 14:05:04

On Fri, 2007-05-25 at 14:38 +1000, David Gibson wrote:
Waah!  Third time lucky?
Not quite. :)
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).
You don't actually do this.  Instead, you specify a
txc-rxc-delay-disable property.
-			PHY1: ethernet-phy@6000 {
-				device_type = "ethernet-phy";
-				compatible = "bcm54xx";
-				reg = <6000 50>;
-				phy-id = <1>;
+			PHY1: ethernet-phy@1 {
+				reg = <1>;
+				txc-rxc-delay-disable;
 			};
I would have rather we left the compatible = "bmc5461A" as well.  Though
perhaps a comment would suffice instead.
quoted hunk
Index: working-2.6/arch/powerpc/sysdev/tsi108_dev.c
===================================================================
--- working-2.6.orig/arch/powerpc/sysdev/tsi108_dev.c	2007-05-25 14:34:00.000000000 +1000
+++ working-2.6/arch/powerpc/sysdev/tsi108_dev.c	2007-05-25 14:38:32.000000000 +1000
+
 		tsi_eth_data.regs = r[0].start;
 		tsi_eth_data.phyregs = res.start;
 		tsi_eth_data.phy = *phy_id;
 		tsi_eth_data.irq_num = irq_of_parse_and_map(np, 0);
-		if (of_device_is_compatible(phy, "bcm54xx"))
+
+		if (of_get_property(phy, "txc-rxc-delay-disable", NULL))
 			tsi_eth_data.phy_type = TSI108_PHY_BCM54XX;
 		of_node_put(phy);
At the very least this needs a comment explaining what exactly is being
done here.  Right now, it's looking for some magical property and
setting the PHY type to a Broadcom PHY...  very confusing to someone
that hasn't followed the email thread.

josh

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: Segher Boessenkool <hidden>
Date: 2007-05-25 14:12:06

Waah!  Third time lucky?
You wish :-)
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).
You completely removed the "compatible" properties instead.
Bad idea.


Segher

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: David Gibson <hidden>
Date: 2007-05-27 23:30:37

On Fri, May 25, 2007 at 04:11:59PM +0200, Segher Boessenkool wrote:
quoted
Waah!  Third time lucky?
You wish :-)
quoted
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).
You completely removed the "compatible" properties instead.
Bad idea.
Um... weren't you the one that was just saying compatible properties
aren't necessary if you can distinguish the hardware in other ways?

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: David Gibson <hidden>
Date: 2007-05-27 23:36:43

On Fri, May 25, 2007 at 09:03:51AM -0500, Josh Boyer wrote:
On Fri, 2007-05-25 at 14:38 +1000, David Gibson wrote:
quoted
Waah!  Third time lucky?
Not quite. :)
quoted
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).
You don't actually do this.  Instead, you specify a
txc-rxc-delay-disable property.
Oops, forgot to update the patch description.
quoted
-			PHY1: ethernet-phy@6000 {
-				device_type = "ethernet-phy";
-				compatible = "bcm54xx";
-				reg = <6000 50>;
-				phy-id = <1>;
+			PHY1: ethernet-phy@1 {
+				reg = <1>;
+				txc-rxc-delay-disable;
 			};
I would have rather we left the compatible = "bmc5461A" as well.  Though
perhaps a comment would suffice instead.
Hrm, I was tending to go with Kumar's point that it's kind of simpler
and safer to probe the PHY type directly.
quoted
Index: working-2.6/arch/powerpc/sysdev/tsi108_dev.c
===================================================================
--- working-2.6.orig/arch/powerpc/sysdev/tsi108_dev.c	2007-05-25 14:34:00.000000000 +1000
+++ working-2.6/arch/powerpc/sysdev/tsi108_dev.c	2007-05-25 14:38:32.000000000 +1000
quoted
+
 		tsi_eth_data.regs = r[0].start;
 		tsi_eth_data.phyregs = res.start;
 		tsi_eth_data.phy = *phy_id;
 		tsi_eth_data.irq_num = irq_of_parse_and_map(np, 0);
-		if (of_device_is_compatible(phy, "bcm54xx"))
+
+		if (of_get_property(phy, "txc-rxc-delay-disable", NULL))
 			tsi_eth_data.phy_type = TSI108_PHY_BCM54XX;
 		of_node_put(phy);
At the very least this needs a comment explaining what exactly is being
done here.  Right now, it's looking for some magical property and
setting the PHY type to a Broadcom PHY...  very confusing to someone
that hasn't followed the email thread.
Yeah, good point.  And more specifically I should put a FIXME comment
in, saying that the ethernet driver itself should be changed to
implement this workaround in a different way, rather than based on
this phy_type field, since the workaround isn't really related to the
PHY type.  I was assuming that cleanup would happen as part of the
port to phylib.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2007-05-28 01:38:49

On Mon, 2007-05-28 at 09:30 +1000, David Gibson wrote:
On Fri, May 25, 2007 at 04:11:59PM +0200, Segher Boessenkool wrote:
quoted
quoted
Waah!  Third time lucky?
You wish :-)
quoted
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).
You completely removed the "compatible" properties instead.
Bad idea.
Um... weren't you the one that was just saying compatible properties
aren't necessary if you can distinguish the hardware in other ways?
In that case however, the 54xx are fairly different from each other, so
please use something more specific.

Ben.

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: Segher Boessenkool <hidden>
Date: 2007-05-28 11:06:40

quoted
quoted
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).
You completely removed the "compatible" properties instead.
Bad idea.
Um... weren't you the one that was just saying compatible properties
aren't necessary if you can distinguish the hardware in other ways?
The OS device driver doesn't need "compatible" if it
can probe the device some other way; it doesn't need
the device node at all, even.  You still should have
a "compatible" property (or, old style, a specific
"name" property) if you want the OS to be able to use
the device node to recognise the device (i.e., if a
device node for the device exists at all: always).


Segher

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: Segher Boessenkool <hidden>
Date: 2007-05-28 11:08:01

In that case however, the 54xx are fairly different from each other, so
please use something more specific.
_Never_ use the "xx" stuff, it's meaningless.


Segher

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2007-05-28 11:15:52

On Mon, 2007-05-28 at 13:07 +0200, Segher Boessenkool wrote:
quoted
In that case however, the 54xx are fairly different from each other, so
please use something more specific.
_Never_ use the "xx" stuff, it's meaningless.
Pretty much yeah... either you are compatible with a precise version
(the first one of the serie, like an hypothetical 5400) or you are not
in which case you have your precise model name in there.

Ben.

Re: Fix problems with Holly's DT representation of ethernet PHYs

From: David Gibson <hidden>
Date: 2007-05-29 04:49:17

On Mon, May 28, 2007 at 01:06:11PM +0200, Segher Boessenkool wrote:
quoted
quoted
quoted
	- Second, the PHYs give only "bcm54xx" as a compatible
property.  This is unfortunate, because there are many bcm54xx PHY
models, and they have differences which can matter.  We add a more
precise compatible string, giving the precise PHY model (bcm5461A in
this case).
You completely removed the "compatible" properties instead.
Bad idea.
Um... weren't you the one that was just saying compatible properties
aren't necessary if you can distinguish the hardware in other ways?
The OS device driver doesn't need "compatible" if it
can probe the device some other way; it doesn't need
the device node at all, even.  You still should have
a "compatible" property (or, old style, a specific
"name" property) if you want the OS to be able to use
the device node to recognise the device (i.e., if a
device node for the device exists at all: always).
Hrm.  Ok.  compatible property restored.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help