[PATCH] Revert "net: fsl_pq_mdio: fix non tbi phy access"

[PATCH] Revert "net: fsl_pq_mdio: fix non tbi phy access"

[Andy Fleming]

This reverts commit c3e072f8a6c5625028531c40ec65f7e301531be2.

The TBI PHY Address must always be set to something sensible.
If not, the value currently in the register may interfere
with MDIO transactions to that address. The architected
solution is to have a TBI node in the device tree, which corresponds
to the desired address (and which should be chosen so that it does
not conflict with other PHYs on the bus). If that node is not there,
it is incorrect to just continue. We must return an error, so that
developers have a chance to realize they've mis-configured their
device trees. A separate patch has been submitted to add such
a node to the device trees for boards which were missing that node.

Signed-off-by: Andy Fleming <afleming@freescale.com>
---
 drivers/net/ethernet/freescale/fsl_pq_mdio.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fsl_pq_mdio.c b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
index f109602..6ff124c 100644
--- a/drivers/net/ethernet/freescale/fsl_pq_mdio.c
+++ b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
@@ -356,16 +356,16 @@ static int fsl_pq_mdio_probe(struct platform_device *ofdev)
 
 		if (prop)
 			tbiaddr = *prop;
+	}
 
-		if (tbiaddr == -1) {
-			err = -EBUSY;
+	if (tbiaddr == -1) {
+		err = -EBUSY;
 
-			goto err_free_irqs;
-		} else {
-			out_be32(tbipa, tbiaddr);
-		}
+		goto err_free_irqs;
 	}
 
+	out_be32(tbipa, tbiaddr);
+
 	err = of_mdiobus_register(new_bus, np);
 	if (err) {
 		printk (KERN_ERR "%s: Cannot register as MDIO bus\n",
-- 
1.7.3.4

[PATCH] pq3: Add default tbi address

[Andy Fleming]

The MDIO driver has been changed so that it no longer supports
scanning the MDIO bus for a free address for the TBI PHY. This
feature was fragile, and required scanning the bus before the bus
was fully up and registered. The intended way for FSL devices to
specify the TBI PHY's address is via a tbi node in the device tree.
All of the device trees had such a node, except for the recent p1/p2
trees. Rather than hand-fixing all of those boards, set a default
value (most boards were using 0x11, anyway), and any board which
wants to change it from that default can then override it in its
board dts file.

This fixes an issue where p1/p2 boards would fail to bring up
Ethernet, due to not finding a tbi node.

Signed-off-by: Andy Fleming <afleming@freescale.com>
---
 arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi |    5 +++++
 arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi |    5 +++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi b/arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi
index a1979ae..0a42e21 100644
--- a/arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi
+++ b/arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi
@@ -50,4 +50,9 @@ mdio@24520 {
 	#size-cells = <0>;
 	compatible = "fsl,gianfar-mdio";
 	reg = <0x24520 0x20>;
+
+	tbi-phy@11 {
+		device-type = "tbi-phy";
+		reg = <0x11>;
+	};
 };
diff --git a/arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi b/arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi
index 1382fec..964670a 100644
--- a/arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi
+++ b/arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi
@@ -38,6 +38,11 @@ mdio@24000 {
 	#size-cells = <0>;
 	compatible = "fsl,etsec2-mdio";
 	reg = <0x24000 0x1000 0xb0030 0x4>;
+
+	tbi-phy@11 {
+		device-type = "tbi-phy";
+		reg = <0x11>;
+	};
 };
 
 ethernet@b0000 {
-- 
1.7.3.4

Re: [PATCH] pq3: Add default tbi address

[Kumar Gala]

On Dec 2, 2011, at 1:05 AM, Andy Fleming wrote:

> The MDIO driver has been changed so that it no longer supports
> scanning the MDIO bus for a free address for the TBI PHY. This
> feature was fragile, and required scanning the bus before the bus
> was fully up and registered. The intended way for FSL devices to
> specify the TBI PHY's address is via a tbi node in the device tree.
> All of the device trees had such a node, except for the recent p1/p2
> trees. Rather than hand-fixing all of those boards, set a default
> value (most boards were using 0x11, anyway), and any board which
> wants to change it from that default can then override it in its
> board dts file.
>=20
> This fixes an issue where p1/p2 boards would fail to bring up
> Ethernet, due to not finding a tbi node.

Is this only needed on 1st controller because its what has external PHY =
control?

> Signed-off-by: Andy Fleming <afleming@freescale.com>
> ---
> arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi |    5 +++++
> arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi |    5 +++++
> 2 files changed, 10 insertions(+), 0 deletions(-)

This doesn't seem correct, meaning this should really be in the board =
.dts not in the IP.

I think the driver should check and warn if this property doesn't exist.

>=20
> diff --git a/arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi =
b/arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi
> index a1979ae..0a42e21 100644
> --- a/arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi
> @@ -50,4 +50,9 @@ mdio@24520 {
> 	#size-cells =3D <0>;
> 	compatible =3D "fsl,gianfar-mdio";
> 	reg =3D <0x24520 0x20>;
> +
> +	tbi-phy@11 {
> +		device-type =3D "tbi-phy";
> +		reg =3D <0x11>;
> +	};
> };
> diff --git a/arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi =
b/arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi
> index 1382fec..964670a 100644
> --- a/arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi
> @@ -38,6 +38,11 @@ mdio@24000 {
> 	#size-cells =3D <0>;
> 	compatible =3D "fsl,etsec2-mdio";
> 	reg =3D <0x24000 0x1000 0xb0030 0x4>;
> +
> +	tbi-phy@11 {
> +		device-type =3D "tbi-phy";
> +		reg =3D <0x11>;
> +	};
> };
>=20
> ethernet@b0000 {
> --=20
> 1.7.3.4
>=20

Re: [PATCH] pq3: Add default tbi address

[Andy Fleming]

>> arch/powerpc/boot/dts/fsl/pq3-etsec1-0.dtsi | =A0 =A05 +++++
>> arch/powerpc/boot/dts/fsl/pq3-etsec2-0.dtsi | =A0 =A05 +++++
>> 2 files changed, 10 insertions(+), 0 deletions(-)
>
> This doesn't seem correct, meaning this should really be in the board .dt=
s not in the IP.
>
> I think the driver should check and warn if this property doesn't exist.

Hmm... in principle, I agree with you. I suppose I was just being
lazy, and avoiding changing all the device trees again. However, it's
not an unreasonable default. A search through *all* of the device
trees for the tbi node indicates that 0x11 is chosen about 50% of the
time.

However, I'll devise a patch which complains if the node isn't found,
and another patch which modifies all of the offending device trees.

Andy