[PATCH 1/3] [NET] phy/fixed.c: rework to not duplicate PHY layer functionality

[PATCH 1/3] [NET] phy/fixed.c: rework to not duplicate PHY layer functionality

[Vitaly Bordug]

With that patch fixed.c now fully emulates MDIO bus, thus no need
to duplicate PHY layer functionality. That, in turn, drastically
simplifies the code, and drops down line count.

As an additional bonus, now there is no need to register MDIO bus
for each PHY, all emulated PHYs placed on the platform fixed MDIO bus.
There is also no more need to pre-allocate PHYs via .config option,
this is all now handled dynamically.

p.s. Don't even try to understand patch content! Better: apply patch
and look into resulting drivers/net/phy/fixed.c.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>

---

 drivers/net/phy/Kconfig   |   32 +--
 drivers/net/phy/fixed.c   |  427 ++++++++++++++++-----------------------------
 include/linux/phy_fixed.h |   49 ++---
 3 files changed, 176 insertions(+), 332 deletions(-)


diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 54b2ba9..a05c614 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -61,34 +61,12 @@ config ICPLUS_PHY
 	  Currently supports the IP175C PHY.
 
 config FIXED_PHY
-	tristate "Drivers for PHY emulation on fixed speed/link"
+	tristate "Drivers for MDIO Bus/PHY emulation on fixed speed/link"
 	---help---
-	  Adds the driver to PHY layer to cover the boards that do not have any PHY bound,
-	  but with the ability to manipulate the speed/link in software. The relevant MII
-	  speed/duplex parameters could be effectively handled in a user-specified function.
-	  Currently tested with mpc866ads.
-
-config FIXED_MII_10_FDX
-	bool "Emulation for 10M Fdx fixed PHY behavior"
-	depends on FIXED_PHY
-
-config FIXED_MII_100_FDX
-	bool "Emulation for 100M Fdx fixed PHY behavior"
-	depends on FIXED_PHY
-
-config FIXED_MII_1000_FDX
-	bool "Emulation for 1000M Fdx fixed PHY behavior"
-	depends on FIXED_PHY
-
-config FIXED_MII_AMNT
-        int "Number of emulated PHYs to allocate "
-        depends on FIXED_PHY
-        default "1"
-        ---help---
-        Sometimes it is required to have several independent emulated
-        PHYs on the bus (in case of multi-eth but phy-less HW for instance).
-        This control will have specified number allocated for each fixed
-        PHY type enabled.
+	  Adds the platform "fixed" MDIO Bus to cover the boards that use
+	  PHYs that are not connected to the real MDIO bus.
+
+	  Currently tested with mpc866ads and mpc8349e-mitx.
 
 config MDIO_BITBANG
 	tristate "Support for bitbanged MDIO buses"
diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
index 5619182..31719b3 100644
--- a/drivers/net/phy/fixed.c
+++ b/drivers/net/phy/fixed.c
@@ -1,362 +1,237 @@
 /*
- * drivers/net/phy/fixed.c
+ * Fixed MDIO bus (MDIO bus emulation with fixed PHYs)
  *
- * Driver for fixed PHYs, when transceiver is able to operate in one fixed mode.
+ * Author: Vitaly Bordug <vbordug@ru.mvista.com>
+ *         Anton Vorontsov <avorontsov@ru.mvista.com>
  *
- * Author: Vitaly Bordug
- *
- * Copyright (c) 2006 MontaVista Software, Inc.
+ * Copyright (c) 2006-2007 MontaVista Software, Inc.
  *
  * This program is free software; you can redistribute  it and/or modify it
  * under  the terms of  the GNU General  Public License as published by the
  * Free Software Foundation;  either version 2 of the  License, or (at your
  * option) any later version.
- *
  */
+
 #include <linux/kernel.h>
-#include <linux/string.h>
-#include <linux/errno.h>
-#include <linux/unistd.h>
-#include <linux/slab.h>
-#include <linux/interrupt.h>
-#include <linux/init.h>
-#include <linux/delay.h>
-#include <linux/netdevice.h>
-#include <linux/etherdevice.h>
-#include <linux/skbuff.h>
-#include <linux/spinlock.h>
-#include <linux/mm.h>
 #include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/list.h>
 #include <linux/mii.h>
-#include <linux/ethtool.h>
 #include <linux/phy.h>
 #include <linux/phy_fixed.h>
 
-#include <asm/io.h>
-#include <asm/irq.h>
-#include <asm/uaccess.h>
-
-/* we need to track the allocated pointers in order to free them on exit */
-static struct fixed_info *fixed_phy_ptrs[CONFIG_FIXED_MII_AMNT*MAX_PHY_AMNT];
-
-/*-----------------------------------------------------------------------------
- *  If something weird is required to be done with link/speed,
- * network driver is able to assign a function to implement this.
- * May be useful for PHY's that need to be software-driven.
- *-----------------------------------------------------------------------------*/
-int fixed_mdio_set_link_update(struct phy_device *phydev,
-			       int (*link_update) (struct net_device *,
-						   struct fixed_phy_status *))
-{
-	struct fixed_info *fixed;
-
-	if (link_update == NULL)
-		return -EINVAL;
-
-	if (phydev) {
-		if (phydev->bus) {
-			fixed = phydev->bus->priv;
-			fixed->link_update = link_update;
-			return 0;
-		}
-	}
-	return -EINVAL;
-}
+#define MII_REGS_NUM 29
 
-EXPORT_SYMBOL(fixed_mdio_set_link_update);
+struct fixed_mdio_bus {
+	int irqs[PHY_MAX_ADDR];
+	struct mii_bus mii_bus;
+	struct list_head phys;
+};
 
-struct fixed_info *fixed_mdio_get_phydev (int phydev_ind)
-{
-	if (phydev_ind >= MAX_PHY_AMNT)
-		return NULL;
-	return fixed_phy_ptrs[phydev_ind];
-}
+struct fixed_phy {
+	int id;
+	u16 regs[MII_REGS_NUM];
+	struct phy_device *phydev;
+	struct fixed_phy_status status;
+	int (*link_update)(struct net_device *, struct fixed_phy_status *);
+	struct list_head node;
+};
 
-EXPORT_SYMBOL(fixed_mdio_get_phydev);
+static struct platform_device *pdev;
+static struct fixed_mdio_bus platform_fmb = {
+	.phys = LIST_HEAD_INIT(platform_fmb.phys),
+};
 
-/*-----------------------------------------------------------------------------
- *  This is used for updating internal mii regs from the status
- *-----------------------------------------------------------------------------*/
-#if defined(CONFIG_FIXED_MII_100_FDX) || defined(CONFIG_FIXED_MII_10_FDX) || defined(CONFIG_FIXED_MII_1000_FDX)
-static int fixed_mdio_update_regs(struct fixed_info *fixed)
+static int fixed_phy_update_regs(struct fixed_phy *fp)
 {
-	u16 *regs = fixed->regs;
 	u16 bmsr = 0;
 	u16 bmcr = 0;
 
-	if (!regs) {
-		printk(KERN_ERR "%s: regs not set up", __FUNCTION__);
-		return -EINVAL;
-	}
-
-	if (fixed->phy_status.link)
-		bmsr |= BMSR_LSTATUS;
-
-	if (fixed->phy_status.duplex) {
+	if (fp->status.duplex) {
 		bmcr |= BMCR_FULLDPLX;
 
-		switch (fixed->phy_status.speed) {
+		switch (fp->status.speed) {
+		case 1000:
+			bmsr |= BMSR_ESTATEN;
+			bmcr |= BMCR_SPEED1000;
+			break;
 		case 100:
 			bmsr |= BMSR_100FULL;
 			bmcr |= BMCR_SPEED100;
 			break;
-
 		case 10:
 			bmsr |= BMSR_10FULL;
 			break;
+		default:
+			printk(KERN_WARNING "fixed phy: unknown speed\n");
+			return -EINVAL;
 		}
 	} else {
-		switch (fixed->phy_status.speed) {
+		switch (fp->status.speed) {
+		case 1000:
+			bmsr |= BMSR_ESTATEN;
+			bmcr |= BMCR_SPEED1000;
+			break;
 		case 100:
 			bmsr |= BMSR_100HALF;
 			bmcr |= BMCR_SPEED100;
 			break;
-
 		case 10:
-			bmsr |= BMSR_100HALF;
+			bmsr |= BMSR_10HALF;
 			break;
+		default:
+			printk(KERN_WARNING "fixed phy: unknown speed\n");
+			return -EINVAL;
 		}
 	}
 
-	regs[MII_BMCR] = bmcr;
-	regs[MII_BMSR] = bmsr | 0x800;	/*we are always capable of 10 hdx */
+	if (fp->status.link)
+		bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
+
+	fp->regs[MII_PHYSID1] = fp->id >> 16;
+	fp->regs[MII_PHYSID2] = fp->id;
+
+	fp->regs[MII_BMSR] = bmsr;
+	fp->regs[MII_BMCR] = bmcr;
 
 	return 0;
 }
 
-static int fixed_mii_read(struct mii_bus *bus, int phy_id, int location)
+static int fixed_mdio_read(struct mii_bus *bus, int phy_id, int reg_num)
 {
-	struct fixed_info *fixed = bus->priv;
-
-	/* if user has registered link update callback, use it */
-	if (fixed->phydev)
-		if (fixed->phydev->attached_dev) {
-			if (fixed->link_update) {
-				fixed->link_update(fixed->phydev->attached_dev,
-						   &fixed->phy_status);
-				fixed_mdio_update_regs(fixed);
+	struct fixed_mdio_bus *fmb = container_of(bus, struct fixed_mdio_bus,
+						  mii_bus);
+	struct fixed_phy *fp;
+
+	if (reg_num >= MII_REGS_NUM)
+		return -1;
+
+	list_for_each_entry(fp, &fmb->phys, node) {
+		if (fp->id == phy_id) {
+			/* Issue callback if user registered it. */
+			if (fp->link_update) {
+				fp->link_update(fp->phydev->attached_dev,
+						&fp->status);
+				fixed_phy_update_regs(fp);
 			}
+			return fp->regs[reg_num];
 		}
+	}
 
-	if ((unsigned int)location >= fixed->regs_num)
-		return -1;
-	return fixed->regs[location];
+	return 0xFFFF;
 }
 
-static int fixed_mii_write(struct mii_bus *bus, int phy_id, int location,
-			   u16 val)
+static int fixed_mdio_write(struct mii_bus *bus, int phy_id, int reg_num,
+			    u16 val)
 {
-	/* do nothing for now */
 	return 0;
 }
 
-static int fixed_mii_reset(struct mii_bus *bus)
+/*
+ * If something weird is required to be done with link/speed,
+ * network driver is able to assign a function to implement this.
+ * May be useful for PHY's that need to be software-driven.
+ */
+int fixed_phy_set_link_update(struct phy_device *phydev,
+			      int (*link_update)(struct net_device *,
+						 struct fixed_phy_status *))
 {
-	/*nothing here - no way/need to reset it */
-	return 0;
-}
-#endif
+	struct fixed_mdio_bus *fmb = &platform_fmb;
+	struct fixed_phy *fp;
 
-static int fixed_config_aneg(struct phy_device *phydev)
-{
-	/* :TODO:03/13/2006 09:45:37 PM::
-	   The full autoneg funcionality can be emulated,
-	   but no need to have anything here for now
-	 */
-	return 0;
-}
+	if (!link_update || !phydev || !phydev->bus)
+		return -EINVAL;
 
-/*-----------------------------------------------------------------------------
- * the manual bind will do the magic - with phy_id_mask == 0
- * match will never return true...
- *-----------------------------------------------------------------------------*/
-static struct phy_driver fixed_mdio_driver = {
-	.name = "Fixed PHY",
-#ifdef CONFIG_FIXED_MII_1000_FDX
-	.features = PHY_GBIT_FEATURES,
-#else
-	.features = PHY_BASIC_FEATURES,
-#endif
-	.config_aneg = fixed_config_aneg,
-	.read_status = genphy_read_status,
-	.driver = { .owner = THIS_MODULE, },
-};
+	list_for_each_entry(fp, &fmb->phys, node) {
+		if (fp->id == phydev->phy_id) {
+			fp->link_update = link_update;
+			fp->phydev = phydev;
+			return 0;
+		}
+	}
 
-static void fixed_mdio_release(struct device *dev)
-{
-	struct phy_device *phydev = container_of(dev, struct phy_device, dev);
-	struct mii_bus *bus = phydev->bus;
-	struct fixed_info *fixed = bus->priv;
-
-	kfree(phydev);
-	kfree(bus->dev);
-	kfree(bus);
-	kfree(fixed->regs);
-	kfree(fixed);
+	return -ENOENT;
 }
+EXPORT_SYMBOL_GPL(fixed_phy_set_link_update);
 
-/*-----------------------------------------------------------------------------
- *  This func is used to create all the necessary stuff, bind
- * the fixed phy driver and register all it on the mdio_bus_type.
- * speed is either 10 or 100 or 1000, duplex is boolean.
- * number is used to create multiple fixed PHYs, so that several devices can
- * utilize them simultaneously.
- *
- * The device on mdio bus will look like [bus_id]:[phy_id],
- * bus_id = number
- * phy_id = speed+duplex.
- *-----------------------------------------------------------------------------*/
-#if defined(CONFIG_FIXED_MII_100_FDX) || defined(CONFIG_FIXED_MII_10_FDX) || defined(CONFIG_FIXED_MII_1000_FDX)
-struct fixed_info *fixed_mdio_register_device(
-	int bus_id, int speed, int duplex, u8 phy_id)
+int fixed_phy_add(unsigned int irq, int phy_id,
+		  struct fixed_phy_status *status)
 {
-	struct mii_bus *new_bus;
-	struct fixed_info *fixed;
-	struct phy_device *phydev;
-	int err;
+	int ret;
+	struct fixed_mdio_bus *fmb = &platform_fmb;
+	struct fixed_phy *fp;
 
-	struct device *dev = kzalloc(sizeof(struct device), GFP_KERNEL);
+	fp = kzalloc(sizeof(*fp), GFP_KERNEL);
+	if (!fp)
+		return -ENOMEM;
 
-	if (dev == NULL)
-		goto err_dev_alloc;
+	memset(fp->regs, 0xFF,  sizeof(fp->regs[0]) * MII_REGS_NUM);
 
-	new_bus = kzalloc(sizeof(struct mii_bus), GFP_KERNEL);
+	fmb->irqs[phy_id] = irq;
 
-	if (new_bus == NULL)
-		goto err_bus_alloc;
+	fp->id = phy_id;
+	fp->status = *status;
 
-	fixed = kzalloc(sizeof(struct fixed_info), GFP_KERNEL);
+	ret = fixed_phy_update_regs(fp);
+	if (ret)
+		goto err_regs;
 
-	if (fixed == NULL)
-		goto err_fixed_alloc;
+	list_add_tail(&fp->node, &fmb->phys);
 
-	fixed->regs = kzalloc(MII_REGS_NUM * sizeof(int), GFP_KERNEL);
-	if (NULL == fixed->regs)
-		goto err_fixed_regs_alloc;
+	return 0;
 
-	fixed->regs_num = MII_REGS_NUM;
-	fixed->phy_status.speed = speed;
-	fixed->phy_status.duplex = duplex;
-	fixed->phy_status.link = 1;
+err_regs:
+	kfree(fp);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(fixed_phy_add);
 
-	new_bus->name = "Fixed MII Bus";
-	new_bus->read = &fixed_mii_read;
-	new_bus->write = &fixed_mii_write;
-	new_bus->reset = &fixed_mii_reset;
-	/*set up workspace */
-	fixed_mdio_update_regs(fixed);
-	new_bus->priv = fixed;
+static int __init fixed_mdio_bus_init(void)
+{
+	struct fixed_mdio_bus *fmb = &platform_fmb;
+	int ret;
 
-	new_bus->dev = dev;
-	dev_set_drvdata(dev, new_bus);
+	pdev = platform_device_register_simple("Fixed MDIO bus", 0, NULL, 0);
+	if (!pdev) {
+		ret = -ENOMEM;
+		goto err_pdev;
+	}
 
-	/* create phy_device and register it on the mdio bus */
-	phydev = phy_device_create(new_bus, 0, 0);
-	if (phydev == NULL)
-		goto err_phy_dev_create;
+	fmb->mii_bus.id = 0;
+	fmb->mii_bus.name = "Fixed MDIO Bus";
+	fmb->mii_bus.dev = &pdev->dev;
+	fmb->mii_bus.read = &fixed_mdio_read;
+	fmb->mii_bus.write = &fixed_mdio_write;
+	fmb->mii_bus.irq = fmb->irqs;
 
-	/*
-	 * Put the phydev pointer into the fixed pack so that bus read/write
-	 * code could be able to access for instance attached netdev. Well it
-	 * doesn't have to do so, only in case of utilizing user-specified
-	 * link-update...
-	 */
+	ret = mdiobus_register(&fmb->mii_bus);
+	if (ret)
+		goto err_mdiobus_reg;
 
-	fixed->phydev = phydev;
-	phydev->speed = speed;
-	phydev->duplex = duplex;
+	return 0;
 
-	phydev->irq = PHY_IGNORE_INTERRUPT;
-	phydev->dev.bus = &mdio_bus_type;
+err_mdiobus_reg:
+	platform_device_unregister(pdev);
+err_pdev:
+	return ret;
+}
+module_init(fixed_mdio_bus_init);
 
-	snprintf(phydev->dev.bus_id, BUS_ID_SIZE,
-		 PHY_ID_FMT, bus_id, phy_id);
+static void __exit fixed_mdio_bus_exit(void)
+{
+	struct fixed_mdio_bus *fmb = &platform_fmb;
+	struct fixed_phy *fp;
 
-	phydev->bus = new_bus;
+	mdiobus_unregister(&fmb->mii_bus);
+	platform_device_unregister(pdev);
 
-	phydev->dev.driver = &fixed_mdio_driver.driver;
-	phydev->dev.release = fixed_mdio_release;
-	err = phydev->dev.driver->probe(&phydev->dev);
-	if (err < 0) {
-		printk(KERN_ERR "Phy %s: problems with fixed driver\n",
-		       phydev->dev.bus_id);
-		goto err_out;
-	}
-	err = device_register(&phydev->dev);
-	if (err) {
-		printk(KERN_ERR "Phy %s failed to register\n",
-		       phydev->dev.bus_id);
-		goto err_out;
+	list_for_each_entry(fp, &fmb->phys, node) {
+		list_del(&fp->node);
+		kfree(fp);
 	}
-	//phydev->state = PHY_RUNNING; /* make phy go up quick, but in 10Mbit/HDX
-	return fixed;
-
-err_out:
-	kfree(phydev);
-err_phy_dev_create:
-	kfree(fixed->regs);
-err_fixed_regs_alloc:
-	kfree(fixed);
-err_fixed_alloc:
-	kfree(new_bus);
-err_bus_alloc:
-	kfree(dev);
-err_dev_alloc:
-
-	return NULL;
-
 }
-#endif
+module_exit(fixed_mdio_bus_exit);
 
-MODULE_DESCRIPTION("Fixed PHY device & driver for PAL");
+MODULE_DESCRIPTION("Fixed MDIO bus (MDIO bus emulation with fixed PHYs)");
 MODULE_AUTHOR("Vitaly Bordug");
 MODULE_LICENSE("GPL");
-
-static int __init fixed_init(void)
-{
-	int cnt = 0;
-	int i;
-/* register on the bus... Not expected to be matched
- * with anything there...
- *
- */
-	phy_driver_register(&fixed_mdio_driver);
-
-/* We will create several mdio devices here, and will bound the upper
- * driver to them.
- *
- * Then the external software can lookup the phy bus by searching
- * for 0:101, to be connected to the virtual 100M Fdx phy.
- *
- * In case several virtual PHYs required, the bus_id will be in form
- * [num]:[duplex]+[speed], which make it able even to define
- * driver-specific link control callback, if for instance PHY is
- * completely SW-driven.
- */
-	for (i=1; i <= CONFIG_FIXED_MII_AMNT; i++) {
-#ifdef CONFIG_FIXED_MII_1000_FDX
-		fixed_phy_ptrs[cnt++] = fixed_mdio_register_device(0, 1000, 1, i);
-#endif
-#ifdef CONFIG_FIXED_MII_100_FDX
-		fixed_phy_ptrs[cnt++] = fixed_mdio_register_device(1, 100, 1, i);
-#endif
-#ifdef CONFIG_FIXED_MII_10_FDX
-		fixed_phy_ptrs[cnt++] = fixed_mdio_register_device(2, 10, 1, i);
-#endif
-	}
-
-	return 0;
-}
-
-static void __exit fixed_exit(void)
-{
-	int i;
-
-	phy_driver_unregister(&fixed_mdio_driver);
-	for (i=0; i < MAX_PHY_AMNT; i++)
-		if ( fixed_phy_ptrs[i] )
-			device_unregister(&fixed_phy_ptrs[i]->phydev->dev);
-}
-
-module_init(fixed_init);
-module_exit(fixed_exit);
diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h
index 04ba70d..01669d1 100644
--- a/include/linux/phy_fixed.h
+++ b/include/linux/phy_fixed.h
@@ -1,38 +1,29 @@
 #ifndef __PHY_FIXED_H
 #define __PHY_FIXED_H
 
-#define MII_REGS_NUM	29
-
-/* max number of virtual phy stuff */
-#define MAX_PHY_AMNT	10
-/*
-    The idea is to emulate normal phy behavior by responding with
-    pre-defined values to mii BMCR read, so that read_status hook could
-    take all the needed info.
-*/
-
 struct fixed_phy_status {
-	u8 link;
-	u16 speed;
-	u8 duplex;
+	int link;
+	int speed;
+	int duplex;
 };
 
-/*-----------------------------------------------------------------------------
- *  Private information hoder for mii_bus
- *-----------------------------------------------------------------------------*/
-struct fixed_info {
-	u16 *regs;
-	u8 regs_num;
-	struct fixed_phy_status phy_status;
-	struct phy_device *phydev;	/* pointer to the container */
-	/* link & speed cb */
-	int (*link_update) (struct net_device *, struct fixed_phy_status *);
+#ifdef CONFIG_FIXED_PHY
+extern int fixed_phy_add(unsigned int irq, int phy_id,
+			 struct fixed_phy_status *status);
+#else
+static inline int fixed_phy_add(unsigned int irq, int phy_id,
+				struct fixed_phy_status *status)
+{
+	return -ENODEV;
+}
+#endif /* CONFIG_FIXED_PHY */
 
-};
-
-
-int fixed_mdio_set_link_update(struct phy_device *,
-       int (*link_update) (struct net_device *, struct fixed_phy_status *));
-struct fixed_info *fixed_mdio_get_phydev (int phydev_ind);
+/*
+ * This function issued only by fixed_phy-aware drivers, no need
+ * protect it with #ifdef
+ */
+extern int fixed_phy_set_link_update(struct phy_device *phydev,
+			int (*link_update)(struct net_device *,
+					   struct fixed_phy_status *));
 
 #endif /* __PHY_FIXED_H */

[PATCH 2/3] [POWERPC] fsl_soc: add support for gianfar for fixed-link property

[Vitaly Bordug]

fixed-link says: register new "Fixed/emulated PHY", i.e. PHY that
not connected to the real MDIO bus.

Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>

---

 Documentation/powerpc/booting-without-of.txt |    3 +
 arch/powerpc/sysdev/fsl_soc.c                |   56 ++++++++++++++++++--------
 2 files changed, 42 insertions(+), 17 deletions(-)


diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index e9a3cb1..cf25070 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -1254,6 +1254,9 @@ platforms are moved over to use the flattened-device-tree model.
       services interrupts for this device.
     - phy-handle : The phandle for the PHY connected to this ethernet
       controller.
+    - fixed-link : <a b c> where a is emulated phy id - choose any,
+      but unique to the all specified fixed-links, b is duplex - 0 half,
+      1 full, c is link speed - d#10/d#100/d#1000.
 
   Recommended properties:
 
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 3ace747..e06a5c9 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -24,6 +24,7 @@
 #include <linux/platform_device.h>
 #include <linux/of_platform.h>
 #include <linux/phy.h>
+#include <linux/phy_fixed.h>
 #include <linux/spi/spi.h>
 #include <linux/fsl_devices.h>
 #include <linux/fs_enet_pd.h>
@@ -193,7 +194,6 @@ static const char *gfar_tx_intr = "tx";
 static const char *gfar_rx_intr = "rx";
 static const char *gfar_err_intr = "error";
 
-
 static int __init gfar_of_init(void)
 {
 	struct device_node *np;
@@ -277,29 +277,51 @@ static int __init gfar_of_init(void)
 			gfar_data.interface = PHY_INTERFACE_MODE_MII;
 
 		ph = of_get_property(np, "phy-handle", NULL);
-		phy = of_find_node_by_phandle(*ph);
+		if (ph == NULL) {
+			struct fixed_phy_status status = {};
+			u32 *fixed_link;
 
-		if (phy == NULL) {
-			ret = -ENODEV;
-			goto unreg;
-		}
+			fixed_link = (u32*)of_get_property(np, "fixed-link",NULL);
+			if (!fixed_link) {
+				ret = -ENODEV;
+				goto unreg;
+			}
 
-		mdio = of_get_parent(phy);
+			status.link = 1;
+			status.duplex = fixed_link[1];
+			status.speed = fixed_link[2];
+
+			ret = fixed_phy_add(PHY_POLL, fixed_link[0], &status);
+			if (ret)
+				goto unreg;
+
+			gfar_data.bus_id = 0;
+			gfar_data.phy_id = fixed_link[0];
+		} else {
+			phy = of_find_node_by_phandle(*ph);
+
+			if (phy == NULL) {
+				ret = -ENODEV;
+				goto unreg;
+			}
+
+			mdio = of_get_parent(phy);
+
+			id = of_get_property(phy, "reg", NULL);
+			ret = of_address_to_resource(mdio, 0, &res);
+			if (ret) {
+				of_node_put(phy);
+				of_node_put(mdio);
+				goto unreg;
+			}
+
+			gfar_data.phy_id = *id;
+			gfar_data.bus_id = res.start;
 
-		id = of_get_property(phy, "reg", NULL);
-		ret = of_address_to_resource(mdio, 0, &res);
-		if (ret) {
 			of_node_put(phy);
 			of_node_put(mdio);
-			goto unreg;
 		}
 
-		gfar_data.phy_id = *id;
-		gfar_data.bus_id = res.start;
-
-		of_node_put(phy);
-		of_node_put(mdio);
-
 		ret =
 		    platform_device_add_data(gfar_dev, &gfar_data,
 					     sizeof(struct

[PATCH 3/3] [POWERPC] MPC8349E-mITX: Vitesse 7385 PHY is not connected to the MDIO bus

[Vitaly Bordug]

...thus use fixed-link to register proper "Fixed PHY"

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>

---

 arch/powerpc/boot/dts/mpc8349emitx.dts |   11 ++---------
 1 files changed, 2 insertions(+), 9 deletions(-)


diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts b/arch/powerpc/boot/dts/mpc8349emitx.dts
index 5072f6d..e2d00f1 100644
--- a/arch/powerpc/boot/dts/mpc8349emitx.dts
+++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
@@ -115,14 +115,6 @@
 				reg = <1c>;
 				device_type = "ethernet-phy";
 			};
-
-			/* Vitesse 7385 */
-			phy1f: ethernet-phy@1f {
-				interrupt-parent = < &ipic >;
-				interrupts = <12 8>;
-				reg = <1f>;
-				device_type = "ethernet-phy";
-			};
 		};
 
 		ethernet@24000 {
@@ -159,7 +151,8 @@
 			local-mac-address = [ 00 00 00 00 00 00 ];
 			interrupts = <23 8 24 8 25 8>;
 			interrupt-parent = < &ipic >;
-			phy-handle = < &phy1f >;
+			/* Vitesse 7385 isn't on the MDIO bus */
+			fixed-link = <1 1 d#1000>;
 			linux,network-index = <1>;
 		};
 

Re: [PATCH 2/3] [POWERPC] fsl_soc: add support for gianfar forfixed-link property

[Joakim Tjernlund]

On Mon, 2007-11-26 at 17:29 +0300, Vitaly Bordug wrote:
> fixed-link says: register new "Fixed/emulated PHY", i.e. PHY that
> not connected to the real MDIO bus.
> 
> Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> 
> ---
> 
>  Documentation/powerpc/booting-without-of.txt |    3 +
>  arch/powerpc/sysdev/fsl_soc.c                |   56 ++++++++++++++++++--------
>  2 files changed, 42 insertions(+), 17 deletions(-)
> 
> 
> diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
> index e9a3cb1..cf25070 100644
> --- a/Documentation/powerpc/booting-without-of.txt
> +++ b/Documentation/powerpc/booting-without-of.txt
> @@ -1254,6 +1254,9 @@ platforms are moved over to use the flattened-device-tree model.
>        services interrupts for this device.
>      - phy-handle : The phandle for the PHY connected to this ethernet
>        controller.
> +    - fixed-link : <a b c> where a is emulated phy id - choose any,
> +      but unique to the all specified fixed-links, b is duplex - 0 half,
> +      1 full, c is link speed - d#10/d#100/d#1000.

Good work!
May I suggest adding a "d" to <a b c> where d is flow control - 0 no, 1 yes

flow control or not just popped up here today so I got a use for it.

 Jocke 

Re: [PATCH 2/3] [POWERPC] fsl_soc: add support for gianfar for fixed-link property

[Anton Vorontsov]

On Mon, Nov 26, 2007 at 04:04:08PM +0100, Joakim Tjernlund wrote:
> On Mon, 2007-11-26 at 17:29 +0300, Vitaly Bordug wrote:
> > fixed-link says: register new "Fixed/emulated PHY", i.e. PHY that
> > not connected to the real MDIO bus.
> > 
> > Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
> > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> > 
> > ---
> > 
> >  Documentation/powerpc/booting-without-of.txt |    3 +
> >  arch/powerpc/sysdev/fsl_soc.c                |   56 ++++++++++++++++++--------
> >  2 files changed, 42 insertions(+), 17 deletions(-)
> > 
> > 
> > diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
> > index e9a3cb1..cf25070 100644
> > --- a/Documentation/powerpc/booting-without-of.txt
> > +++ b/Documentation/powerpc/booting-without-of.txt
> > @@ -1254,6 +1254,9 @@ platforms are moved over to use the flattened-device-tree model.
> >        services interrupts for this device.
> >      - phy-handle : The phandle for the PHY connected to this ethernet
> >        controller.
> > +    - fixed-link : <a b c> where a is emulated phy id - choose any,
> > +      but unique to the all specified fixed-links, b is duplex - 0 half,
> > +      1 full, c is link speed - d#10/d#100/d#1000.
> 
> Good work!
> May I suggest adding a "d" to <a b c> where d is flow control - 0 no, 1 yes

Well, I see no reference of the "flow" neither in the include/linux/mii.h
nor in the drivers/net/phy/*. :-/ Thus today there is no such register
bit we can emulate?..

> flow control or not just popped up here today so I got a use for it.

..and I looked into few drivers (gianfar, ucc), and they seem to use
hard-coded flow control values, thus they don't speak to the PHYs
about that.

Can you give any reference to the driver that needs that parameter?
Does it use PHY subsystem to obtain flow-control on/off information?

Thanks,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

Re: [PATCH 2/3] [POWERPC] fsl_soc: add support for gianfar for fixed-link property

[Joakim Tjernlund]

On Tue, 2007-11-27 at 14:39 +0300, Anton Vorontsov wrote:
> On Mon, Nov 26, 2007 at 04:04:08PM +0100, Joakim Tjernlund wrote:
> > On Mon, 2007-11-26 at 17:29 +0300, Vitaly Bordug wrote:
> > > fixed-link says: register new "Fixed/emulated PHY", i.e. PHY that
> > > not connected to the real MDIO bus.
> > > 
> > > Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
> > > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> > > 
> > > ---
> > > 
> > >  Documentation/powerpc/booting-without-of.txt |    3 +
> > >  arch/powerpc/sysdev/fsl_soc.c                |   56 ++++++++++++++++++--------
> > >  2 files changed, 42 insertions(+), 17 deletions(-)
> > > 
> > > 
> > > diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
> > > index e9a3cb1..cf25070 100644
> > > --- a/Documentation/powerpc/booting-without-of.txt
> > > +++ b/Documentation/powerpc/booting-without-of.txt
> > > @@ -1254,6 +1254,9 @@ platforms are moved over to use the flattened-device-tree model.
> > >        services interrupts for this device.
> > >      - phy-handle : The phandle for the PHY connected to this ethernet
> > >        controller.
> > > +    - fixed-link : <a b c> where a is emulated phy id - choose any,
> > > +      but unique to the all specified fixed-links, b is duplex - 0 half,
> > > +      1 full, c is link speed - d#10/d#100/d#1000.
> > 
> > Good work!
> > May I suggest adding a "d" to <a b c> where d is flow control - 0 no, 1 yes
> 
> Well, I see no reference of the "flow" neither in the include/linux/mii.h
> nor in the drivers/net/phy/*. :-/ Thus today there is no such register
> bit we can emulate?..

Well, as good as I can recall, flow control(pause) is something that the
PHY negotiates, just like FDX/HDX and should be dealt with in the
adjust_link callback but not many do currently.

If you seach for pause in phy_device.c you will find it.

> 
> > flow control or not just popped up here today so I got a use for it.
> 
> ..and I looked into few drivers (gianfar, ucc), and they seem to use
> hard-coded flow control values, thus they don't speak to the PHYs
> about that.

Yes, but they should.

> 
> Can you give any reference to the driver that needs that parameter?
> Does it use PHY subsystem to obtain flow-control on/off information?
> 
> Thanks,
> 

Re: [PATCH 2/3] [POWERPC] fsl_soc: add support for gianfar for fixed-link property

[Anton Vorontsov]

On Tue, Nov 27, 2007 at 02:17:11PM +0100, Joakim Tjernlund wrote:
> 
> On Tue, 2007-11-27 at 14:39 +0300, Anton Vorontsov wrote:
> > On Mon, Nov 26, 2007 at 04:04:08PM +0100, Joakim Tjernlund wrote:
> > > On Mon, 2007-11-26 at 17:29 +0300, Vitaly Bordug wrote:
> > > > fixed-link says: register new "Fixed/emulated PHY", i.e. PHY that
> > > > not connected to the real MDIO bus.
> > > > 
> > > > Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
> > > > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> > > > 
> > > > ---
> > > > 
> > > >  Documentation/powerpc/booting-without-of.txt |    3 +
> > > >  arch/powerpc/sysdev/fsl_soc.c                |   56 ++++++++++++++++++--------
> > > >  2 files changed, 42 insertions(+), 17 deletions(-)
> > > > 
> > > > 
> > > > diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
> > > > index e9a3cb1..cf25070 100644
> > > > --- a/Documentation/powerpc/booting-without-of.txt
> > > > +++ b/Documentation/powerpc/booting-without-of.txt
> > > > @@ -1254,6 +1254,9 @@ platforms are moved over to use the flattened-device-tree model.
> > > >        services interrupts for this device.
> > > >      - phy-handle : The phandle for the PHY connected to this ethernet
> > > >        controller.
> > > > +    - fixed-link : <a b c> where a is emulated phy id - choose any,
> > > > +      but unique to the all specified fixed-links, b is duplex - 0 half,
> > > > +      1 full, c is link speed - d#10/d#100/d#1000.
> > > 
> > > Good work!
> > > May I suggest adding a "d" to <a b c> where d is flow control - 0 no, 1 yes
> > 
> > Well, I see no reference of the "flow" neither in the include/linux/mii.h
> > nor in the drivers/net/phy/*. :-/ Thus today there is no such register
> > bit we can emulate?..
> 
> Well, as good as I can recall, flow control(pause) is something that the
> PHY negotiates, just like FDX/HDX and should be dealt with in the
> adjust_link callback but not many do currently.
> 
> If you seach for pause in phy_device.c you will find it.

Ah, pause. Sure, we can emulate that.

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

Re: [PATCH 2/3] [POWERPC] fsl_soc: add support for gianfar for fixed-link property

[Joakim Tjernlund]

On Tue, 2007-11-27 at 16:59 +0300, Anton Vorontsov wrote:
> On Tue, Nov 27, 2007 at 02:17:11PM +0100, Joakim Tjernlund wrote:
> > 
> > On Tue, 2007-11-27 at 14:39 +0300, Anton Vorontsov wrote:
> > > On Mon, Nov 26, 2007 at 04:04:08PM +0100, Joakim Tjernlund wrote:
> > > > On Mon, 2007-11-26 at 17:29 +0300, Vitaly Bordug wrote:
> > > > > fixed-link says: register new "Fixed/emulated PHY", i.e. PHY that
> > > > > not connected to the real MDIO bus.
> > > > > 
> > > > > Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
> > > > > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> > > > > 
> > > > > ---
> > > > > 
> > > > >  Documentation/powerpc/booting-without-of.txt |    3 +
> > > > >  arch/powerpc/sysdev/fsl_soc.c                |   56 ++++++++++++++++++--------
> > > > >  2 files changed, 42 insertions(+), 17 deletions(-)
> > > > > 
> > > > > 
> > > > > diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
> > > > > index e9a3cb1..cf25070 100644
> > > > > --- a/Documentation/powerpc/booting-without-of.txt
> > > > > +++ b/Documentation/powerpc/booting-without-of.txt
> > > > > @@ -1254,6 +1254,9 @@ platforms are moved over to use the flattened-device-tree model.
> > > > >        services interrupts for this device.
> > > > >      - phy-handle : The phandle for the PHY connected to this ethernet
> > > > >        controller.
> > > > > +    - fixed-link : <a b c> where a is emulated phy id - choose any,
> > > > > +      but unique to the all specified fixed-links, b is duplex - 0 half,
> > > > > +      1 full, c is link speed - d#10/d#100/d#1000.
> > > > 
> > > > Good work!
> > > > May I suggest adding a "d" to <a b c> where d is flow control - 0 no, 1 yes
> > > 
> > > Well, I see no reference of the "flow" neither in the include/linux/mii.h
> > > nor in the drivers/net/phy/*. :-/ Thus today there is no such register
> > > bit we can emulate?..
> > 
> > Well, as good as I can recall, flow control(pause) is something that the
> > PHY negotiates, just like FDX/HDX and should be dealt with in the
> > adjust_link callback but not many do currently.
> > 
> > If you seach for pause in phy_device.c you will find it.
> 
> Ah, pause. Sure, we can emulate that.

Good, looking into phy_device there seems to be 2 pauses:
ADVERTISED_Pause and ADVERTISE_PAUSE_ASYM which
maps to phydev->pause and phydev->asym_pause
so "d" should reflect that or a "e" should be added
for the asym pause.

 Jocke

 Jocke

Re: [PATCH 1/3] [NET] phy/fixed.c: rework to not duplicate PHY layer functionality

[Jochen Friedrich]

Hi Vitaly,

> With that patch fixed.c now fully emulates MDIO bus, thus no need
> to duplicate PHY layer functionality. That, in turn, drastically
> simplifies the code, and drops down line count.
>
> As an additional bonus, now there is no need to register MDIO bus
> for each PHY, all emulated PHYs placed on the platform fixed MDIO bus.
> There is also no more need to pre-allocate PHYs via .config option,
> this is all now handled dynamically.
>
> p.s. Don't even try to understand patch content! Better: apply patch
> and look into resulting drivers/net/phy/fixed.c.
>   
If i understand your code correctly, you seem to rely on the fact 
that fixed_phy_add() is called before the fixed MDIO bus is scanned for 
devices. How is this supposed to work for modules or for the 
PPC_CPM_NEW_BINDING mode where the device tree is no longer scanned 
during fs_soc initialization but during device initialization?

I tried to add fixed-phy support to fs_enet, but the fixed phy is not 
found this way.

--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -36,6 +36,7 @@
 #include <linux/fs.h>
 #include <linux/platform_device.h>
 #include <linux/phy.h>
+#include <linux/phy_fixed.h>
 
 #include <linux/vmalloc.h>
 #include <asm/pgtable.h>
@@ -1174,8 +1175,24 @@ static int __devinit find_phy(struct device_node *np,
        struct device_node *phynode, *mdionode;
        struct resource res;
        int ret = 0, len;
+       const u32 *data;
+       struct fixed_phy_status status = {};
+
+       data  = of_get_property(np, "fixed-link", NULL);
+       if (data) {
+               status.link = 1;
+               status.duplex = data[1];
+               status.speed  = data[2];
+
+               ret = fixed_phy_add(PHY_POLL, data[0], &status);
+               if (ret)
+                       return ret;
+
+               snprintf(fpi->bus_id, 16, PHY_ID_FMT, 0, *data);
+               return 0;
+       }
 
-       const u32 *data = of_get_property(np, "phy-handle", &len);
+       data = of_get_property(np, "phy-handle", &len);
        if (!data || len != 4)
                return -EINVAL;

Thanks,
Jochen

Re: [PATCH 1/3] [NET] phy/fixed.c: rework to not duplicate PHY layer functionality

[Vitaly Bordug]

On Sat, 01 Dec 2007 14:48:54 +0100
Jochen Friedrich wrote:

> Hi Vitaly,
> 
> > With that patch fixed.c now fully emulates MDIO bus, thus no need
> > to duplicate PHY layer functionality. That, in turn, drastically
> > simplifies the code, and drops down line count.
> >
> > As an additional bonus, now there is no need to register MDIO bus
> > for each PHY, all emulated PHYs placed on the platform fixed MDIO
> > bus. There is also no more need to pre-allocate PHYs via .config
> > option, this is all now handled dynamically.
> >
> > p.s. Don't even try to understand patch content! Better: apply patch
> > and look into resulting drivers/net/phy/fixed.c.
> >   
> If i understand your code correctly, you seem to rely on the fact 
> that fixed_phy_add() is called before the fixed MDIO bus is scanned
> for devices. How is this supposed to work for modules or for the 
> PPC_CPM_NEW_BINDING mode where the device tree is no longer scanned 
> during fs_soc initialization but during device initialization?
>
Well, this is kind of known issue - to work it around for now, place PHY lib after fs_enet in
Makefile. This way it works for me for _NEW_BINDING and mpc866ads.

> I tried to add fixed-phy support to fs_enet, but the fixed phy is not 
> found this way.
> 
The point is I have the code and it works now(for fs_enet etc.), but I need to find the way for the fixed phy pinning to work in either order with phylib. If you have ideas, please go ahead :)


> --- a/drivers/net/fs_enet/fs_enet-main.c
> +++ b/drivers/net/fs_enet/fs_enet-main.c
> @@ -36,6 +36,7 @@
>  #include <linux/fs.h>
>  #include <linux/platform_device.h>
>  #include <linux/phy.h>
> +#include <linux/phy_fixed.h>
>  
>  #include <linux/vmalloc.h>
>  #include <asm/pgtable.h>
> @@ -1174,8 +1175,24 @@ static int __devinit find_phy(struct
> device_node *np, struct device_node *phynode, *mdionode;
>         struct resource res;
>         int ret = 0, len;
> +       const u32 *data;
> +       struct fixed_phy_status status = {};
> +
> +       data  = of_get_property(np, "fixed-link", NULL);
> +       if (data) {
> +               status.link = 1;
> +               status.duplex = data[1];
> +               status.speed  = data[2];
> +
> +               ret = fixed_phy_add(PHY_POLL, data[0], &status);
> +               if (ret)
> +                       return ret;
> +
> +               snprintf(fpi->bus_id, 16, PHY_ID_FMT, 0, *data);
> +               return 0;
> +       }
>  
> -       const u32 *data = of_get_property(np, "phy-handle", &len);
> +       data = of_get_property(np, "phy-handle", &len);
>         if (!data || len != 4)
>                 return -EINVAL;
> 
> Thanks,
> Jochen


-- 
Sincerely, Vitaly

Re: [PATCH 1/3] [NET] phy/fixed.c: rework to not duplicate PHY layer functionality

[Anton Vorontsov]

On Sat, Dec 01, 2007 at 02:48:54PM +0100, Jochen Friedrich wrote:
> Hi Vitaly,
> 
> > With that patch fixed.c now fully emulates MDIO bus, thus no need
> > to duplicate PHY layer functionality. That, in turn, drastically
> > simplifies the code, and drops down line count.
> >
> > As an additional bonus, now there is no need to register MDIO bus
> > for each PHY, all emulated PHYs placed on the platform fixed MDIO bus.
> > There is also no more need to pre-allocate PHYs via .config option,
> > this is all now handled dynamically.
> >
> > p.s. Don't even try to understand patch content! Better: apply patch
> > and look into resulting drivers/net/phy/fixed.c.
> >   
> If i understand your code correctly, you seem to rely on the fact 
> that fixed_phy_add() is called before the fixed MDIO bus is scanned for 
> devices.

Yes, indeed. The other name of "fixed phys" are "platform phys"
or "platform MDIO bus" on which virtual PHYs are placed.

That is, these phys supposed to be created by the platform setup
code (arch/). The rationale here is: we do hardware emulation, thus
to make drivers actually see that "hardware", we have to create it
early.

> I tried to add fixed-phy support to fs_enet, but the fixed phy is not 
> found this way.
> 
> --- a/drivers/net/fs_enet/fs_enet-main.c
> +++ b/drivers/net/fs_enet/fs_enet-main.c
> @@ -36,6 +36,7 @@
>  #include <linux/fs.h>
>  #include <linux/platform_device.h>
>  #include <linux/phy.h>
> +#include <linux/phy_fixed.h>
>  
>  #include <linux/vmalloc.h>
>  #include <asm/pgtable.h>
> @@ -1174,8 +1175,24 @@ static int __devinit find_phy(struct device_node *np,
>         struct device_node *phynode, *mdionode;
>         struct resource res;
>         int ret = 0, len;
> +       const u32 *data;
> +       struct fixed_phy_status status = {};
> +
> +       data  = of_get_property(np, "fixed-link", NULL);
> +       if (data) {
> +               status.link = 1;
> +               status.duplex = data[1];
> +               status.speed  = data[2];
> +
> +               ret = fixed_phy_add(PHY_POLL, data[0], &status);
> +               if (ret)
> +                       return ret;
> +
> +               snprintf(fpi->bus_id, 16, PHY_ID_FMT, 0, *data);
> +               return 0;
> +       }
>  
> -       const u32 *data = of_get_property(np, "phy-handle", &len);
> +       data = of_get_property(np, "phy-handle", &len);
>         if (!data || len != 4)
>                 return -EINVAL;

^^ the correct solution is to implement arch_initcall function
which will create fixed PHYs, and then leave only
snprintf(fpi->bus_id, 16, PHY_ID_FMT, 0, *data); part in the
fs_enet's find_phy().

Try add something like this to the fsl_soc.c (compile untested):

- - - -
static int __init of_add_fixed_phys(void)
{
	struct device_node *np;
	const u32 *prop;
	struct fixed_phy_status status = {};

	while ((np = of_find_node_by_name(NULL, "ethernet"))) {
		data  = of_get_property(np, "fixed-link", NULL);
		if (!data)
			continue;

		status.link = 1;
		status.duplex = data[1];
		status.speed  = data[2];

		ret = fixed_phy_add(PHY_POLL, data[0], &status);
		if (ret)
			return ret;
	}

	return 0;
}
arch_initcall(of_add_fixed_phys);
- - - -

And remove fixed_phy_add() from the fs_enet. This should work
nicely and also should be ideologically correct. ;-)

> How is this supposed to work for modules or for the
> PPC_CPM_NEW_BINDING mode where the device tree is no longer scanned
> during fs_soc initialization but during device initialization?

We should mark fixed.c as bool. Fake/virtual/fixed/platform PHYs
creation is architecture code anyway, can't be =m.

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

Re: [PATCH 1/3] [NET] phy/fixed.c: rework to not duplicate PHY layer functionality

[Jeff Garzik]

Vitaly Bordug wrote:
> With that patch fixed.c now fully emulates MDIO bus, thus no need
> to duplicate PHY layer functionality. That, in turn, drastically
> simplifies the code, and drops down line count.
> 
> As an additional bonus, now there is no need to register MDIO bus
> for each PHY, all emulated PHYs placed on the platform fixed MDIO bus.
> There is also no more need to pre-allocate PHYs via .config option,
> this is all now handled dynamically.
> 
> p.s. Don't even try to understand patch content! Better: apply patch
> and look into resulting drivers/net/phy/fixed.c.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>

what's the context of this patchset?  2.6.25?

it's late for 2.6.24-rc, IMO.

Do I have the latest version (sent Nov 26 @ 9:29am)?

	Jeff

Re: [PATCH 1/3] [NET] phy/fixed.c: rework to not duplicate PHY layer functionality

[Vitaly Bordug]

On Sat, 01 Dec 2007 16:59:52 -0500
Jeff Garzik wrote:

> Vitaly Bordug wrote:
> > With that patch fixed.c now fully emulates MDIO bus, thus no need
> > to duplicate PHY layer functionality. That, in turn, drastically
> > simplifies the code, and drops down line count.
> > 
> > As an additional bonus, now there is no need to register MDIO bus
> > for each PHY, all emulated PHYs placed on the platform fixed MDIO
> > bus. There is also no more need to pre-allocate PHYs via .config
> > option, this is all now handled dynamically.
> > 
> > p.s. Don't even try to understand patch content! Better: apply patch
> > and look into resulting drivers/net/phy/fixed.c.
> > 
> > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> > Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
> 
> what's the context of this patchset?  2.6.25?
> 
Fine with it.

> it's late for 2.6.24-rc, IMO.
> 
> Do I have the latest version (sent Nov 26 @ 9:29am)?
yes, that's it.

-- 
Sincerely, Vitaly

Re: [PATCH 1/3] [NET] phy/fixed.c: rework to not duplicate PHY layer functionality

[Vitaly Bordug]

On Sun, 2 Dec 2007 00:34:03 +0300
Anton Vorontsov wrote:

> > If i understand your code correctly, you seem to rely on the fact 
> > that fixed_phy_add() is called before the fixed MDIO bus is scanned
> > for devices.  
> 
> Yes, indeed. The other name of "fixed phys" are "platform phys"
> or "platform MDIO bus" on which virtual PHYs are placed.
> 
> That is, these phys supposed to be created by the platform setup
> code (arch/). The rationale here is: we do hardware emulation, thus
> to make drivers actually see that "hardware", we have to create it
> early.

well that was the intention but... The point is - as device is emulated, (nearly) everything is doable,
and the only tradeoff to consider, is how far will we go with that emulation. IOW, PHYlib could be tricked
to "do the right thing", and I thought about adding module flexibility...

But thinking more about it, it seems that BSP-code-phy-creation just sucks less and is clear enough yet flexible.
-- 
Sincerely, Vitaly

Re: [PATCH 1/3] [NET] phy/fixed.c: rework to not duplicate PHY layer functionality

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 893 bytes --]

Just a little reminder ...

On Sun, 2 Dec 2007 00:34:03 +0300 Anton Vorontsov <cbou@mail.ru> wrote:
>
> static int __init of_add_fixed_phys(void)
> {
> 	struct device_node *np;
> 	const u32 *prop;
> 	struct fixed_phy_status status = {};
> 
> 	while ((np = of_find_node_by_name(NULL, "ethernet"))) {

	for_each_node_by_name(np, "ethernet") {
(this probably does what you want instead of finding just the first
ethernet over and over again. :-))

> 		data  = of_get_property(np, "fixed-link", NULL);
> 		if (!data)
> 			continue;
> 
> 		status.link = 1;
> 		status.duplex = data[1];
> 		status.speed  = data[2];
> 
> 		ret = fixed_phy_add(PHY_POLL, data[0], &status);
> 		if (ret)
> 			return ret;
		if (ret) {
			of_put_node(np);
			retun ret;
		}
> 	}

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

RE: [PATCH 1/3] [NET] phy/fixed.c: rework to not duplicate PHYlayer functionality

[Joakim Tjernlund]

[SNIP]
> ^^ the correct solution is to implement arch_initcall function
> which will create fixed PHYs, and then leave only
> snprintf(fpi->bus_id, 16, PHY_ID_FMT, 0, *data); part in the
> fs_enet's find_phy().
> 
> Try add something like this to the fsl_soc.c (compile untested):
> 
> - - - -
> static int __init of_add_fixed_phys(void)
> {
> 	struct device_node *np;
> 	const u32 *prop;
> 	struct fixed_phy_status status = {};
> 
> 	while ((np = of_find_node_by_name(NULL, "ethernet"))) {
> 		data  = of_get_property(np, "fixed-link", NULL);
> 		if (!data)
> 			continue;
> 
> 		status.link = 1;
> 		status.duplex = data[1];
> 		status.speed  = data[2];

What about Pause and Asym_Pause? Dunno why so few, if any, eth drivers
impl. it, but the PHY lib supports it.
Even if fixed PHYs doesn't support it directly I think the OF interface
should have it.

    - fixed-link : <a b c d e> where a is emulated phy id - choose any,
      but unique to the all specified fixed-links, b is duplex - 0 half,
      1 full, c is link speed - d#10/d#100/d#1000, d is pause - 0 no pause,
      1 pause, d asym_pause - 0 no asym_pause, 1 asym_pause.

Jocke

> 
> 		ret = fixed_phy_add(PHY_POLL, data[0], &status);
> 		if (ret)
> 			return ret;
> 	}
> 
> 	return 0;
> }
> arch_initcall(of_add_fixed_phys);
> - - - -
> 
> And remove fixed_phy_add() from the fs_enet. This should work
> nicely and also should be ideologically correct. ;-)
> 
> > How is this supposed to work for modules or for the
> > PPC_CPM_NEW_BINDING mode where the device tree is no longer scanned
> > during fs_soc initialization but during device initialization?
> 
> We should mark fixed.c as bool. Fake/virtual/fixed/platform PHYs
> creation is architecture code anyway, can't be =m.
> 
> -- 
> Anton Vorontsov
> email: cbou@mail.ru
> backup email: ya-cbou@yandex.ru
> irc://irc.freenode.net/bd2

Re: [PATCH 1/3] [NET] phy/fixed.c: rework to not duplicate PHYlayer functionality

[Anton Vorontsov]

On Sun, Dec 02, 2007 at 12:54:36PM +0100, Joakim Tjernlund wrote:
> [SNIP]
> > ^^ the correct solution is to implement arch_initcall function
> > which will create fixed PHYs, and then leave only
> > snprintf(fpi->bus_id, 16, PHY_ID_FMT, 0, *data); part in the
> > fs_enet's find_phy().
> > 
> > Try add something like this to the fsl_soc.c (compile untested):
> > 
> > - - - -
> > static int __init of_add_fixed_phys(void)
> > {
> > 	struct device_node *np;
> > 	const u32 *prop;
> > 	struct fixed_phy_status status = {};
> > 
> > 	while ((np = of_find_node_by_name(NULL, "ethernet"))) {
> > 		data  = of_get_property(np, "fixed-link", NULL);
> > 		if (!data)
> > 			continue;
> > 
> > 		status.link = 1;
> > 		status.duplex = data[1];
> > 		status.speed  = data[2];
> 
> What about Pause and Asym_Pause?

Will be addressed in the next respin of these patches. Let's
hope on Monday.

> Dunno why so few, if any, eth drivers
> impl. it, but the PHY lib supports it.
> Even if fixed PHYs doesn't support it directly I think the OF interface
> should have it.
> 
>     - fixed-link : <a b c d e> where a is emulated phy id - choose any,
>       but unique to the all specified fixed-links, b is duplex - 0 half,
>       1 full, c is link speed - d#10/d#100/d#1000, d is pause - 0 no pause,
>       1 pause, d asym_pause - 0 no asym_pause, 1 asym_pause.

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

Re: [PATCH 1/3] [NET] phy/fixed.c: rework to not duplicate PHY layer functionality

[Jeff Garzik]

Vitaly Bordug wrote:
> With that patch fixed.c now fully emulates MDIO bus, thus no need
> to duplicate PHY layer functionality. That, in turn, drastically
> simplifies the code, and drops down line count.
> 
> As an additional bonus, now there is no need to register MDIO bus
> for each PHY, all emulated PHYs placed on the platform fixed MDIO bus.
> There is also no more need to pre-allocate PHYs via .config option,
> this is all now handled dynamically.
> 
> p.s. Don't even try to understand patch content! Better: apply patch
> and look into resulting drivers/net/phy/fixed.c.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>

ACK, I presume this will go via the ppc tree?

Re: [PATCH 1/3] [NET] phy/fixed.c: rework to not duplicate PHY layer functionality

[Vitaly Bordug]

On Tue, 04 Dec 2007 15:07:49 -0500
Jeff Garzik wrote:

> Vitaly Bordug wrote:
> > With that patch fixed.c now fully emulates MDIO bus, thus no need
> > to duplicate PHY layer functionality. That, in turn, drastically
> > simplifies the code, and drops down line count.
> > 
> > As an additional bonus, now there is no need to register MDIO bus
> > for each PHY, all emulated PHYs placed on the platform fixed MDIO
> > bus. There is also no more need to pre-allocate PHYs via .config
> > option, this is all now handled dynamically.
> > 
> > p.s. Don't even try to understand patch content! Better: apply patch
> > and look into resulting drivers/net/phy/fixed.c.
> > 
> > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> > Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
> 
> ACK, I presume this will go via the ppc tree?
> 

Yes, I'll add your ack in next respin and will ask Paul to consider it,
if you don't mind.

-- 
Sincerely, Vitaly