[PATCH] POWERPC: add support of the GiGE switch for mpc8313RDB via fixed PHY

[PATCH] POWERPC: add support of the GiGE switch for mpc8313RDB via fixed PHY

[Vitaly Bordug]

This utilises pretend phy, making access to it via device tree. GiGE switch
is connected to TSEC1 with fixed gigE link, so we need to emulate it
via artificial PHY to make it work.

Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>

---

 arch/powerpc/boot/dts/mpc8313erdb.dts     |    1 +
 arch/powerpc/platforms/83xx/mpc8313_rdb.c |   43 +++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8313erdb.dts b/arch/powerpc/boot/dts/mpc8313erdb.dts
index a1533cc..1b351dc 100644
--- a/arch/powerpc/boot/dts/mpc8313erdb.dts
+++ b/arch/powerpc/boot/dts/mpc8313erdb.dts
@@ -100,6 +100,7 @@
 			#size-cells = <0>;
 			phy1: ethernet-phy@1 {
 				interrupt-parent = < &ipic >;
+				compatible = "fixed";
 				interrupts = <13 8>;
 				reg = <1>;
 				device_type = "ethernet-phy";
diff --git a/arch/powerpc/platforms/83xx/mpc8313_rdb.c b/arch/powerpc/platforms/83xx/mpc8313_rdb.c
index b410e67..5ee08fc 100644
--- a/arch/powerpc/platforms/83xx/mpc8313_rdb.c
+++ b/arch/powerpc/platforms/83xx/mpc8313_rdb.c
@@ -115,6 +115,49 @@ static int __init rtc_hookup(void)
 late_initcall(rtc_hookup);
 #endif
 
+#if defined(CONFIG_FIXED_MII_1000_FDX)
+
+static int fixed_set_link (void)
+{
+	struct fixed_info *phyinfo = fixed_mdio_get_phydev(0);	/* only one fixed phy on this platform */
+	struct phy_device *phydev;
+	struct device_node *np;
+	struct device_node *child;
+	unsigned int i;
+	struct resource res;
+	int ret;
+	u32 *id = NULL;
+
+	if (!phyinfo)
+		return -ENXIO;
+	phydev = phyinfo->phydev;
+	if (!phydev)
+		return -ENXIO;
+	for (np = NULL, i = 0;
+	     (np = of_find_compatible_node(np, "mdio", "gianfar")) != NULL;
+	     i++) {
+
+		memset(&res, 0, sizeof(res));
+
+		ret = of_address_to_resource(np, 0, &res);
+		if (ret)
+			return ret;
+		child = of_find_compatible_node(np, "ethernet-phy","fixed");
+		if (!child)
+			return -ENXIO;
+		id = (u32*)of_get_property(child, "reg", NULL);
+		if (!id)
+			return -ENXIO;
+		break;
+	}
+	snprintf(phydev->dev.bus_id, BUS_ID_SIZE, PHY_ID_FMT,  res.start, *id);
+	memset(phyinfo->regs,0xff,sizeof(phyinfo->regs[0])*phyinfo->regs_num);
+
+	return 0;
+}
+late_initcall(fixed_set_link);
+#endif
+
 /*
  * Called very early, MMU is off, device-tree isn't unflattened
  */

Re: [PATCH] POWERPC: add support of the GiGE switch for mpc8313RDB via fixed PHY

[Segher Boessenkool]

> This utilises pretend phy, making access to it via device tree.  
> GiGE switch
> is connected to TSEC1 with fixed gigE link, so we need to emulate it
> via artificial PHY to make it work.

Even if "pretend phy" in the device tree would be a good
idea (and it's not)...

>  			#size-cells = <0>;
>  			phy1: ethernet-phy@1 {
>  				interrupt-parent = < &ipic >;
> +				compatible = "fixed";

...you cannot seriously believe you are allowed to claim
the name "fixed" for it.

NAK.


Segher

Re: [PATCH] POWERPC: add support of the GiGE switch for mpc8313RDB via fixed PHY

[Scott Wood]

This just comments on code style, not semantics...  I agree with Segher
that this isn't the way to do it.

On Tue, Jul 17, 2007 at 04:49:37AM +0400, Vitaly Bordug wrote:
> +#if defined(CONFIG_FIXED_MII_1000_FDX)
> +
> +static int fixed_set_link (void)
> +{
> +	struct fixed_info *phyinfo = fixed_mdio_get_phydev(0);	/* only one fixed phy on this platform */

Line length.

> +	for (np = NULL, i = 0;
> +	     (np = of_find_compatible_node(np, "mdio", "gianfar")) != NULL;
> +	     i++) {

Can't we just initialize np and i above, and use a while loop?

> +		memset(&res, 0, sizeof(res));

Not necessary.

> +		ret = of_address_to_resource(np, 0, &res);
> +		if (ret)
> +			return ret;
> +		child = of_find_compatible_node(np, "ethernet-phy","fixed");

Space after comma.

> +		if (!child)
> +			return -ENXIO;
> +		id = (u32*)of_get_property(child, "reg", NULL);

Cast not required.

> +		if (!id)
> +			return -ENXIO;
> +		break;

Why are you using a loop at all if there's a break at the end and no
continue?

> +	}
> +	snprintf(phydev->dev.bus_id, BUS_ID_SIZE, PHY_ID_FMT,  res.start, *id);

Only one space before res.start.

> +	memset(phyinfo->regs,0xff,sizeof(phyinfo->regs[0])*phyinfo->regs_num);

Spaces after commas.

-Scott

Re: [PATCH] POWERPC: add support of the GiGE switch for mpc8313RDB via fixed PHY

[Vitaly Bordug]

On Tue, 17 Jul 2007 11:43:31 -0500
Scott Wood <scottwood@freescale.com> wrote:

> This just comments on code style, not semantics...  I agree with Segher
> that this isn't the way to do it.

That's nm, I'll rather include specific field into mac node,
so the function below will be rewritten.

-- 
Sincerely, 
Vitaly