On 23. 01. 19 19:20, Joe Hershberger wrote: > On Tue, Nov 27, 2018 at 12:20 AM Siva Durga Prasad Paladugu > <siva.durga.palad...@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.palad...@xilinx.com> >> Signed-off-by: Michal Simek <michal.si...@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@0 { + reg = <0x0>; + }; + gmiitorgmii: gmiitorgmii@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@lists.denx.de >> https://lists.denx.de/listinfo/u-boot Thanks, Michal _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot