[U-Boot] [PATCH 1/2] net: phy: Move fixed link code to separate routine

[U-Boot] [PATCH 1/2] net: phy: Move fixed link code to separate routine

[Siva Durga Prasad Paladugu]

This patch moves fixed-link functionality code to a separate
routine inorder to make it more modular and cleaner.

Signed-off-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/net/phy/phy.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index e837eb7..3cb2785 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -875,18 +875,18 @@ void phy_connect_dev(struct phy_device *phydev, struct eth_device *dev)
 	debug("%s connected to %s\n", dev->name, phydev->drv->name);
 }
 
+#ifdef CONFIG_PHY_FIXED
 #ifdef CONFIG_DM_ETH
-struct phy_device *phy_connect(struct mii_dev *bus, int addr,
-			       struct udevice *dev,
-			       phy_interface_t interface)
+static struct phy_device *phy_connect_fixed(struct mii_dev *bus,
+					    struct udevice *dev,
+					    phy_interface_t interface)
 #else
-struct phy_device *phy_connect(struct mii_dev *bus, int addr,
-			       struct eth_device *dev,
-			       phy_interface_t interface)
+static struct phy_device *phy_connect_fixed(struct mii_dev *bus,
+					    struct eth_device *dev,
+					    phy_interface_t interface)
 #endif
 {
 	struct phy_device *phydev = NULL;
-#ifdef CONFIG_PHY_FIXED
 	int sn;
 	const char *name;
 
@@ -900,7 +900,27 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr,
 		}
 		sn = fdt_next_subnode(gd->fdt_blob, sn);
 	}
+
+	return phydev;
+}
 #endif
+
+#ifdef CONFIG_DM_ETH
+struct phy_device *phy_connect(struct mii_dev *bus, int addr,
+			       struct udevice *dev,
+			       phy_interface_t interface)
+#else
+struct phy_device *phy_connect(struct mii_dev *bus, int addr,
+			       struct eth_device *dev,
+			       phy_interface_t interface)
+#endif
+{
+	struct phy_device *phydev = NULL;
+
+#ifdef CONFIG_PHY_FIXED
+	phydev = phy_connect_fixed(bus, dev, interface);
+#endif
+
 	if (!phydev)
 		phydev = phy_find_by_mask(bus, 1 << addr, interface);
 
-- 
2.7.4

[U-Boot] [PATCH 2/2] net: phy: Add gmiitorgmii converter support

[Siva Durga Prasad Paladugu]

This patch adds support for gmiitorgmii converter.
This converter sits between the MAC and the external phy
MAC <==> GMII2RGMII <==> RGMII_PHY.
The ethernet driver probes this bridge and this bridge driver
probes real phy driver and invokes the real phy functionalities
as requested. This bridge just needs to be configured based on
real phy negotiated speed and duplex.

Signed-off-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/net/phy/Kconfig             |   7 +++
 drivers/net/phy/Makefile            |   1 +
 drivers/net/phy/phy.c               |  41 ++++++++++++++
 drivers/net/phy/xilinx_gmii2rgmii.c | 103 ++++++++++++++++++++++++++++++++++++
 include/phy.h                       |   6 +++
 5 files changed, 158 insertions(+)
 create mode 100644 drivers/net/phy/xilinx_gmii2rgmii.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 3dc0822..a68e167 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -217,6 +217,13 @@ config PHY_VITESSE
 config PHY_XILINX
 	bool "Xilinx Ethernet PHYs support"
 
+config PHY_XILINX_GMII2RGMII
+	bool "Xilinx GMII to RGMII Ethernet PHYs support"
+	help
+	  This adds support for Xilinx GMII to RGMII IP core. This IP acts
+	  as bridge between MAC connected over GMII and external phy that
+	  is connected over RGMII interface.
+
 config PHY_FIXED
 	bool "Fixed-Link PHY"
 	depends on DM_ETH
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 555da83..76b6197 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_PHY_SMSC) += smsc.o
 obj-$(CONFIG_PHY_TERANETICS) += teranetics.o
 obj-$(CONFIG_PHY_TI) += ti.o
 obj-$(CONFIG_PHY_XILINX) += xilinx_phy.o
+obj-$(CONFIG_PHY_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o
 obj-$(CONFIG_PHY_VITESSE) += vitesse.o
 obj-$(CONFIG_PHY_MSCC) += mscc.o
 obj-$(CONFIG_PHY_FIXED) += fixed.o
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 3cb2785..d02c4d8 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -528,6 +528,9 @@ int phy_init(void)
 #ifdef CONFIG_PHY_FIXED
 	phy_fixed_init();
 #endif
+#ifdef CONFIG_PHY_XILINX_GMII2RGMII
+	phy_xilinx_gmii2rgmii_init();
+#endif
 	return 0;
 }
 
@@ -875,6 +878,41 @@ void phy_connect_dev(struct phy_device *phydev, struct eth_device *dev)
 	debug("%s connected to %s\n", dev->name, phydev->drv->name);
 }
 
+#ifdef CONFIG_PHY_XILINX_GMII2RGMII
+#ifdef CONFIG_DM_ETH
+static struct phy_device *phy_connect_gmii2rgmii(struct mii_dev *bus,
+						 struct udevice *dev,
+						 phy_interface_t interface)
+#else
+static struct phy_device *phy_connect_gmii2rgmii(struct mii_dev *bus,
+						 struct eth_device *dev,
+						 phy_interface_t interface)
+#endif
+{
+	struct phy_device *phydev = NULL;
+	int sn = dev_of_offset(dev);
+	int off;
+
+	while (sn > 0) {
+		off = fdt_node_offset_by_compatible(gd->fdt_blob, sn,
+						    "xlnx,gmii-to-rgmii-1.0");
+		if (off > 0) {
+			phydev = phy_device_create(bus,
+						   off, PHY_GMII2RGMII_ID,
+						   interface);
+			break;
+		}
+		if (off == -FDT_ERR_NOTFOUND)
+			sn = fdt_first_subnode(gd->fdt_blob, sn);
+		else
+			printf("%s: Error finding compat string:%d\n",
+			       __func__, off);
+	}
+
+	return phydev;
+}
+#endif
+
 #ifdef CONFIG_PHY_FIXED
 #ifdef CONFIG_DM_ETH
 static struct phy_device *phy_connect_fixed(struct mii_dev *bus,
@@ -920,6 +958,9 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr,
 #ifdef CONFIG_PHY_FIXED
 	phydev = phy_connect_fixed(bus, dev, interface);
 #endif
+#ifdef CONFIG_PHY_XILINX_GMII2RGMII
+	phydev = phy_connect_gmii2rgmii(bus, dev, interface);
+#endif
 
 	if (!phydev)
 		phydev = phy_find_by_mask(bus, 1 << addr, interface);
diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c b/drivers/net/phy/xilinx_gmii2rgmii.c
new file mode 100644
index 0000000..aa4ce86
--- /dev/null
+++ b/drivers/net/phy/xilinx_gmii2rgmii.c
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx GMII2RGMII phy driver
+ *
+ * Copyright (C) 2018 Xilinx, Inc.
+ */
+
+#include <dm.h>
+#include <phy.h>
+#include <config.h>
+#include <common.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define ZYNQ_GMII2RGMII_REG		0x10
+#define ZYNQ_GMII2RGMII_SPEED_MASK	(BMCR_SPEED1000 | BMCR_SPEED100)
+
+static int xilinxgmiitorgmii_config(struct phy_device *phydev)
+{
+	struct phy_device *ext_phydev = phydev->priv;
+
+	debug("%s\n", __func__);
+	if (ext_phydev->drv->config)
+		ext_phydev->drv->config(ext_phydev);
+
+	return 0;
+}
+
+static int xilinxgmiitorgmii_startup(struct phy_device *phydev)
+{
+	u16 val = 0;
+	struct phy_device *ext_phydev = phydev->priv;
+
+	debug("%s\n", __func__);
+	ext_phydev->dev = phydev->dev;
+	if (ext_phydev->drv->startup)
+		ext_phydev->drv->startup(ext_phydev);
+
+	val = phy_read(phydev, phydev->addr, ZYNQ_GMII2RGMII_REG);
+	val &= ~ZYNQ_GMII2RGMII_SPEED_MASK;
+
+	if (ext_phydev->speed == SPEED_1000)
+		val |= BMCR_SPEED1000;
+	else if (ext_phydev->speed == SPEED_100)
+		val |= BMCR_SPEED100;
+
+	phy_write(phydev, phydev->addr, ZYNQ_GMII2RGMII_REG, val |
+		  BMCR_FULLDPLX);
+
+	phydev->duplex = ext_phydev->duplex;
+	phydev->speed = ext_phydev->speed;
+	phydev->link = ext_phydev->link;
+
+	return 0;
+}
+
+static int xilinxgmiitorgmii_probe(struct phy_device *phydev)
+{
+	int ofnode = phydev->addr;
+	u32 phy_of_handle;
+	int ext_phyaddr = -1;
+
+	debug("%s\n", __func__);
+
+	/*
+	 * Read the phy address again as the one we read in ethernet driver
+	 * was overwritten for the purpose of storing the ofnode
+	 */
+	phydev->addr = fdtdec_get_int(gd->fdt_blob, ofnode, "reg", -1);
+	phy_of_handle = fdtdec_lookup_phandle(gd->fdt_blob, ofnode,
+					      "phy-handle");
+	if (phy_of_handle > 0)
+		ext_phyaddr = fdtdec_get_int(gd->fdt_blob,
+					     phy_of_handle,
+					     "reg", -1);
+	phydev->priv = phy_find_by_mask(phydev->bus,
+					1 << ext_phyaddr,
+					phydev->interface);
+
+	debug("%s, gmii2rgmmi:0x%x, extphy:0x%x\n", __func__, phydev->addr,
+	      ext_phyaddr);
+
+	phydev->flags |= PHY_FLAG_BROKEN_RESET;
+
+	return 0;
+}
+
+static struct phy_driver gmii2rgmii_driver = {
+	.name = "XILINX GMII2RGMII",
+	.uid = PHY_GMII2RGMII_ID,
+	.mask = 0xffffffff,
+	.features = PHY_GBIT_FEATURES,
+	.probe = xilinxgmiitorgmii_probe,
+	.config = xilinxgmiitorgmii_config,
+	.startup = xilinxgmiitorgmii_startup,
+};
+
+int phy_xilinx_gmii2rgmii_init(void)
+{
+	phy_register(&gmii2rgmii_driver);
+
+	return 0;
+}
diff --git a/include/phy.h b/include/phy.h
index b86fdfb..0485701 100644
--- a/include/phy.h
+++ b/include/phy.h
@@ -17,6 +17,11 @@
 #include <phy_interface.h>
 
 #define PHY_FIXED_ID		0xa5a55a5a
+/*
+ * There is no actual id for this.
+ * This is just a dummy id for gmii2rgmmi converter.
+ */
+#define PHY_GMII2RGMII_ID	0x5a5a5a5a
 
 #define PHY_MAX_ADDR 32
 
@@ -240,6 +245,7 @@ int phy_vitesse_init(void);
 int phy_xilinx_init(void);
 int phy_mscc_init(void);
 int phy_fixed_init(void);
+int phy_xilinx_gmii2rgmii_init(void);
 
 int board_phy_config(struct phy_device *phydev);
 int get_phy_id(struct mii_dev *bus, int addr, int devad, u32 *phy_id);
-- 
2.7.4

[U-Boot] [PATCH 2/2] net: phy: Add gmiitorgmii converter support

[Michal Simek]

On 27. 11. 18 7:19, Siva Durga Prasad Paladugu wrote:
> This patch adds support for gmiitorgmii converter.
> This converter sits between the MAC and the external phy
> MAC <==> GMII2RGMII <==> RGMII_PHY.
> The ethernet driver probes this bridge and this bridge driver
> probes real phy driver and invokes the real phy functionalities
> as requested. This bridge just needs to be configured based on
> real phy negotiated speed and duplex.
> 
> Signed-off-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>  drivers/net/phy/Kconfig             |   7 +++
>  drivers/net/phy/Makefile            |   1 +
>  drivers/net/phy/phy.c               |  41 ++++++++++++++
>  drivers/net/phy/xilinx_gmii2rgmii.c | 103 ++++++++++++++++++++++++++++++++++++
>  include/phy.h                       |   6 +++
>  5 files changed, 158 insertions(+)
>  create mode 100644 drivers/net/phy/xilinx_gmii2rgmii.c
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 3dc0822..a68e167 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -217,6 +217,13 @@ config PHY_VITESSE
>  config PHY_XILINX
>  	bool "Xilinx Ethernet PHYs support"
>  
> +config PHY_XILINX_GMII2RGMII
> +	bool "Xilinx GMII to RGMII Ethernet PHYs support"
> +	help
> +	  This adds support for Xilinx GMII to RGMII IP core. This IP acts
> +	  as bridge between MAC connected over GMII and external phy that
> +	  is connected over RGMII interface.
> +
>  config PHY_FIXED
>  	bool "Fixed-Link PHY"
>  	depends on DM_ETH
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 555da83..76b6197 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_PHY_SMSC) += smsc.o
>  obj-$(CONFIG_PHY_TERANETICS) += teranetics.o
>  obj-$(CONFIG_PHY_TI) += ti.o
>  obj-$(CONFIG_PHY_XILINX) += xilinx_phy.o
> +obj-$(CONFIG_PHY_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o
>  obj-$(CONFIG_PHY_VITESSE) += vitesse.o
>  obj-$(CONFIG_PHY_MSCC) += mscc.o
>  obj-$(CONFIG_PHY_FIXED) += fixed.o
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 3cb2785..d02c4d8 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -528,6 +528,9 @@ int phy_init(void)
>  #ifdef CONFIG_PHY_FIXED
>  	phy_fixed_init();
>  #endif
> +#ifdef CONFIG_PHY_XILINX_GMII2RGMII
> +	phy_xilinx_gmii2rgmii_init();
> +#endif
>  	return 0;
>  }
>  
> @@ -875,6 +878,41 @@ void phy_connect_dev(struct phy_device *phydev, struct eth_device *dev)
>  	debug("%s connected to %s\n", dev->name, phydev->drv->name);
>  }
>  
> +#ifdef CONFIG_PHY_XILINX_GMII2RGMII
> +#ifdef CONFIG_DM_ETH
> +static struct phy_device *phy_connect_gmii2rgmii(struct mii_dev *bus,
> +						 struct udevice *dev,
> +						 phy_interface_t interface)
> +#else
> +static struct phy_device *phy_connect_gmii2rgmii(struct mii_dev *bus,
> +						 struct eth_device *dev,
> +						 phy_interface_t interface)
> +#endif
> +{
> +	struct phy_device *phydev = NULL;
> +	int sn = dev_of_offset(dev);
> +	int off;
> +
> +	while (sn > 0) {
> +		off = fdt_node_offset_by_compatible(gd->fdt_blob, sn,
> +						    "xlnx,gmii-to-rgmii-1.0");
> +		if (off > 0) {
> +			phydev = phy_device_create(bus,
> +						   off, PHY_GMII2RGMII_ID,
> +						   interface);
> +			break;
> +		}
> +		if (off == -FDT_ERR_NOTFOUND)
> +			sn = fdt_first_subnode(gd->fdt_blob, sn);
> +		else
> +			printf("%s: Error finding compat string:%d\n",
> +			       __func__, off);
> +	}
> +
> +	return phydev;
> +}
> +#endif
> +
>  #ifdef CONFIG_PHY_FIXED
>  #ifdef CONFIG_DM_ETH
>  static struct phy_device *phy_connect_fixed(struct mii_dev *bus,
> @@ -920,6 +958,9 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr,
>  #ifdef CONFIG_PHY_FIXED
>  	phydev = phy_connect_fixed(bus, dev, interface);
>  #endif
> +#ifdef CONFIG_PHY_XILINX_GMII2RGMII
> +	phydev = phy_connect_gmii2rgmii(bus, dev, interface);
> +#endif

I was debugging one issue in connection to fixed-link phy and I think we
should try to connect gmii2rgmii if phydev is NULL.

It means this should be

#ifdef CONFIG_PHY_XILINX_GMII2RGMII
	if (!phydev)
		phydev = phy_connect_gmii2rgmii(bus, dev, interface);
#endif

Thanks,
Michal

[U-Boot] [PATCH 2/2] net: phy: Add gmiitorgmii converter support

[Joe Hershberger]

On Tue, Nov 27, 2018 at 12:20 AM Siva Durga Prasad Paladugu
<siva.durga.paladugu@xilinx.com> wrote:
>
> This patch adds support for gmiitorgmii converter.
> This converter sits between the MAC and the external phy
> MAC <==> GMII2RGMII <==> RGMII_PHY.
> The ethernet driver probes this bridge and this bridge driver
> probes real phy driver and invokes the real phy functionalities
> as requested. This bridge just needs to be configured based on
> real phy negotiated speed and duplex.
>
> Signed-off-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---

FYI, NI doesn't use the Xilinx adapter, we have several internal
adapters... GMII to RGMII, RMII, and I believe GMII (passing through
EMIO).

>  drivers/net/phy/Kconfig             |   7 +++
>  drivers/net/phy/Makefile            |   1 +
>  drivers/net/phy/phy.c               |  41 ++++++++++++++
>  drivers/net/phy/xilinx_gmii2rgmii.c | 103 ++++++++++++++++++++++++++++++++++++
>  include/phy.h                       |   6 +++
>  5 files changed, 158 insertions(+)
>  create mode 100644 drivers/net/phy/xilinx_gmii2rgmii.c
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 3dc0822..a68e167 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -217,6 +217,13 @@ config PHY_VITESSE
>  config PHY_XILINX
>         bool "Xilinx Ethernet PHYs support"
>
> +config PHY_XILINX_GMII2RGMII
> +       bool "Xilinx GMII to RGMII Ethernet PHYs support"
> +       help
> +         This adds support for Xilinx GMII to RGMII IP core. This IP acts
> +         as bridge between MAC connected over GMII and external phy that
> +         is connected over RGMII interface.
> +
>  config PHY_FIXED
>         bool "Fixed-Link PHY"
>         depends on DM_ETH
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 555da83..76b6197 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_PHY_SMSC) += smsc.o
>  obj-$(CONFIG_PHY_TERANETICS) += teranetics.o
>  obj-$(CONFIG_PHY_TI) += ti.o
>  obj-$(CONFIG_PHY_XILINX) += xilinx_phy.o
> +obj-$(CONFIG_PHY_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o
>  obj-$(CONFIG_PHY_VITESSE) += vitesse.o
>  obj-$(CONFIG_PHY_MSCC) += mscc.o
>  obj-$(CONFIG_PHY_FIXED) += fixed.o
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 3cb2785..d02c4d8 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -528,6 +528,9 @@ int phy_init(void)
>  #ifdef CONFIG_PHY_FIXED
>         phy_fixed_init();
>  #endif
> +#ifdef CONFIG_PHY_XILINX_GMII2RGMII
> +       phy_xilinx_gmii2rgmii_init();
> +#endif

I'm a bit surprised this needs to act as a unique type of device and
not a normal phy driver? When the phy_id is read, what is returned?
The real phy? Nothing? You change the speed by accessing MDIO?

>         return 0;
>  }
>
> @@ -875,6 +878,41 @@ void phy_connect_dev(struct phy_device *phydev, struct eth_device *dev)
>         debug("%s connected to %s\n", dev->name, phydev->drv->name);
>  }
>
> +#ifdef CONFIG_PHY_XILINX_GMII2RGMII
> +#ifdef CONFIG_DM_ETH
> +static struct phy_device *phy_connect_gmii2rgmii(struct mii_dev *bus,
> +                                                struct udevice *dev,
> +                                                phy_interface_t interface)
> +#else
> +static struct phy_device *phy_connect_gmii2rgmii(struct mii_dev *bus,
> +                                                struct eth_device *dev,
> +                                                phy_interface_t interface)
> +#endif
> +{
> +       struct phy_device *phydev = NULL;
> +       int sn = dev_of_offset(dev);
> +       int off;
> +
> +       while (sn > 0) {
> +               off = fdt_node_offset_by_compatible(gd->fdt_blob, sn,
> +                                                   "xlnx,gmii-to-rgmii-1.0");

This seems to be searching under the eth dev, but at least in the
"linux/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt"
docs, this is found under the mdio node. How is this supposed to
handle more than one bridge?

> +               if (off > 0) {
> +                       phydev = phy_device_create(bus,
> +                                                  off, PHY_GMII2RGMII_ID,
> +                                                  interface);
> +                       break;
> +               }
> +               if (off == -FDT_ERR_NOTFOUND)
> +                       sn = fdt_first_subnode(gd->fdt_blob, sn);
> +               else
> +                       printf("%s: Error finding compat string:%d\n",
> +                              __func__, off);
> +       }
> +
> +       return phydev;
> +}
> +#endif
> +
>  #ifdef CONFIG_PHY_FIXED
>  #ifdef CONFIG_DM_ETH
>  static struct phy_device *phy_connect_fixed(struct mii_dev *bus,
> @@ -920,6 +958,9 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr,
>  #ifdef CONFIG_PHY_FIXED
>         phydev = phy_connect_fixed(bus, dev, interface);
>  #endif
> +#ifdef CONFIG_PHY_XILINX_GMII2RGMII
> +       phydev = phy_connect_gmii2rgmii(bus, dev, interface);
> +#endif
>
>         if (!phydev)
>                 phydev = phy_find_by_mask(bus, 1 << addr, interface);
> diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c b/drivers/net/phy/xilinx_gmii2rgmii.c
> new file mode 100644
> index 0000000..aa4ce86
> --- /dev/null
> +++ b/drivers/net/phy/xilinx_gmii2rgmii.c
> @@ -0,0 +1,103 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx GMII2RGMII phy driver
> + *
> + * Copyright (C) 2018 Xilinx, Inc.
> + */
> +
> +#include <dm.h>
> +#include <phy.h>
> +#include <config.h>
> +#include <common.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define ZYNQ_GMII2RGMII_REG            0x10
> +#define ZYNQ_GMII2RGMII_SPEED_MASK     (BMCR_SPEED1000 | BMCR_SPEED100)
> +
> +static int xilinxgmiitorgmii_config(struct phy_device *phydev)
> +{
> +       struct phy_device *ext_phydev = phydev->priv;
> +
> +       debug("%s\n", __func__);
> +       if (ext_phydev->drv->config)
> +               ext_phydev->drv->config(ext_phydev);
> +
> +       return 0;
> +}
> +
> +static int xilinxgmiitorgmii_startup(struct phy_device *phydev)
> +{
> +       u16 val = 0;
> +       struct phy_device *ext_phydev = phydev->priv;
> +
> +       debug("%s\n", __func__);
> +       ext_phydev->dev = phydev->dev;
> +       if (ext_phydev->drv->startup)
> +               ext_phydev->drv->startup(ext_phydev);
> +
> +       val = phy_read(phydev, phydev->addr, ZYNQ_GMII2RGMII_REG);

This seems to imply that the bridge IP is actually on MDIO at a
separate address from the real phy, but the same MDIO bus.

> +       val &= ~ZYNQ_GMII2RGMII_SPEED_MASK;
> +
> +       if (ext_phydev->speed == SPEED_1000)
> +               val |= BMCR_SPEED1000;
> +       else if (ext_phydev->speed == SPEED_100)
> +               val |= BMCR_SPEED100;
> +
> +       phy_write(phydev, phydev->addr, ZYNQ_GMII2RGMII_REG, val |
> +                 BMCR_FULLDPLX);
> +
> +       phydev->duplex = ext_phydev->duplex;
> +       phydev->speed = ext_phydev->speed;
> +       phydev->link = ext_phydev->link;
> +
> +       return 0;
> +}
> +
> +static int xilinxgmiitorgmii_probe(struct phy_device *phydev)
> +{
> +       int ofnode = phydev->addr;
> +       u32 phy_of_handle;
> +       int ext_phyaddr = -1;
> +
> +       debug("%s\n", __func__);
> +
> +       /*
> +        * Read the phy address again as the one we read in ethernet driver
> +        * was overwritten for the purpose of storing the ofnode
> +        */
> +       phydev->addr = fdtdec_get_int(gd->fdt_blob, ofnode, "reg", -1);

Is this phyaddr made up? Or is this IP actually listening on a
specific address on MDIO?

> +       phy_of_handle = fdtdec_lookup_phandle(gd->fdt_blob, ofnode,
> +                                             "phy-handle");
> +       if (phy_of_handle > 0)
> +               ext_phyaddr = fdtdec_get_int(gd->fdt_blob,
> +                                            phy_of_handle,
> +                                            "reg", -1);

I'd like to see a xilinx_gmii2rgmii.txt added to describe how the
device tree is expected to be written. The one in Linux seems a bit
incomplete, also. For instance, does the phy-handle in the ethmac node
point to the bridge? Is it just missing? No mention.

> +       phydev->priv = phy_find_by_mask(phydev->bus,
> +                                       1 << ext_phyaddr,
> +                                       phydev->interface);
> +
> +       debug("%s, gmii2rgmmi:0x%x, extphy:0x%x\n", __func__, phydev->addr,
> +             ext_phyaddr);
> +
> +       phydev->flags |= PHY_FLAG_BROKEN_RESET;
> +
> +       return 0;
> +}
> +
> +static struct phy_driver gmii2rgmii_driver = {
> +       .name = "XILINX GMII2RGMII",
> +       .uid = PHY_GMII2RGMII_ID,
> +       .mask = 0xffffffff,
> +       .features = PHY_GBIT_FEATURES,
> +       .probe = xilinxgmiitorgmii_probe,
> +       .config = xilinxgmiitorgmii_config,
> +       .startup = xilinxgmiitorgmii_startup,
> +};
> +
> +int phy_xilinx_gmii2rgmii_init(void)
> +{
> +       phy_register(&gmii2rgmii_driver);
> +
> +       return 0;
> +}
> diff --git a/include/phy.h b/include/phy.h
> index b86fdfb..0485701 100644
> --- a/include/phy.h
> +++ b/include/phy.h
> @@ -17,6 +17,11 @@
>  #include <phy_interface.h>
>
>  #define PHY_FIXED_ID           0xa5a55a5a
> +/*
> + * There is no actual id for this.
> + * This is just a dummy id for gmii2rgmmi converter.
> + */
> +#define PHY_GMII2RGMII_ID      0x5a5a5a5a
>
>  #define PHY_MAX_ADDR 32
>
> @@ -240,6 +245,7 @@ int phy_vitesse_init(void);
>  int phy_xilinx_init(void);
>  int phy_mscc_init(void);
>  int phy_fixed_init(void);
> +int phy_xilinx_gmii2rgmii_init(void);
>
>  int board_phy_config(struct phy_device *phydev);
>  int get_phy_id(struct mii_dev *bus, int addr, int devad, u32 *phy_id);
> --
> 2.7.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 2/2] net: phy: Add gmiitorgmii converter support

[Michal Simek]

On 23. 01. 19 19:20, Joe Hershberger wrote:
> On Tue, Nov 27, 2018 at 12:20 AM Siva Durga Prasad Paladugu
> <siva.durga.paladugu@xilinx.com> wrote:
>>
>> This patch adds support for gmiitorgmii converter.
>> This converter sits between the MAC and the external phy
>> MAC <==> GMII2RGMII <==> RGMII_PHY.
>> The ethernet driver probes this bridge and this bridge driver
>> probes real phy driver and invokes the real phy functionalities
>> as requested. This bridge just needs to be configured based on
>> real phy negotiated speed and duplex.
>>
>> Signed-off-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
> 
> FYI, NI doesn't use the Xilinx adapter, we have several internal
> adapters... GMII to RGMII, RMII, and I believe GMII (passing through
> EMIO).

ok. How do you manage them?


> 
>>  drivers/net/phy/Kconfig             |   7 +++
>>  drivers/net/phy/Makefile            |   1 +
>>  drivers/net/phy/phy.c               |  41 ++++++++++++++
>>  drivers/net/phy/xilinx_gmii2rgmii.c | 103 ++++++++++++++++++++++++++++++++++++
>>  include/phy.h                       |   6 +++
>>  5 files changed, 158 insertions(+)
>>  create mode 100644 drivers/net/phy/xilinx_gmii2rgmii.c
>>
>> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
>> index 3dc0822..a68e167 100644
>> --- a/drivers/net/phy/Kconfig
>> +++ b/drivers/net/phy/Kconfig
>> @@ -217,6 +217,13 @@ config PHY_VITESSE
>>  config PHY_XILINX
>>         bool "Xilinx Ethernet PHYs support"
>>
>> +config PHY_XILINX_GMII2RGMII
>> +       bool "Xilinx GMII to RGMII Ethernet PHYs support"
>> +       help
>> +         This adds support for Xilinx GMII to RGMII IP core. This IP acts
>> +         as bridge between MAC connected over GMII and external phy that
>> +         is connected over RGMII interface.
>> +
>>  config PHY_FIXED
>>         bool "Fixed-Link PHY"
>>         depends on DM_ETH
>> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
>> index 555da83..76b6197 100644
>> --- a/drivers/net/phy/Makefile
>> +++ b/drivers/net/phy/Makefile
>> @@ -27,6 +27,7 @@ obj-$(CONFIG_PHY_SMSC) += smsc.o
>>  obj-$(CONFIG_PHY_TERANETICS) += teranetics.o
>>  obj-$(CONFIG_PHY_TI) += ti.o
>>  obj-$(CONFIG_PHY_XILINX) += xilinx_phy.o
>> +obj-$(CONFIG_PHY_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o
>>  obj-$(CONFIG_PHY_VITESSE) += vitesse.o
>>  obj-$(CONFIG_PHY_MSCC) += mscc.o
>>  obj-$(CONFIG_PHY_FIXED) += fixed.o
>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>> index 3cb2785..d02c4d8 100644
>> --- a/drivers/net/phy/phy.c
>> +++ b/drivers/net/phy/phy.c
>> @@ -528,6 +528,9 @@ int phy_init(void)
>>  #ifdef CONFIG_PHY_FIXED
>>         phy_fixed_init();
>>  #endif
>> +#ifdef CONFIG_PHY_XILINX_GMII2RGMII
>> +       phy_xilinx_gmii2rgmii_init();
>> +#endif
> 
> I'm a bit surprised this needs to act as a unique type of device and
> not a normal phy driver? When the phy_id is read, what is returned?
> The real phy? Nothing? You change the speed by accessing MDIO?

It is connected on mdio bus but unfortunately there is no phy_id to read
to be able to use that.

https://www.xilinx.com/support/documentation/ip_documentation/gmii_to_rgmii/v4_0/pg160-gmii-to-rgmii.pdf
page 21


> 
>>         return 0;
>>  }
>>
>> @@ -875,6 +878,41 @@ void phy_connect_dev(struct phy_device *phydev, struct eth_device *dev)
>>         debug("%s connected to %s\n", dev->name, phydev->drv->name);
>>  }
>>
>> +#ifdef CONFIG_PHY_XILINX_GMII2RGMII
>> +#ifdef CONFIG_DM_ETH
>> +static struct phy_device *phy_connect_gmii2rgmii(struct mii_dev *bus,
>> +                                                struct udevice *dev,
>> +                                                phy_interface_t interface)
>> +#else
>> +static struct phy_device *phy_connect_gmii2rgmii(struct mii_dev *bus,
>> +                                                struct eth_device *dev,
>> +                                                phy_interface_t interface)
>> +#endif
>> +{
>> +       struct phy_device *phydev = NULL;
>> +       int sn = dev_of_offset(dev);
>> +       int off;
>> +
>> +       while (sn > 0) {
>> +               off = fdt_node_offset_by_compatible(gd->fdt_blob, sn,
>> +                                                   "xlnx,gmii-to-rgmii-1.0");
> 
> This seems to be searching under the eth dev, but at least in the
> "linux/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt"
> docs, this is found under the mdio node. How is this supposed to
> handle more than one bridge?

I haven't seen design with more than one bridge. It is really just one
brigde which goes to regular phy.

Regarding mdio and DT binding. I haven't seen anywhere written if phys
should be inside mdio node or they should be just child in controller node.

I see some examples where mdio node needs to contain compatibility
string and there is separate driver for that.

In dt spec nothing is written about it and also I haven't seen yaml
checking from Rob.

The key point for example is to show how bridge needs description for
phy on the board.

>> +               if (off > 0) {
>> +                       phydev = phy_device_create(bus,
>> +                                                  off, PHY_GMII2RGMII_ID,
>> +                                                  interface);
>> +                       break;
>> +               }
>> +               if (off == -FDT_ERR_NOTFOUND)
>> +                       sn = fdt_first_subnode(gd->fdt_blob, sn);
>> +               else
>> +                       printf("%s: Error finding compat string:%d\n",
>> +                              __func__, off);
>> +       }
>> +
>> +       return phydev;
>> +}
>> +#endif
>> +
>>  #ifdef CONFIG_PHY_FIXED
>>  #ifdef CONFIG_DM_ETH
>>  static struct phy_device *phy_connect_fixed(struct mii_dev *bus,
>> @@ -920,6 +958,9 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr,
>>  #ifdef CONFIG_PHY_FIXED
>>         phydev = phy_connect_fixed(bus, dev, interface);
>>  #endif
>> +#ifdef CONFIG_PHY_XILINX_GMII2RGMII
>> +       phydev = phy_connect_gmii2rgmii(bus, dev, interface);
>> +#endif
>>
>>         if (!phydev)
>>                 phydev = phy_find_by_mask(bus, 1 << addr, interface);
>> diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c b/drivers/net/phy/xilinx_gmii2rgmii.c
>> new file mode 100644
>> index 0000000..aa4ce86
>> --- /dev/null
>> +++ b/drivers/net/phy/xilinx_gmii2rgmii.c
>> @@ -0,0 +1,103 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Xilinx GMII2RGMII phy driver
>> + *
>> + * Copyright (C) 2018 Xilinx, Inc.
>> + */
>> +
>> +#include <dm.h>
>> +#include <phy.h>
>> +#include <config.h>
>> +#include <common.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +#define ZYNQ_GMII2RGMII_REG            0x10
>> +#define ZYNQ_GMII2RGMII_SPEED_MASK     (BMCR_SPEED1000 | BMCR_SPEED100)
>> +
>> +static int xilinxgmiitorgmii_config(struct phy_device *phydev)
>> +{
>> +       struct phy_device *ext_phydev = phydev->priv;
>> +
>> +       debug("%s\n", __func__);
>> +       if (ext_phydev->drv->config)
>> +               ext_phydev->drv->config(ext_phydev);
>> +
>> +       return 0;
>> +}
>> +
>> +static int xilinxgmiitorgmii_startup(struct phy_device *phydev)
>> +{
>> +       u16 val = 0;
>> +       struct phy_device *ext_phydev = phydev->priv;
>> +
>> +       debug("%s\n", __func__);
>> +       ext_phydev->dev = phydev->dev;
>> +       if (ext_phydev->drv->startup)
>> +               ext_phydev->drv->startup(ext_phydev);
>> +
>> +       val = phy_read(phydev, phydev->addr, ZYNQ_GMII2RGMII_REG);
> 
> This seems to imply that the bridge IP is actually on MDIO at a
> separate address from the real phy, but the same MDIO bus.

yes. They are both on the same bus and that's how it is designed. I was
looking in past if there is a code in u-boot for handling several phys
on the same bus and this is not in place.

That's why even cases like this
eth0 -> mdio -> phy_for_eth0, phy_for_eth1
eth1 without mdio bus

are not supported by u-boot. At least I didn't see that code for that
some months ago. Or is it supported now?


> 
>> +       val &= ~ZYNQ_GMII2RGMII_SPEED_MASK;
>> +
>> +       if (ext_phydev->speed == SPEED_1000)
>> +               val |= BMCR_SPEED1000;
>> +       else if (ext_phydev->speed == SPEED_100)
>> +               val |= BMCR_SPEED100;
>> +
>> +       phy_write(phydev, phydev->addr, ZYNQ_GMII2RGMII_REG, val |
>> +                 BMCR_FULLDPLX);
>> +
>> +       phydev->duplex = ext_phydev->duplex;
>> +       phydev->speed = ext_phydev->speed;
>> +       phydev->link = ext_phydev->link;
>> +
>> +       return 0;
>> +}
>> +
>> +static int xilinxgmiitorgmii_probe(struct phy_device *phydev)
>> +{
>> +       int ofnode = phydev->addr;
>> +       u32 phy_of_handle;
>> +       int ext_phyaddr = -1;
>> +
>> +       debug("%s\n", __func__);
>> +
>> +       /*
>> +        * Read the phy address again as the one we read in ethernet driver
>> +        * was overwritten for the purpose of storing the ofnode
>> +        */
>> +       phydev->addr = fdtdec_get_int(gd->fdt_blob, ofnode, "reg", -1);
> 
> Is this phyaddr made up? Or is this IP actually listening on a
> specific address on MDIO?

this IP is in xilinx catalog and you can setup whatever number you want.

> 
>> +       phy_of_handle = fdtdec_lookup_phandle(gd->fdt_blob, ofnode,
>> +                                             "phy-handle");
>> +       if (phy_of_handle > 0)
>> +               ext_phyaddr = fdtdec_get_int(gd->fdt_blob,
>> +                                            phy_of_handle,
>> +                                            "reg", -1);
> 
> I'd like to see a xilinx_gmii2rgmii.txt added to describe how the
> device tree is expected to be written. The one in Linux seems a bit
> incomplete, also. For instance, does the phy-handle in the ethmac node
> point to the bridge? Is it just missing? No mention.

I have sent you wiring for zc1275-revB

Here is that fragment. Phy in root points to bridge. Bridge points to
phy on the board.

+&gem1 {
+       status = "okay";
+       phy-mode = "gmii";
+       phy-handle = <&gmiitorgmii>;
+       phy: ethernet-phy at 0 {
+               reg = <0x0>;
+       };
+       gmiitorgmii: gmiitorgmii at 8 {
+               compatible = "xlnx,gmii-to-rgmii-1.0";
+               reg = <8>;
+               phy-handle = <&phy>;
+       };
+};
+

We can extend that binding in the kernel if it is not clear.
Just let me know.

> 
>> +       phydev->priv = phy_find_by_mask(phydev->bus,
>> +                                       1 << ext_phyaddr,
>> +                                       phydev->interface);
>> +
>> +       debug("%s, gmii2rgmmi:0x%x, extphy:0x%x\n", __func__, phydev->addr,
>> +             ext_phyaddr);
>> +
>> +       phydev->flags |= PHY_FLAG_BROKEN_RESET;
>> +
>> +       return 0;
>> +}
>> +
>> +static struct phy_driver gmii2rgmii_driver = {
>> +       .name = "XILINX GMII2RGMII",
>> +       .uid = PHY_GMII2RGMII_ID,
>> +       .mask = 0xffffffff,
>> +       .features = PHY_GBIT_FEATURES,
>> +       .probe = xilinxgmiitorgmii_probe,
>> +       .config = xilinxgmiitorgmii_config,
>> +       .startup = xilinxgmiitorgmii_startup,
>> +};
>> +
>> +int phy_xilinx_gmii2rgmii_init(void)
>> +{
>> +       phy_register(&gmii2rgmii_driver);
>> +
>> +       return 0;
>> +}
>> diff --git a/include/phy.h b/include/phy.h
>> index b86fdfb..0485701 100644
>> --- a/include/phy.h
>> +++ b/include/phy.h
>> @@ -17,6 +17,11 @@
>>  #include <phy_interface.h>
>>
>>  #define PHY_FIXED_ID           0xa5a55a5a
>> +/*
>> + * There is no actual id for this.
>> + * This is just a dummy id for gmii2rgmmi converter.
>> + */
>> +#define PHY_GMII2RGMII_ID      0x5a5a5a5a
>>
>>  #define PHY_MAX_ADDR 32
>>
>> @@ -240,6 +245,7 @@ int phy_vitesse_init(void);
>>  int phy_xilinx_init(void);
>>  int phy_mscc_init(void);
>>  int phy_fixed_init(void);
>> +int phy_xilinx_gmii2rgmii_init(void);
>>
>>  int board_phy_config(struct phy_device *phydev);
>>  int get_phy_id(struct mii_dev *bus, int addr, int devad, u32 *phy_id);
>> --
>> 2.7.4
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot


Thanks,
Michal

[U-Boot] [PATCH 2/2] net: phy: Add gmiitorgmii converter support

[Joe Hershberger]

On Thu, Jan 24, 2019 at 2:07 AM Michal Simek <michal.simek@xilinx.com> wrote:
>
> On 23. 01. 19 19:20, Joe Hershberger wrote:
> > On Tue, Nov 27, 2018 at 12:20 AM Siva Durga Prasad Paladugu
> > <siva.durga.paladugu@xilinx.com> wrote:
> >>
> >> This patch adds support for gmiitorgmii converter.
> >> This converter sits between the MAC and the external phy
> >> MAC <==> GMII2RGMII <==> RGMII_PHY.
> >> The ethernet driver probes this bridge and this bridge driver
> >> probes real phy driver and invokes the real phy functionalities
> >> as requested. This bridge just needs to be configured based on
> >> real phy negotiated speed and duplex.
> >>
> >> Signed-off-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >> ---
> >
> > FYI, NI doesn't use the Xilinx adapter, we have several internal
> > adapters... GMII to RGMII, RMII, and I believe GMII (passing through
> > EMIO).
>
> ok. How do you manage them?

They are managed by 2 digital lines on EMIO
>
> >
> >>  drivers/net/phy/Kconfig             |   7 +++
> >>  drivers/net/phy/Makefile            |   1 +
> >>  drivers/net/phy/phy.c               |  41 ++++++++++++++
> >>  drivers/net/phy/xilinx_gmii2rgmii.c | 103 ++++++++++++++++++++++++++++++++++++
> >>  include/phy.h                       |   6 +++
> >>  5 files changed, 158 insertions(+)
> >>  create mode 100644 drivers/net/phy/xilinx_gmii2rgmii.c
> >>
> >> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> >> index 3dc0822..a68e167 100644
> >> --- a/drivers/net/phy/Kconfig
> >> +++ b/drivers/net/phy/Kconfig
> >> @@ -217,6 +217,13 @@ config PHY_VITESSE
> >>  config PHY_XILINX
> >>         bool "Xilinx Ethernet PHYs support"
> >>
> >> +config PHY_XILINX_GMII2RGMII
> >> +       bool "Xilinx GMII to RGMII Ethernet PHYs support"
> >> +       help
> >> +         This adds support for Xilinx GMII to RGMII IP core. This IP acts
> >> +         as bridge between MAC connected over GMII and external phy that
> >> +         is connected over RGMII interface.
> >> +
> >>  config PHY_FIXED
> >>         bool "Fixed-Link PHY"
> >>         depends on DM_ETH
> >> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> >> index 555da83..76b6197 100644
> >> --- a/drivers/net/phy/Makefile
> >> +++ b/drivers/net/phy/Makefile
> >> @@ -27,6 +27,7 @@ obj-$(CONFIG_PHY_SMSC) += smsc.o
> >>  obj-$(CONFIG_PHY_TERANETICS) += teranetics.o
> >>  obj-$(CONFIG_PHY_TI) += ti.o
> >>  obj-$(CONFIG_PHY_XILINX) += xilinx_phy.o
> >> +obj-$(CONFIG_PHY_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o
> >>  obj-$(CONFIG_PHY_VITESSE) += vitesse.o
> >>  obj-$(CONFIG_PHY_MSCC) += mscc.o
> >>  obj-$(CONFIG_PHY_FIXED) += fixed.o
> >> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> >> index 3cb2785..d02c4d8 100644
> >> --- a/drivers/net/phy/phy.c
> >> +++ b/drivers/net/phy/phy.c
> >> @@ -528,6 +528,9 @@ int phy_init(void)
> >>  #ifdef CONFIG_PHY_FIXED
> >>         phy_fixed_init();
> >>  #endif
> >> +#ifdef CONFIG_PHY_XILINX_GMII2RGMII
> >> +       phy_xilinx_gmii2rgmii_init();
> >> +#endif
> >
> > I'm a bit surprised this needs to act as a unique type of device and
> > not a normal phy driver? When the phy_id is read, what is returned?
> > The real phy? Nothing? You change the speed by accessing MDIO?
>
> It is connected on mdio bus but unfortunately there is no phy_id to read
> to be able to use that.
>
> https://www.xilinx.com/support/documentation/ip_documentation/gmii_to_rgmii/v4_0/pg160-gmii-to-rgmii.pdf
> page 21

That's very unfortunate. Is it not something you can have added to the IP?

> >
> >>         return 0;
> >>  }
> >>
> >> @@ -875,6 +878,41 @@ void phy_connect_dev(struct phy_device *phydev, struct eth_device *dev)
> >>         debug("%s connected to %s\n", dev->name, phydev->drv->name);
> >>  }
> >>
> >> +#ifdef CONFIG_PHY_XILINX_GMII2RGMII
> >> +#ifdef CONFIG_DM_ETH
> >> +static struct phy_device *phy_connect_gmii2rgmii(struct mii_dev *bus,
> >> +                                                struct udevice *dev,
> >> +                                                phy_interface_t interface)
> >> +#else
> >> +static struct phy_device *phy_connect_gmii2rgmii(struct mii_dev *bus,
> >> +                                                struct eth_device *dev,
> >> +                                                phy_interface_t interface)
> >> +#endif
> >> +{
> >> +       struct phy_device *phydev = NULL;
> >> +       int sn = dev_of_offset(dev);
> >> +       int off;
> >> +
> >> +       while (sn > 0) {
> >> +               off = fdt_node_offset_by_compatible(gd->fdt_blob, sn,
> >> +                                                   "xlnx,gmii-to-rgmii-1.0");
> >
> > This seems to be searching under the eth dev, but at least in the
> > "linux/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt"
> > docs, this is found under the mdio node. How is this supposed to
> > handle more than one bridge?
>
> I haven't seen design with more than one bridge. It is really just one
> brigde which goes to regular phy.

A number of our boards have more than one, but they are not yet ported
to modern U-Boot.

> Regarding mdio and DT binding. I haven't seen anywhere written if phys
> should be inside mdio node or they should be just child in controller node.

I think the inconsistency is a large part of why we can't have a
generic MDIO or ENET PHY driver class enumeration in U-Boot.

> I see some examples where mdio node needs to contain compatibility
> string and there is separate driver for that.
>
> In dt spec nothing is written about it and also I haven't seen yaml
> checking from Rob.
>
> The key point for example is to show how bridge needs description for
> phy on the board.

Sure.

> >> +               if (off > 0) {
> >> +                       phydev = phy_device_create(bus,
> >> +                                                  off, PHY_GMII2RGMII_ID,
> >> +                                                  interface);
> >> +                       break;
> >> +               }
> >> +               if (off == -FDT_ERR_NOTFOUND)
> >> +                       sn = fdt_first_subnode(gd->fdt_blob, sn);
> >> +               else
> >> +                       printf("%s: Error finding compat string:%d\n",
> >> +                              __func__, off);
> >> +       }
> >> +
> >> +       return phydev;
> >> +}
> >> +#endif
> >> +
> >>  #ifdef CONFIG_PHY_FIXED
> >>  #ifdef CONFIG_DM_ETH
> >>  static struct phy_device *phy_connect_fixed(struct mii_dev *bus,
> >> @@ -920,6 +958,9 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr,
> >>  #ifdef CONFIG_PHY_FIXED
> >>         phydev = phy_connect_fixed(bus, dev, interface);
> >>  #endif
> >> +#ifdef CONFIG_PHY_XILINX_GMII2RGMII
> >> +       phydev = phy_connect_gmii2rgmii(bus, dev, interface);
> >> +#endif
> >>
> >>         if (!phydev)
> >>                 phydev = phy_find_by_mask(bus, 1 << addr, interface);
> >> diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c b/drivers/net/phy/xilinx_gmii2rgmii.c
> >> new file mode 100644
> >> index 0000000..aa4ce86
> >> --- /dev/null
> >> +++ b/drivers/net/phy/xilinx_gmii2rgmii.c
> >> @@ -0,0 +1,103 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Xilinx GMII2RGMII phy driver
> >> + *
> >> + * Copyright (C) 2018 Xilinx, Inc.
> >> + */
> >> +
> >> +#include <dm.h>
> >> +#include <phy.h>
> >> +#include <config.h>
> >> +#include <common.h>
> >> +
> >> +DECLARE_GLOBAL_DATA_PTR;
> >> +
> >> +#define ZYNQ_GMII2RGMII_REG            0x10
> >> +#define ZYNQ_GMII2RGMII_SPEED_MASK     (BMCR_SPEED1000 | BMCR_SPEED100)
> >> +
> >> +static int xilinxgmiitorgmii_config(struct phy_device *phydev)
> >> +{
> >> +       struct phy_device *ext_phydev = phydev->priv;
> >> +
> >> +       debug("%s\n", __func__);
> >> +       if (ext_phydev->drv->config)
> >> +               ext_phydev->drv->config(ext_phydev);
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static int xilinxgmiitorgmii_startup(struct phy_device *phydev)
> >> +{
> >> +       u16 val = 0;
> >> +       struct phy_device *ext_phydev = phydev->priv;
> >> +
> >> +       debug("%s\n", __func__);
> >> +       ext_phydev->dev = phydev->dev;
> >> +       if (ext_phydev->drv->startup)
> >> +               ext_phydev->drv->startup(ext_phydev);
> >> +
> >> +       val = phy_read(phydev, phydev->addr, ZYNQ_GMII2RGMII_REG);
> >
> > This seems to imply that the bridge IP is actually on MDIO at a
> > separate address from the real phy, but the same MDIO bus.
>
> yes. They are both on the same bus and that's how it is designed. I was
> looking in past if there is a code in u-boot for handling several phys
> on the same bus and this is not in place.
>
> That's why even cases like this
> eth0 -> mdio -> phy_for_eth0, phy_for_eth1
> eth1 without mdio bus

We have exactly this example on one of our Zynq 7000 boards, and I
expect to be able to describe that.

> are not supported by u-boot. At least I didn't see that code for that
> some months ago. Or is it supported now?

I have not yet tried it to know what is lacking to support it.

> >
> >> +       val &= ~ZYNQ_GMII2RGMII_SPEED_MASK;
> >> +
> >> +       if (ext_phydev->speed == SPEED_1000)
> >> +               val |= BMCR_SPEED1000;
> >> +       else if (ext_phydev->speed == SPEED_100)
> >> +               val |= BMCR_SPEED100;
> >> +
> >> +       phy_write(phydev, phydev->addr, ZYNQ_GMII2RGMII_REG, val |
> >> +                 BMCR_FULLDPLX);
> >> +
> >> +       phydev->duplex = ext_phydev->duplex;
> >> +       phydev->speed = ext_phydev->speed;
> >> +       phydev->link = ext_phydev->link;
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static int xilinxgmiitorgmii_probe(struct phy_device *phydev)
> >> +{
> >> +       int ofnode = phydev->addr;
> >> +       u32 phy_of_handle;
> >> +       int ext_phyaddr = -1;
> >> +
> >> +       debug("%s\n", __func__);
> >> +
> >> +       /*
> >> +        * Read the phy address again as the one we read in ethernet driver
> >> +        * was overwritten for the purpose of storing the ofnode
> >> +        */
> >> +       phydev->addr = fdtdec_get_int(gd->fdt_blob, ofnode, "reg", -1);
> >
> > Is this phyaddr made up? Or is this IP actually listening on a
> > specific address on MDIO?
>
> this IP is in xilinx catalog and you can setup whatever number you want.

OK, so the IP configuration and the DTS just need to match.

> >
> >> +       phy_of_handle = fdtdec_lookup_phandle(gd->fdt_blob, ofnode,
> >> +                                             "phy-handle");
> >> +       if (phy_of_handle > 0)
> >> +               ext_phyaddr = fdtdec_get_int(gd->fdt_blob,
> >> +                                            phy_of_handle,
> >> +                                            "reg", -1);
> >
> > I'd like to see a xilinx_gmii2rgmii.txt added to describe how the
> > device tree is expected to be written. The one in Linux seems a bit
> > incomplete, also. For instance, does the phy-handle in the ethmac node
> > point to the bridge? Is it just missing? No mention.
>
> I have sent you wiring for zc1275-revB
>
> Here is that fragment. Phy in root points to bridge. Bridge points to
> phy on the board.
>
> +&gem1 {
> +       status = "okay";
> +       phy-mode = "gmii";
> +       phy-handle = <&gmiitorgmii>;
> +       phy: ethernet-phy at 0 {
> +               reg = <0x0>;
> +       };
> +       gmiitorgmii: gmiitorgmii at 8 {
> +               compatible = "xlnx,gmii-to-rgmii-1.0";
> +               reg = <8>;
> +               phy-handle = <&phy>;
> +       };
> +};
> +

That looks reasonable.

> We can extend that binding in the kernel if it is not clear.
> Just let me know.

It would be good to have the binding include the relationship to the
gem as well.

> >
> >> +       phydev->priv = phy_find_by_mask(phydev->bus,
> >> +                                       1 << ext_phyaddr,
> >> +                                       phydev->interface);
> >> +
> >> +       debug("%s, gmii2rgmmi:0x%x, extphy:0x%x\n", __func__, phydev->addr,
> >> +             ext_phyaddr);
> >> +
> >> +       phydev->flags |= PHY_FLAG_BROKEN_RESET;
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static struct phy_driver gmii2rgmii_driver = {
> >> +       .name = "XILINX GMII2RGMII",
> >> +       .uid = PHY_GMII2RGMII_ID,
> >> +       .mask = 0xffffffff,
> >> +       .features = PHY_GBIT_FEATURES,
> >> +       .probe = xilinxgmiitorgmii_probe,
> >> +       .config = xilinxgmiitorgmii_config,
> >> +       .startup = xilinxgmiitorgmii_startup,
> >> +};
> >> +
> >> +int phy_xilinx_gmii2rgmii_init(void)
> >> +{
> >> +       phy_register(&gmii2rgmii_driver);
> >> +
> >> +       return 0;
> >> +}
> >> diff --git a/include/phy.h b/include/phy.h
> >> index b86fdfb..0485701 100644
> >> --- a/include/phy.h
> >> +++ b/include/phy.h
> >> @@ -17,6 +17,11 @@
> >>  #include <phy_interface.h>
> >>
> >>  #define PHY_FIXED_ID           0xa5a55a5a
> >> +/*
> >> + * There is no actual id for this.
> >> + * This is just a dummy id for gmii2rgmmi converter.
> >> + */
> >> +#define PHY_GMII2RGMII_ID      0x5a5a5a5a
> >>
> >>  #define PHY_MAX_ADDR 32
> >>
> >> @@ -240,6 +245,7 @@ int phy_vitesse_init(void);
> >>  int phy_xilinx_init(void);
> >>  int phy_mscc_init(void);
> >>  int phy_fixed_init(void);
> >> +int phy_xilinx_gmii2rgmii_init(void);
> >>
> >>  int board_phy_config(struct phy_device *phydev);
> >>  int get_phy_id(struct mii_dev *bus, int addr, int devad, u32 *phy_id);
> >> --
> >> 2.7.4
> >>
> >> _______________________________________________
> >> U-Boot mailing list
> >> U-Boot at lists.denx.de
> >> https://lists.denx.de/listinfo/u-boot
>
>
> Thanks,
> Michal
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 2/2] net: phy: Add gmiitorgmii converter support

[Michal Simek]

On 24. 01. 19 18:24, Joe Hershberger wrote:
> On Thu, Jan 24, 2019 at 2:07 AM Michal Simek <michal.simek@xilinx.com> wrote:
>>
>> On 23. 01. 19 19:20, Joe Hershberger wrote:
>>> On Tue, Nov 27, 2018 at 12:20 AM Siva Durga Prasad Paladugu
>>> <siva.durga.paladugu@xilinx.com> wrote:
>>>>
>>>> This patch adds support for gmiitorgmii converter.
>>>> This converter sits between the MAC and the external phy
>>>> MAC <==> GMII2RGMII <==> RGMII_PHY.
>>>> The ethernet driver probes this bridge and this bridge driver
>>>> probes real phy driver and invokes the real phy functionalities
>>>> as requested. This bridge just needs to be configured based on
>>>> real phy negotiated speed and duplex.
>>>>
>>>> Signed-off-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>> ---
>>>
>>> FYI, NI doesn't use the Xilinx adapter, we have several internal
>>> adapters... GMII to RGMII, RMII, and I believe GMII (passing through
>>> EMIO).
>>
>> ok. How do you manage them?
> 
> They are managed by 2 digital lines on EMIO

ok.

>>
>>>
>>>>  drivers/net/phy/Kconfig             |   7 +++
>>>>  drivers/net/phy/Makefile            |   1 +
>>>>  drivers/net/phy/phy.c               |  41 ++++++++++++++
>>>>  drivers/net/phy/xilinx_gmii2rgmii.c | 103 ++++++++++++++++++++++++++++++++++++
>>>>  include/phy.h                       |   6 +++
>>>>  5 files changed, 158 insertions(+)
>>>>  create mode 100644 drivers/net/phy/xilinx_gmii2rgmii.c
>>>>
>>>> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
>>>> index 3dc0822..a68e167 100644
>>>> --- a/drivers/net/phy/Kconfig
>>>> +++ b/drivers/net/phy/Kconfig
>>>> @@ -217,6 +217,13 @@ config PHY_VITESSE
>>>>  config PHY_XILINX
>>>>         bool "Xilinx Ethernet PHYs support"
>>>>
>>>> +config PHY_XILINX_GMII2RGMII
>>>> +       bool "Xilinx GMII to RGMII Ethernet PHYs support"
>>>> +       help
>>>> +         This adds support for Xilinx GMII to RGMII IP core. This IP acts
>>>> +         as bridge between MAC connected over GMII and external phy that
>>>> +         is connected over RGMII interface.
>>>> +
>>>>  config PHY_FIXED
>>>>         bool "Fixed-Link PHY"
>>>>         depends on DM_ETH
>>>> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
>>>> index 555da83..76b6197 100644
>>>> --- a/drivers/net/phy/Makefile
>>>> +++ b/drivers/net/phy/Makefile
>>>> @@ -27,6 +27,7 @@ obj-$(CONFIG_PHY_SMSC) += smsc.o
>>>>  obj-$(CONFIG_PHY_TERANETICS) += teranetics.o
>>>>  obj-$(CONFIG_PHY_TI) += ti.o
>>>>  obj-$(CONFIG_PHY_XILINX) += xilinx_phy.o
>>>> +obj-$(CONFIG_PHY_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o
>>>>  obj-$(CONFIG_PHY_VITESSE) += vitesse.o
>>>>  obj-$(CONFIG_PHY_MSCC) += mscc.o
>>>>  obj-$(CONFIG_PHY_FIXED) += fixed.o
>>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>>> index 3cb2785..d02c4d8 100644
>>>> --- a/drivers/net/phy/phy.c
>>>> +++ b/drivers/net/phy/phy.c
>>>> @@ -528,6 +528,9 @@ int phy_init(void)
>>>>  #ifdef CONFIG_PHY_FIXED
>>>>         phy_fixed_init();
>>>>  #endif
>>>> +#ifdef CONFIG_PHY_XILINX_GMII2RGMII
>>>> +       phy_xilinx_gmii2rgmii_init();
>>>> +#endif
>>>
>>> I'm a bit surprised this needs to act as a unique type of device and
>>> not a normal phy driver? When the phy_id is read, what is returned?
>>> The real phy? Nothing? You change the speed by accessing MDIO?
>>
>> It is connected on mdio bus but unfortunately there is no phy_id to read
>> to be able to use that.
>>
>> https://www.xilinx.com/support/documentation/ip_documentation/gmii_to_rgmii/v4_0/pg160-gmii-to-rgmii.pdf
>> page 21
> 
> That's very unfortunate. Is it not something you can have added to the IP?

This is official IP in IP catalog and there is no way to update current
version.


>>>
>>>>         return 0;
>>>>  }
>>>>
>>>> @@ -875,6 +878,41 @@ void phy_connect_dev(struct phy_device *phydev, struct eth_device *dev)
>>>>         debug("%s connected to %s\n", dev->name, phydev->drv->name);
>>>>  }
>>>>
>>>> +#ifdef CONFIG_PHY_XILINX_GMII2RGMII
>>>> +#ifdef CONFIG_DM_ETH
>>>> +static struct phy_device *phy_connect_gmii2rgmii(struct mii_dev *bus,
>>>> +                                                struct udevice *dev,
>>>> +                                                phy_interface_t interface)
>>>> +#else
>>>> +static struct phy_device *phy_connect_gmii2rgmii(struct mii_dev *bus,
>>>> +                                                struct eth_device *dev,
>>>> +                                                phy_interface_t interface)
>>>> +#endif
>>>> +{
>>>> +       struct phy_device *phydev = NULL;
>>>> +       int sn = dev_of_offset(dev);
>>>> +       int off;
>>>> +
>>>> +       while (sn > 0) {
>>>> +               off = fdt_node_offset_by_compatible(gd->fdt_blob, sn,
>>>> +                                                   "xlnx,gmii-to-rgmii-1.0");
>>>
>>> This seems to be searching under the eth dev, but at least in the
>>> "linux/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt"
>>> docs, this is found under the mdio node. How is this supposed to
>>> handle more than one bridge?
>>
>> I haven't seen design with more than one bridge. It is really just one
>> brigde which goes to regular phy.
> 
> A number of our boards have more than one, but they are not yet ported
> to modern U-Boot.

ok.


>> Regarding mdio and DT binding. I haven't seen anywhere written if phys
>> should be inside mdio node or they should be just child in controller node.
> 
> I think the inconsistency is a large part of why we can't have a
> generic MDIO or ENET PHY driver class enumeration in U-Boot.

There was one attempt in past to support multiple phys on gem. I created
one patch in past to support case where you have all 4 gems on zynqmp
enabled and because there is only single MDIO line coming directy out of
PS they have to share it.
This works fine in Linux but it doesn't work with current u-boot
infrastructure.


>> I see some examples where mdio node needs to contain compatibility
>> string and there is separate driver for that.
>>
>> In dt spec nothing is written about it and also I haven't seen yaml
>> checking from Rob.
>>
>> The key point for example is to show how bridge needs description for
>> phy on the board.
> 
> Sure.
> 
>>>> +               if (off > 0) {
>>>> +                       phydev = phy_device_create(bus,
>>>> +                                                  off, PHY_GMII2RGMII_ID,
>>>> +                                                  interface);
>>>> +                       break;
>>>> +               }
>>>> +               if (off == -FDT_ERR_NOTFOUND)
>>>> +                       sn = fdt_first_subnode(gd->fdt_blob, sn);
>>>> +               else
>>>> +                       printf("%s: Error finding compat string:%d\n",
>>>> +                              __func__, off);
>>>> +       }
>>>> +
>>>> +       return phydev;
>>>> +}
>>>> +#endif
>>>> +
>>>>  #ifdef CONFIG_PHY_FIXED
>>>>  #ifdef CONFIG_DM_ETH
>>>>  static struct phy_device *phy_connect_fixed(struct mii_dev *bus,
>>>> @@ -920,6 +958,9 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr,
>>>>  #ifdef CONFIG_PHY_FIXED
>>>>         phydev = phy_connect_fixed(bus, dev, interface);
>>>>  #endif
>>>> +#ifdef CONFIG_PHY_XILINX_GMII2RGMII
>>>> +       phydev = phy_connect_gmii2rgmii(bus, dev, interface);
>>>> +#endif
>>>>
>>>>         if (!phydev)
>>>>                 phydev = phy_find_by_mask(bus, 1 << addr, interface);
>>>> diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c b/drivers/net/phy/xilinx_gmii2rgmii.c
>>>> new file mode 100644
>>>> index 0000000..aa4ce86
>>>> --- /dev/null
>>>> +++ b/drivers/net/phy/xilinx_gmii2rgmii.c
>>>> @@ -0,0 +1,103 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * Xilinx GMII2RGMII phy driver
>>>> + *
>>>> + * Copyright (C) 2018 Xilinx, Inc.
>>>> + */
>>>> +
>>>> +#include <dm.h>
>>>> +#include <phy.h>
>>>> +#include <config.h>
>>>> +#include <common.h>
>>>> +
>>>> +DECLARE_GLOBAL_DATA_PTR;
>>>> +
>>>> +#define ZYNQ_GMII2RGMII_REG            0x10
>>>> +#define ZYNQ_GMII2RGMII_SPEED_MASK     (BMCR_SPEED1000 | BMCR_SPEED100)
>>>> +
>>>> +static int xilinxgmiitorgmii_config(struct phy_device *phydev)
>>>> +{
>>>> +       struct phy_device *ext_phydev = phydev->priv;
>>>> +
>>>> +       debug("%s\n", __func__);
>>>> +       if (ext_phydev->drv->config)
>>>> +               ext_phydev->drv->config(ext_phydev);
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static int xilinxgmiitorgmii_startup(struct phy_device *phydev)
>>>> +{
>>>> +       u16 val = 0;
>>>> +       struct phy_device *ext_phydev = phydev->priv;
>>>> +
>>>> +       debug("%s\n", __func__);
>>>> +       ext_phydev->dev = phydev->dev;
>>>> +       if (ext_phydev->drv->startup)
>>>> +               ext_phydev->drv->startup(ext_phydev);
>>>> +
>>>> +       val = phy_read(phydev, phydev->addr, ZYNQ_GMII2RGMII_REG);
>>>
>>> This seems to imply that the bridge IP is actually on MDIO at a
>>> separate address from the real phy, but the same MDIO bus.
>>
>> yes. They are both on the same bus and that's how it is designed. I was
>> looking in past if there is a code in u-boot for handling several phys
>> on the same bus and this is not in place.
>>
>> That's why even cases like this
>> eth0 -> mdio -> phy_for_eth0, phy_for_eth1
>> eth1 without mdio bus
> 
> We have exactly this example on one of our Zynq 7000 boards, and I
> expect to be able to describe that.

It is not a problem to describe it. In Linux is already handled properly
but not in u-boot.


> 
>> are not supported by u-boot. At least I didn't see that code for that
>> some months ago. Or is it supported now?
> 
> I have not yet tried it to know what is lacking to support it.

If you have time definitely please try it. Normally if you have multiple
ethernet controllers you don't want to use all of them for booting. You
choose one with attached phy and all is good.

>>>
>>>> +       val &= ~ZYNQ_GMII2RGMII_SPEED_MASK;
>>>> +
>>>> +       if (ext_phydev->speed == SPEED_1000)
>>>> +               val |= BMCR_SPEED1000;
>>>> +       else if (ext_phydev->speed == SPEED_100)
>>>> +               val |= BMCR_SPEED100;
>>>> +
>>>> +       phy_write(phydev, phydev->addr, ZYNQ_GMII2RGMII_REG, val |
>>>> +                 BMCR_FULLDPLX);
>>>> +
>>>> +       phydev->duplex = ext_phydev->duplex;
>>>> +       phydev->speed = ext_phydev->speed;
>>>> +       phydev->link = ext_phydev->link;
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static int xilinxgmiitorgmii_probe(struct phy_device *phydev)
>>>> +{
>>>> +       int ofnode = phydev->addr;
>>>> +       u32 phy_of_handle;
>>>> +       int ext_phyaddr = -1;
>>>> +
>>>> +       debug("%s\n", __func__);
>>>> +
>>>> +       /*
>>>> +        * Read the phy address again as the one we read in ethernet driver
>>>> +        * was overwritten for the purpose of storing the ofnode
>>>> +        */
>>>> +       phydev->addr = fdtdec_get_int(gd->fdt_blob, ofnode, "reg", -1);
>>>
>>> Is this phyaddr made up? Or is this IP actually listening on a
>>> specific address on MDIO?
>>
>> this IP is in xilinx catalog and you can setup whatever number you want.
> 
> OK, so the IP configuration and the DTS just need to match.

yep.


>>>
>>>> +       phy_of_handle = fdtdec_lookup_phandle(gd->fdt_blob, ofnode,
>>>> +                                             "phy-handle");
>>>> +       if (phy_of_handle > 0)
>>>> +               ext_phyaddr = fdtdec_get_int(gd->fdt_blob,
>>>> +                                            phy_of_handle,
>>>> +                                            "reg", -1);
>>>
>>> I'd like to see a xilinx_gmii2rgmii.txt added to describe how the
>>> device tree is expected to be written. The one in Linux seems a bit
>>> incomplete, also. For instance, does the phy-handle in the ethmac node
>>> point to the bridge? Is it just missing? No mention.
>>
>> I have sent you wiring for zc1275-revB
>>
>> Here is that fragment. Phy in root points to bridge. Bridge points to
>> phy on the board.
>>
>> +&gem1 {
>> +       status = "okay";
>> +       phy-mode = "gmii";
>> +       phy-handle = <&gmiitorgmii>;
>> +       phy: ethernet-phy at 0 {
>> +               reg = <0x0>;
>> +       };
>> +       gmiitorgmii: gmiitorgmii at 8 {
>> +               compatible = "xlnx,gmii-to-rgmii-1.0";
>> +               reg = <8>;
>> +               phy-handle = <&phy>;
>> +       };
>> +};
>> +
> 
> That looks reasonable.
> 
>> We can extend that binding in the kernel if it is not clear.
>> Just let me know.
> 
> It would be good to have the binding include the relationship to the
> gem as well.

This should work in the same way with other ethernet controllers. I
haven't tried that myself but I can't see an issue if you connect this
with others ethernet controllers. You point to bridge and bridge points
to phy in the same way.

Thanks,
Michal

[U-Boot] [PATCH 1/2] net: phy: Move fixed link code to separate routine

[Michal Simek]

Hi Joe,

On 27. 11. 18 7:19, Siva Durga Prasad Paladugu wrote:
> This patch moves fixed-link functionality code to a separate
> routine inorder to make it more modular and cleaner.
> 
> Signed-off-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>  drivers/net/phy/phy.c | 34 +++++++++++++++++++++++++++-------
>  1 file changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index e837eb7..3cb2785 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -875,18 +875,18 @@ void phy_connect_dev(struct phy_device *phydev, struct eth_device *dev)
>  	debug("%s connected to %s\n", dev->name, phydev->drv->name);
>  }
>  
> +#ifdef CONFIG_PHY_FIXED
>  #ifdef CONFIG_DM_ETH
> -struct phy_device *phy_connect(struct mii_dev *bus, int addr,
> -			       struct udevice *dev,
> -			       phy_interface_t interface)
> +static struct phy_device *phy_connect_fixed(struct mii_dev *bus,
> +					    struct udevice *dev,
> +					    phy_interface_t interface)
>  #else
> -struct phy_device *phy_connect(struct mii_dev *bus, int addr,
> -			       struct eth_device *dev,
> -			       phy_interface_t interface)
> +static struct phy_device *phy_connect_fixed(struct mii_dev *bus,
> +					    struct eth_device *dev,
> +					    phy_interface_t interface)
>  #endif
>  {
>  	struct phy_device *phydev = NULL;
> -#ifdef CONFIG_PHY_FIXED
>  	int sn;
>  	const char *name;
>  
> @@ -900,7 +900,27 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr,
>  		}
>  		sn = fdt_next_subnode(gd->fdt_blob, sn);
>  	}
> +
> +	return phydev;
> +}
>  #endif
> +
> +#ifdef CONFIG_DM_ETH
> +struct phy_device *phy_connect(struct mii_dev *bus, int addr,
> +			       struct udevice *dev,
> +			       phy_interface_t interface)
> +#else
> +struct phy_device *phy_connect(struct mii_dev *bus, int addr,
> +			       struct eth_device *dev,
> +			       phy_interface_t interface)
> +#endif
> +{
> +	struct phy_device *phydev = NULL;
> +
> +#ifdef CONFIG_PHY_FIXED
> +	phydev = phy_connect_fixed(bus, dev, interface);
> +#endif
> +
>  	if (!phydev)
>  		phydev = phy_find_by_mask(bus, 1 << addr, interface);
>  
> 

can you please take a look at this?
Unfortunately there are no IDs to check that's why we had to do it in
this way. I would like to get this to v2019.01.

Thanks,
Michal

[U-Boot] [PATCH 1/2] net: phy: Move fixed link code to separate routine

[Joe Hershberger]

On Tue, Nov 27, 2018 at 12:20 AM Siva Durga Prasad Paladugu
<siva.durga.paladugu@xilinx.com> wrote:
>
> This patch moves fixed-link functionality code to a separate
> routine inorder to make it more modular and cleaner.
>
> Signed-off-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>