[PATCH 1/4] dts/mpc8569mds: Cleanup tbi phy to support rtbi

[PATCH 1/4] dts/mpc8569mds: Cleanup tbi phy to support rtbi

[Liu Yu]

After these change, when need to work in rtbi mode,
just change phy-connection-type to "rtbi".

Also, this work can be done by u-boot.

Signed-off-by: Liu Yu <yu.liu@freescale.com>
---
 arch/powerpc/boot/dts/mpc8569mds.dts |   47 ++++++++++++++++++++++++++++++---
 1 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8569mds.dts b/arch/powerpc/boot/dts/mpc8569mds.dts
index 795eb36..8b72eaf 100644
--- a/arch/powerpc/boot/dts/mpc8569mds.dts
+++ b/arch/powerpc/boot/dts/mpc8569mds.dts
@@ -535,6 +535,7 @@
 			rx-clock-name = "none";
 			tx-clock-name = "clk12";
 			pio-handle = <&pio1>;
+			tbi-handle = <&tbi1>;
 			phy-handle = <&qe_phy0>;
 			phy-connection-type = "rgmii-id";
 		};
@@ -579,7 +580,7 @@
 				reg = <0x6>;
 				device_type = "ethernet-phy";
 			};
-			tbi-phy@11 {
+			tbi1: tbi-phy@11 {
 				reg = <0x11>;
 				device_type = "tbi-phy";
 			};
@@ -590,7 +591,7 @@
 			reg = <0x3520 0x18>;
 			compatible = "fsl,ucc-mdio";
 
-			tbi0: tbi-phy@15 {
+			tbi6: tbi-phy@15 {
 			reg = <0x15>;
 			device_type = "tbi-phy";
 			};
@@ -600,7 +601,7 @@
 			#size-cells = <0>;
 			reg = <0x3720 0x38>;
 			compatible = "fsl,ucc-mdio";
-			tbi1: tbi-phy@17 {
+			tbi8: tbi-phy@17 {
 				reg = <0x17>;
 				device_type = "tbi-phy";
 			};
@@ -617,10 +618,22 @@
 			rx-clock-name = "none";
 			tx-clock-name = "clk12";
 			pio-handle = <&pio3>;
+			tbi-handle = <&tbi3>;
 			phy-handle = <&qe_phy2>;
 			phy-connection-type = "rgmii-id";
 		};
 
+		mdio@2320 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x2320 0x18>;
+			compatible = "fsl,ucc-mdio";
+			tbi3: tbi-phy@11 {
+				reg = <0x11>;
+				device_type = "tbi-phy";
+			};
+		};
+
 		enet1: ucc@3000 {
 			device_type = "network";
 			compatible = "ucc_geth";
@@ -632,10 +645,22 @@
 			rx-clock-name = "none";
 			tx-clock-name = "clk17";
 			pio-handle = <&pio2>;
+			tbi-handle = <&tbi2>;
 			phy-handle = <&qe_phy1>;
 			phy-connection-type = "rgmii-id";
 		};
 
+		mdio@3120 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x3120 0x18>;
+			compatible = "fsl,ucc-mdio";
+			tbi2: tbi-phy@11 {
+				reg = <0x11>;
+				device_type = "tbi-phy";
+			};
+		};
+
 		enet3: ucc@3200 {
 			device_type = "network";
 			compatible = "ucc_geth";
@@ -647,10 +672,22 @@
 			rx-clock-name = "none";
 			tx-clock-name = "clk17";
 			pio-handle = <&pio4>;
+			tbi-handle = <&tbi4>;
 			phy-handle = <&qe_phy3>;
 			phy-connection-type = "rgmii-id";
 		};
 
+		mdio@3320 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x3320 0x18>;
+			compatible = "fsl,ucc-mdio";
+			tbi4: tbi-phy@11 {
+				reg = <0x11>;
+				device_type = "tbi-phy";
+			};
+		};
+
 		enet5: ucc@3400 {
 			device_type = "network";
 			compatible = "ucc_geth";
@@ -661,7 +698,7 @@
 			local-mac-address = [ 00 00 00 00 00 00 ];
 			rx-clock-name = "none";
 			tx-clock-name = "none";
-			tbi-handle = <&tbi0>;
+			tbi-handle = <&tbi6>;
 			phy-handle = <&qe_phy5>;
 			phy-connection-type = "sgmii";
 		};
@@ -676,7 +713,7 @@
 			local-mac-address = [ 00 00 00 00 00 00 ];
 			rx-clock-name = "none";
 			tx-clock-name = "none";
-			tbi-handle = <&tbi1>;
+			tbi-handle = <&tbi8>;
 			phy-handle = <&qe_phy7>;
 			phy-connection-type = "sgmii";
 		};
-- 
1.6.4

[PATCH 2/4] mpc8569mds: Add bscr setting for rtbi mode

[Liu Yu]

Signed-off-by: Liu Yu <yu.liu@freescale.com>
---
 arch/powerpc/platforms/85xx/mpc85xx_mds.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
index c5028a2..0872e4a 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
@@ -237,6 +237,8 @@ static void __init mpc85xx_mds_setup_arch(void)
 		} else if (machine_is(mpc8569_mds)) {
 #define BCSR7_UCC12_GETHnRST	(0x1 << 2)
 #define BCSR8_UEM_MARVELL_RST	(0x1 << 1)
+#define BCSR_UCC_RGMII		(0x1 << 6)
+#define BCSR_UCC_RTBI		(0x1 << 5)
 			/*
 			 * U-Boot mangles interrupt polarity for Marvell PHYs,
 			 * so reset built-in and UEM Marvell PHYs, this puts
@@ -247,6 +249,28 @@ static void __init mpc85xx_mds_setup_arch(void)
 
 			setbits8(&bcsr_regs[7], BCSR7_UCC12_GETHnRST);
 			clrbits8(&bcsr_regs[8], BCSR8_UEM_MARVELL_RST);
+
+			for (np = NULL; (np = of_find_compatible_node(np,
+							"network",
+							"ucc_geth")) != NULL;) {
+				const unsigned int *prop;
+				int ucc_num;
+
+				prop = of_get_property(np, "cell-index", NULL);
+				if (prop == NULL)
+					continue;
+
+				ucc_num = *prop - 1;
+
+				prop = of_get_property(np, "phy-connection-type", NULL);
+				if (prop == NULL)
+					continue;
+
+				if (strcmp("rtbi", (const char *)prop) == 0)
+					clrsetbits_8(&bcsr_regs[7 + ucc_num],
+						BCSR_UCC_RGMII, BCSR_UCC_RTBI);
+			}
+
 		}
 		iounmap(bcsr_regs);
 	}
-- 
1.6.4

[PATCH 3/4] ucc_geth: update the tbi-phy setting

[Liu Yu]

Old method only set tbi-phy for eth0.

Signed-off-by: Liu Yu <yu.liu@freescale.com>
---
 drivers/net/ucc_geth.c |   23 +++++++++++++++--------
 1 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 750d86d..0e72013 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -37,6 +37,7 @@
 #include <asm/qe.h>
 #include <asm/ucc.h>
 #include <asm/ucc_fast.h>
+#include <asm/machdep.h>
 
 #include "ucc_geth.h"
 #include "fsl_pq_mdio.h"
@@ -1334,7 +1335,7 @@ static int adjust_enet_interface(struct ucc_geth_private *ugeth)
 	struct ucc_geth __iomem *ug_regs;
 	struct ucc_fast __iomem *uf_regs;
 	int ret_val;
-	u32 upsmr, maccfg2, tbiBaseAddress;
+	u32 upsmr, maccfg2;
 	u16 value;
 
 	ugeth_vdbg("%s: IN", __func__);
@@ -1389,14 +1390,20 @@ static int adjust_enet_interface(struct ucc_geth_private *ugeth)
 	/* Note that this depends on proper setting in utbipar register. */
 	if ((ugeth->phy_interface == PHY_INTERFACE_MODE_TBI) ||
 	    (ugeth->phy_interface == PHY_INTERFACE_MODE_RTBI)) {
-		tbiBaseAddress = in_be32(&ug_regs->utbipar);
-		tbiBaseAddress &= UTBIPAR_PHY_ADDRESS_MASK;
-		tbiBaseAddress >>= UTBIPAR_PHY_ADDRESS_SHIFT;
-		value = ugeth->phydev->bus->read(ugeth->phydev->bus,
-				(u8) tbiBaseAddress, ENET_TBI_MII_CR);
+		struct ucc_geth_info *ug_info = ugeth->ug_info;
+		struct phy_device *tbiphy;
+
+		if (!ug_info->tbi_node)
+			ugeth_warn("TBI mode requires that the device "
+				"tree specify a tbi-handle\n");
+
+		tbiphy = of_phy_find_device(ug_info->tbi_node);
+		if (!tbiphy)
+			ugeth_warn("Could not get TBI device\n");
+
+		value = phy_read(tbiphy, ENET_TBI_MII_CR);
 		value &= ~0x1000;	/* Turn off autonegotiation */
-		ugeth->phydev->bus->write(ugeth->phydev->bus,
-				(u8) tbiBaseAddress, ENET_TBI_MII_CR, value);
+		phy_write(tbiphy, ENET_TBI_MII_CR, value);
 	}
 
 	init_check_frame_length_mode(ug_info->lengthCheckRx, &ug_regs->maccfg2);
-- 
1.6.4

[PATCH 4/4] phy: add RTBI mode for m88e1111

[Liu Yu]

Signed-off-by: Liu Yu <yu.liu@freescale.com>
---
 drivers/net/phy/marvell.c |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 6f69b9b..65ed385 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -63,6 +63,7 @@
 #define MII_M1111_HWCFG_MODE_COPPER_RGMII	0xb
 #define MII_M1111_HWCFG_MODE_FIBER_RGMII	0x3
 #define MII_M1111_HWCFG_MODE_SGMII_NO_CLK	0x4
+#define MII_M1111_HWCFG_MODE_COPPER_RTBI	0x9
 #define MII_M1111_HWCFG_FIBER_COPPER_AUTO	0x8000
 #define MII_M1111_HWCFG_FIBER_COPPER_RES	0x2000
 
@@ -269,6 +270,43 @@ static int m88e1111_config_init(struct phy_device *phydev)
 			return err;
 	}
 
+	if (phydev->interface == PHY_INTERFACE_MODE_RTBI) {
+		temp = phy_read(phydev, MII_M1111_PHY_EXT_CR);
+		if (temp < 0)
+			return temp;
+		temp |= (MII_M1111_RX_DELAY | MII_M1111_TX_DELAY);
+		err = phy_write(phydev, MII_M1111_PHY_EXT_CR, temp);
+		if (err < 0)
+			return err;
+
+		temp = phy_read(phydev, MII_M1111_PHY_EXT_SR);
+		if (temp < 0)
+			return temp;
+		temp &= ~(MII_M1111_HWCFG_MODE_MASK | MII_M1111_HWCFG_FIBER_COPPER_RES);
+		temp |= 0x7 | MII_M1111_HWCFG_FIBER_COPPER_AUTO;
+		err = phy_write(phydev, MII_M1111_PHY_EXT_SR, temp);
+		if (err < 0)
+			return err;
+
+		/* soft reset */
+		err = phy_write(phydev, MII_BMCR, BMCR_RESET);
+		if (err < 0)
+			return err;
+		do
+			temp = phy_read(phydev, MII_BMCR);
+		while (temp & BMCR_RESET);
+
+		temp = phy_read(phydev, MII_M1111_PHY_EXT_SR);
+		if (temp < 0)
+			return temp;
+		temp &= ~(MII_M1111_HWCFG_MODE_MASK | MII_M1111_HWCFG_FIBER_COPPER_RES);
+		temp |= MII_M1111_HWCFG_MODE_COPPER_RTBI | MII_M1111_HWCFG_FIBER_COPPER_AUTO;
+		err = phy_write(phydev, MII_M1111_PHY_EXT_SR, temp);
+		if (err < 0)
+			return err;
+	}
+
+
 	err = phy_write(phydev, MII_BMCR, BMCR_RESET);
 	if (err < 0)
 		return err;
-- 
1.6.4

Re: [PATCH 4/4] phy: add RTBI mode for m88e1111

[Kumar Gala]

On Jan 14, 2010, at 2:13 AM, Liu Yu wrote:

> Signed-off-by: Liu Yu <yu.liu@freescale.com>
> ---
> drivers/net/phy/marvell.c |   38 =
++++++++++++++++++++++++++++++++++++++
> 1 files changed, 38 insertions(+), 0 deletions(-)
>=20
> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> index 6f69b9b..65ed385 100644
> --- a/drivers/net/phy/marvell.c
> +++ b/drivers/net/phy/marvell.c
> @@ -63,6 +63,7 @@
> #define MII_M1111_HWCFG_MODE_COPPER_RGMII	0xb
> #define MII_M1111_HWCFG_MODE_FIBER_RGMII	0x3
> #define MII_M1111_HWCFG_MODE_SGMII_NO_CLK	0x4
> +#define MII_M1111_HWCFG_MODE_COPPER_RTBI	0x9
> #define MII_M1111_HWCFG_FIBER_COPPER_AUTO	0x8000
> #define MII_M1111_HWCFG_FIBER_COPPER_RES	0x2000
>=20
> @@ -269,6 +270,43 @@ static int m88e1111_config_init(struct phy_device =
*phydev)
> 			return err;
> 	}
>=20
> +	if (phydev->interface =3D=3D PHY_INTERFACE_MODE_RTBI) {
> +		temp =3D phy_read(phydev, MII_M1111_PHY_EXT_CR);
> +		if (temp < 0)
> +			return temp;
> +		temp |=3D (MII_M1111_RX_DELAY | MII_M1111_TX_DELAY);
> +		err =3D phy_write(phydev, MII_M1111_PHY_EXT_CR, temp);
> +		if (err < 0)
> +			return err;
> +
> +		temp =3D phy_read(phydev, MII_M1111_PHY_EXT_SR);
> +		if (temp < 0)
> +			return temp;
> +		temp &=3D ~(MII_M1111_HWCFG_MODE_MASK | =
MII_M1111_HWCFG_FIBER_COPPER_RES);
> +		temp |=3D 0x7 | MII_M1111_HWCFG_FIBER_COPPER_AUTO;

Does this magic 0x7 have some meaning?

> +		err =3D phy_write(phydev, MII_M1111_PHY_EXT_SR, temp);
> +		if (err < 0)
> +			return err;
> +
> +		/* soft reset */
> +		err =3D phy_write(phydev, MII_BMCR, BMCR_RESET);
> +		if (err < 0)
> +			return err;
> +		do
> +			temp =3D phy_read(phydev, MII_BMCR);
> +		while (temp & BMCR_RESET);
> +
> +		temp =3D phy_read(phydev, MII_M1111_PHY_EXT_SR);
> +		if (temp < 0)
> +			return temp;
> +		temp &=3D ~(MII_M1111_HWCFG_MODE_MASK | =
MII_M1111_HWCFG_FIBER_COPPER_RES);
> +		temp |=3D MII_M1111_HWCFG_MODE_COPPER_RTBI | =
MII_M1111_HWCFG_FIBER_COPPER_AUTO;
> +		err =3D phy_write(phydev, MII_M1111_PHY_EXT_SR, temp);
> +		if (err < 0)
> +			return err;
> +	}
> +
> +
> 	err =3D phy_write(phydev, MII_BMCR, BMCR_RESET);
> 	if (err < 0)
> 		return err;
> --=20
> 1.6.4

RE: [PATCH 4/4] phy: add RTBI mode for m88e1111

[Liu Yu-B13201]

=20

> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]=20
> Sent: Friday, January 15, 2010 12:20 AM
> To: Liu Yu-B13201
> Cc: davem@davemloft.net; linuxppc-dev@lists.ozlabs.org;=20
> netdev@vger.kernel.org
> Subject: Re: [PATCH 4/4] phy: add RTBI mode for m88e1111
>=20
>=20
> On Jan 14, 2010, at 2:13 AM, Liu Yu wrote:
>=20
> > Signed-off-by: Liu Yu <yu.liu@freescale.com>
> > ---
> > drivers/net/phy/marvell.c |   38=20
> ++++++++++++++++++++++++++++++++++++++
> > 1 files changed, 38 insertions(+), 0 deletions(-)
> >=20
> > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> > index 6f69b9b..65ed385 100644
> > --- a/drivers/net/phy/marvell.c
> > +++ b/drivers/net/phy/marvell.c
> > @@ -63,6 +63,7 @@
> > #define MII_M1111_HWCFG_MODE_COPPER_RGMII	0xb
> > #define MII_M1111_HWCFG_MODE_FIBER_RGMII	0x3
> > #define MII_M1111_HWCFG_MODE_SGMII_NO_CLK	0x4
> > +#define MII_M1111_HWCFG_MODE_COPPER_RTBI	0x9
> > #define MII_M1111_HWCFG_FIBER_COPPER_AUTO	0x8000
> > #define MII_M1111_HWCFG_FIBER_COPPER_RES	0x2000
> >=20
> > @@ -269,6 +270,43 @@ static int m88e1111_config_init(struct=20
> phy_device *phydev)
> > 			return err;
> > 	}
> >=20
> > +	if (phydev->interface =3D=3D PHY_INTERFACE_MODE_RTBI) {
> > +		temp =3D phy_read(phydev, MII_M1111_PHY_EXT_CR);
> > +		if (temp < 0)
> > +			return temp;
> > +		temp |=3D (MII_M1111_RX_DELAY | MII_M1111_TX_DELAY);
> > +		err =3D phy_write(phydev, MII_M1111_PHY_EXT_CR, temp);
> > +		if (err < 0)
> > +			return err;
> > +
> > +		temp =3D phy_read(phydev, MII_M1111_PHY_EXT_SR);
> > +		if (temp < 0)
> > +			return temp;
> > +		temp &=3D ~(MII_M1111_HWCFG_MODE_MASK |=20
> MII_M1111_HWCFG_FIBER_COPPER_RES);
> > +		temp |=3D 0x7 | MII_M1111_HWCFG_FIBER_COPPER_AUTO;
>=20
> Does this magic 0x7 have some meaning?
>=20

Hrr...  it's GMII to fibre mode.
Document 88E1111_erata_RevB2  chapter 4.35 describ the reason.
Without this sometimes phys couldnot work.

Re: [PATCH 4/4] phy: add RTBI mode for m88e1111

[David Miller]

From: "Liu Yu-B13201" <B13201@freescale.com>
Date: Fri, 15 Jan 2010 10:49:37 +0800

>> -----Original Message-----
>> From: Kumar Gala [mailto:galak@kernel.crashing.org] 
>> Sent: Friday, January 15, 2010 12:20 AM
>> To: Liu Yu-B13201
>> Cc: davem@davemloft.net; linuxppc-dev@lists.ozlabs.org; 
>> netdev@vger.kernel.org
>> Subject: Re: [PATCH 4/4] phy: add RTBI mode for m88e1111
>> 
>> 
>> Does this magic 0x7 have some meaning?
>> 
> 
> Hrr...  it's GMII to fibre mode.
> Document 88E1111_erata_RevB2  chapter 4.35 describ the reason.
> Without this sometimes phys couldnot work.

Just FYI, I've applied all of these patches to net-next-2.6

Re: [PATCH 1/4] dts/mpc8569mds: Cleanup tbi phy to support rtbi

[Grant Likely]

T24gVGh1LCBKYW4gMTQsIDIwMTAgYXQgMToxMyBBTSwgTGl1IFl1IDx5dS5saXVAZnJlZXNjYWxl
LmNvbT4gd3JvdGU6Cj4gQWZ0ZXIgdGhlc2UgY2hhbmdlLCB3aGVuIG5lZWQgdG8gd29yayBpbiBy
dGJpIG1vZGUsCj4ganVzdCBjaGFuZ2UgcGh5LWNvbm5lY3Rpb24tdHlwZSB0byAicnRiaSIuCj4K
PiBBbHNvLCB0aGlzIHdvcmsgY2FuIGJlIGRvbmUgYnkgdS1ib290Lgo+Cj4gU2lnbmVkLW9mZi1i
eTogTGl1IFl1IDx5dS5saXVAZnJlZXNjYWxlLmNvbT4KCkxvb2tzIG9rYXkgYnkgbWUsCkFja2Vk
LWJ5OiBHcmFudCBMaWtlbHkgPGdyYW50Lmxpa2VseUBzZWNyZXRsYWIuY2E+Cgo+IC0tLQo+IKBh
cmNoL3Bvd2VycGMvYm9vdC9kdHMvbXBjODU2OW1kcy5kdHMgfCCgIDQ3ICsrKysrKysrKysrKysr
KysrKysrKysrKysrKysrKy0tLQo+IKAxIGZpbGVzIGNoYW5nZWQsIDQyIGluc2VydGlvbnMoKyks
IDUgZGVsZXRpb25zKC0pCj4KPiBkaWZmIC0tZ2l0IGEvYXJjaC9wb3dlcnBjL2Jvb3QvZHRzL21w
Yzg1NjltZHMuZHRzIGIvYXJjaC9wb3dlcnBjL2Jvb3QvZHRzL21wYzg1NjltZHMuZHRzCj4gaW5k
ZXggNzk1ZWIzNi4uOGI3MmVhZiAxMDA2NDQKPiAtLS0gYS9hcmNoL3Bvd2VycGMvYm9vdC9kdHMv
bXBjODU2OW1kcy5kdHMKPiArKysgYi9hcmNoL3Bvd2VycGMvYm9vdC9kdHMvbXBjODU2OW1kcy5k
dHMKPiBAQCAtNTM1LDYgKzUzNSw3IEBACj4goCCgIKAgoCCgIKAgoCCgIKAgoCCgIKByeC1jbG9j
ay1uYW1lID0gIm5vbmUiOwo+IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgdHgtY2xvY2stbmFtZSA9
ICJjbGsxMiI7Cj4goCCgIKAgoCCgIKAgoCCgIKAgoCCgIKBwaW8taGFuZGxlID0gPCZwaW8xPjsK
PiArIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCB0YmktaGFuZGxlID0gPCZ0YmkxPjsKPiCgIKAgoCCg
IKAgoCCgIKAgoCCgIKAgoHBoeS1oYW5kbGUgPSA8JnFlX3BoeTA+Owo+IKAgoCCgIKAgoCCgIKAg
oCCgIKAgoCCgcGh5LWNvbm5lY3Rpb24tdHlwZSA9ICJyZ21paS1pZCI7Cj4goCCgIKAgoCCgIKAg
oCCgfTsKPiBAQCAtNTc5LDcgKzU4MCw3IEBACj4goCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCg
IKAgoHJlZyA9IDwweDY+Owo+IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKBkZXZpY2Vf
dHlwZSA9ICJldGhlcm5ldC1waHkiOwo+IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgfTsKPiAtIKAg
oCCgIKAgoCCgIKAgoCCgIKAgoCB0YmktcGh5QDExIHsKPiArIKAgoCCgIKAgoCCgIKAgoCCgIKAg
oCB0YmkxOiB0YmktcGh5QDExIHsKPiCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgcmVn
ID0gPDB4MTE+Owo+IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKBkZXZpY2VfdHlwZSA9
ICJ0YmktcGh5IjsKPiCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoH07Cj4gQEAgLTU5MCw3ICs1OTEs
NyBAQAo+IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgcmVnID0gPDB4MzUyMCAweDE4PjsKPiCgIKAg
oCCgIKAgoCCgIKAgoCCgIKAgoGNvbXBhdGlibGUgPSAiZnNsLHVjYy1tZGlvIjsKPgo+IC0goCCg
IKAgoCCgIKAgoCCgIKAgoCCgIHRiaTA6IHRiaS1waHlAMTUgewo+ICsgoCCgIKAgoCCgIKAgoCCg
IKAgoCCgIHRiaTY6IHRiaS1waHlAMTUgewo+IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgcmVnID0g
PDB4MTU+Owo+IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgZGV2aWNlX3R5cGUgPSAidGJpLXBoeSI7
Cj4goCCgIKAgoCCgIKAgoCCgIKAgoCCgIKB9Owo+IEBAIC02MDAsNyArNjAxLDcgQEAKPiCgIKAg
oCCgIKAgoCCgIKAgoCCgIKAgoCNzaXplLWNlbGxzID0gPDA+Owo+IKAgoCCgIKAgoCCgIKAgoCCg
IKAgoCCgcmVnID0gPDB4MzcyMCAweDM4PjsKPiCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoGNvbXBh
dGlibGUgPSAiZnNsLHVjYy1tZGlvIjsKPiAtIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCB0YmkxOiB0
YmktcGh5QDE3IHsKPiArIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCB0Ymk4OiB0YmktcGh5QDE3IHsK
PiCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgcmVnID0gPDB4MTc+Owo+IKAgoCCgIKAg
oCCgIKAgoCCgIKAgoCCgIKAgoCCgIKBkZXZpY2VfdHlwZSA9ICJ0YmktcGh5IjsKPiCgIKAgoCCg
IKAgoCCgIKAgoCCgIKAgoH07Cj4gQEAgLTYxNywxMCArNjE4LDIyIEBACj4goCCgIKAgoCCgIKAg
oCCgIKAgoCCgIKByeC1jbG9jay1uYW1lID0gIm5vbmUiOwo+IKAgoCCgIKAgoCCgIKAgoCCgIKAg
oCCgdHgtY2xvY2stbmFtZSA9ICJjbGsxMiI7Cj4goCCgIKAgoCCgIKAgoCCgIKAgoCCgIKBwaW8t
aGFuZGxlID0gPCZwaW8zPjsKPiArIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCB0YmktaGFuZGxlID0g
PCZ0YmkzPjsKPiCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoHBoeS1oYW5kbGUgPSA8JnFlX3BoeTI+
Owo+IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgcGh5LWNvbm5lY3Rpb24tdHlwZSA9ICJyZ21paS1p
ZCI7Cj4goCCgIKAgoCCgIKAgoCCgfTsKPgo+ICsgoCCgIKAgoCCgIKAgoCBtZGlvQDIzMjAgewo+
ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCgICNhZGRyZXNzLWNlbGxzID0gPDE+Owo+ICsgoCCgIKAg
oCCgIKAgoCCgIKAgoCCgICNzaXplLWNlbGxzID0gPDA+Owo+ICsgoCCgIKAgoCCgIKAgoCCgIKAg
oCCgIHJlZyA9IDwweDIzMjAgMHgxOD47Cj4gKyCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgY29tcGF0
aWJsZSA9ICJmc2wsdWNjLW1kaW8iOwo+ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIHRiaTM6IHRi
aS1waHlAMTEgewo+ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgcmVnID0gPDB4MTE+
Owo+ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgZGV2aWNlX3R5cGUgPSAidGJpLXBo
eSI7Cj4gKyCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgfTsKPiArIKAgoCCgIKAgoCCgIKAgfTsKPiAr
Cj4goCCgIKAgoCCgIKAgoCCgZW5ldDE6IHVjY0AzMDAwIHsKPiCgIKAgoCCgIKAgoCCgIKAgoCCg
IKAgoGRldmljZV90eXBlID0gIm5ldHdvcmsiOwo+IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgY29t
cGF0aWJsZSA9ICJ1Y2NfZ2V0aCI7Cj4gQEAgLTYzMiwxMCArNjQ1LDIyIEBACj4goCCgIKAgoCCg
IKAgoCCgIKAgoCCgIKByeC1jbG9jay1uYW1lID0gIm5vbmUiOwo+IKAgoCCgIKAgoCCgIKAgoCCg
IKAgoCCgdHgtY2xvY2stbmFtZSA9ICJjbGsxNyI7Cj4goCCgIKAgoCCgIKAgoCCgIKAgoCCgIKBw
aW8taGFuZGxlID0gPCZwaW8yPjsKPiArIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCB0YmktaGFuZGxl
ID0gPCZ0YmkyPjsKPiCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoHBoeS1oYW5kbGUgPSA8JnFlX3Bo
eTE+Owo+IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgcGh5LWNvbm5lY3Rpb24tdHlwZSA9ICJyZ21p
aS1pZCI7Cj4goCCgIKAgoCCgIKAgoCCgfTsKPgo+ICsgoCCgIKAgoCCgIKAgoCBtZGlvQDMxMjAg
ewo+ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCgICNhZGRyZXNzLWNlbGxzID0gPDE+Owo+ICsgoCCg
IKAgoCCgIKAgoCCgIKAgoCCgICNzaXplLWNlbGxzID0gPDA+Owo+ICsgoCCgIKAgoCCgIKAgoCCg
IKAgoCCgIHJlZyA9IDwweDMxMjAgMHgxOD47Cj4gKyCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgY29t
cGF0aWJsZSA9ICJmc2wsdWNjLW1kaW8iOwo+ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIHRiaTI6
IHRiaS1waHlAMTEgewo+ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgcmVnID0gPDB4
MTE+Owo+ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgZGV2aWNlX3R5cGUgPSAidGJp
LXBoeSI7Cj4gKyCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgfTsKPiArIKAgoCCgIKAgoCCgIKAgfTsK
PiArCj4goCCgIKAgoCCgIKAgoCCgZW5ldDM6IHVjY0AzMjAwIHsKPiCgIKAgoCCgIKAgoCCgIKAg
oCCgIKAgoGRldmljZV90eXBlID0gIm5ldHdvcmsiOwo+IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCg
Y29tcGF0aWJsZSA9ICJ1Y2NfZ2V0aCI7Cj4gQEAgLTY0NywxMCArNjcyLDIyIEBACj4goCCgIKAg
oCCgIKAgoCCgIKAgoCCgIKByeC1jbG9jay1uYW1lID0gIm5vbmUiOwo+IKAgoCCgIKAgoCCgIKAg
oCCgIKAgoCCgdHgtY2xvY2stbmFtZSA9ICJjbGsxNyI7Cj4goCCgIKAgoCCgIKAgoCCgIKAgoCCg
IKBwaW8taGFuZGxlID0gPCZwaW80PjsKPiArIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCB0YmktaGFu
ZGxlID0gPCZ0Ymk0PjsKPiCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoHBoeS1oYW5kbGUgPSA8JnFl
X3BoeTM+Owo+IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgcGh5LWNvbm5lY3Rpb24tdHlwZSA9ICJy
Z21paS1pZCI7Cj4goCCgIKAgoCCgIKAgoCCgfTsKPgo+ICsgoCCgIKAgoCCgIKAgoCBtZGlvQDMz
MjAgewo+ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCgICNhZGRyZXNzLWNlbGxzID0gPDE+Owo+ICsg
oCCgIKAgoCCgIKAgoCCgIKAgoCCgICNzaXplLWNlbGxzID0gPDA+Owo+ICsgoCCgIKAgoCCgIKAg
oCCgIKAgoCCgIHJlZyA9IDwweDMzMjAgMHgxOD47Cj4gKyCgIKAgoCCgIKAgoCCgIKAgoCCgIKAg
Y29tcGF0aWJsZSA9ICJmc2wsdWNjLW1kaW8iOwo+ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIHRi
aTQ6IHRiaS1waHlAMTEgewo+ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgcmVnID0g
PDB4MTE+Owo+ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgZGV2aWNlX3R5cGUgPSAi
dGJpLXBoeSI7Cj4gKyCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgfTsKPiArIKAgoCCgIKAgoCCgIKAg
fTsKPiArCj4goCCgIKAgoCCgIKAgoCCgZW5ldDU6IHVjY0AzNDAwIHsKPiCgIKAgoCCgIKAgoCCg
IKAgoCCgIKAgoGRldmljZV90eXBlID0gIm5ldHdvcmsiOwo+IKAgoCCgIKAgoCCgIKAgoCCgIKAg
oCCgY29tcGF0aWJsZSA9ICJ1Y2NfZ2V0aCI7Cj4gQEAgLTY2MSw3ICs2OTgsNyBAQAo+IKAgoCCg
IKAgoCCgIKAgoCCgIKAgoCCgbG9jYWwtbWFjLWFkZHJlc3MgPSBbIDAwIDAwIDAwIDAwIDAwIDAw
IF07Cj4goCCgIKAgoCCgIKAgoCCgIKAgoCCgIKByeC1jbG9jay1uYW1lID0gIm5vbmUiOwo+IKAg
oCCgIKAgoCCgIKAgoCCgIKAgoCCgdHgtY2xvY2stbmFtZSA9ICJub25lIjsKPiAtIKAgoCCgIKAg
oCCgIKAgoCCgIKAgoCB0YmktaGFuZGxlID0gPCZ0YmkwPjsKPiArIKAgoCCgIKAgoCCgIKAgoCCg
IKAgoCB0YmktaGFuZGxlID0gPCZ0Ymk2PjsKPiCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoHBoeS1o
YW5kbGUgPSA8JnFlX3BoeTU+Owo+IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgcGh5LWNvbm5lY3Rp
b24tdHlwZSA9ICJzZ21paSI7Cj4goCCgIKAgoCCgIKAgoCCgfTsKPiBAQCAtNjc2LDcgKzcxMyw3
IEBACj4goCCgIKAgoCCgIKAgoCCgIKAgoCCgIKBsb2NhbC1tYWMtYWRkcmVzcyA9IFsgMDAgMDAg
MDAgMDAgMDAgMDAgXTsKPiCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoHJ4LWNsb2NrLW5hbWUgPSAi
bm9uZSI7Cj4goCCgIKAgoCCgIKAgoCCgIKAgoCCgIKB0eC1jbG9jay1uYW1lID0gIm5vbmUiOwo+
IC0goCCgIKAgoCCgIKAgoCCgIKAgoCCgIHRiaS1oYW5kbGUgPSA8JnRiaTE+Owo+ICsgoCCgIKAg
oCCgIKAgoCCgIKAgoCCgIHRiaS1oYW5kbGUgPSA8JnRiaTg+Owo+IKAgoCCgIKAgoCCgIKAgoCCg
IKAgoCCgcGh5LWhhbmRsZSA9IDwmcWVfcGh5Nz47Cj4goCCgIKAgoCCgIKAgoCCgIKAgoCCgIKBw
aHktY29ubmVjdGlvbi10eXBlID0gInNnbWlpIjsKPiCgIKAgoCCgIKAgoCCgIKB9Owo+IC0tCj4g
MS42LjQKPgo+IF9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
Cj4gTGludXhwcGMtZGV2IG1haWxpbmcgbGlzdAo+IExpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMu
b3JnCj4gaHR0cHM6Ly9saXN0cy5vemxhYnMub3JnL2xpc3RpbmZvL2xpbnV4cHBjLWRldgo+CgoK
Ci0tIApHcmFudCBMaWtlbHksIEIuU2MuLCBQLkVuZy4KU2VjcmV0IExhYiBUZWNobm9sb2dpZXMg
THRkLgo=

Re: [PATCH 2/4] mpc8569mds: Add bscr setting for rtbi mode

[Grant Likely]

On Thu, Jan 14, 2010 at 1:13 AM, Liu Yu <yu.liu@freescale.com> wrote:
> Signed-off-by: Liu Yu <yu.liu@freescale.com>
> ---
> =A0arch/powerpc/platforms/85xx/mpc85xx_mds.c | =A0 24 +++++++++++++++++++=
+++++
> =A01 files changed, 24 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/pla=
tforms/85xx/mpc85xx_mds.c
> index c5028a2..0872e4a 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> @@ -237,6 +237,8 @@ static void __init mpc85xx_mds_setup_arch(void)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} else if (machine_is(mpc8569_mds)) {
> =A0#define BCSR7_UCC12_GETHnRST =A0 (0x1 << 2)
> =A0#define BCSR8_UEM_MARVELL_RST =A0(0x1 << 1)
> +#define BCSR_UCC_RGMII =A0 =A0 =A0 =A0 (0x1 << 6)
> +#define BCSR_UCC_RTBI =A0 =A0 =A0 =A0 =A0(0x1 << 5)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/*
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * U-Boot mangles interrup=
t polarity for Marvell PHYs,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * so reset built-in and U=
EM Marvell PHYs, this puts
> @@ -247,6 +249,28 @@ static void __init mpc85xx_mds_setup_arch(void)
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0setbits8(&bcsr_regs[7], BC=
SR7_UCC12_GETHnRST);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0clrbits8(&bcsr_regs[8], BC=
SR8_UEM_MARVELL_RST);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 for (np =3D NULL; (np =3D o=
f_find_compatible_node(np,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "network",

Don't match on the 'type' field.  Replace "network" with NULL and just
rely on "ucc_geth" for matching.

> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "ucc_geth")) !=3D NULL;) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 const unsig=
ned int *prop;

u32 please.

Also, rather than reusing 'prop' for both char* and u32 values, which
forces you to use ugly casts, use 2 local variables here.

> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int ucc_num=
;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 prop =3D of=
_get_property(np, "cell-index", NULL);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (prop =
=3D=3D NULL)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 continue;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ucc_num =3D=
 *prop - 1;

Ugh.  No bounds checking...

> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 prop =3D of=
_get_property(np, "phy-connection-type", NULL);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (prop =
=3D=3D NULL)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 continue;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (strcmp(=
"rtbi", (const char *)prop) =3D=3D 0)

(This is the ugly cast I was talking about.)

> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 clrsetbits_8(&bcsr_regs[7 + ucc_num],

...not having bounds checking could result in badness in this array index.

This patch is dangerous as written.

Finally, while using cell-index seems convenient, I think it would be
better to have a lookup table of the index into the BCSR register
block from the UCC base address, which also gives you implicit bounds
checking.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH 3/4] ucc_geth: update the tbi-phy setting

[Grant Likely]

On Thu, Jan 14, 2010 at 1:13 AM, Liu Yu <yu.liu@freescale.com> wrote:
> Old method only set tbi-phy for eth0.
>
> Signed-off-by: Liu Yu <yu.liu@freescale.com>
> ---
> =A0drivers/net/ucc_geth.c | =A0 23 +++++++++++++++--------
> =A01 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
> index 750d86d..0e72013 100644
> --- a/drivers/net/ucc_geth.c
> +++ b/drivers/net/ucc_geth.c
> @@ -37,6 +37,7 @@
> =A0#include <asm/qe.h>
> =A0#include <asm/ucc.h>
> =A0#include <asm/ucc_fast.h>
> +#include <asm/machdep.h>
>
> =A0#include "ucc_geth.h"
> =A0#include "fsl_pq_mdio.h"
> @@ -1334,7 +1335,7 @@ static int adjust_enet_interface(struct ucc_geth_pr=
ivate *ugeth)
> =A0 =A0 =A0 =A0struct ucc_geth __iomem *ug_regs;
> =A0 =A0 =A0 =A0struct ucc_fast __iomem *uf_regs;
> =A0 =A0 =A0 =A0int ret_val;
> - =A0 =A0 =A0 u32 upsmr, maccfg2, tbiBaseAddress;
> + =A0 =A0 =A0 u32 upsmr, maccfg2;
> =A0 =A0 =A0 =A0u16 value;
>
> =A0 =A0 =A0 =A0ugeth_vdbg("%s: IN", __func__);
> @@ -1389,14 +1390,20 @@ static int adjust_enet_interface(struct ucc_geth_=
private *ugeth)
> =A0 =A0 =A0 =A0/* Note that this depends on proper setting in utbipar reg=
ister. */
> =A0 =A0 =A0 =A0if ((ugeth->phy_interface =3D=3D PHY_INTERFACE_MODE_TBI) |=
|
> =A0 =A0 =A0 =A0 =A0 =A0(ugeth->phy_interface =3D=3D PHY_INTERFACE_MODE_RT=
BI)) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 tbiBaseAddress =3D in_be32(&ug_regs->utbipa=
r);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 tbiBaseAddress &=3D UTBIPAR_PHY_ADDRESS_MAS=
K;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 tbiBaseAddress >>=3D UTBIPAR_PHY_ADDRESS_SH=
IFT;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 value =3D ugeth->phydev->bus->read(ugeth->p=
hydev->bus,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (u8) tbiBas=
eAddress, ENET_TBI_MII_CR);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct ucc_geth_info *ug_info =3D ugeth->ug=
_info;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct phy_device *tbiphy;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!ug_info->tbi_node)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ugeth_warn("TBI mode requir=
es that the device "
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "tree speci=
fy a tbi-handle\n");

Shouldn't the driver avoid calling of_phy_find_device() if this error
path is taken?

> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 tbiphy =3D of_phy_find_device(ug_info->tbi_=
node);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!tbiphy)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ugeth_warn("Could not get T=
BI device\n");
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 value =3D phy_read(tbiphy, ENET_TBI_MII_CR)=
;

Ditto here; the code unconditionally tries to read the tbiphy, even if
it isn't there.

Looks wrong.

g.

> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0value &=3D ~0x1000; =A0 =A0 =A0 /* Turn of=
f autonegotiation */
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 ugeth->phydev->bus->write(ugeth->phydev->bu=
s,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (u8) tbiBas=
eAddress, ENET_TBI_MII_CR, value);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 phy_write(tbiphy, ENET_TBI_MII_CR, value);
> =A0 =A0 =A0 =A0}
>
> =A0 =A0 =A0 =A0init_check_frame_length_mode(ug_info->lengthCheckRx, &ug_r=
egs->maccfg2);
> --
> 1.6.4
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

RE: [PATCH 2/4] mpc8569mds: Add bscr setting for rtbi mode

[Liu Yu-B13201]

Thanks.
As the patchset is accepted.
Will commit another one to fix them.

=20
> -----Original Message-----
> From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On=20
> Behalf Of Grant Likely
> Sent: Tuesday, February 02, 2010 5:58 AM
> To: Liu Yu-B13201
> Cc: galak@kernel.crashing.org; davem@davemloft.net;=20
> netdev@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 2/4] mpc8569mds: Add bscr setting for rtbi mode
>=20
> On Thu, Jan 14, 2010 at 1:13 AM, Liu Yu <yu.liu@freescale.com> wrote:
> > Signed-off-by: Liu Yu <yu.liu@freescale.com>
> > ---
> > =A0arch/powerpc/platforms/85xx/mpc85xx_mds.c | =A0 24=20
> ++++++++++++++++++++++++
> > =A01 files changed, 24 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c=20
> b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> > index c5028a2..0872e4a 100644
> > --- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> > +++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> > @@ -237,6 +237,8 @@ static void __init mpc85xx_mds_setup_arch(void)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} else if (machine_is(mpc8569_mds)) {
> > =A0#define BCSR7_UCC12_GETHnRST =A0 (0x1 << 2)
> > =A0#define BCSR8_UEM_MARVELL_RST =A0(0x1 << 1)
> > +#define BCSR_UCC_RGMII =A0 =A0 =A0 =A0 (0x1 << 6)
> > +#define BCSR_UCC_RTBI =A0 =A0 =A0 =A0 =A0(0x1 << 5)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/*
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * U-Boot mangles =
interrupt polarity=20
> for Marvell PHYs,
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * so reset built-in =
and UEM Marvell=20
> PHYs, this puts
> > @@ -247,6 +249,28 @@ static void __init mpc85xx_mds_setup_arch(void)
> >
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0setbits8(&bcsr_regs[7],=20
> BCSR7_UCC12_GETHnRST);
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0clrbits8(&bcsr_regs[8],=20
> BCSR8_UEM_MARVELL_RST);
> > +
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 for (np =3D NULL; (np =
=3D=20
> of_find_compatible_node(np,
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "network",
>=20
> Don't match on the 'type' field.  Replace "network" with NULL and just
> rely on "ucc_geth" for matching.
>=20
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=20
> "ucc_geth")) !=3D NULL;) {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 const =
unsigned int *prop;
>=20
> u32 please.
>=20
> Also, rather than reusing 'prop' for both char* and u32 values, which
> forces you to use ugly casts, use 2 local variables here.
>=20
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int =
ucc_num;
> > +
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 prop =
=3D of_get_property(np,=20
> "cell-index", NULL);
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if =
(prop =3D=3D NULL)
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 continue;
> > +
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
ucc_num =3D *prop - 1;
>=20
> Ugh.  No bounds checking...
>=20
> > +
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 prop =
=3D of_get_property(np,=20
> "phy-connection-type", NULL);
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if =
(prop =3D=3D NULL)
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 continue;
> > +
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if =
(strcmp("rtbi", (const=20
> char *)prop) =3D=3D 0)
>=20
> (This is the ugly cast I was talking about.)
>=20
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0=20
> clrsetbits_8(&bcsr_regs[7 + ucc_num],
>=20
> ...not having bounds checking could result in badness in this=20
> array index.
>=20
> This patch is dangerous as written.
>=20
> Finally, while using cell-index seems convenient, I think it would be
> better to have a lookup table of the index into the BCSR register
> block from the UCC base address, which also gives you implicit bounds
> checking.
>=20
> g.
>=20
> --=20
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
>=20
>=20