[PATCHv4 0/5] Add DT support for fixed PHYs

[PATCHv4 0/5] Add DT support for fixed PHYs

[Thomas Petazzoni]

Subject: Add DT support for fixed PHYs

Hello,

Here is a fourth version of the patch set that adds a Device Tree
binding and the related code to support fixed PHYs. I'm hoping to get
this merged in 3.16.

Changes since v3:

 * Rebased on top of v3.15-rc5

 * In patch "net: phy: decouple PHY id and PHY address in fixed PHY
   driver", changed the PHY ID of fixed PHYs from 0xdeadbeef to 0x0,
   as suggested by Grant Likely.

 * Fixed the !CONFIG_PHY_FIXED case in patch "net: phy: extend fixed
   driver with fixed_phy_register()". Noticed by Florian Fainelli.

 * Added Acked-by from Grant Likely and Florian Fainelli on patch
   "net: phy: extend fixed driver with fixed_phy_register()".

 * Reworked the new fixed-link DT binding to be just a sub-node of the
   Ethernet MAC node, and not a node referenced by the 'phy'
   property. This was requested by Grant Likely.

 * Reworked the code implementing the new DT binding to also make it
   accept the old, single property based, DT binding.

 * Added a patch that actually uses the new fixed link DT binding for
   the Armada XP Matrix board.

Changes since v2:

 * Rebased on top of v3.14-rc1, and re-tested on hardware.

 * Removed the RFC tag, since there seems to be some real interest in
   this feature, and the code has gone through several iterations
   already.

 * The error handling in fixed_phy_register() has been fixed.

Changes since v1:

 * Instead of using a 'fixed-link' property inside the Ethernet device
   DT node, with a fairly cryptic succession of integer values, we now
   use a PHY subnode under the Ethernet device DT node, with explicit
   properties to configure the duplex, speed, pause and other PHY
   properties.

 * The PHY address is automatically allocated by the kernel and no
   longer visible in the Device Tree binding.

 * The PHY device is created directly when the network driver calls
   of_phy_connect_fixed_link(), and associated to the PHY DT node,
   which allows the existing of_phy_connect() function to work,
   without the need to use the deprecated of_phy_connect_fixed_link().

Posts of previous versions:

  RFCv1:   http://www.spinics.net/lists/netdev/msg243253.html
  RFCv2:   http://lists.infradead.org/pipermail/linux-arm-kernel/2013-September/196919.html
  PATCHv3: http://www.spinics.net/lists/netdev/msg273117.html

Thanks,

Thomas

Thomas Petazzoni (5):
  net: phy: decouple PHY id and PHY address in fixed PHY driver
  net: phy: extend fixed driver with fixed_phy_register()
  of: provide a binding for fixed link PHYs
  net: mvneta: add support for fixed links
  ARM: mvebu: use the fixed-link PHY DT binding for the Armada XP Matrix
    board

 .../devicetree/bindings/net/fixed-link.txt         | 31 +++++++++
 arch/arm/boot/dts/armada-xp-matrix.dts             |  4 ++
 drivers/net/ethernet/marvell/mvneta.c              | 19 ++++-
 drivers/net/phy/fixed.c                            | 81 +++++++++++++++++++---
 drivers/of/of_mdio.c                               | 67 ++++++++++++++++++
 include/linux/of_mdio.h                            | 15 ++++
 include/linux/phy_fixed.h                          | 11 +++
 7 files changed, 215 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/fixed-link.txt

-- 
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 0/5] Add DT support for fixed PHYs

[Thomas Petazzoni]

Subject: Add DT support for fixed PHYs

Hello,

Here is a fourth version of the patch set that adds a Device Tree
binding and the related code to support fixed PHYs. I'm hoping to get
this merged in 3.16.

Changes since v3:

 * Rebased on top of v3.15-rc5

 * In patch "net: phy: decouple PHY id and PHY address in fixed PHY
   driver", changed the PHY ID of fixed PHYs from 0xdeadbeef to 0x0,
   as suggested by Grant Likely.

 * Fixed the !CONFIG_PHY_FIXED case in patch "net: phy: extend fixed
   driver with fixed_phy_register()". Noticed by Florian Fainelli.

 * Added Acked-by from Grant Likely and Florian Fainelli on patch
   "net: phy: extend fixed driver with fixed_phy_register()".

 * Reworked the new fixed-link DT binding to be just a sub-node of the
   Ethernet MAC node, and not a node referenced by the 'phy'
   property. This was requested by Grant Likely.

 * Reworked the code implementing the new DT binding to also make it
   accept the old, single property based, DT binding.

 * Added a patch that actually uses the new fixed link DT binding for
   the Armada XP Matrix board.

Changes since v2:

 * Rebased on top of v3.14-rc1, and re-tested on hardware.

 * Removed the RFC tag, since there seems to be some real interest in
   this feature, and the code has gone through several iterations
   already.

 * The error handling in fixed_phy_register() has been fixed.

Changes since v1:

 * Instead of using a 'fixed-link' property inside the Ethernet device
   DT node, with a fairly cryptic succession of integer values, we now
   use a PHY subnode under the Ethernet device DT node, with explicit
   properties to configure the duplex, speed, pause and other PHY
   properties.

 * The PHY address is automatically allocated by the kernel and no
   longer visible in the Device Tree binding.

 * The PHY device is created directly when the network driver calls
   of_phy_connect_fixed_link(), and associated to the PHY DT node,
   which allows the existing of_phy_connect() function to work,
   without the need to use the deprecated of_phy_connect_fixed_link().

Posts of previous versions:

  RFCv1:   http://www.spinics.net/lists/netdev/msg243253.html
  RFCv2:   http://lists.infradead.org/pipermail/linux-arm-kernel/2013-September/196919.html
  PATCHv3: http://www.spinics.net/lists/netdev/msg273117.html

Thanks,

Thomas

Thomas Petazzoni (5):
  net: phy: decouple PHY id and PHY address in fixed PHY driver
  net: phy: extend fixed driver with fixed_phy_register()
  of: provide a binding for fixed link PHYs
  net: mvneta: add support for fixed links
  ARM: mvebu: use the fixed-link PHY DT binding for the Armada XP Matrix
    board

 .../devicetree/bindings/net/fixed-link.txt         | 31 +++++++++
 arch/arm/boot/dts/armada-xp-matrix.dts             |  4 ++
 drivers/net/ethernet/marvell/mvneta.c              | 19 ++++-
 drivers/net/phy/fixed.c                            | 81 +++++++++++++++++++---
 drivers/of/of_mdio.c                               | 67 ++++++++++++++++++
 include/linux/of_mdio.h                            | 15 ++++
 include/linux/phy_fixed.h                          | 11 +++
 7 files changed, 215 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/fixed-link.txt

-- 
1.9.3

[PATCHv4 1/5] net: phy: decouple PHY id and PHY address in fixed PHY driver

[Thomas Petazzoni]

Until now, the fixed_phy_add() function was taking as argument
'phy_id', which was used both as the PHY address on the fake fixed
MDIO bus, and as the PHY id, as available in the MII_PHYSID1 and
MII_PHYSID2 registers. However, those two informations are completely
unrelated.

This patch decouples them. The PHY id of fixed PHYs is hardcoded to be
0x0. Ideally, a really reserved value would be nicer, but there
doesn't seem to be an easy of making sure a dummy value can be
assigned to the Linux kernel for such usage.

The PHY address remains passed by the caller of phy_fixed_add().

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/phy/fixed.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
index ba55adf..e41546d 100644
--- a/drivers/net/phy/fixed.c
+++ b/drivers/net/phy/fixed.c
@@ -31,7 +31,7 @@ struct fixed_mdio_bus {
 };
 
 struct fixed_phy {
-	int id;
+	int addr;
 	u16 regs[MII_REGS_NUM];
 	struct phy_device *phydev;
 	struct fixed_phy_status status;
@@ -104,8 +104,8 @@ static int fixed_phy_update_regs(struct fixed_phy *fp)
 	if (fp->status.asym_pause)
 		lpa |= LPA_PAUSE_ASYM;
 
-	fp->regs[MII_PHYSID1] = fp->id >> 16;
-	fp->regs[MII_PHYSID2] = fp->id;
+	fp->regs[MII_PHYSID1] = 0;
+	fp->regs[MII_PHYSID2] = 0;
 
 	fp->regs[MII_BMSR] = bmsr;
 	fp->regs[MII_BMCR] = bmcr;
@@ -115,7 +115,7 @@ static int fixed_phy_update_regs(struct fixed_phy *fp)
 	return 0;
 }
 
-static int fixed_mdio_read(struct mii_bus *bus, int phy_id, int reg_num)
+static int fixed_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num)
 {
 	struct fixed_mdio_bus *fmb = bus->priv;
 	struct fixed_phy *fp;
@@ -124,7 +124,7 @@ static int fixed_mdio_read(struct mii_bus *bus, int phy_id, int reg_num)
 		return -1;
 
 	list_for_each_entry(fp, &fmb->phys, node) {
-		if (fp->id == phy_id) {
+		if (fp->addr == phy_addr) {
 			/* Issue callback if user registered it. */
 			if (fp->link_update) {
 				fp->link_update(fp->phydev->attached_dev,
@@ -138,7 +138,7 @@ static int fixed_mdio_read(struct mii_bus *bus, int phy_id, int reg_num)
 	return 0xFFFF;
 }
 
-static int fixed_mdio_write(struct mii_bus *bus, int phy_id, int reg_num,
+static int fixed_mdio_write(struct mii_bus *bus, int phy_addr, int reg_num,
 			    u16 val)
 {
 	return 0;
@@ -160,7 +160,7 @@ int fixed_phy_set_link_update(struct phy_device *phydev,
 		return -EINVAL;
 
 	list_for_each_entry(fp, &fmb->phys, node) {
-		if (fp->id == phydev->phy_id) {
+		if (fp->addr == phydev->addr) {
 			fp->link_update = link_update;
 			fp->phydev = phydev;
 			return 0;
@@ -171,7 +171,7 @@ int fixed_phy_set_link_update(struct phy_device *phydev,
 }
 EXPORT_SYMBOL_GPL(fixed_phy_set_link_update);
 
-int fixed_phy_add(unsigned int irq, int phy_id,
+int fixed_phy_add(unsigned int irq, int phy_addr,
 		  struct fixed_phy_status *status)
 {
 	int ret;
@@ -184,9 +184,9 @@ int fixed_phy_add(unsigned int irq, int phy_id,
 
 	memset(fp->regs, 0xFF,  sizeof(fp->regs[0]) * MII_REGS_NUM);
 
-	fmb->irqs[phy_id] = irq;
+	fmb->irqs[phy_addr] = irq;
 
-	fp->id = phy_id;
+	fp->addr = phy_addr;
 	fp->status = *status;
 
 	ret = fixed_phy_update_regs(fp);
-- 
1.9.3

[PATCHv4 1/5] net: phy: decouple PHY id and PHY address in fixed PHY driver

[Thomas Petazzoni]

Until now, the fixed_phy_add() function was taking as argument
'phy_id', which was used both as the PHY address on the fake fixed
MDIO bus, and as the PHY id, as available in the MII_PHYSID1 and
MII_PHYSID2 registers. However, those two informations are completely
unrelated.

This patch decouples them. The PHY id of fixed PHYs is hardcoded to be
0x0. Ideally, a really reserved value would be nicer, but there
doesn't seem to be an easy of making sure a dummy value can be
assigned to the Linux kernel for such usage.

The PHY address remains passed by the caller of phy_fixed_add().

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/phy/fixed.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
index ba55adf..e41546d 100644
--- a/drivers/net/phy/fixed.c
+++ b/drivers/net/phy/fixed.c
@@ -31,7 +31,7 @@ struct fixed_mdio_bus {
 };
 
 struct fixed_phy {
-	int id;
+	int addr;
 	u16 regs[MII_REGS_NUM];
 	struct phy_device *phydev;
 	struct fixed_phy_status status;
@@ -104,8 +104,8 @@ static int fixed_phy_update_regs(struct fixed_phy *fp)
 	if (fp->status.asym_pause)
 		lpa |= LPA_PAUSE_ASYM;
 
-	fp->regs[MII_PHYSID1] = fp->id >> 16;
-	fp->regs[MII_PHYSID2] = fp->id;
+	fp->regs[MII_PHYSID1] = 0;
+	fp->regs[MII_PHYSID2] = 0;
 
 	fp->regs[MII_BMSR] = bmsr;
 	fp->regs[MII_BMCR] = bmcr;
@@ -115,7 +115,7 @@ static int fixed_phy_update_regs(struct fixed_phy *fp)
 	return 0;
 }
 
-static int fixed_mdio_read(struct mii_bus *bus, int phy_id, int reg_num)
+static int fixed_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num)
 {
 	struct fixed_mdio_bus *fmb = bus->priv;
 	struct fixed_phy *fp;
@@ -124,7 +124,7 @@ static int fixed_mdio_read(struct mii_bus *bus, int phy_id, int reg_num)
 		return -1;
 
 	list_for_each_entry(fp, &fmb->phys, node) {
-		if (fp->id == phy_id) {
+		if (fp->addr == phy_addr) {
 			/* Issue callback if user registered it. */
 			if (fp->link_update) {
 				fp->link_update(fp->phydev->attached_dev,
@@ -138,7 +138,7 @@ static int fixed_mdio_read(struct mii_bus *bus, int phy_id, int reg_num)
 	return 0xFFFF;
 }
 
-static int fixed_mdio_write(struct mii_bus *bus, int phy_id, int reg_num,
+static int fixed_mdio_write(struct mii_bus *bus, int phy_addr, int reg_num,
 			    u16 val)
 {
 	return 0;
@@ -160,7 +160,7 @@ int fixed_phy_set_link_update(struct phy_device *phydev,
 		return -EINVAL;
 
 	list_for_each_entry(fp, &fmb->phys, node) {
-		if (fp->id == phydev->phy_id) {
+		if (fp->addr == phydev->addr) {
 			fp->link_update = link_update;
 			fp->phydev = phydev;
 			return 0;
@@ -171,7 +171,7 @@ int fixed_phy_set_link_update(struct phy_device *phydev,
 }
 EXPORT_SYMBOL_GPL(fixed_phy_set_link_update);
 
-int fixed_phy_add(unsigned int irq, int phy_id,
+int fixed_phy_add(unsigned int irq, int phy_addr,
 		  struct fixed_phy_status *status)
 {
 	int ret;
@@ -184,9 +184,9 @@ int fixed_phy_add(unsigned int irq, int phy_id,
 
 	memset(fp->regs, 0xFF,  sizeof(fp->regs[0]) * MII_REGS_NUM);
 
-	fmb->irqs[phy_id] = irq;
+	fmb->irqs[phy_addr] = irq;
 
-	fp->id = phy_id;
+	fp->addr = phy_addr;
 	fp->status = *status;
 
 	ret = fixed_phy_update_regs(fp);
-- 
1.9.3

[PATCHv4 2/5] net: phy: extend fixed driver with fixed_phy_register()

[Thomas Petazzoni]

The existing fixed_phy_add() function has several drawbacks that
prevents it from being used as is for OF-based declaration of fixed
PHYs:

 * The address of the PHY on the fake bus needs to be passed, while a
   dynamic allocation is desired.

 * Since the phy_device instantiation is post-poned until the next
   mdiobus scan, there is no way to associate the fixed PHY with its
   OF node, which later prevents of_phy_connect() from finding this
   fixed PHY from a given OF node.

To solve this, this commit introduces fixed_phy_register(), which will
allocate an available PHY address, add the PHY using fixed_phy_add()
and instantiate the phy_device structure associated with the provided
OF node.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Grant Likely <grant.likely@linaro.org>
---
 drivers/net/phy/fixed.c   | 61 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/phy_fixed.h | 11 +++++++++
 2 files changed, 72 insertions(+)

diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
index e41546d..d60d875 100644
--- a/drivers/net/phy/fixed.c
+++ b/drivers/net/phy/fixed.c
@@ -21,6 +21,7 @@
 #include <linux/phy_fixed.h>
 #include <linux/err.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 
 #define MII_REGS_NUM 29
 
@@ -203,6 +204,66 @@ err_regs:
 }
 EXPORT_SYMBOL_GPL(fixed_phy_add);
 
+void fixed_phy_del(int phy_addr)
+{
+	struct fixed_mdio_bus *fmb = &platform_fmb;
+	struct fixed_phy *fp, *tmp;
+
+	list_for_each_entry_safe(fp, tmp, &fmb->phys, node) {
+		if (fp->addr == phy_addr) {
+			list_del(&fp->node);
+			kfree(fp);
+			return;
+		}
+	}
+}
+EXPORT_SYMBOL_GPL(fixed_phy_del);
+
+static int phy_fixed_addr;
+static DEFINE_SPINLOCK(phy_fixed_addr_lock);
+
+int fixed_phy_register(unsigned int irq,
+		       struct fixed_phy_status *status,
+		       struct device_node *np)
+{
+	struct fixed_mdio_bus *fmb = &platform_fmb;
+	struct phy_device *phy;
+	int phy_addr;
+	int ret;
+
+	/* Get the next available PHY address, up to PHY_MAX_ADDR */
+	spin_lock(&phy_fixed_addr_lock);
+	if (phy_fixed_addr == PHY_MAX_ADDR) {
+		spin_unlock(&phy_fixed_addr_lock);
+		return -ENOSPC;
+	}
+	phy_addr = phy_fixed_addr++;
+	spin_unlock(&phy_fixed_addr_lock);
+
+	ret = fixed_phy_add(PHY_POLL, phy_addr, status);
+	if (ret < 0)
+		return ret;
+
+	phy = get_phy_device(fmb->mii_bus, phy_addr, false);
+	if (!phy || IS_ERR(phy)) {
+		fixed_phy_del(phy_addr);
+		return -EINVAL;
+	}
+
+	of_node_get(np);
+	phy->dev.of_node = np;
+
+	ret = phy_device_register(phy);
+	if (ret) {
+		phy_device_free(phy);
+		of_node_put(np);
+		fixed_phy_del(phy_addr);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int __init fixed_mdio_bus_init(void)
 {
 	struct fixed_mdio_bus *fmb = &platform_fmb;
diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h
index 509d8f5..4f2478b 100644
--- a/include/linux/phy_fixed.h
+++ b/include/linux/phy_fixed.h
@@ -9,15 +9,26 @@ struct fixed_phy_status {
 	int asym_pause;
 };
 
+struct device_node;
+
 #ifdef CONFIG_FIXED_PHY
 extern int fixed_phy_add(unsigned int irq, int phy_id,
 			 struct fixed_phy_status *status);
+extern int fixed_phy_register(unsigned int irq,
+			      struct fixed_phy_status *status,
+			      struct device_node *np);
 #else
 static inline int fixed_phy_add(unsigned int irq, int phy_id,
 				struct fixed_phy_status *status)
 {
 	return -ENODEV;
 }
+static inline int fixed_phy_register(unsigned int irq,
+				     struct fixed_phy_status *status,
+				     struct device_node *np)
+{
+	return -ENODEV;
+}
 #endif /* CONFIG_FIXED_PHY */
 
 /*
-- 
1.9.3

[PATCHv4 2/5] net: phy: extend fixed driver with fixed_phy_register()

[Thomas Petazzoni]

The existing fixed_phy_add() function has several drawbacks that
prevents it from being used as is for OF-based declaration of fixed
PHYs:

 * The address of the PHY on the fake bus needs to be passed, while a
   dynamic allocation is desired.

 * Since the phy_device instantiation is post-poned until the next
   mdiobus scan, there is no way to associate the fixed PHY with its
   OF node, which later prevents of_phy_connect() from finding this
   fixed PHY from a given OF node.

To solve this, this commit introduces fixed_phy_register(), which will
allocate an available PHY address, add the PHY using fixed_phy_add()
and instantiate the phy_device structure associated with the provided
OF node.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Grant Likely <grant.likely@linaro.org>
---
 drivers/net/phy/fixed.c   | 61 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/phy_fixed.h | 11 +++++++++
 2 files changed, 72 insertions(+)

diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
index e41546d..d60d875 100644
--- a/drivers/net/phy/fixed.c
+++ b/drivers/net/phy/fixed.c
@@ -21,6 +21,7 @@
 #include <linux/phy_fixed.h>
 #include <linux/err.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 
 #define MII_REGS_NUM 29
 
@@ -203,6 +204,66 @@ err_regs:
 }
 EXPORT_SYMBOL_GPL(fixed_phy_add);
 
+void fixed_phy_del(int phy_addr)
+{
+	struct fixed_mdio_bus *fmb = &platform_fmb;
+	struct fixed_phy *fp, *tmp;
+
+	list_for_each_entry_safe(fp, tmp, &fmb->phys, node) {
+		if (fp->addr == phy_addr) {
+			list_del(&fp->node);
+			kfree(fp);
+			return;
+		}
+	}
+}
+EXPORT_SYMBOL_GPL(fixed_phy_del);
+
+static int phy_fixed_addr;
+static DEFINE_SPINLOCK(phy_fixed_addr_lock);
+
+int fixed_phy_register(unsigned int irq,
+		       struct fixed_phy_status *status,
+		       struct device_node *np)
+{
+	struct fixed_mdio_bus *fmb = &platform_fmb;
+	struct phy_device *phy;
+	int phy_addr;
+	int ret;
+
+	/* Get the next available PHY address, up to PHY_MAX_ADDR */
+	spin_lock(&phy_fixed_addr_lock);
+	if (phy_fixed_addr == PHY_MAX_ADDR) {
+		spin_unlock(&phy_fixed_addr_lock);
+		return -ENOSPC;
+	}
+	phy_addr = phy_fixed_addr++;
+	spin_unlock(&phy_fixed_addr_lock);
+
+	ret = fixed_phy_add(PHY_POLL, phy_addr, status);
+	if (ret < 0)
+		return ret;
+
+	phy = get_phy_device(fmb->mii_bus, phy_addr, false);
+	if (!phy || IS_ERR(phy)) {
+		fixed_phy_del(phy_addr);
+		return -EINVAL;
+	}
+
+	of_node_get(np);
+	phy->dev.of_node = np;
+
+	ret = phy_device_register(phy);
+	if (ret) {
+		phy_device_free(phy);
+		of_node_put(np);
+		fixed_phy_del(phy_addr);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int __init fixed_mdio_bus_init(void)
 {
 	struct fixed_mdio_bus *fmb = &platform_fmb;
diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h
index 509d8f5..4f2478b 100644
--- a/include/linux/phy_fixed.h
+++ b/include/linux/phy_fixed.h
@@ -9,15 +9,26 @@ struct fixed_phy_status {
 	int asym_pause;
 };
 
+struct device_node;
+
 #ifdef CONFIG_FIXED_PHY
 extern int fixed_phy_add(unsigned int irq, int phy_id,
 			 struct fixed_phy_status *status);
+extern int fixed_phy_register(unsigned int irq,
+			      struct fixed_phy_status *status,
+			      struct device_node *np);
 #else
 static inline int fixed_phy_add(unsigned int irq, int phy_id,
 				struct fixed_phy_status *status)
 {
 	return -ENODEV;
 }
+static inline int fixed_phy_register(unsigned int irq,
+				     struct fixed_phy_status *status,
+				     struct device_node *np)
+{
+	return -ENODEV;
+}
 #endif /* CONFIG_FIXED_PHY */
 
 /*
-- 
1.9.3

[PATCHv4 3/5] of: provide a binding for fixed link PHYs

[Thomas Petazzoni]

Some Ethernet MACs have a "fixed link", and are not connected to a
normal MDIO-managed PHY device. For those situations, a Device Tree
binding allows to describe a "fixed link" using a special PHY node.

This patch adds:

 * A documentation for the fixed PHY Device Tree binding.

 * An of_phy_is_fixed_link() function that an Ethernet driver can call
   on its PHY phandle to find out whether it's a fixed link PHY or
   not. It should typically be used to know if
   of_phy_register_fixed_link() should be called.

 * An of_phy_register_fixed_link() function that instantiates the
   fixed PHY into the PHY subsystem, so that when the driver calls
   of_phy_connect(), the PHY device associated to the OF node will be
   found.

These two additional functions also support the old fixed-link Device
Tree binding used on PowerPC platforms, so that ultimately, the
network device drivers for those platforms could be converted to use
of_phy_is_fixed_link() and of_phy_register_fixed_link() instead of
of_phy_connect_fixed_link(), while keeping compatibility with their
respective Device Tree bindings.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 .../devicetree/bindings/net/fixed-link.txt         | 31 ++++++++++
 drivers/of/of_mdio.c                               | 67 ++++++++++++++++++++++
 include/linux/of_mdio.h                            | 15 +++++
 3 files changed, 113 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/fixed-link.txt

diff --git a/Documentation/devicetree/bindings/net/fixed-link.txt b/Documentation/devicetree/bindings/net/fixed-link.txt
new file mode 100644
index 0000000..377f775
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/fixed-link.txt
@@ -0,0 +1,31 @@
+Fixed link Device Tree binding
+------------------------------
+
+Some Ethernet MACs have a "fixed link", and are not connected to a
+normal MDIO-managed PHY device. For those situations, a Device Tree
+binding allows to describe a "fixed link".
+
+Such a fixed link situation is described by creating a 'fixed-link'
+sub-node of the Ethernet MAC device node, with the following
+properties:
+
+* 'speed' (integer, mandatory), to indicate the link speed. Accepted
+  values are 10, 100 and 1000
+* 'full-duplex' (boolean, optional), to indicate that full duplex is
+  used. When absent, half duplex is assumed.
+* 'pause' (boolean, optional), to indicate that pause should be
+  enabled.
+* 'asym-pause' (boolean, optional), to indicate that asym_pause should
+  be enabled.
+
+Example:
+
+ethernet@0 {
+	...
+	fixed-link {
+	      speed = <1000>;
+	      full-duplex;
+	};
+	...
+};
+
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 9a95831..1def0bb 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -14,6 +14,7 @@
 #include <linux/netdevice.h>
 #include <linux/err.h>
 #include <linux/phy.h>
+#include <linux/phy_fixed.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
 #include <linux/of_mdio.h>
@@ -301,3 +302,69 @@ struct phy_device *of_phy_attach(struct net_device *dev,
 	return phy_attach_direct(dev, phy, flags, iface) ? NULL : phy;
 }
 EXPORT_SYMBOL(of_phy_attach);
+
+#if defined(CONFIG_FIXED_PHY)
+/*
+ * of_phy_is_fixed_link() and of_phy_register_fixed_link() must
+ * support two DT bindings:
+ * - the old DT binding, where 'fixed-link' was a property with 5
+ *   cells encoding various informations about the fixed PHY
+ * - the new DT binding, where 'fixed-link' is a sub-node of the
+ *   Ethernet device.
+ */
+bool of_phy_is_fixed_link(struct device_node *np)
+{
+	struct device_node *dn;
+	int len;
+
+	/* New binding */
+	dn = of_get_child_by_name(np, "fixed-link");
+	if (dn) {
+		of_node_put(dn);
+		return true;
+	}
+
+	/* Old binding */
+	if (of_get_property(np, "fixed-link", &len) &&
+	    len == (5 * sizeof(__be32)))
+		return true;
+
+	return false;
+}
+EXPORT_SYMBOL(of_phy_is_fixed_link);
+
+int of_phy_register_fixed_link(struct device_node *np)
+{
+	struct fixed_phy_status status = {};
+	struct device_node *fixed_link_node;
+	const __be32 *fixed_link_prop;
+	int len;
+
+	/* New binding */
+	fixed_link_node = of_get_child_by_name(np, "fixed-link");
+	if (fixed_link_node) {
+		status.link = 1;
+		status.duplex = of_property_read_bool(np, "full-duplex");
+		if (of_property_read_u32(fixed_link_node, "speed", &status.speed))
+			return -EINVAL;
+		status.pause = of_property_read_bool(np, "pause");
+		status.asym_pause = of_property_read_bool(np, "asym-pause");
+		of_node_put(fixed_link_node);
+		return fixed_phy_register(PHY_POLL, &status, np);
+	}
+
+	/* Old binding */
+	fixed_link_prop = of_get_property(np, "fixed-link", &len);
+	if (fixed_link_prop && len == (5 * sizeof(__be32))) {
+		status.link = 1;
+		status.duplex = be32_to_cpu(fixed_link_prop[1]);
+		status.speed = be32_to_cpu(fixed_link_prop[2]);
+		status.pause = be32_to_cpu(fixed_link_prop[3]);
+		status.asym_pause = be32_to_cpu(fixed_link_prop[4]);
+		return fixed_phy_register(PHY_POLL, &status, np);
+	}
+
+	return -ENODEV;
+}
+EXPORT_SYMBOL(of_phy_register_fixed_link);
+#endif
diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
index 6fe8464..77a6e32 100644
--- a/include/linux/of_mdio.h
+++ b/include/linux/of_mdio.h
@@ -67,4 +67,19 @@ static inline struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np)
 }
 #endif /* CONFIG_OF */
 
+#if defined(CONFIG_OF) && defined(CONFIG_FIXED_PHY)
+extern int of_phy_register_fixed_link(struct device_node *np);
+extern bool of_phy_is_fixed_link(struct device_node *np);
+#else
+static inline int of_phy_register_fixed_link(struct device_node *np)
+{
+	return -ENOSYS;
+}
+static inline bool of_phy_is_fixed_link(struct device_node *np)
+{
+	return false;
+}
+#endif
+
+
 #endif /* __LINUX_OF_MDIO_H */
-- 
1.9.3

[PATCHv4 3/5] of: provide a binding for fixed link PHYs

[Thomas Petazzoni]

Some Ethernet MACs have a "fixed link", and are not connected to a
normal MDIO-managed PHY device. For those situations, a Device Tree
binding allows to describe a "fixed link" using a special PHY node.

This patch adds:

 * A documentation for the fixed PHY Device Tree binding.

 * An of_phy_is_fixed_link() function that an Ethernet driver can call
   on its PHY phandle to find out whether it's a fixed link PHY or
   not. It should typically be used to know if
   of_phy_register_fixed_link() should be called.

 * An of_phy_register_fixed_link() function that instantiates the
   fixed PHY into the PHY subsystem, so that when the driver calls
   of_phy_connect(), the PHY device associated to the OF node will be
   found.

These two additional functions also support the old fixed-link Device
Tree binding used on PowerPC platforms, so that ultimately, the
network device drivers for those platforms could be converted to use
of_phy_is_fixed_link() and of_phy_register_fixed_link() instead of
of_phy_connect_fixed_link(), while keeping compatibility with their
respective Device Tree bindings.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 .../devicetree/bindings/net/fixed-link.txt         | 31 ++++++++++
 drivers/of/of_mdio.c                               | 67 ++++++++++++++++++++++
 include/linux/of_mdio.h                            | 15 +++++
 3 files changed, 113 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/fixed-link.txt

diff --git a/Documentation/devicetree/bindings/net/fixed-link.txt b/Documentation/devicetree/bindings/net/fixed-link.txt
new file mode 100644
index 0000000..377f775
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/fixed-link.txt
@@ -0,0 +1,31 @@
+Fixed link Device Tree binding
+------------------------------
+
+Some Ethernet MACs have a "fixed link", and are not connected to a
+normal MDIO-managed PHY device. For those situations, a Device Tree
+binding allows to describe a "fixed link".
+
+Such a fixed link situation is described by creating a 'fixed-link'
+sub-node of the Ethernet MAC device node, with the following
+properties:
+
+* 'speed' (integer, mandatory), to indicate the link speed. Accepted
+  values are 10, 100 and 1000
+* 'full-duplex' (boolean, optional), to indicate that full duplex is
+  used. When absent, half duplex is assumed.
+* 'pause' (boolean, optional), to indicate that pause should be
+  enabled.
+* 'asym-pause' (boolean, optional), to indicate that asym_pause should
+  be enabled.
+
+Example:
+
+ethernet at 0 {
+	...
+	fixed-link {
+	      speed = <1000>;
+	      full-duplex;
+	};
+	...
+};
+
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 9a95831..1def0bb 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -14,6 +14,7 @@
 #include <linux/netdevice.h>
 #include <linux/err.h>
 #include <linux/phy.h>
+#include <linux/phy_fixed.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
 #include <linux/of_mdio.h>
@@ -301,3 +302,69 @@ struct phy_device *of_phy_attach(struct net_device *dev,
 	return phy_attach_direct(dev, phy, flags, iface) ? NULL : phy;
 }
 EXPORT_SYMBOL(of_phy_attach);
+
+#if defined(CONFIG_FIXED_PHY)
+/*
+ * of_phy_is_fixed_link() and of_phy_register_fixed_link() must
+ * support two DT bindings:
+ * - the old DT binding, where 'fixed-link' was a property with 5
+ *   cells encoding various informations about the fixed PHY
+ * - the new DT binding, where 'fixed-link' is a sub-node of the
+ *   Ethernet device.
+ */
+bool of_phy_is_fixed_link(struct device_node *np)
+{
+	struct device_node *dn;
+	int len;
+
+	/* New binding */
+	dn = of_get_child_by_name(np, "fixed-link");
+	if (dn) {
+		of_node_put(dn);
+		return true;
+	}
+
+	/* Old binding */
+	if (of_get_property(np, "fixed-link", &len) &&
+	    len == (5 * sizeof(__be32)))
+		return true;
+
+	return false;
+}
+EXPORT_SYMBOL(of_phy_is_fixed_link);
+
+int of_phy_register_fixed_link(struct device_node *np)
+{
+	struct fixed_phy_status status = {};
+	struct device_node *fixed_link_node;
+	const __be32 *fixed_link_prop;
+	int len;
+
+	/* New binding */
+	fixed_link_node = of_get_child_by_name(np, "fixed-link");
+	if (fixed_link_node) {
+		status.link = 1;
+		status.duplex = of_property_read_bool(np, "full-duplex");
+		if (of_property_read_u32(fixed_link_node, "speed", &status.speed))
+			return -EINVAL;
+		status.pause = of_property_read_bool(np, "pause");
+		status.asym_pause = of_property_read_bool(np, "asym-pause");
+		of_node_put(fixed_link_node);
+		return fixed_phy_register(PHY_POLL, &status, np);
+	}
+
+	/* Old binding */
+	fixed_link_prop = of_get_property(np, "fixed-link", &len);
+	if (fixed_link_prop && len == (5 * sizeof(__be32))) {
+		status.link = 1;
+		status.duplex = be32_to_cpu(fixed_link_prop[1]);
+		status.speed = be32_to_cpu(fixed_link_prop[2]);
+		status.pause = be32_to_cpu(fixed_link_prop[3]);
+		status.asym_pause = be32_to_cpu(fixed_link_prop[4]);
+		return fixed_phy_register(PHY_POLL, &status, np);
+	}
+
+	return -ENODEV;
+}
+EXPORT_SYMBOL(of_phy_register_fixed_link);
+#endif
diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
index 6fe8464..77a6e32 100644
--- a/include/linux/of_mdio.h
+++ b/include/linux/of_mdio.h
@@ -67,4 +67,19 @@ static inline struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np)
 }
 #endif /* CONFIG_OF */
 
+#if defined(CONFIG_OF) && defined(CONFIG_FIXED_PHY)
+extern int of_phy_register_fixed_link(struct device_node *np);
+extern bool of_phy_is_fixed_link(struct device_node *np);
+#else
+static inline int of_phy_register_fixed_link(struct device_node *np)
+{
+	return -ENOSYS;
+}
+static inline bool of_phy_is_fixed_link(struct device_node *np)
+{
+	return false;
+}
+#endif
+
+
 #endif /* __LINUX_OF_MDIO_H */
-- 
1.9.3

[PATCHv4 4/5] net: mvneta: add support for fixed links

[Thomas Petazzoni]

Following the introduction of of_phy_register_fixed_link(), this patch
introduces fixed link support in the mvneta driver, for Marvell Armada
370/XP SOCs.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvneta.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 14786c8..f232e3b 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -2797,9 +2797,22 @@ static int mvneta_probe(struct platform_device *pdev)
 
 	phy_node = of_parse_phandle(dn, "phy", 0);
 	if (!phy_node) {
-		dev_err(&pdev->dev, "no associated PHY\n");
-		err = -ENODEV;
-		goto err_free_irq;
+		if (!of_phy_is_fixed_link(dn)) {
+			dev_err(&pdev->dev, "no PHY specified\n");
+			err = -ENODEV;
+			goto err_free_irq;
+		}
+
+		err = of_phy_register_fixed_link(dn);
+		if (err < 0) {
+			dev_err(&pdev->dev, "cannot register fixed PHY\n");
+			goto err_free_irq;
+		}
+
+		/* In the case of a fixed PHY, the DT node associated
+		 * to the PHY is the Ethernet MAC DT node.
+		 */
+		phy_node = dn;
 	}
 
 	phy_mode = of_get_phy_mode(dn);
-- 
1.9.3

[PATCHv4 4/5] net: mvneta: add support for fixed links

[Thomas Petazzoni]

Following the introduction of of_phy_register_fixed_link(), this patch
introduces fixed link support in the mvneta driver, for Marvell Armada
370/XP SOCs.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvneta.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 14786c8..f232e3b 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -2797,9 +2797,22 @@ static int mvneta_probe(struct platform_device *pdev)
 
 	phy_node = of_parse_phandle(dn, "phy", 0);
 	if (!phy_node) {
-		dev_err(&pdev->dev, "no associated PHY\n");
-		err = -ENODEV;
-		goto err_free_irq;
+		if (!of_phy_is_fixed_link(dn)) {
+			dev_err(&pdev->dev, "no PHY specified\n");
+			err = -ENODEV;
+			goto err_free_irq;
+		}
+
+		err = of_phy_register_fixed_link(dn);
+		if (err < 0) {
+			dev_err(&pdev->dev, "cannot register fixed PHY\n");
+			goto err_free_irq;
+		}
+
+		/* In the case of a fixed PHY, the DT node associated
+		 * to the PHY is the Ethernet MAC DT node.
+		 */
+		phy_node = dn;
 	}
 
 	phy_mode = of_get_phy_mode(dn);
-- 
1.9.3

[PATCHv4 5/5] ARM: mvebu: use the fixed-link PHY DT binding for the Armada XP Matrix board

[Thomas Petazzoni]

The Armada XP Matrix board has an Ethernet PHY that isn't configurable
through the MDIO bus, so we use the newly introduced fixed-link PHY DT
binding to represent the PHY of this platform and get network working.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/boot/dts/armada-xp-matrix.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/armada-xp-matrix.dts b/arch/arm/boot/dts/armada-xp-matrix.dts
index c224274..3bb8c00 100644
--- a/arch/arm/boot/dts/armada-xp-matrix.dts
+++ b/arch/arm/boot/dts/armada-xp-matrix.dts
@@ -61,6 +61,10 @@
 			ethernet@30000 {
 				status = "okay";
 				phy-mode = "sgmii";
+				fixed-link {
+					speed = <1000>;
+					full-duplex;
+				};
 			};
 
 			pcie-controller {
-- 
1.9.3

[PATCHv4 5/5] ARM: mvebu: use the fixed-link PHY DT binding for the Armada XP Matrix board

[Thomas Petazzoni]

The Armada XP Matrix board has an Ethernet PHY that isn't configurable
through the MDIO bus, so we use the newly introduced fixed-link PHY DT
binding to represent the PHY of this platform and get network working.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/boot/dts/armada-xp-matrix.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/armada-xp-matrix.dts b/arch/arm/boot/dts/armada-xp-matrix.dts
index c224274..3bb8c00 100644
--- a/arch/arm/boot/dts/armada-xp-matrix.dts
+++ b/arch/arm/boot/dts/armada-xp-matrix.dts
@@ -61,6 +61,10 @@
 			ethernet at 30000 {
 				status = "okay";
 				phy-mode = "sgmii";
+				fixed-link {
+					speed = <1000>;
+					full-duplex;
+				};
 			};
 
 			pcie-controller {
-- 
1.9.3

Re: [PATCHv4 0/5] Add DT support for fixed PHYs

[Florian Fainelli]

Hi Thomas,

2014-05-16 7:14 GMT-07:00 Thomas Petazzoni
<thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
> Subject: Add DT support for fixed PHYs
>
> Hello,
>
> Here is a fourth version of the patch set that adds a Device Tree
> binding and the related code to support fixed PHYs. I'm hoping to get
> this merged in 3.16.

Thanks for being persistent on this and coming back with these
patches, that is really appreciated.

I gave this patch set a try on a brcmstb system with SYSTEMPORT and my
latest changes to net/dsa/ to allow for different PHY setups to be
used with the SoC integrated switch. This switch has a mix of fixed
and real PHYs. This works like a charm, so long as you consistently
use the "new" functions, and not the old ones, which is totally
expected. This is excellent work!

As we discussed offline, I think we need to progressively get rid of
of_phy_connect_fixed() to avoid having to debug situations where the
old 'fixed-link' property used some sparse addressing scheme, which is
now incompatible with the dynamic address allocation scheme.

>
> Changes since v3:
>
>  * Rebased on top of v3.15-rc5
>
>  * In patch "net: phy: decouple PHY id and PHY address in fixed PHY
>    driver", changed the PHY ID of fixed PHYs from 0xdeadbeef to 0x0,
>    as suggested by Grant Likely.
>
>  * Fixed the !CONFIG_PHY_FIXED case in patch "net: phy: extend fixed
>    driver with fixed_phy_register()". Noticed by Florian Fainelli.
>
>  * Added Acked-by from Grant Likely and Florian Fainelli on patch
>    "net: phy: extend fixed driver with fixed_phy_register()".
>
>  * Reworked the new fixed-link DT binding to be just a sub-node of the
>    Ethernet MAC node, and not a node referenced by the 'phy'
>    property. This was requested by Grant Likely.
>
>  * Reworked the code implementing the new DT binding to also make it
>    accept the old, single property based, DT binding.
>
>  * Added a patch that actually uses the new fixed link DT binding for
>    the Armada XP Matrix board.
>
> Changes since v2:
>
>  * Rebased on top of v3.14-rc1, and re-tested on hardware.
>
>  * Removed the RFC tag, since there seems to be some real interest in
>    this feature, and the code has gone through several iterations
>    already.
>
>  * The error handling in fixed_phy_register() has been fixed.
>
> Changes since v1:
>
>  * Instead of using a 'fixed-link' property inside the Ethernet device
>    DT node, with a fairly cryptic succession of integer values, we now
>    use a PHY subnode under the Ethernet device DT node, with explicit
>    properties to configure the duplex, speed, pause and other PHY
>    properties.
>
>  * The PHY address is automatically allocated by the kernel and no
>    longer visible in the Device Tree binding.
>
>  * The PHY device is created directly when the network driver calls
>    of_phy_connect_fixed_link(), and associated to the PHY DT node,
>    which allows the existing of_phy_connect() function to work,
>    without the need to use the deprecated of_phy_connect_fixed_link().
>
> Posts of previous versions:
>
>   RFCv1:   http://www.spinics.net/lists/netdev/msg243253.html
>   RFCv2:   http://lists.infradead.org/pipermail/linux-arm-kernel/2013-September/196919.html
>   PATCHv3: http://www.spinics.net/lists/netdev/msg273117.html
>
> Thanks,
>
> Thomas
>
> Thomas Petazzoni (5):
>   net: phy: decouple PHY id and PHY address in fixed PHY driver
>   net: phy: extend fixed driver with fixed_phy_register()
>   of: provide a binding for fixed link PHYs
>   net: mvneta: add support for fixed links
>   ARM: mvebu: use the fixed-link PHY DT binding for the Armada XP Matrix
>     board
>
>  .../devicetree/bindings/net/fixed-link.txt         | 31 +++++++++
>  arch/arm/boot/dts/armada-xp-matrix.dts             |  4 ++
>  drivers/net/ethernet/marvell/mvneta.c              | 19 ++++-
>  drivers/net/phy/fixed.c                            | 81 +++++++++++++++++++---
>  drivers/of/of_mdio.c                               | 67 ++++++++++++++++++
>  include/linux/of_mdio.h                            | 15 ++++
>  include/linux/phy_fixed.h                          | 11 +++
>  7 files changed, 215 insertions(+), 13 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/fixed-link.txt
>
> --
> 1.9.3

-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 0/5] Add DT support for fixed PHYs

[Florian Fainelli]

Hi Thomas,

2014-05-16 7:14 GMT-07:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Subject: Add DT support for fixed PHYs
>
> Hello,
>
> Here is a fourth version of the patch set that adds a Device Tree
> binding and the related code to support fixed PHYs. I'm hoping to get
> this merged in 3.16.

Thanks for being persistent on this and coming back with these
patches, that is really appreciated.

I gave this patch set a try on a brcmstb system with SYSTEMPORT and my
latest changes to net/dsa/ to allow for different PHY setups to be
used with the SoC integrated switch. This switch has a mix of fixed
and real PHYs. This works like a charm, so long as you consistently
use the "new" functions, and not the old ones, which is totally
expected. This is excellent work!

As we discussed offline, I think we need to progressively get rid of
of_phy_connect_fixed() to avoid having to debug situations where the
old 'fixed-link' property used some sparse addressing scheme, which is
now incompatible with the dynamic address allocation scheme.

>
> Changes since v3:
>
>  * Rebased on top of v3.15-rc5
>
>  * In patch "net: phy: decouple PHY id and PHY address in fixed PHY
>    driver", changed the PHY ID of fixed PHYs from 0xdeadbeef to 0x0,
>    as suggested by Grant Likely.
>
>  * Fixed the !CONFIG_PHY_FIXED case in patch "net: phy: extend fixed
>    driver with fixed_phy_register()". Noticed by Florian Fainelli.
>
>  * Added Acked-by from Grant Likely and Florian Fainelli on patch
>    "net: phy: extend fixed driver with fixed_phy_register()".
>
>  * Reworked the new fixed-link DT binding to be just a sub-node of the
>    Ethernet MAC node, and not a node referenced by the 'phy'
>    property. This was requested by Grant Likely.
>
>  * Reworked the code implementing the new DT binding to also make it
>    accept the old, single property based, DT binding.
>
>  * Added a patch that actually uses the new fixed link DT binding for
>    the Armada XP Matrix board.
>
> Changes since v2:
>
>  * Rebased on top of v3.14-rc1, and re-tested on hardware.
>
>  * Removed the RFC tag, since there seems to be some real interest in
>    this feature, and the code has gone through several iterations
>    already.
>
>  * The error handling in fixed_phy_register() has been fixed.
>
> Changes since v1:
>
>  * Instead of using a 'fixed-link' property inside the Ethernet device
>    DT node, with a fairly cryptic succession of integer values, we now
>    use a PHY subnode under the Ethernet device DT node, with explicit
>    properties to configure the duplex, speed, pause and other PHY
>    properties.
>
>  * The PHY address is automatically allocated by the kernel and no
>    longer visible in the Device Tree binding.
>
>  * The PHY device is created directly when the network driver calls
>    of_phy_connect_fixed_link(), and associated to the PHY DT node,
>    which allows the existing of_phy_connect() function to work,
>    without the need to use the deprecated of_phy_connect_fixed_link().
>
> Posts of previous versions:
>
>   RFCv1:   http://www.spinics.net/lists/netdev/msg243253.html
>   RFCv2:   http://lists.infradead.org/pipermail/linux-arm-kernel/2013-September/196919.html
>   PATCHv3: http://www.spinics.net/lists/netdev/msg273117.html
>
> Thanks,
>
> Thomas
>
> Thomas Petazzoni (5):
>   net: phy: decouple PHY id and PHY address in fixed PHY driver
>   net: phy: extend fixed driver with fixed_phy_register()
>   of: provide a binding for fixed link PHYs
>   net: mvneta: add support for fixed links
>   ARM: mvebu: use the fixed-link PHY DT binding for the Armada XP Matrix
>     board
>
>  .../devicetree/bindings/net/fixed-link.txt         | 31 +++++++++
>  arch/arm/boot/dts/armada-xp-matrix.dts             |  4 ++
>  drivers/net/ethernet/marvell/mvneta.c              | 19 ++++-
>  drivers/net/phy/fixed.c                            | 81 +++++++++++++++++++---
>  drivers/of/of_mdio.c                               | 67 ++++++++++++++++++
>  include/linux/of_mdio.h                            | 15 ++++
>  include/linux/phy_fixed.h                          | 11 +++
>  7 files changed, 215 insertions(+), 13 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/fixed-link.txt
>
> --
> 1.9.3

-- 
Florian

Re: [PATCHv4 2/5] net: phy: extend fixed driver with fixed_phy_register()

[Florian Fainelli]

Hi Thomas,

2014-05-16 7:14 GMT-07:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> The existing fixed_phy_add() function has several drawbacks that
> prevents it from being used as is for OF-based declaration of fixed
> PHYs:
>
>  * The address of the PHY on the fake bus needs to be passed, while a
>    dynamic allocation is desired.
>
>  * Since the phy_device instantiation is post-poned until the next
>    mdiobus scan, there is no way to associate the fixed PHY with its
>    OF node, which later prevents of_phy_connect() from finding this
>    fixed PHY from a given OF node.
>
> To solve this, this commit introduces fixed_phy_register(), which will
> allocate an available PHY address, add the PHY using fixed_phy_add()
> and instantiate the phy_device structure associated with the provided
> OF node.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> Acked-by: Grant Likely <grant.likely@linaro.org>

Tested-by: Florian Fainelli <f.fainelli@gmail.com>

> ---
>  drivers/net/phy/fixed.c   | 61 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/phy_fixed.h | 11 +++++++++
>  2 files changed, 72 insertions(+)
>
> diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
> index e41546d..d60d875 100644
> --- a/drivers/net/phy/fixed.c
> +++ b/drivers/net/phy/fixed.c
> @@ -21,6 +21,7 @@
>  #include <linux/phy_fixed.h>
>  #include <linux/err.h>
>  #include <linux/slab.h>
> +#include <linux/of.h>
>
>  #define MII_REGS_NUM 29
>
> @@ -203,6 +204,66 @@ err_regs:
>  }
>  EXPORT_SYMBOL_GPL(fixed_phy_add);
>
> +void fixed_phy_del(int phy_addr)
> +{
> +       struct fixed_mdio_bus *fmb = &platform_fmb;
> +       struct fixed_phy *fp, *tmp;
> +
> +       list_for_each_entry_safe(fp, tmp, &fmb->phys, node) {
> +               if (fp->addr == phy_addr) {
> +                       list_del(&fp->node);
> +                       kfree(fp);
> +                       return;
> +               }
> +       }
> +}
> +EXPORT_SYMBOL_GPL(fixed_phy_del);
> +
> +static int phy_fixed_addr;
> +static DEFINE_SPINLOCK(phy_fixed_addr_lock);
> +
> +int fixed_phy_register(unsigned int irq,
> +                      struct fixed_phy_status *status,
> +                      struct device_node *np)
> +{
> +       struct fixed_mdio_bus *fmb = &platform_fmb;
> +       struct phy_device *phy;
> +       int phy_addr;
> +       int ret;
> +
> +       /* Get the next available PHY address, up to PHY_MAX_ADDR */
> +       spin_lock(&phy_fixed_addr_lock);
> +       if (phy_fixed_addr == PHY_MAX_ADDR) {
> +               spin_unlock(&phy_fixed_addr_lock);
> +               return -ENOSPC;
> +       }
> +       phy_addr = phy_fixed_addr++;
> +       spin_unlock(&phy_fixed_addr_lock);
> +
> +       ret = fixed_phy_add(PHY_POLL, phy_addr, status);
> +       if (ret < 0)
> +               return ret;
> +
> +       phy = get_phy_device(fmb->mii_bus, phy_addr, false);
> +       if (!phy || IS_ERR(phy)) {
> +               fixed_phy_del(phy_addr);
> +               return -EINVAL;
> +       }
> +
> +       of_node_get(np);
> +       phy->dev.of_node = np;
> +
> +       ret = phy_device_register(phy);
> +       if (ret) {
> +               phy_device_free(phy);
> +               of_node_put(np);
> +               fixed_phy_del(phy_addr);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
>  static int __init fixed_mdio_bus_init(void)
>  {
>         struct fixed_mdio_bus *fmb = &platform_fmb;
> diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h
> index 509d8f5..4f2478b 100644
> --- a/include/linux/phy_fixed.h
> +++ b/include/linux/phy_fixed.h
> @@ -9,15 +9,26 @@ struct fixed_phy_status {
>         int asym_pause;
>  };
>
> +struct device_node;
> +
>  #ifdef CONFIG_FIXED_PHY
>  extern int fixed_phy_add(unsigned int irq, int phy_id,
>                          struct fixed_phy_status *status);
> +extern int fixed_phy_register(unsigned int irq,
> +                             struct fixed_phy_status *status,
> +                             struct device_node *np);
>  #else
>  static inline int fixed_phy_add(unsigned int irq, int phy_id,
>                                 struct fixed_phy_status *status)
>  {
>         return -ENODEV;
>  }
> +static inline int fixed_phy_register(unsigned int irq,
> +                                    struct fixed_phy_status *status,
> +                                    struct device_node *np)
> +{
> +       return -ENODEV;
> +}
>  #endif /* CONFIG_FIXED_PHY */
>
>  /*
> --
> 1.9.3



-- 
Florian

[PATCHv4 2/5] net: phy: extend fixed driver with fixed_phy_register()

[Florian Fainelli]

Hi Thomas,

2014-05-16 7:14 GMT-07:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> The existing fixed_phy_add() function has several drawbacks that
> prevents it from being used as is for OF-based declaration of fixed
> PHYs:
>
>  * The address of the PHY on the fake bus needs to be passed, while a
>    dynamic allocation is desired.
>
>  * Since the phy_device instantiation is post-poned until the next
>    mdiobus scan, there is no way to associate the fixed PHY with its
>    OF node, which later prevents of_phy_connect() from finding this
>    fixed PHY from a given OF node.
>
> To solve this, this commit introduces fixed_phy_register(), which will
> allocate an available PHY address, add the PHY using fixed_phy_add()
> and instantiate the phy_device structure associated with the provided
> OF node.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> Acked-by: Grant Likely <grant.likely@linaro.org>

Tested-by: Florian Fainelli <f.fainelli@gmail.com>

> ---
>  drivers/net/phy/fixed.c   | 61 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/phy_fixed.h | 11 +++++++++
>  2 files changed, 72 insertions(+)
>
> diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
> index e41546d..d60d875 100644
> --- a/drivers/net/phy/fixed.c
> +++ b/drivers/net/phy/fixed.c
> @@ -21,6 +21,7 @@
>  #include <linux/phy_fixed.h>
>  #include <linux/err.h>
>  #include <linux/slab.h>
> +#include <linux/of.h>
>
>  #define MII_REGS_NUM 29
>
> @@ -203,6 +204,66 @@ err_regs:
>  }
>  EXPORT_SYMBOL_GPL(fixed_phy_add);
>
> +void fixed_phy_del(int phy_addr)
> +{
> +       struct fixed_mdio_bus *fmb = &platform_fmb;
> +       struct fixed_phy *fp, *tmp;
> +
> +       list_for_each_entry_safe(fp, tmp, &fmb->phys, node) {
> +               if (fp->addr == phy_addr) {
> +                       list_del(&fp->node);
> +                       kfree(fp);
> +                       return;
> +               }
> +       }
> +}
> +EXPORT_SYMBOL_GPL(fixed_phy_del);
> +
> +static int phy_fixed_addr;
> +static DEFINE_SPINLOCK(phy_fixed_addr_lock);
> +
> +int fixed_phy_register(unsigned int irq,
> +                      struct fixed_phy_status *status,
> +                      struct device_node *np)
> +{
> +       struct fixed_mdio_bus *fmb = &platform_fmb;
> +       struct phy_device *phy;
> +       int phy_addr;
> +       int ret;
> +
> +       /* Get the next available PHY address, up to PHY_MAX_ADDR */
> +       spin_lock(&phy_fixed_addr_lock);
> +       if (phy_fixed_addr == PHY_MAX_ADDR) {
> +               spin_unlock(&phy_fixed_addr_lock);
> +               return -ENOSPC;
> +       }
> +       phy_addr = phy_fixed_addr++;
> +       spin_unlock(&phy_fixed_addr_lock);
> +
> +       ret = fixed_phy_add(PHY_POLL, phy_addr, status);
> +       if (ret < 0)
> +               return ret;
> +
> +       phy = get_phy_device(fmb->mii_bus, phy_addr, false);
> +       if (!phy || IS_ERR(phy)) {
> +               fixed_phy_del(phy_addr);
> +               return -EINVAL;
> +       }
> +
> +       of_node_get(np);
> +       phy->dev.of_node = np;
> +
> +       ret = phy_device_register(phy);
> +       if (ret) {
> +               phy_device_free(phy);
> +               of_node_put(np);
> +               fixed_phy_del(phy_addr);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
>  static int __init fixed_mdio_bus_init(void)
>  {
>         struct fixed_mdio_bus *fmb = &platform_fmb;
> diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h
> index 509d8f5..4f2478b 100644
> --- a/include/linux/phy_fixed.h
> +++ b/include/linux/phy_fixed.h
> @@ -9,15 +9,26 @@ struct fixed_phy_status {
>         int asym_pause;
>  };
>
> +struct device_node;
> +
>  #ifdef CONFIG_FIXED_PHY
>  extern int fixed_phy_add(unsigned int irq, int phy_id,
>                          struct fixed_phy_status *status);
> +extern int fixed_phy_register(unsigned int irq,
> +                             struct fixed_phy_status *status,
> +                             struct device_node *np);
>  #else
>  static inline int fixed_phy_add(unsigned int irq, int phy_id,
>                                 struct fixed_phy_status *status)
>  {
>         return -ENODEV;
>  }
> +static inline int fixed_phy_register(unsigned int irq,
> +                                    struct fixed_phy_status *status,
> +                                    struct device_node *np)
> +{
> +       return -ENODEV;
> +}
>  #endif /* CONFIG_FIXED_PHY */
>
>  /*
> --
> 1.9.3



-- 
Florian

Re: [PATCHv4 3/5] of: provide a binding for fixed link PHYs

[Florian Fainelli]

2014-05-16 7:14 GMT-07:00 Thomas Petazzoni
<thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
> Some Ethernet MACs have a "fixed link", and are not connected to a
> normal MDIO-managed PHY device. For those situations, a Device Tree
> binding allows to describe a "fixed link" using a special PHY node.
>
> This patch adds:
>
>  * A documentation for the fixed PHY Device Tree binding.
>
>  * An of_phy_is_fixed_link() function that an Ethernet driver can call
>    on its PHY phandle to find out whether it's a fixed link PHY or
>    not. It should typically be used to know if
>    of_phy_register_fixed_link() should be called.
>
>  * An of_phy_register_fixed_link() function that instantiates the
>    fixed PHY into the PHY subsystem, so that when the driver calls
>    of_phy_connect(), the PHY device associated to the OF node will be
>    found.
>
> These two additional functions also support the old fixed-link Device
> Tree binding used on PowerPC platforms, so that ultimately, the
> network device drivers for those platforms could be converted to use
> of_phy_is_fixed_link() and of_phy_register_fixed_link() instead of
> of_phy_connect_fixed_link(), while keeping compatibility with their
> respective Device Tree bindings.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Reviewed-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Tested-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

> ---
>  .../devicetree/bindings/net/fixed-link.txt         | 31 ++++++++++
>  drivers/of/of_mdio.c                               | 67 ++++++++++++++++++++++
>  include/linux/of_mdio.h                            | 15 +++++
>  3 files changed, 113 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/fixed-link.txt
>
> diff --git a/Documentation/devicetree/bindings/net/fixed-link.txt b/Documentation/devicetree/bindings/net/fixed-link.txt
> new file mode 100644
> index 0000000..377f775
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/fixed-link.txt
> @@ -0,0 +1,31 @@
> +Fixed link Device Tree binding
> +------------------------------
> +
> +Some Ethernet MACs have a "fixed link", and are not connected to a
> +normal MDIO-managed PHY device. For those situations, a Device Tree
> +binding allows to describe a "fixed link".
> +
> +Such a fixed link situation is described by creating a 'fixed-link'
> +sub-node of the Ethernet MAC device node, with the following
> +properties:
> +
> +* 'speed' (integer, mandatory), to indicate the link speed. Accepted
> +  values are 10, 100 and 1000
> +* 'full-duplex' (boolean, optional), to indicate that full duplex is
> +  used. When absent, half duplex is assumed.
> +* 'pause' (boolean, optional), to indicate that pause should be
> +  enabled.
> +* 'asym-pause' (boolean, optional), to indicate that asym_pause should
> +  be enabled.
> +
> +Example:
> +
> +ethernet@0 {
> +       ...
> +       fixed-link {
> +             speed = <1000>;
> +             full-duplex;
> +       };
> +       ...
> +};
> +
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 9a95831..1def0bb 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -14,6 +14,7 @@
>  #include <linux/netdevice.h>
>  #include <linux/err.h>
>  #include <linux/phy.h>
> +#include <linux/phy_fixed.h>
>  #include <linux/of.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_mdio.h>
> @@ -301,3 +302,69 @@ struct phy_device *of_phy_attach(struct net_device *dev,
>         return phy_attach_direct(dev, phy, flags, iface) ? NULL : phy;
>  }
>  EXPORT_SYMBOL(of_phy_attach);
> +
> +#if defined(CONFIG_FIXED_PHY)
> +/*
> + * of_phy_is_fixed_link() and of_phy_register_fixed_link() must
> + * support two DT bindings:
> + * - the old DT binding, where 'fixed-link' was a property with 5
> + *   cells encoding various informations about the fixed PHY
> + * - the new DT binding, where 'fixed-link' is a sub-node of the
> + *   Ethernet device.
> + */
> +bool of_phy_is_fixed_link(struct device_node *np)
> +{
> +       struct device_node *dn;
> +       int len;
> +
> +       /* New binding */
> +       dn = of_get_child_by_name(np, "fixed-link");
> +       if (dn) {
> +               of_node_put(dn);
> +               return true;
> +       }
> +
> +       /* Old binding */
> +       if (of_get_property(np, "fixed-link", &len) &&
> +           len == (5 * sizeof(__be32)))
> +               return true;
> +
> +       return false;
> +}
> +EXPORT_SYMBOL(of_phy_is_fixed_link);
> +
> +int of_phy_register_fixed_link(struct device_node *np)
> +{
> +       struct fixed_phy_status status = {};
> +       struct device_node *fixed_link_node;
> +       const __be32 *fixed_link_prop;
> +       int len;
> +
> +       /* New binding */
> +       fixed_link_node = of_get_child_by_name(np, "fixed-link");
> +       if (fixed_link_node) {
> +               status.link = 1;
> +               status.duplex = of_property_read_bool(np, "full-duplex");
> +               if (of_property_read_u32(fixed_link_node, "speed", &status.speed))
> +                       return -EINVAL;
> +               status.pause = of_property_read_bool(np, "pause");
> +               status.asym_pause = of_property_read_bool(np, "asym-pause");
> +               of_node_put(fixed_link_node);
> +               return fixed_phy_register(PHY_POLL, &status, np);
> +       }
> +
> +       /* Old binding */
> +       fixed_link_prop = of_get_property(np, "fixed-link", &len);
> +       if (fixed_link_prop && len == (5 * sizeof(__be32))) {
> +               status.link = 1;
> +               status.duplex = be32_to_cpu(fixed_link_prop[1]);
> +               status.speed = be32_to_cpu(fixed_link_prop[2]);
> +               status.pause = be32_to_cpu(fixed_link_prop[3]);
> +               status.asym_pause = be32_to_cpu(fixed_link_prop[4]);
> +               return fixed_phy_register(PHY_POLL, &status, np);
> +       }
> +
> +       return -ENODEV;
> +}
> +EXPORT_SYMBOL(of_phy_register_fixed_link);
> +#endif
> diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
> index 6fe8464..77a6e32 100644
> --- a/include/linux/of_mdio.h
> +++ b/include/linux/of_mdio.h
> @@ -67,4 +67,19 @@ static inline struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np)
>  }
>  #endif /* CONFIG_OF */
>
> +#if defined(CONFIG_OF) && defined(CONFIG_FIXED_PHY)
> +extern int of_phy_register_fixed_link(struct device_node *np);
> +extern bool of_phy_is_fixed_link(struct device_node *np);
> +#else
> +static inline int of_phy_register_fixed_link(struct device_node *np)
> +{
> +       return -ENOSYS;
> +}
> +static inline bool of_phy_is_fixed_link(struct device_node *np)
> +{
> +       return false;
> +}
> +#endif
> +
> +
>  #endif /* __LINUX_OF_MDIO_H */
> --
> 1.9.3



-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 3/5] of: provide a binding for fixed link PHYs

[Florian Fainelli]

2014-05-16 7:14 GMT-07:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Some Ethernet MACs have a "fixed link", and are not connected to a
> normal MDIO-managed PHY device. For those situations, a Device Tree
> binding allows to describe a "fixed link" using a special PHY node.
>
> This patch adds:
>
>  * A documentation for the fixed PHY Device Tree binding.
>
>  * An of_phy_is_fixed_link() function that an Ethernet driver can call
>    on its PHY phandle to find out whether it's a fixed link PHY or
>    not. It should typically be used to know if
>    of_phy_register_fixed_link() should be called.
>
>  * An of_phy_register_fixed_link() function that instantiates the
>    fixed PHY into the PHY subsystem, so that when the driver calls
>    of_phy_connect(), the PHY device associated to the OF node will be
>    found.
>
> These two additional functions also support the old fixed-link Device
> Tree binding used on PowerPC platforms, so that ultimately, the
> network device drivers for those platforms could be converted to use
> of_phy_is_fixed_link() and of_phy_register_fixed_link() instead of
> of_phy_connect_fixed_link(), while keeping compatibility with their
> respective Device Tree bindings.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>

> ---
>  .../devicetree/bindings/net/fixed-link.txt         | 31 ++++++++++
>  drivers/of/of_mdio.c                               | 67 ++++++++++++++++++++++
>  include/linux/of_mdio.h                            | 15 +++++
>  3 files changed, 113 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/fixed-link.txt
>
> diff --git a/Documentation/devicetree/bindings/net/fixed-link.txt b/Documentation/devicetree/bindings/net/fixed-link.txt
> new file mode 100644
> index 0000000..377f775
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/fixed-link.txt
> @@ -0,0 +1,31 @@
> +Fixed link Device Tree binding
> +------------------------------
> +
> +Some Ethernet MACs have a "fixed link", and are not connected to a
> +normal MDIO-managed PHY device. For those situations, a Device Tree
> +binding allows to describe a "fixed link".
> +
> +Such a fixed link situation is described by creating a 'fixed-link'
> +sub-node of the Ethernet MAC device node, with the following
> +properties:
> +
> +* 'speed' (integer, mandatory), to indicate the link speed. Accepted
> +  values are 10, 100 and 1000
> +* 'full-duplex' (boolean, optional), to indicate that full duplex is
> +  used. When absent, half duplex is assumed.
> +* 'pause' (boolean, optional), to indicate that pause should be
> +  enabled.
> +* 'asym-pause' (boolean, optional), to indicate that asym_pause should
> +  be enabled.
> +
> +Example:
> +
> +ethernet at 0 {
> +       ...
> +       fixed-link {
> +             speed = <1000>;
> +             full-duplex;
> +       };
> +       ...
> +};
> +
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 9a95831..1def0bb 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -14,6 +14,7 @@
>  #include <linux/netdevice.h>
>  #include <linux/err.h>
>  #include <linux/phy.h>
> +#include <linux/phy_fixed.h>
>  #include <linux/of.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_mdio.h>
> @@ -301,3 +302,69 @@ struct phy_device *of_phy_attach(struct net_device *dev,
>         return phy_attach_direct(dev, phy, flags, iface) ? NULL : phy;
>  }
>  EXPORT_SYMBOL(of_phy_attach);
> +
> +#if defined(CONFIG_FIXED_PHY)
> +/*
> + * of_phy_is_fixed_link() and of_phy_register_fixed_link() must
> + * support two DT bindings:
> + * - the old DT binding, where 'fixed-link' was a property with 5
> + *   cells encoding various informations about the fixed PHY
> + * - the new DT binding, where 'fixed-link' is a sub-node of the
> + *   Ethernet device.
> + */
> +bool of_phy_is_fixed_link(struct device_node *np)
> +{
> +       struct device_node *dn;
> +       int len;
> +
> +       /* New binding */
> +       dn = of_get_child_by_name(np, "fixed-link");
> +       if (dn) {
> +               of_node_put(dn);
> +               return true;
> +       }
> +
> +       /* Old binding */
> +       if (of_get_property(np, "fixed-link", &len) &&
> +           len == (5 * sizeof(__be32)))
> +               return true;
> +
> +       return false;
> +}
> +EXPORT_SYMBOL(of_phy_is_fixed_link);
> +
> +int of_phy_register_fixed_link(struct device_node *np)
> +{
> +       struct fixed_phy_status status = {};
> +       struct device_node *fixed_link_node;
> +       const __be32 *fixed_link_prop;
> +       int len;
> +
> +       /* New binding */
> +       fixed_link_node = of_get_child_by_name(np, "fixed-link");
> +       if (fixed_link_node) {
> +               status.link = 1;
> +               status.duplex = of_property_read_bool(np, "full-duplex");
> +               if (of_property_read_u32(fixed_link_node, "speed", &status.speed))
> +                       return -EINVAL;
> +               status.pause = of_property_read_bool(np, "pause");
> +               status.asym_pause = of_property_read_bool(np, "asym-pause");
> +               of_node_put(fixed_link_node);
> +               return fixed_phy_register(PHY_POLL, &status, np);
> +       }
> +
> +       /* Old binding */
> +       fixed_link_prop = of_get_property(np, "fixed-link", &len);
> +       if (fixed_link_prop && len == (5 * sizeof(__be32))) {
> +               status.link = 1;
> +               status.duplex = be32_to_cpu(fixed_link_prop[1]);
> +               status.speed = be32_to_cpu(fixed_link_prop[2]);
> +               status.pause = be32_to_cpu(fixed_link_prop[3]);
> +               status.asym_pause = be32_to_cpu(fixed_link_prop[4]);
> +               return fixed_phy_register(PHY_POLL, &status, np);
> +       }
> +
> +       return -ENODEV;
> +}
> +EXPORT_SYMBOL(of_phy_register_fixed_link);
> +#endif
> diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
> index 6fe8464..77a6e32 100644
> --- a/include/linux/of_mdio.h
> +++ b/include/linux/of_mdio.h
> @@ -67,4 +67,19 @@ static inline struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np)
>  }
>  #endif /* CONFIG_OF */
>
> +#if defined(CONFIG_OF) && defined(CONFIG_FIXED_PHY)
> +extern int of_phy_register_fixed_link(struct device_node *np);
> +extern bool of_phy_is_fixed_link(struct device_node *np);
> +#else
> +static inline int of_phy_register_fixed_link(struct device_node *np)
> +{
> +       return -ENOSYS;
> +}
> +static inline bool of_phy_is_fixed_link(struct device_node *np)
> +{
> +       return false;
> +}
> +#endif
> +
> +
>  #endif /* __LINUX_OF_MDIO_H */
> --
> 1.9.3



-- 
Florian

Re: [PATCHv4 1/5] net: phy: decouple PHY id and PHY address in fixed PHY driver

[Florian Fainelli]

2014-05-16 7:14 GMT-07:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Until now, the fixed_phy_add() function was taking as argument
> 'phy_id', which was used both as the PHY address on the fake fixed
> MDIO bus, and as the PHY id, as available in the MII_PHYSID1 and
> MII_PHYSID2 registers. However, those two informations are completely
> unrelated.
>
> This patch decouples them. The PHY id of fixed PHYs is hardcoded to be
> 0x0. Ideally, a really reserved value would be nicer, but there
> doesn't seem to be an easy of making sure a dummy value can be
> assigned to the Linux kernel for such usage.
>
> The PHY address remains passed by the caller of phy_fixed_add().
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>

> ---
>  drivers/net/phy/fixed.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
> index ba55adf..e41546d 100644
> --- a/drivers/net/phy/fixed.c
> +++ b/drivers/net/phy/fixed.c
> @@ -31,7 +31,7 @@ struct fixed_mdio_bus {
>  };
>
>  struct fixed_phy {
> -       int id;
> +       int addr;
>         u16 regs[MII_REGS_NUM];
>         struct phy_device *phydev;
>         struct fixed_phy_status status;
> @@ -104,8 +104,8 @@ static int fixed_phy_update_regs(struct fixed_phy *fp)
>         if (fp->status.asym_pause)
>                 lpa |= LPA_PAUSE_ASYM;
>
> -       fp->regs[MII_PHYSID1] = fp->id >> 16;
> -       fp->regs[MII_PHYSID2] = fp->id;
> +       fp->regs[MII_PHYSID1] = 0;
> +       fp->regs[MII_PHYSID2] = 0;
>
>         fp->regs[MII_BMSR] = bmsr;
>         fp->regs[MII_BMCR] = bmcr;
> @@ -115,7 +115,7 @@ static int fixed_phy_update_regs(struct fixed_phy *fp)
>         return 0;
>  }
>
> -static int fixed_mdio_read(struct mii_bus *bus, int phy_id, int reg_num)
> +static int fixed_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num)
>  {
>         struct fixed_mdio_bus *fmb = bus->priv;
>         struct fixed_phy *fp;
> @@ -124,7 +124,7 @@ static int fixed_mdio_read(struct mii_bus *bus, int phy_id, int reg_num)
>                 return -1;
>
>         list_for_each_entry(fp, &fmb->phys, node) {
> -               if (fp->id == phy_id) {
> +               if (fp->addr == phy_addr) {
>                         /* Issue callback if user registered it. */
>                         if (fp->link_update) {
>                                 fp->link_update(fp->phydev->attached_dev,
> @@ -138,7 +138,7 @@ static int fixed_mdio_read(struct mii_bus *bus, int phy_id, int reg_num)
>         return 0xFFFF;
>  }
>
> -static int fixed_mdio_write(struct mii_bus *bus, int phy_id, int reg_num,
> +static int fixed_mdio_write(struct mii_bus *bus, int phy_addr, int reg_num,
>                             u16 val)
>  {
>         return 0;
> @@ -160,7 +160,7 @@ int fixed_phy_set_link_update(struct phy_device *phydev,
>                 return -EINVAL;
>
>         list_for_each_entry(fp, &fmb->phys, node) {
> -               if (fp->id == phydev->phy_id) {
> +               if (fp->addr == phydev->addr) {
>                         fp->link_update = link_update;
>                         fp->phydev = phydev;
>                         return 0;
> @@ -171,7 +171,7 @@ int fixed_phy_set_link_update(struct phy_device *phydev,
>  }
>  EXPORT_SYMBOL_GPL(fixed_phy_set_link_update);
>
> -int fixed_phy_add(unsigned int irq, int phy_id,
> +int fixed_phy_add(unsigned int irq, int phy_addr,
>                   struct fixed_phy_status *status)
>  {
>         int ret;
> @@ -184,9 +184,9 @@ int fixed_phy_add(unsigned int irq, int phy_id,
>
>         memset(fp->regs, 0xFF,  sizeof(fp->regs[0]) * MII_REGS_NUM);
>
> -       fmb->irqs[phy_id] = irq;
> +       fmb->irqs[phy_addr] = irq;
>
> -       fp->id = phy_id;
> +       fp->addr = phy_addr;
>         fp->status = *status;
>
>         ret = fixed_phy_update_regs(fp);
> --
> 1.9.3



-- 
Florian

[PATCHv4 1/5] net: phy: decouple PHY id and PHY address in fixed PHY driver

[Florian Fainelli]

2014-05-16 7:14 GMT-07:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Until now, the fixed_phy_add() function was taking as argument
> 'phy_id', which was used both as the PHY address on the fake fixed
> MDIO bus, and as the PHY id, as available in the MII_PHYSID1 and
> MII_PHYSID2 registers. However, those two informations are completely
> unrelated.
>
> This patch decouples them. The PHY id of fixed PHYs is hardcoded to be
> 0x0. Ideally, a really reserved value would be nicer, but there
> doesn't seem to be an easy of making sure a dummy value can be
> assigned to the Linux kernel for such usage.
>
> The PHY address remains passed by the caller of phy_fixed_add().
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>

> ---
>  drivers/net/phy/fixed.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
> index ba55adf..e41546d 100644
> --- a/drivers/net/phy/fixed.c
> +++ b/drivers/net/phy/fixed.c
> @@ -31,7 +31,7 @@ struct fixed_mdio_bus {
>  };
>
>  struct fixed_phy {
> -       int id;
> +       int addr;
>         u16 regs[MII_REGS_NUM];
>         struct phy_device *phydev;
>         struct fixed_phy_status status;
> @@ -104,8 +104,8 @@ static int fixed_phy_update_regs(struct fixed_phy *fp)
>         if (fp->status.asym_pause)
>                 lpa |= LPA_PAUSE_ASYM;
>
> -       fp->regs[MII_PHYSID1] = fp->id >> 16;
> -       fp->regs[MII_PHYSID2] = fp->id;
> +       fp->regs[MII_PHYSID1] = 0;
> +       fp->regs[MII_PHYSID2] = 0;
>
>         fp->regs[MII_BMSR] = bmsr;
>         fp->regs[MII_BMCR] = bmcr;
> @@ -115,7 +115,7 @@ static int fixed_phy_update_regs(struct fixed_phy *fp)
>         return 0;
>  }
>
> -static int fixed_mdio_read(struct mii_bus *bus, int phy_id, int reg_num)
> +static int fixed_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num)
>  {
>         struct fixed_mdio_bus *fmb = bus->priv;
>         struct fixed_phy *fp;
> @@ -124,7 +124,7 @@ static int fixed_mdio_read(struct mii_bus *bus, int phy_id, int reg_num)
>                 return -1;
>
>         list_for_each_entry(fp, &fmb->phys, node) {
> -               if (fp->id == phy_id) {
> +               if (fp->addr == phy_addr) {
>                         /* Issue callback if user registered it. */
>                         if (fp->link_update) {
>                                 fp->link_update(fp->phydev->attached_dev,
> @@ -138,7 +138,7 @@ static int fixed_mdio_read(struct mii_bus *bus, int phy_id, int reg_num)
>         return 0xFFFF;
>  }
>
> -static int fixed_mdio_write(struct mii_bus *bus, int phy_id, int reg_num,
> +static int fixed_mdio_write(struct mii_bus *bus, int phy_addr, int reg_num,
>                             u16 val)
>  {
>         return 0;
> @@ -160,7 +160,7 @@ int fixed_phy_set_link_update(struct phy_device *phydev,
>                 return -EINVAL;
>
>         list_for_each_entry(fp, &fmb->phys, node) {
> -               if (fp->id == phydev->phy_id) {
> +               if (fp->addr == phydev->addr) {
>                         fp->link_update = link_update;
>                         fp->phydev = phydev;
>                         return 0;
> @@ -171,7 +171,7 @@ int fixed_phy_set_link_update(struct phy_device *phydev,
>  }
>  EXPORT_SYMBOL_GPL(fixed_phy_set_link_update);
>
> -int fixed_phy_add(unsigned int irq, int phy_id,
> +int fixed_phy_add(unsigned int irq, int phy_addr,
>                   struct fixed_phy_status *status)
>  {
>         int ret;
> @@ -184,9 +184,9 @@ int fixed_phy_add(unsigned int irq, int phy_id,
>
>         memset(fp->regs, 0xFF,  sizeof(fp->regs[0]) * MII_REGS_NUM);
>
> -       fmb->irqs[phy_id] = irq;
> +       fmb->irqs[phy_addr] = irq;
>
> -       fp->id = phy_id;
> +       fp->addr = phy_addr;
>         fp->status = *status;
>
>         ret = fixed_phy_update_regs(fp);
> --
> 1.9.3



-- 
Florian

Re: [PATCHv4 0/5] Add DT support for fixed PHYs

[Thomas Petazzoni]

Dear Florian Fainelli,

On Fri, 16 May 2014 11:27:36 -0700, Florian Fainelli wrote:

> > Here is a fourth version of the patch set that adds a Device Tree
> > binding and the related code to support fixed PHYs. I'm hoping to get
> > this merged in 3.16.
> 
> Thanks for being persistent on this and coming back with these
> patches, that is really appreciated.
> 
> I gave this patch set a try on a brcmstb system with SYSTEMPORT and my
> latest changes to net/dsa/ to allow for different PHY setups to be
> used with the SoC integrated switch. This switch has a mix of fixed
> and real PHYs. This works like a charm, so long as you consistently
> use the "new" functions, and not the old ones, which is totally
> expected. This is excellent work!

Thanks a lot for your review and testing.

> As we discussed offline, I think we need to progressively get rid of
> of_phy_connect_fixed() to avoid having to debug situations where the
> old 'fixed-link' property used some sparse addressing scheme, which is
> now incompatible with the dynamic address allocation scheme.

Yes, agreed. As we discussed, my plan is to send a followup patch
series to fix the users of the old API to finally remove it. But I'd
like to do this once this series gets accepted and applied.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCHv4 0/5] Add DT support for fixed PHYs

[Thomas Petazzoni]

Dear Florian Fainelli,

On Fri, 16 May 2014 11:27:36 -0700, Florian Fainelli wrote:

> > Here is a fourth version of the patch set that adds a Device Tree
> > binding and the related code to support fixed PHYs. I'm hoping to get
> > this merged in 3.16.
> 
> Thanks for being persistent on this and coming back with these
> patches, that is really appreciated.
> 
> I gave this patch set a try on a brcmstb system with SYSTEMPORT and my
> latest changes to net/dsa/ to allow for different PHY setups to be
> used with the SoC integrated switch. This switch has a mix of fixed
> and real PHYs. This works like a charm, so long as you consistently
> use the "new" functions, and not the old ones, which is totally
> expected. This is excellent work!

Thanks a lot for your review and testing.

> As we discussed offline, I think we need to progressively get rid of
> of_phy_connect_fixed() to avoid having to debug situations where the
> old 'fixed-link' property used some sparse addressing scheme, which is
> now incompatible with the dynamic address allocation scheme.

Yes, agreed. As we discussed, my plan is to send a followup patch
series to fix the users of the old API to finally remove it. But I'd
like to do this once this series gets accepted and applied.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCHv4 0/5] Add DT support for fixed PHYs

[David Miller]

From: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Date: Fri, 16 May 2014 16:14:02 +0200

> Here is a fourth version of the patch set that adds a Device Tree
> binding and the related code to support fixed PHYs. I'm hoping to get
> this merged in 3.16.

Series applied, thanks Thomas.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 0/5] Add DT support for fixed PHYs

[David Miller]

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Date: Fri, 16 May 2014 16:14:02 +0200

> Here is a fourth version of the patch set that adds a Device Tree
> binding and the related code to support fixed PHYs. I'm hoping to get
> this merged in 3.16.

Series applied, thanks Thomas.

Re: [PATCHv4 0/5] Add DT support for fixed PHYs

[Thomas Petazzoni]

Dear David Miller,

On Fri, 16 May 2014 17:20:09 -0400 (EDT), David Miller wrote:
> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Date: Fri, 16 May 2014 16:14:02 +0200
> 
> > Here is a fourth version of the patch set that adds a Device Tree
> > binding and the related code to support fixed PHYs. I'm hoping to get
> > this merged in 3.16.
> 
> Series applied, thanks Thomas.

Thanks! Did you also pull PATCH 5/5, which changes the Armada XP Matrix
Device Tree file?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCHv4 0/5] Add DT support for fixed PHYs

[Thomas Petazzoni]

Dear David Miller,

On Fri, 16 May 2014 17:20:09 -0400 (EDT), David Miller wrote:
> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Date: Fri, 16 May 2014 16:14:02 +0200
> 
> > Here is a fourth version of the patch set that adds a Device Tree
> > binding and the related code to support fixed PHYs. I'm hoping to get
> > this merged in 3.16.
> 
> Series applied, thanks Thomas.

Thanks! Did you also pull PATCH 5/5, which changes the Armada XP Matrix
Device Tree file?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCHv4 0/5] Add DT support for fixed PHYs

[Florian Fainelli]

2014-05-18 3:18 GMT-07:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Dear David Miller,
>
> On Fri, 16 May 2014 17:20:09 -0400 (EDT), David Miller wrote:
>> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Date: Fri, 16 May 2014 16:14:02 +0200
>>
>> > Here is a fourth version of the patch set that adds a Device Tree
>> > binding and the related code to support fixed PHYs. I'm hoping to get
>> > this merged in 3.16.
>>
>> Series applied, thanks Thomas.
>
> Thanks! Did you also pull PATCH 5/5, which changes the Armada XP Matrix
> Device Tree file?

It has been applied:
http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=84f6e11f21d40025ae9b0ebbf1c9b3be91ea8e1a
-- 
Florian

[PATCHv4 0/5] Add DT support for fixed PHYs

[Florian Fainelli]

2014-05-18 3:18 GMT-07:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Dear David Miller,
>
> On Fri, 16 May 2014 17:20:09 -0400 (EDT), David Miller wrote:
>> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Date: Fri, 16 May 2014 16:14:02 +0200
>>
>> > Here is a fourth version of the patch set that adds a Device Tree
>> > binding and the related code to support fixed PHYs. I'm hoping to get
>> > this merged in 3.16.
>>
>> Series applied, thanks Thomas.
>
> Thanks! Did you also pull PATCH 5/5, which changes the Armada XP Matrix
> Device Tree file?

It has been applied:
http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=84f6e11f21d40025ae9b0ebbf1c9b3be91ea8e1a
-- 
Florian

Re: [PATCHv4 0/5] Add DT support for fixed PHYs

[Thomas Petazzoni]

Dear Florian Fainelli,

On Sun, 18 May 2014 10:21:10 -0700, Florian Fainelli wrote:

> > On Fri, 16 May 2014 17:20:09 -0400 (EDT), David Miller wrote:
> >> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> >> Date: Fri, 16 May 2014 16:14:02 +0200
> >>
> >> > Here is a fourth version of the patch set that adds a Device Tree
> >> > binding and the related code to support fixed PHYs. I'm hoping to get
> >> > this merged in 3.16.
> >>
> >> Series applied, thanks Thomas.
> >
> > Thanks! Did you also pull PATCH 5/5, which changes the Armada XP Matrix
> > Device Tree file?
> 
> It has been applied:
> http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=84f6e11f21d40025ae9b0ebbf1c9b3be91ea8e1a

I think it should have been routed through the mvebu tree, or at least
get the hack of the mvebu maintainers. That being said, I don't think
we have other patches for armada-xp-matrix.dts in this cycle, so maybe
having this patch go through David's tree will be OK. I'll let Jason
Cooper (Cc'ed) answer on this.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCHv4 0/5] Add DT support for fixed PHYs

[Thomas Petazzoni]

Dear Florian Fainelli,

On Sun, 18 May 2014 10:21:10 -0700, Florian Fainelli wrote:

> > On Fri, 16 May 2014 17:20:09 -0400 (EDT), David Miller wrote:
> >> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> >> Date: Fri, 16 May 2014 16:14:02 +0200
> >>
> >> > Here is a fourth version of the patch set that adds a Device Tree
> >> > binding and the related code to support fixed PHYs. I'm hoping to get
> >> > this merged in 3.16.
> >>
> >> Series applied, thanks Thomas.
> >
> > Thanks! Did you also pull PATCH 5/5, which changes the Armada XP Matrix
> > Device Tree file?
> 
> It has been applied:
> http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=84f6e11f21d40025ae9b0ebbf1c9b3be91ea8e1a

I think it should have been routed through the mvebu tree, or at least
get the hack of the mvebu maintainers. That being said, I don't think
we have other patches for armada-xp-matrix.dts in this cycle, so maybe
having this patch go through David's tree will be OK. I'll let Jason
Cooper (Cc'ed) answer on this.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCHv4 0/5] Add DT support for fixed PHYs

[Jason Cooper]

On Sun, May 18, 2014 at 07:46:28PM +0200, Thomas Petazzoni wrote:
> Dear Florian Fainelli,
> 
> On Sun, 18 May 2014 10:21:10 -0700, Florian Fainelli wrote:
> 
> > > On Fri, 16 May 2014 17:20:09 -0400 (EDT), David Miller wrote:
> > >> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > >> Date: Fri, 16 May 2014 16:14:02 +0200
> > >>
> > >> > Here is a fourth version of the patch set that adds a Device Tree
> > >> > binding and the related code to support fixed PHYs. I'm hoping to get
> > >> > this merged in 3.16.
> > >>
> > >> Series applied, thanks Thomas.
> > >
> > > Thanks! Did you also pull PATCH 5/5, which changes the Armada XP Matrix
> > > Device Tree file?
> > 
> > It has been applied:
> > http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=84f6e11f21d40025ae9b0ebbf1c9b3be91ea8e1a
> 
> I think it should have been routed through the mvebu tree, or at least
> get the hack of the mvebu maintainers. That being said, I don't think
> we have other patches for armada-xp-matrix.dts in this cycle, so maybe
> having this patch go through David's tree will be OK. I'll let Jason
> Cooper (Cc'ed) answer on this.

There's just one patch:

  0d9179fb3369 ARM: mvebu: remove clock-frequency of serial port Device Tree nodes

Which removes a single line.  So it should be trivial to resolve any conflict.

In the future: Yes, we prefer to take the dts file changes.  But I don't
see a problem here.  Thanks for the heads up, though.

thx,

Jason.

[PATCHv4 0/5] Add DT support for fixed PHYs

[Jason Cooper]

On Sun, May 18, 2014 at 07:46:28PM +0200, Thomas Petazzoni wrote:
> Dear Florian Fainelli,
> 
> On Sun, 18 May 2014 10:21:10 -0700, Florian Fainelli wrote:
> 
> > > On Fri, 16 May 2014 17:20:09 -0400 (EDT), David Miller wrote:
> > >> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > >> Date: Fri, 16 May 2014 16:14:02 +0200
> > >>
> > >> > Here is a fourth version of the patch set that adds a Device Tree
> > >> > binding and the related code to support fixed PHYs. I'm hoping to get
> > >> > this merged in 3.16.
> > >>
> > >> Series applied, thanks Thomas.
> > >
> > > Thanks! Did you also pull PATCH 5/5, which changes the Armada XP Matrix
> > > Device Tree file?
> > 
> > It has been applied:
> > http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=84f6e11f21d40025ae9b0ebbf1c9b3be91ea8e1a
> 
> I think it should have been routed through the mvebu tree, or at least
> get the hack of the mvebu maintainers. That being said, I don't think
> we have other patches for armada-xp-matrix.dts in this cycle, so maybe
> having this patch go through David's tree will be OK. I'll let Jason
> Cooper (Cc'ed) answer on this.

There's just one patch:

  0d9179fb3369 ARM: mvebu: remove clock-frequency of serial port Device Tree nodes

Which removes a single line.  So it should be trivial to resolve any conflict.

In the future: Yes, we prefer to take the dts file changes.  But I don't
see a problem here.  Thanks for the heads up, though.

thx,

Jason.

Re: [PATCHv4 2/5] net: phy: extend fixed driver with fixed_phy_register()

[Sergei Shtylyov]

Hello.

On 05/16/2014 06:14 PM, Thomas Petazzoni wrote:

> The existing fixed_phy_add() function has several drawbacks that
> prevents it from being used as is for OF-based declaration of fixed
> PHYs:
>
>   * The address of the PHY on the fake bus needs to be passed, while a
>     dynamic allocation is desired.
>
>   * Since the phy_device instantiation is post-poned until the next
>     mdiobus scan, there is no way to associate the fixed PHY with its
>     OF node, which later prevents of_phy_connect() from finding this
>     fixed PHY from a given OF node.
>
> To solve this, this commit introduces fixed_phy_register(), which will
> allocate an available PHY address, add the PHY using fixed_phy_add()
> and instantiate the phy_device structure associated with the provided
> OF node.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> Acked-by: Grant Likely <grant.likely@linaro.org>
> ---
>   drivers/net/phy/fixed.c   | 61 +++++++++++++++++++++++++++++++++++++++++++++++
>   include/linux/phy_fixed.h | 11 +++++++++
>   2 files changed, 72 insertions(+)
>
> diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
> index e41546d..d60d875 100644
> --- a/drivers/net/phy/fixed.c
> +++ b/drivers/net/phy/fixed.c
[...]
> @@ -203,6 +204,66 @@ err_regs:
[...]
> +int fixed_phy_register(unsigned int irq,
> +		       struct fixed_phy_status *status,
> +		       struct device_node *np)
> +{
> +	struct fixed_mdio_bus *fmb = &platform_fmb;
> +	struct phy_device *phy;
> +	int phy_addr;
> +	int ret;
> +
> +	/* Get the next available PHY address, up to PHY_MAX_ADDR */
> +	spin_lock(&phy_fixed_addr_lock);
> +	if (phy_fixed_addr == PHY_MAX_ADDR) {
> +		spin_unlock(&phy_fixed_addr_lock);
> +		return -ENOSPC;
> +	}
> +	phy_addr = phy_fixed_addr++;
> +	spin_unlock(&phy_fixed_addr_lock);
> +
> +	ret = fixed_phy_add(PHY_POLL, phy_addr, status);

    Was rummaging in the fixed_phy driver and a bug sprang right at me: 'phy' 
should have been passed here, not PHY_POLL. Luckily, all callers pass PHY_POLL 
anyway...

[...]

MBR, Sergei

[PATCHv4 2/5] net: phy: extend fixed driver with fixed_phy_register()

[Sergei Shtylyov]

Hello.

On 05/16/2014 06:14 PM, Thomas Petazzoni wrote:

> The existing fixed_phy_add() function has several drawbacks that
> prevents it from being used as is for OF-based declaration of fixed
> PHYs:
>
>   * The address of the PHY on the fake bus needs to be passed, while a
>     dynamic allocation is desired.
>
>   * Since the phy_device instantiation is post-poned until the next
>     mdiobus scan, there is no way to associate the fixed PHY with its
>     OF node, which later prevents of_phy_connect() from finding this
>     fixed PHY from a given OF node.
>
> To solve this, this commit introduces fixed_phy_register(), which will
> allocate an available PHY address, add the PHY using fixed_phy_add()
> and instantiate the phy_device structure associated with the provided
> OF node.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> Acked-by: Grant Likely <grant.likely@linaro.org>
> ---
>   drivers/net/phy/fixed.c   | 61 +++++++++++++++++++++++++++++++++++++++++++++++
>   include/linux/phy_fixed.h | 11 +++++++++
>   2 files changed, 72 insertions(+)
>
> diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
> index e41546d..d60d875 100644
> --- a/drivers/net/phy/fixed.c
> +++ b/drivers/net/phy/fixed.c
[...]
> @@ -203,6 +204,66 @@ err_regs:
[...]
> +int fixed_phy_register(unsigned int irq,
> +		       struct fixed_phy_status *status,
> +		       struct device_node *np)
> +{
> +	struct fixed_mdio_bus *fmb = &platform_fmb;
> +	struct phy_device *phy;
> +	int phy_addr;
> +	int ret;
> +
> +	/* Get the next available PHY address, up to PHY_MAX_ADDR */
> +	spin_lock(&phy_fixed_addr_lock);
> +	if (phy_fixed_addr == PHY_MAX_ADDR) {
> +		spin_unlock(&phy_fixed_addr_lock);
> +		return -ENOSPC;
> +	}
> +	phy_addr = phy_fixed_addr++;
> +	spin_unlock(&phy_fixed_addr_lock);
> +
> +	ret = fixed_phy_add(PHY_POLL, phy_addr, status);

    Was rummaging in the fixed_phy driver and a bug sprang right at me: 'phy' 
should have been passed here, not PHY_POLL. Luckily, all callers pass PHY_POLL 
anyway...

[...]

MBR, Sergei

Re: [PATCHv4 2/5] net: phy: extend fixed driver with fixed_phy_register()

[Florian Fainelli]

On 03/09/15 12:20, Sergei Shtylyov wrote:
> Hello.
> 
> On 05/16/2014 06:14 PM, Thomas Petazzoni wrote:
> 
>> The existing fixed_phy_add() function has several drawbacks that
>> prevents it from being used as is for OF-based declaration of fixed
>> PHYs:
>>
>>   * The address of the PHY on the fake bus needs to be passed, while a
>>     dynamic allocation is desired.
>>
>>   * Since the phy_device instantiation is post-poned until the next
>>     mdiobus scan, there is no way to associate the fixed PHY with its
>>     OF node, which later prevents of_phy_connect() from finding this
>>     fixed PHY from a given OF node.
>>
>> To solve this, this commit introduces fixed_phy_register(), which will
>> allocate an available PHY address, add the PHY using fixed_phy_add()
>> and instantiate the phy_device structure associated with the provided
>> OF node.
>>
>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>> Acked-by: Grant Likely <grant.likely@linaro.org>
>> ---
>>   drivers/net/phy/fixed.c   | 61
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/phy_fixed.h | 11 +++++++++
>>   2 files changed, 72 insertions(+)
>>
>> diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
>> index e41546d..d60d875 100644
>> --- a/drivers/net/phy/fixed.c
>> +++ b/drivers/net/phy/fixed.c
> [...]
>> @@ -203,6 +204,66 @@ err_regs:
> [...]
>> +int fixed_phy_register(unsigned int irq,
>> +               struct fixed_phy_status *status,
>> +               struct device_node *np)
>> +{
>> +    struct fixed_mdio_bus *fmb = &platform_fmb;
>> +    struct phy_device *phy;
>> +    int phy_addr;
>> +    int ret;
>> +
>> +    /* Get the next available PHY address, up to PHY_MAX_ADDR */
>> +    spin_lock(&phy_fixed_addr_lock);
>> +    if (phy_fixed_addr == PHY_MAX_ADDR) {
>> +        spin_unlock(&phy_fixed_addr_lock);
>> +        return -ENOSPC;
>> +    }
>> +    phy_addr = phy_fixed_addr++;
>> +    spin_unlock(&phy_fixed_addr_lock);
>> +
>> +    ret = fixed_phy_add(PHY_POLL, phy_addr, status);
> 
>    Was rummaging in the fixed_phy driver and a bug sprang right at me:
> 'phy' should have been passed here, not PHY_POLL. Luckily, all callers
> pass PHY_POLL anyway...

Are we looking at the same header file for the prototype of fixed_phy_add()?

extern int fixed_phy_add(unsigned int irq, int phy_id,
                         struct fixed_phy_status *status,
                         int link_gpio);

First argument is correct here.. at any rate, if something needs fixing,
just go ahead and submit a patch.
--
Florian

[PATCHv4 2/5] net: phy: extend fixed driver with fixed_phy_register()

[Florian Fainelli]

On 03/09/15 12:20, Sergei Shtylyov wrote:
> Hello.
> 
> On 05/16/2014 06:14 PM, Thomas Petazzoni wrote:
> 
>> The existing fixed_phy_add() function has several drawbacks that
>> prevents it from being used as is for OF-based declaration of fixed
>> PHYs:
>>
>>   * The address of the PHY on the fake bus needs to be passed, while a
>>     dynamic allocation is desired.
>>
>>   * Since the phy_device instantiation is post-poned until the next
>>     mdiobus scan, there is no way to associate the fixed PHY with its
>>     OF node, which later prevents of_phy_connect() from finding this
>>     fixed PHY from a given OF node.
>>
>> To solve this, this commit introduces fixed_phy_register(), which will
>> allocate an available PHY address, add the PHY using fixed_phy_add()
>> and instantiate the phy_device structure associated with the provided
>> OF node.
>>
>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>> Acked-by: Grant Likely <grant.likely@linaro.org>
>> ---
>>   drivers/net/phy/fixed.c   | 61
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/phy_fixed.h | 11 +++++++++
>>   2 files changed, 72 insertions(+)
>>
>> diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
>> index e41546d..d60d875 100644
>> --- a/drivers/net/phy/fixed.c
>> +++ b/drivers/net/phy/fixed.c
> [...]
>> @@ -203,6 +204,66 @@ err_regs:
> [...]
>> +int fixed_phy_register(unsigned int irq,
>> +               struct fixed_phy_status *status,
>> +               struct device_node *np)
>> +{
>> +    struct fixed_mdio_bus *fmb = &platform_fmb;
>> +    struct phy_device *phy;
>> +    int phy_addr;
>> +    int ret;
>> +
>> +    /* Get the next available PHY address, up to PHY_MAX_ADDR */
>> +    spin_lock(&phy_fixed_addr_lock);
>> +    if (phy_fixed_addr == PHY_MAX_ADDR) {
>> +        spin_unlock(&phy_fixed_addr_lock);
>> +        return -ENOSPC;
>> +    }
>> +    phy_addr = phy_fixed_addr++;
>> +    spin_unlock(&phy_fixed_addr_lock);
>> +
>> +    ret = fixed_phy_add(PHY_POLL, phy_addr, status);
> 
>    Was rummaging in the fixed_phy driver and a bug sprang right at me:
> 'phy' should have been passed here, not PHY_POLL. Luckily, all callers
> pass PHY_POLL anyway...

Are we looking at the same header file for the prototype of fixed_phy_add()?

extern int fixed_phy_add(unsigned int irq, int phy_id,
                         struct fixed_phy_status *status,
                         int link_gpio);

First argument is correct here.. at any rate, if something needs fixing,
just go ahead and submit a patch.
--
Florian

Re: [PATCHv4 2/5] net: phy: extend fixed driver with fixed_phy_register()

[Sergei Shtylyov]

Hello.

On 09/03/2015 10:23 PM, Florian Fainelli wrote:

>>> The existing fixed_phy_add() function has several drawbacks that
>>> prevents it from being used as is for OF-based declaration of fixed
>>> PHYs:
>>>
>>>    * The address of the PHY on the fake bus needs to be passed, while a
>>>      dynamic allocation is desired.
>>>
>>>    * Since the phy_device instantiation is post-poned until the next
>>>      mdiobus scan, there is no way to associate the fixed PHY with its
>>>      OF node, which later prevents of_phy_connect() from finding this
>>>      fixed PHY from a given OF node.
>>>
>>> To solve this, this commit introduces fixed_phy_register(), which will
>>> allocate an available PHY address, add the PHY using fixed_phy_add()
>>> and instantiate the phy_device structure associated with the provided
>>> OF node.
>>>
>>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>>> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>>> Acked-by: Grant Likely <grant.likely@linaro.org>
>>> ---
>>>    drivers/net/phy/fixed.c   | 61
>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>    include/linux/phy_fixed.h | 11 +++++++++
>>>    2 files changed, 72 insertions(+)
>>>
>>> diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
>>> index e41546d..d60d875 100644
>>> --- a/drivers/net/phy/fixed.c
>>> +++ b/drivers/net/phy/fixed.c
>> [...]
>>> @@ -203,6 +204,66 @@ err_regs:
>> [...]
>>> +int fixed_phy_register(unsigned int irq,
>>> +               struct fixed_phy_status *status,
>>> +               struct device_node *np)
>>> +{
>>> +    struct fixed_mdio_bus *fmb = &platform_fmb;
>>> +    struct phy_device *phy;
>>> +    int phy_addr;
>>> +    int ret;
>>> +
>>> +    /* Get the next available PHY address, up to PHY_MAX_ADDR */
>>> +    spin_lock(&phy_fixed_addr_lock);
>>> +    if (phy_fixed_addr == PHY_MAX_ADDR) {
>>> +        spin_unlock(&phy_fixed_addr_lock);
>>> +        return -ENOSPC;
>>> +    }
>>> +    phy_addr = phy_fixed_addr++;
>>> +    spin_unlock(&phy_fixed_addr_lock);
>>> +
>>> +    ret = fixed_phy_add(PHY_POLL, phy_addr, status);
>>
>>     Was rummaging in the fixed_phy driver and a bug sprang right at me:
>> 'phy' should have been passed here, not PHY_POLL. Luckily, all callers
>> pass PHY_POLL anyway...

> Are we looking at the same header file for the prototype of fixed_phy_add()?

    Probably not. I was looking at Linus' tree, yours is probably net-next. :-)

> extern int fixed_phy_add(unsigned int irq, int phy_id,
>                           struct fixed_phy_status *status,
>                           int link_gpio);

> First argument is correct here..

    No, fixed_phy_register() gets 'irq' passed to it and it should in its turn 
call fixed_phy_add() with this argument, not PHY_POLL; otherwise the 'irq' 
parameter gets completely ignored...

> at any rate, if something needs fixing, just go ahead and submit a patch.

    OK.

> --
> Florian

MBR, Sergei

[PATCHv4 2/5] net: phy: extend fixed driver with fixed_phy_register()

[Sergei Shtylyov]

Hello.

On 09/03/2015 10:23 PM, Florian Fainelli wrote:

>>> The existing fixed_phy_add() function has several drawbacks that
>>> prevents it from being used as is for OF-based declaration of fixed
>>> PHYs:
>>>
>>>    * The address of the PHY on the fake bus needs to be passed, while a
>>>      dynamic allocation is desired.
>>>
>>>    * Since the phy_device instantiation is post-poned until the next
>>>      mdiobus scan, there is no way to associate the fixed PHY with its
>>>      OF node, which later prevents of_phy_connect() from finding this
>>>      fixed PHY from a given OF node.
>>>
>>> To solve this, this commit introduces fixed_phy_register(), which will
>>> allocate an available PHY address, add the PHY using fixed_phy_add()
>>> and instantiate the phy_device structure associated with the provided
>>> OF node.
>>>
>>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>>> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>>> Acked-by: Grant Likely <grant.likely@linaro.org>
>>> ---
>>>    drivers/net/phy/fixed.c   | 61
>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>    include/linux/phy_fixed.h | 11 +++++++++
>>>    2 files changed, 72 insertions(+)
>>>
>>> diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
>>> index e41546d..d60d875 100644
>>> --- a/drivers/net/phy/fixed.c
>>> +++ b/drivers/net/phy/fixed.c
>> [...]
>>> @@ -203,6 +204,66 @@ err_regs:
>> [...]
>>> +int fixed_phy_register(unsigned int irq,
>>> +               struct fixed_phy_status *status,
>>> +               struct device_node *np)
>>> +{
>>> +    struct fixed_mdio_bus *fmb = &platform_fmb;
>>> +    struct phy_device *phy;
>>> +    int phy_addr;
>>> +    int ret;
>>> +
>>> +    /* Get the next available PHY address, up to PHY_MAX_ADDR */
>>> +    spin_lock(&phy_fixed_addr_lock);
>>> +    if (phy_fixed_addr == PHY_MAX_ADDR) {
>>> +        spin_unlock(&phy_fixed_addr_lock);
>>> +        return -ENOSPC;
>>> +    }
>>> +    phy_addr = phy_fixed_addr++;
>>> +    spin_unlock(&phy_fixed_addr_lock);
>>> +
>>> +    ret = fixed_phy_add(PHY_POLL, phy_addr, status);
>>
>>     Was rummaging in the fixed_phy driver and a bug sprang right at me:
>> 'phy' should have been passed here, not PHY_POLL. Luckily, all callers
>> pass PHY_POLL anyway...

> Are we looking at the same header file for the prototype of fixed_phy_add()?

    Probably not. I was looking at Linus' tree, yours is probably net-next. :-)

> extern int fixed_phy_add(unsigned int irq, int phy_id,
>                           struct fixed_phy_status *status,
>                           int link_gpio);

> First argument is correct here..

    No, fixed_phy_register() gets 'irq' passed to it and it should in its turn 
call fixed_phy_add() with this argument, not PHY_POLL; otherwise the 'irq' 
parameter gets completely ignored...

> at any rate, if something needs fixing, just go ahead and submit a patch.

    OK.

> --
> Florian

MBR, Sergei

Re: [PATCHv4 2/5] net: phy: extend fixed driver with fixed_phy_register()

[Sergei Shtylyov]

On 09/03/2015 10:20 PM, Sergei Shtylyov wrote:

>> The existing fixed_phy_add() function has several drawbacks that
>> prevents it from being used as is for OF-based declaration of fixed
>> PHYs:
>>
>>   * The address of the PHY on the fake bus needs to be passed, while a
>>     dynamic allocation is desired.
>>
>>   * Since the phy_device instantiation is post-poned until the next
>>     mdiobus scan, there is no way to associate the fixed PHY with its
>>     OF node, which later prevents of_phy_connect() from finding this
>>     fixed PHY from a given OF node.
>>
>> To solve this, this commit introduces fixed_phy_register(), which will
>> allocate an available PHY address, add the PHY using fixed_phy_add()
>> and instantiate the phy_device structure associated with the provided
>> OF node.
>>
>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>> Acked-by: Grant Likely <grant.likely@linaro.org>
>> ---
>>   drivers/net/phy/fixed.c   | 61
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/phy_fixed.h | 11 +++++++++
>>   2 files changed, 72 insertions(+)
>>
>> diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
>> index e41546d..d60d875 100644
>> --- a/drivers/net/phy/fixed.c
>> +++ b/drivers/net/phy/fixed.c
> [...]
>> @@ -203,6 +204,66 @@ err_regs:
> [...]
>> +int fixed_phy_register(unsigned int irq,
>> +               struct fixed_phy_status *status,
>> +               struct device_node *np)
>> +{
>> +    struct fixed_mdio_bus *fmb = &platform_fmb;
>> +    struct phy_device *phy;
>> +    int phy_addr;
>> +    int ret;
>> +
>> +    /* Get the next available PHY address, up to PHY_MAX_ADDR */
>> +    spin_lock(&phy_fixed_addr_lock);
>> +    if (phy_fixed_addr == PHY_MAX_ADDR) {
>> +        spin_unlock(&phy_fixed_addr_lock);
>> +        return -ENOSPC;
>> +    }
>> +    phy_addr = phy_fixed_addr++;
>> +    spin_unlock(&phy_fixed_addr_lock);
>> +
>> +    ret = fixed_phy_add(PHY_POLL, phy_addr, status);
>
>     Was rummaging in the fixed_phy driver and a bug sprang right at me: 'phy'

    Sorry, s/phy/irq/ of course. Just noticed. :-/

> should have been passed here, not PHY_POLL. Luckily, all callers pass PHY_POLL
> anyway...

> [...]

MBR, Sergei

[PATCHv4 2/5] net: phy: extend fixed driver with fixed_phy_register()

[Sergei Shtylyov]

On 09/03/2015 10:20 PM, Sergei Shtylyov wrote:

>> The existing fixed_phy_add() function has several drawbacks that
>> prevents it from being used as is for OF-based declaration of fixed
>> PHYs:
>>
>>   * The address of the PHY on the fake bus needs to be passed, while a
>>     dynamic allocation is desired.
>>
>>   * Since the phy_device instantiation is post-poned until the next
>>     mdiobus scan, there is no way to associate the fixed PHY with its
>>     OF node, which later prevents of_phy_connect() from finding this
>>     fixed PHY from a given OF node.
>>
>> To solve this, this commit introduces fixed_phy_register(), which will
>> allocate an available PHY address, add the PHY using fixed_phy_add()
>> and instantiate the phy_device structure associated with the provided
>> OF node.
>>
>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>> Acked-by: Grant Likely <grant.likely@linaro.org>
>> ---
>>   drivers/net/phy/fixed.c   | 61
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/phy_fixed.h | 11 +++++++++
>>   2 files changed, 72 insertions(+)
>>
>> diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
>> index e41546d..d60d875 100644
>> --- a/drivers/net/phy/fixed.c
>> +++ b/drivers/net/phy/fixed.c
> [...]
>> @@ -203,6 +204,66 @@ err_regs:
> [...]
>> +int fixed_phy_register(unsigned int irq,
>> +               struct fixed_phy_status *status,
>> +               struct device_node *np)
>> +{
>> +    struct fixed_mdio_bus *fmb = &platform_fmb;
>> +    struct phy_device *phy;
>> +    int phy_addr;
>> +    int ret;
>> +
>> +    /* Get the next available PHY address, up to PHY_MAX_ADDR */
>> +    spin_lock(&phy_fixed_addr_lock);
>> +    if (phy_fixed_addr == PHY_MAX_ADDR) {
>> +        spin_unlock(&phy_fixed_addr_lock);
>> +        return -ENOSPC;
>> +    }
>> +    phy_addr = phy_fixed_addr++;
>> +    spin_unlock(&phy_fixed_addr_lock);
>> +
>> +    ret = fixed_phy_add(PHY_POLL, phy_addr, status);
>
>     Was rummaging in the fixed_phy driver and a bug sprang right at me: 'phy'

    Sorry, s/phy/irq/ of course. Just noticed. :-/

> should have been passed here, not PHY_POLL. Luckily, all callers pass PHY_POLL
> anyway...

> [...]

MBR, Sergei

Re: [PATCHv4 2/5] net: phy: extend fixed driver with fixed_phy_register()

[Florian Fainelli]

On 03/09/15 12:37, Sergei Shtylyov wrote:
> On 09/03/2015 10:20 PM, Sergei Shtylyov wrote:
> 
>>> The existing fixed_phy_add() function has several drawbacks that
>>> prevents it from being used as is for OF-based declaration of fixed
>>> PHYs:
>>>
>>>   * The address of the PHY on the fake bus needs to be passed, while a
>>>     dynamic allocation is desired.
>>>
>>>   * Since the phy_device instantiation is post-poned until the next
>>>     mdiobus scan, there is no way to associate the fixed PHY with its
>>>     OF node, which later prevents of_phy_connect() from finding this
>>>     fixed PHY from a given OF node.
>>>
>>> To solve this, this commit introduces fixed_phy_register(), which will
>>> allocate an available PHY address, add the PHY using fixed_phy_add()
>>> and instantiate the phy_device structure associated with the provided
>>> OF node.
>>>
>>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>>> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>>> Acked-by: Grant Likely <grant.likely@linaro.org>
>>> ---
>>>   drivers/net/phy/fixed.c   | 61
>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>   include/linux/phy_fixed.h | 11 +++++++++
>>>   2 files changed, 72 insertions(+)
>>>
>>> diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
>>> index e41546d..d60d875 100644
>>> --- a/drivers/net/phy/fixed.c
>>> +++ b/drivers/net/phy/fixed.c
>> [...]
>>> @@ -203,6 +204,66 @@ err_regs:
>> [...]
>>> +int fixed_phy_register(unsigned int irq,
>>> +               struct fixed_phy_status *status,
>>> +               struct device_node *np)
>>> +{
>>> +    struct fixed_mdio_bus *fmb = &platform_fmb;
>>> +    struct phy_device *phy;
>>> +    int phy_addr;
>>> +    int ret;
>>> +
>>> +    /* Get the next available PHY address, up to PHY_MAX_ADDR */
>>> +    spin_lock(&phy_fixed_addr_lock);
>>> +    if (phy_fixed_addr == PHY_MAX_ADDR) {
>>> +        spin_unlock(&phy_fixed_addr_lock);
>>> +        return -ENOSPC;
>>> +    }
>>> +    phy_addr = phy_fixed_addr++;
>>> +    spin_unlock(&phy_fixed_addr_lock);
>>> +
>>> +    ret = fixed_phy_add(PHY_POLL, phy_addr, status);
>>
>>     Was rummaging in the fixed_phy driver and a bug sprang right at
>> me: 'phy'
> 
>    Sorry, s/phy/irq/ of course. Just noticed. :-/

Ok, that makes sense then, and yes, this "irq" argument should have been
passed down to fixed_phy_add(). Might be worth adding a WARN_ON(irq !=
PHY_POLL) just to catch callers that expect something else.

Thanks!
--
Florian

[PATCHv4 2/5] net: phy: extend fixed driver with fixed_phy_register()

[Florian Fainelli]

On 03/09/15 12:37, Sergei Shtylyov wrote:
> On 09/03/2015 10:20 PM, Sergei Shtylyov wrote:
> 
>>> The existing fixed_phy_add() function has several drawbacks that
>>> prevents it from being used as is for OF-based declaration of fixed
>>> PHYs:
>>>
>>>   * The address of the PHY on the fake bus needs to be passed, while a
>>>     dynamic allocation is desired.
>>>
>>>   * Since the phy_device instantiation is post-poned until the next
>>>     mdiobus scan, there is no way to associate the fixed PHY with its
>>>     OF node, which later prevents of_phy_connect() from finding this
>>>     fixed PHY from a given OF node.
>>>
>>> To solve this, this commit introduces fixed_phy_register(), which will
>>> allocate an available PHY address, add the PHY using fixed_phy_add()
>>> and instantiate the phy_device structure associated with the provided
>>> OF node.
>>>
>>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>>> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>>> Acked-by: Grant Likely <grant.likely@linaro.org>
>>> ---
>>>   drivers/net/phy/fixed.c   | 61
>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>   include/linux/phy_fixed.h | 11 +++++++++
>>>   2 files changed, 72 insertions(+)
>>>
>>> diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
>>> index e41546d..d60d875 100644
>>> --- a/drivers/net/phy/fixed.c
>>> +++ b/drivers/net/phy/fixed.c
>> [...]
>>> @@ -203,6 +204,66 @@ err_regs:
>> [...]
>>> +int fixed_phy_register(unsigned int irq,
>>> +               struct fixed_phy_status *status,
>>> +               struct device_node *np)
>>> +{
>>> +    struct fixed_mdio_bus *fmb = &platform_fmb;
>>> +    struct phy_device *phy;
>>> +    int phy_addr;
>>> +    int ret;
>>> +
>>> +    /* Get the next available PHY address, up to PHY_MAX_ADDR */
>>> +    spin_lock(&phy_fixed_addr_lock);
>>> +    if (phy_fixed_addr == PHY_MAX_ADDR) {
>>> +        spin_unlock(&phy_fixed_addr_lock);
>>> +        return -ENOSPC;
>>> +    }
>>> +    phy_addr = phy_fixed_addr++;
>>> +    spin_unlock(&phy_fixed_addr_lock);
>>> +
>>> +    ret = fixed_phy_add(PHY_POLL, phy_addr, status);
>>
>>     Was rummaging in the fixed_phy driver and a bug sprang right at
>> me: 'phy'
> 
>    Sorry, s/phy/irq/ of course. Just noticed. :-/

Ok, that makes sense then, and yes, this "irq" argument should have been
passed down to fixed_phy_add(). Might be worth adding a WARN_ON(irq !=
PHY_POLL) just to catch callers that expect something else.

Thanks!
--
Florian

Re: [PATCHv4 2/5] net: phy: extend fixed driver with fixed_phy_register()

[Sergei Shtylyov]

On 09/03/2015 10:38 PM, Florian Fainelli wrote:

>>>> The existing fixed_phy_add() function has several drawbacks that
>>>> prevents it from being used as is for OF-based declaration of fixed
>>>> PHYs:
>>>>
>>>>    * The address of the PHY on the fake bus needs to be passed, while a
>>>>      dynamic allocation is desired.
>>>>
>>>>    * Since the phy_device instantiation is post-poned until the next
>>>>      mdiobus scan, there is no way to associate the fixed PHY with its
>>>>      OF node, which later prevents of_phy_connect() from finding this
>>>>      fixed PHY from a given OF node.
>>>>
>>>> To solve this, this commit introduces fixed_phy_register(), which will
>>>> allocate an available PHY address, add the PHY using fixed_phy_add()
>>>> and instantiate the phy_device structure associated with the provided
>>>> OF node.
>>>>
>>>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>>>> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>>>> Acked-by: Grant Likely <grant.likely@linaro.org>
>>>> ---
>>>>    drivers/net/phy/fixed.c   | 61
>>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>>    include/linux/phy_fixed.h | 11 +++++++++
>>>>    2 files changed, 72 insertions(+)
>>>>
>>>> diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
>>>> index e41546d..d60d875 100644
>>>> --- a/drivers/net/phy/fixed.c
>>>> +++ b/drivers/net/phy/fixed.c
>>> [...]
>>>> @@ -203,6 +204,66 @@ err_regs:
>>> [...]
>>>> +int fixed_phy_register(unsigned int irq,
>>>> +               struct fixed_phy_status *status,
>>>> +               struct device_node *np)
>>>> +{
>>>> +    struct fixed_mdio_bus *fmb = &platform_fmb;
>>>> +    struct phy_device *phy;
>>>> +    int phy_addr;
>>>> +    int ret;
>>>> +
>>>> +    /* Get the next available PHY address, up to PHY_MAX_ADDR */
>>>> +    spin_lock(&phy_fixed_addr_lock);
>>>> +    if (phy_fixed_addr == PHY_MAX_ADDR) {
>>>> +        spin_unlock(&phy_fixed_addr_lock);
>>>> +        return -ENOSPC;
>>>> +    }
>>>> +    phy_addr = phy_fixed_addr++;
>>>> +    spin_unlock(&phy_fixed_addr_lock);
>>>> +
>>>> +    ret = fixed_phy_add(PHY_POLL, phy_addr, status);
>>>
>>>      Was rummaging in the fixed_phy driver and a bug sprang right at
>>> me: 'phy'
>>
>>     Sorry, s/phy/irq/ of course. Just noticed. :-/

    I've reported the bug on #miplsinux, there I used the correct word. :-)

> Ok, that makes sense then, and yes, this "irq" argument should have been
> passed down to fixed_phy_add(). Might be worth adding a WARN_ON(irq !=
> PHY_POLL) just to catch callers that expect something else.

    In-tree callers all seem to pass PHY_POLL to fixed_phy_register(). Do we 
care about out of tree stuff?

> Thanks!
> --
> Florian

MBR, Sergei

[PATCHv4 2/5] net: phy: extend fixed driver with fixed_phy_register()

[Sergei Shtylyov]

On 09/03/2015 10:38 PM, Florian Fainelli wrote:

>>>> The existing fixed_phy_add() function has several drawbacks that
>>>> prevents it from being used as is for OF-based declaration of fixed
>>>> PHYs:
>>>>
>>>>    * The address of the PHY on the fake bus needs to be passed, while a
>>>>      dynamic allocation is desired.
>>>>
>>>>    * Since the phy_device instantiation is post-poned until the next
>>>>      mdiobus scan, there is no way to associate the fixed PHY with its
>>>>      OF node, which later prevents of_phy_connect() from finding this
>>>>      fixed PHY from a given OF node.
>>>>
>>>> To solve this, this commit introduces fixed_phy_register(), which will
>>>> allocate an available PHY address, add the PHY using fixed_phy_add()
>>>> and instantiate the phy_device structure associated with the provided
>>>> OF node.
>>>>
>>>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>>>> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>>>> Acked-by: Grant Likely <grant.likely@linaro.org>
>>>> ---
>>>>    drivers/net/phy/fixed.c   | 61
>>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>>    include/linux/phy_fixed.h | 11 +++++++++
>>>>    2 files changed, 72 insertions(+)
>>>>
>>>> diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c
>>>> index e41546d..d60d875 100644
>>>> --- a/drivers/net/phy/fixed.c
>>>> +++ b/drivers/net/phy/fixed.c
>>> [...]
>>>> @@ -203,6 +204,66 @@ err_regs:
>>> [...]
>>>> +int fixed_phy_register(unsigned int irq,
>>>> +               struct fixed_phy_status *status,
>>>> +               struct device_node *np)
>>>> +{
>>>> +    struct fixed_mdio_bus *fmb = &platform_fmb;
>>>> +    struct phy_device *phy;
>>>> +    int phy_addr;
>>>> +    int ret;
>>>> +
>>>> +    /* Get the next available PHY address, up to PHY_MAX_ADDR */
>>>> +    spin_lock(&phy_fixed_addr_lock);
>>>> +    if (phy_fixed_addr == PHY_MAX_ADDR) {
>>>> +        spin_unlock(&phy_fixed_addr_lock);
>>>> +        return -ENOSPC;
>>>> +    }
>>>> +    phy_addr = phy_fixed_addr++;
>>>> +    spin_unlock(&phy_fixed_addr_lock);
>>>> +
>>>> +    ret = fixed_phy_add(PHY_POLL, phy_addr, status);
>>>
>>>      Was rummaging in the fixed_phy driver and a bug sprang right at
>>> me: 'phy'
>>
>>     Sorry, s/phy/irq/ of course. Just noticed. :-/

    I've reported the bug on #miplsinux, there I used the correct word. :-)

> Ok, that makes sense then, and yes, this "irq" argument should have been
> passed down to fixed_phy_add(). Might be worth adding a WARN_ON(irq !=
> PHY_POLL) just to catch callers that expect something else.

    In-tree callers all seem to pass PHY_POLL to fixed_phy_register(). Do we 
care about out of tree stuff?

> Thanks!
> --
> Florian

MBR, Sergei