[U-Boot] [RFC PATCH] net: gem: Add support for more PHYs on MDIO bus
Find out MDIO bus and enable MDIO access to it if this is done via different controller.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
Hi Joe,
this is the code I have hacked a year ago for ZynqMP where we can have configuration that 4 gems are enabled but they share the same MDIO bus which can be assigned to only gem. Normally recommendation is that you should assign it to IP which is required for boot and this is suitable I would say for almost everybody. But for testing purpose it will be good to support sharing mdio bus between others IPs. This hack is "enabling" this for others gem but not across different ethernet drivers. And my question is if there is any solution which can be used now for handling it. Or even this should work even now but we do something wrong.
I am refreshing this topic based on communication with Tomasz Gorochowik when he wanted to separate mdio part but it is not compatible with solution used in Linux which is pattern we should follow.
Note: I tested it only on zcu102 with the standard configuration but Tomasz confirmed that this is still working with shared mdio bus.
Thanks, Michal
--- drivers/net/zynq_gem.c | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 2cc49bca922a..33245ec36e67 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -177,6 +177,7 @@ struct zynq_gem_priv { int phyaddr; int init; struct zynq_gem_regs *iobase; + struct zynq_gem_regs *mdiobase; phy_interface_t interface; struct phy_device *phydev; int phy_of_handle; @@ -189,7 +190,7 @@ static u32 phy_setup_op(struct zynq_gem_priv *priv, u32 phy_addr, u32 regnum, u32 op, u16 *data) { u32 mgtcr; - struct zynq_gem_regs *regs = priv->iobase; + struct zynq_gem_regs *regs = priv->mdiobase; int err;
err = wait_for_bit_le32(®s->nwsr, ZYNQ_GEM_NWSR_MDIOIDLE_MASK, @@ -314,7 +315,7 @@ static int zynq_phy_init(struct udevice *dev) { int ret; struct zynq_gem_priv *priv = dev_get_priv(dev); - struct zynq_gem_regs *regs = priv->iobase; + struct zynq_gem_regs *regs_mdio = priv->mdiobase; const u32 supported = SUPPORTED_10baseT_Half | SUPPORTED_10baseT_Full | SUPPORTED_100baseT_Half | @@ -323,7 +324,7 @@ static int zynq_phy_init(struct udevice *dev) SUPPORTED_1000baseT_Full;
/* Enable only MDIO bus */ - writel(ZYNQ_GEM_NWCTRL_MDEN_MASK, ®s->nwctrl); + writel(ZYNQ_GEM_NWCTRL_MDEN_MASK, ®s_mdio->nwctrl);
if (priv->interface != PHY_INTERFACE_MODE_SGMII) { ret = phy_detection(dev); @@ -355,6 +356,7 @@ static int zynq_gem_init(struct udevice *dev) unsigned long clk_rate = 0; struct zynq_gem_priv *priv = dev_get_priv(dev); struct zynq_gem_regs *regs = priv->iobase; + struct zynq_gem_regs *regs_mdio = priv->mdiobase; struct emac_bd *dummy_tx_bd = &priv->tx_bd[TX_FREE_DESC]; struct emac_bd *dummy_rx_bd = &priv->tx_bd[TX_FREE_DESC + 2];
@@ -397,7 +399,7 @@ static int zynq_gem_init(struct udevice *dev) writel(ZYNQ_GEM_DMACR_INIT, ®s->dmacr);
/* Setup for Network Control register, MDIO, Rx and Tx enable */ - setbits_le32(®s->nwctrl, ZYNQ_GEM_NWCTRL_MDEN_MASK); + setbits_le32(®s_mdio->nwctrl, ZYNQ_GEM_NWCTRL_MDEN_MASK);
/* Disable the second priority queue */ dummy_tx_bd->addr = 0; @@ -623,6 +625,7 @@ static int zynq_gem_probe(struct udevice *dev) void *bd_space; struct zynq_gem_priv *priv = dev_get_priv(dev); int ret; + char name[MDIO_NAME_LEN];
/* Align rxbuffers to ARCH_DMA_MINALIGN */ priv->rxbuffers = memalign(ARCH_DMA_MINALIGN, RX_BUF * PKTSIZE_ALIGN); @@ -648,6 +651,9 @@ static int zynq_gem_probe(struct udevice *dev) priv->bus->write = zynq_gem_miiphy_write; priv->bus->priv = priv;
+ snprintf(name, MDIO_NAME_LEN, "gem%lx", (ulong)priv->iobase); + strncpy(priv->bus->name, name, MDIO_NAME_LEN); + ret = mdio_register_seq(priv->bus, dev->seq); if (ret) return ret; @@ -682,6 +688,8 @@ static int zynq_gem_ofdata_to_platdata(struct udevice *dev) struct zynq_gem_priv *priv = dev_get_priv(dev); int node = dev_of_offset(dev); const char *phy_mode; + fdt_addr_t addr; + int parent;
pdata->iobase = (phys_addr_t)devfdt_get_addr(dev); priv->iobase = (struct zynq_gem_regs *)pdata->iobase; @@ -690,10 +698,24 @@ static int zynq_gem_ofdata_to_platdata(struct udevice *dev)
priv->phy_of_handle = fdtdec_lookup_phandle(gd->fdt_blob, node, "phy-handle"); - if (priv->phy_of_handle > 0) + if (priv->phy_of_handle > 0) { priv->phyaddr = fdtdec_get_int(gd->fdt_blob, priv->phy_of_handle, "reg", -1);
+ parent = fdt_parent_offset(gd->fdt_blob, priv->phy_of_handle); + addr = fdtdec_get_addr(gd->fdt_blob, parent, "reg"); + + if (addr == FDT_ADDR_T_NONE) { + printf("MDIO bus not found %x %x, %x\n", + node, priv->phy_of_handle, parent); + return -ENODEV; + } + + priv->mdiobase = (struct zynq_gem_regs *)addr; + } else { + priv->mdiobase = priv->iobase; + } + phy_mode = fdt_getprop(gd->fdt_blob, node, "phy-mode", NULL); if (phy_mode) pdata->phy_interface = phy_get_interface_by_name(phy_mode); @@ -706,8 +728,9 @@ static int zynq_gem_ofdata_to_platdata(struct udevice *dev) priv->int_pcs = fdtdec_get_bool(gd->fdt_blob, node, "is-internal-pcspma");
- printf("ZYNQ GEM: %lx, phyaddr %x, interface %s\n", (ulong)priv->iobase, - priv->phyaddr, phy_string_for_interface(priv->interface)); + printf("ZYNQ GEM: %lx, mdio bus %lx, phyaddr %d, interface %s\n", + (ulong)priv->iobase, (ulong)priv->mdiobase, priv->phyaddr, + phy_string_for_interface(priv->interface));
return 0; }
Hi Michal,
On Thu, Feb 1, 2018 at 6:42 AM, Michal Simek michal.simek@xilinx.com wrote:
Find out MDIO bus and enable MDIO access to it if this is done via different controller.
Signed-off-by: Michal Simek michal.simek@xilinx.com
Hi Joe,
this is the code I have hacked a year ago for ZynqMP where we can have configuration that 4 gems are enabled but they share the same MDIO bus which can be assigned to only gem. Normally recommendation is that you should assign it to IP which is required for boot and this is suitable I would say for almost everybody. But for testing purpose it will be good to support sharing mdio bus between others IPs. This hack is "enabling" this for others gem but not across different ethernet drivers.
Seems practical.
And my question is if there is any solution which can be used now for handling it. Or even this should work even now but we do something wrong.
I would envision some DM way to model the NIC, the MDIO bus, and the PHY separately. The big roadblock is that the DTS format is different for every NIC.
I am refreshing this topic based on communication with Tomasz Gorochowik when he wanted to separate mdio part but it is not compatible with solution used in Linux which is pattern we should follow.
We do the same thing on our 7020 products.
Note: I tested it only on zcu102 with the standard configuration but Tomasz confirmed that this is still working with shared mdio bus.
Thanks, Michal
drivers/net/zynq_gem.c | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 2cc49bca922a..33245ec36e67 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -177,6 +177,7 @@ struct zynq_gem_priv { int phyaddr; int init; struct zynq_gem_regs *iobase;
struct zynq_gem_regs *mdiobase; phy_interface_t interface; struct phy_device *phydev; int phy_of_handle;@@ -189,7 +190,7 @@ static u32 phy_setup_op(struct zynq_gem_priv *priv, u32 phy_addr, u32 regnum, u32 op, u16 *data) { u32 mgtcr;
struct zynq_gem_regs *regs = priv->iobase;
struct zynq_gem_regs *regs = priv->mdiobase; int err; err = wait_for_bit_le32(®s->nwsr, ZYNQ_GEM_NWSR_MDIOIDLE_MASK,@@ -314,7 +315,7 @@ static int zynq_phy_init(struct udevice *dev) { int ret; struct zynq_gem_priv *priv = dev_get_priv(dev);
struct zynq_gem_regs *regs = priv->iobase;
struct zynq_gem_regs *regs_mdio = priv->mdiobase; const u32 supported = SUPPORTED_10baseT_Half | SUPPORTED_10baseT_Full | SUPPORTED_100baseT_Half |@@ -323,7 +324,7 @@ static int zynq_phy_init(struct udevice *dev) SUPPORTED_1000baseT_Full;
/* Enable only MDIO bus */
writel(ZYNQ_GEM_NWCTRL_MDEN_MASK, ®s->nwctrl);
writel(ZYNQ_GEM_NWCTRL_MDEN_MASK, ®s_mdio->nwctrl); if (priv->interface != PHY_INTERFACE_MODE_SGMII) { ret = phy_detection(dev);@@ -355,6 +356,7 @@ static int zynq_gem_init(struct udevice *dev) unsigned long clk_rate = 0; struct zynq_gem_priv *priv = dev_get_priv(dev); struct zynq_gem_regs *regs = priv->iobase;
struct zynq_gem_regs *regs_mdio = priv->mdiobase; struct emac_bd *dummy_tx_bd = &priv->tx_bd[TX_FREE_DESC]; struct emac_bd *dummy_rx_bd = &priv->tx_bd[TX_FREE_DESC + 2];@@ -397,7 +399,7 @@ static int zynq_gem_init(struct udevice *dev) writel(ZYNQ_GEM_DMACR_INIT, ®s->dmacr);
/* Setup for Network Control register, MDIO, Rx and Tx enable */
setbits_le32(®s->nwctrl, ZYNQ_GEM_NWCTRL_MDEN_MASK);
setbits_le32(®s_mdio->nwctrl, ZYNQ_GEM_NWCTRL_MDEN_MASK); /* Disable the second priority queue */ dummy_tx_bd->addr = 0;@@ -623,6 +625,7 @@ static int zynq_gem_probe(struct udevice *dev) void *bd_space; struct zynq_gem_priv *priv = dev_get_priv(dev); int ret;
char name[MDIO_NAME_LEN]; /* Align rxbuffers to ARCH_DMA_MINALIGN */ priv->rxbuffers = memalign(ARCH_DMA_MINALIGN, RX_BUF * PKTSIZE_ALIGN);@@ -648,6 +651,9 @@ static int zynq_gem_probe(struct udevice *dev) priv->bus->write = zynq_gem_miiphy_write; priv->bus->priv = priv;
snprintf(name, MDIO_NAME_LEN, "gem%lx", (ulong)priv->iobase);strncpy(priv->bus->name, name, MDIO_NAME_LEN);ret = mdio_register_seq(priv->bus, dev->seq); if (ret) return ret;@@ -682,6 +688,8 @@ static int zynq_gem_ofdata_to_platdata(struct udevice *dev) struct zynq_gem_priv *priv = dev_get_priv(dev); int node = dev_of_offset(dev); const char *phy_mode;
fdt_addr_t addr;int parent; pdata->iobase = (phys_addr_t)devfdt_get_addr(dev); priv->iobase = (struct zynq_gem_regs *)pdata->iobase;@@ -690,10 +698,24 @@ static int zynq_gem_ofdata_to_platdata(struct udevice *dev)
priv->phy_of_handle = fdtdec_lookup_phandle(gd->fdt_blob, node, "phy-handle");
if (priv->phy_of_handle > 0)
if (priv->phy_of_handle > 0) { priv->phyaddr = fdtdec_get_int(gd->fdt_blob, priv->phy_of_handle, "reg", -1);parent = fdt_parent_offset(gd->fdt_blob, priv->phy_of_handle);addr = fdtdec_get_addr(gd->fdt_blob, parent, "reg");if (addr == FDT_ADDR_T_NONE) {printf("MDIO bus not found %x %x, %x\n",node, priv->phy_of_handle, parent);return -ENODEV;}priv->mdiobase = (struct zynq_gem_regs *)addr;} else {priv->mdiobase = priv->iobase;}phy_mode = fdt_getprop(gd->fdt_blob, node, "phy-mode", NULL); if (phy_mode) pdata->phy_interface = phy_get_interface_by_name(phy_mode);@@ -706,8 +728,9 @@ static int zynq_gem_ofdata_to_platdata(struct udevice *dev) priv->int_pcs = fdtdec_get_bool(gd->fdt_blob, node, "is-internal-pcspma");
printf("ZYNQ GEM: %lx, phyaddr %x, interface %s\n", (ulong)priv->iobase,priv->phyaddr, phy_string_for_interface(priv->interface));
printf("ZYNQ GEM: %lx, mdio bus %lx, phyaddr %d, interface %s\n",(ulong)priv->iobase, (ulong)priv->mdiobase, priv->phyaddr,phy_string_for_interface(priv->interface)); return 0;}
1.9.1
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Hi Joe,
On 2.2.2018 20:35, Joe Hershberger wrote:
Hi Michal,
On Thu, Feb 1, 2018 at 6:42 AM, Michal Simek michal.simek@xilinx.com wrote:
Find out MDIO bus and enable MDIO access to it if this is done via different controller.
Signed-off-by: Michal Simek michal.simek@xilinx.com
Hi Joe,
this is the code I have hacked a year ago for ZynqMP where we can have configuration that 4 gems are enabled but they share the same MDIO bus which can be assigned to only gem. Normally recommendation is that you should assign it to IP which is required for boot and this is suitable I would say for almost everybody. But for testing purpose it will be good to support sharing mdio bus between others IPs. This hack is "enabling" this for others gem but not across different ethernet drivers.
Seems practical.
And my question is if there is any solution which can be used now for handling it. Or even this should work even now but we do something wrong.
I would envision some DM way to model the NIC, the MDIO bus, and the PHY separately. The big roadblock is that the DTS format is different for every NIC.
Is it really that different? There should be phy handle which is pointing to actual phy and that should be enough to recognize parents and IP responsible for MDIO bus.
I am refreshing this topic based on communication with Tomasz Gorochowik when he wanted to separate mdio part but it is not compatible with solution used in Linux which is pattern we should follow.
We do the same thing on our 7020 products.
What exactly are you doing on 7020?
Thanks, Michal
On Mon, Feb 5, 2018 at 1:17 AM, Michal Simek michal.simek@xilinx.com wrote:
Hi Joe,
On 2.2.2018 20:35, Joe Hershberger wrote:
Hi Michal,
On Thu, Feb 1, 2018 at 6:42 AM, Michal Simek michal.simek@xilinx.com wrote:
Find out MDIO bus and enable MDIO access to it if this is done via different controller.
Signed-off-by: Michal Simek michal.simek@xilinx.com
Hi Joe,
this is the code I have hacked a year ago for ZynqMP where we can have configuration that 4 gems are enabled but they share the same MDIO bus which can be assigned to only gem. Normally recommendation is that you should assign it to IP which is required for boot and this is suitable I would say for almost everybody. But for testing purpose it will be good to support sharing mdio bus between others IPs. This hack is "enabling" this for others gem but not across different ethernet drivers.
Seems practical.
And my question is if there is any solution which can be used now for handling it. Or even this should work even now but we do something wrong.
I would envision some DM way to model the NIC, the MDIO bus, and the PHY separately. The big roadblock is that the DTS format is different for every NIC.
Is it really that different? There should be phy handle which is pointing to actual phy and that should be enough to recognize parents and IP responsible for MDIO bus.
The point is different drivers express the relationship with varying levels of implication, so there are a number of ways people describe it.
I am refreshing this topic based on communication with Tomasz Gorochowik when he wanted to separate mdio part but it is not compatible with solution used in Linux which is pattern we should follow.
We do the same thing on our 7020 products.
What exactly are you doing on 7020?
We are using a single MDIO interface to connect the PHYs for 2 GEMs.
-Joe
On 6.2.2018 22:11, Joe Hershberger wrote:
On Mon, Feb 5, 2018 at 1:17 AM, Michal Simek michal.simek@xilinx.com wrote:
Hi Joe,
On 2.2.2018 20:35, Joe Hershberger wrote:
Hi Michal,
On Thu, Feb 1, 2018 at 6:42 AM, Michal Simek michal.simek@xilinx.com wrote:
Find out MDIO bus and enable MDIO access to it if this is done via different controller.
Signed-off-by: Michal Simek michal.simek@xilinx.com
Hi Joe,
this is the code I have hacked a year ago for ZynqMP where we can have configuration that 4 gems are enabled but they share the same MDIO bus which can be assigned to only gem. Normally recommendation is that you should assign it to IP which is required for boot and this is suitable I would say for almost everybody. But for testing purpose it will be good to support sharing mdio bus between others IPs. This hack is "enabling" this for others gem but not across different ethernet drivers.
Seems practical.
And my question is if there is any solution which can be used now for handling it. Or even this should work even now but we do something wrong.
I would envision some DM way to model the NIC, the MDIO bus, and the PHY separately. The big roadblock is that the DTS format is different for every NIC.
Is it really that different? There should be phy handle which is pointing to actual phy and that should be enough to recognize parents and IP responsible for MDIO bus.
The point is different drivers express the relationship with varying levels of implication, so there are a number of ways people describe it.
ok.
I am refreshing this topic based on communication with Tomasz Gorochowik when he wanted to separate mdio part but it is not compatible with solution used in Linux which is pattern we should follow.
We do the same thing on our 7020 products.
What exactly are you doing on 7020?
We are using a single MDIO interface to connect the PHYs for 2 GEMs.
And I expect you are using only one interface for boot right?
M
On Wed, Feb 7, 2018 at 1:46 AM, Michal Simek michal.simek@xilinx.com wrote:
On 6.2.2018 22:11, Joe Hershberger wrote:
On Mon, Feb 5, 2018 at 1:17 AM, Michal Simek michal.simek@xilinx.com wrote:
Hi Joe,
On 2.2.2018 20:35, Joe Hershberger wrote:
Hi Michal,
On Thu, Feb 1, 2018 at 6:42 AM, Michal Simek michal.simek@xilinx.com wrote:
Find out MDIO bus and enable MDIO access to it if this is done via different controller.
Signed-off-by: Michal Simek michal.simek@xilinx.com
Hi Joe,
this is the code I have hacked a year ago for ZynqMP where we can have configuration that 4 gems are enabled but they share the same MDIO bus which can be assigned to only gem. Normally recommendation is that you should assign it to IP which is required for boot and this is suitable I would say for almost everybody. But for testing purpose it will be good to support sharing mdio bus between others IPs. This hack is "enabling" this for others gem but not across different ethernet drivers.
Seems practical.
And my question is if there is any solution which can be used now for handling it. Or even this should work even now but we do something wrong.
I would envision some DM way to model the NIC, the MDIO bus, and the PHY separately. The big roadblock is that the DTS format is different for every NIC.
Is it really that different? There should be phy handle which is pointing to actual phy and that should be enough to recognize parents and IP responsible for MDIO bus.
The point is different drivers express the relationship with varying levels of implication, so there are a number of ways people describe it.
ok.
I am refreshing this topic based on communication with Tomasz Gorochowik when he wanted to separate mdio part but it is not compatible with solution used in Linux which is pattern we should follow.
We do the same thing on our 7020 products.
What exactly are you doing on 7020?
We are using a single MDIO interface to connect the PHYs for 2 GEMs.
And I expect you are using only one interface for boot right?
That's right. Currently we only have one defined in U-Boot, but of course use both from Linux.
-Joe
Joe, Michal,
Please allow me to include the patches I originally sent to Michal.
We are well aware of the fact that only one network interface is used to boot the board, but it is quite convenient to be able to plug the ethernet cable to any eth socket, and still be able to boot (assuming 'ethrotate' is set).
We also accept the fact that the patch not acceptable in its current form, but maybe you could point us in the right direction, Joe?
I think that the main advantage of our patch is that the mdio_alloc is called just once, not separately for each gem.
Just to note: we're also okay with the solution from Michal, but we'd like to get one of the patches to the official repo (or to Michals fork first for that matter). We're also willing to spend more time on that if you provide us some hints on how would you like us to proceed.
Thanks, Tomasz
---
From 718a7c7bfdbd4323bd47197c2d00cd215a3e8f74 Mon Sep 17 00:00:00 2001
From: Tomasz Gorochowik tgorochowik@antmicro.com Date: Fri, 15 Dec 2017 12:03:34 +0100 Subject: [PATCH] net: zynq_gem: add zynq gem mdio bus driver
The purpose of this driver is to allow accessing multiple PHYs from separate GEMs, but with a shared MDIO bus.
This commit explores the same idea as the Linux macb_mdio patch submitted by Harini Katakam (Xilinx).
To make this driver usable with all the currently existing ZynqMP platforms, and not to break regular Zynq support, the MDIO bus has to be used as a virtual bus that wraps all the GEM interfaces that it is going to be used by (thus the zynqmp.dtsi changes).
If the parent node of the GEM driver is something else than the MDIO bus node - the driver will fallback to using built-in MDIO - this ensures compatibility for current Zynq boards and ZynqMP boards that do not want to use the shared MDIO functionality.
By default the 'compatible' property of the virtual bus node is set to 'simple-bus', so the child nodes get probed normally without the need to modify any existing dts files for specific boards, but also to make the MDIO bus instantiation easy.
To instantiate it, lets say for GEM0 and GEM3 and make both of them use the MDIO bus from GEM0, the following lines have to be added to the main devicetree file of the board:
&mdio { compatible = "cdns,zynqmp-gem-mdio"; reg = <0x0 0xff0b0000>; phy7: ethernet-phy@7 { reg = <7>; }; phy3: ethernet-phy@3 { reg = <3>; }; };
&gem0 { status = "okay"; phy-handle = <&phy3>; phy-mode = "rgmii-id"; };
&gem3 { status = "okay"; phy-handle = <&phy7>; phy-mode = "rgmii-id"; };
Note how the compatible string is changed to bind the device to the gem-mdio driver. The reg value of the MDIO bus is defined to the address of the GEM in which the MDIO resides (GEM0 in this case). Also note how the phy handles are defined inside the mdio bus for clarity.
Signed-off-by: Tomasz Gorochowik tgorochowik@antmicro.com --- arch/arm/dts/zynqmp.dtsi | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------------------------- drivers/net/Makefile | 2 +- drivers/net/zynq_gem.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------- drivers/net/zynq_gem.h | 8 ++++++++ drivers/net/zynq_gem_mdio.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 221 insertions(+), 87 deletions(-) create mode 100644 drivers/net/zynq_gem.h create mode 100644 drivers/net/zynq_gem_mdio.c
diff --git a/arch/arm/dts/zynqmp.dtsi b/arch/arm/dts/zynqmp.dtsi index 6a5c902..7499e52 100644 --- a/arch/arm/dts/zynqmp.dtsi +++ b/arch/arm/dts/zynqmp.dtsi @@ -568,60 +568,67 @@ power-domains = <&pd_nand>; };
- gem0: ethernet@ff0b0000 { - compatible = "cdns,zynqmp-gem"; - status = "disabled"; - interrupt-parent = <&gic>; - interrupts = <0 57 4>, <0 57 4>; - reg = <0x0 0xff0b0000 0x0 0x1000>; - clock-names = "pclk", "hclk", "tx_clk"; - #address-cells = <1>; - #size-cells = <0>; - #stream-id-cells = <1>; - iommus = <&smmu 0x874>; - power-domains = <&pd_eth0>; - }; + mdio: mdio { + compatible = "simple-bus"; + status = "okay"; + #address-cells = <2>; + #size-cells = <2>;
- gem1: ethernet@ff0c0000 { - compatible = "cdns,zynqmp-gem"; - status = "disabled"; - interrupt-parent = <&gic>; - interrupts = <0 59 4>, <0 59 4>; - reg = <0x0 0xff0c0000 0x0 0x1000>; - clock-names = "pclk", "hclk", "tx_clk"; - #address-cells = <1>; - #size-cells = <0>; - #stream-id-cells = <1>; - iommus = <&smmu 0x875>; - power-domains = <&pd_eth1>; - }; + gem0: ethernet@ff0b0000 { + compatible = "cdns,zynqmp-gem"; + status = "disabled"; + interrupt-parent = <&gic>; + interrupts = <0 57 4>, <0 57 4>; + reg = <0x0 0xff0b0000 0x0 0x1000>; + clock-names = "pclk", "hclk", "tx_clk"; + #address-cells = <1>; + #size-cells = <0>; + #stream-id-cells = <1>; + iommus = <&smmu 0x874>; + power-domains = <&pd_eth0>; + };
- gem2: ethernet@ff0d0000 { - compatible = "cdns,zynqmp-gem"; - status = "disabled"; - interrupt-parent = <&gic>; - interrupts = <0 61 4>, <0 61 4>; - reg = <0x0 0xff0d0000 0x0 0x1000>; - clock-names = "pclk", "hclk", "tx_clk"; - #address-cells = <1>; - #size-cells = <0>; - #stream-id-cells = <1>; - iommus = <&smmu 0x876>; - power-domains = <&pd_eth2>; - }; + gem1: ethernet@ff0c0000 { + compatible = "cdns,zynqmp-gem"; + status = "disabled"; + interrupt-parent = <&gic>; + interrupts = <0 59 4>, <0 59 4>; + reg = <0x0 0xff0c0000 0x0 0x1000>; + clock-names = "pclk", "hclk", "tx_clk"; + #address-cells = <1>; + #size-cells = <0>; + #stream-id-cells = <1>; + iommus = <&smmu 0x875>; + power-domains = <&pd_eth1>; + };
- gem3: ethernet@ff0e0000 { - compatible = "cdns,zynqmp-gem"; - status = "disabled"; - interrupt-parent = <&gic>; - interrupts = <0 63 4>, <0 63 4>; - reg = <0x0 0xff0e0000 0x0 0x1000>; - clock-names = "pclk", "hclk", "tx_clk"; - #address-cells = <1>; - #size-cells = <0>; - #stream-id-cells = <1>; - iommus = <&smmu 0x877>; - power-domains = <&pd_eth3>; + gem2: ethernet@ff0d0000 { + compatible = "cdns,zynqmp-gem"; + status = "disabled"; + interrupt-parent = <&gic>; + interrupts = <0 61 4>, <0 61 4>; + reg = <0x0 0xff0d0000 0x0 0x1000>; + clock-names = "pclk", "hclk", "tx_clk"; + #address-cells = <1>; + #size-cells = <0>; + #stream-id-cells = <1>; + iommus = <&smmu 0x876>; + power-domains = <&pd_eth2>; + }; + + gem3: ethernet@ff0e0000 { + compatible = "cdns,zynqmp-gem"; + status = "disabled"; + interrupt-parent = <&gic>; + interrupts = <0 63 4>, <0 63 4>; + reg = <0x0 0xff0e0000 0x0 0x1000>; + clock-names = "pclk", "hclk", "tx_clk"; + #address-cells = <1>; + #size-cells = <0>; + #stream-id-cells = <1>; + iommus = <&smmu 0x877>; + power-domains = <&pd_eth3>; + }; };
gpio: gpio@ff0a0000 { diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 5702592..fd8a923 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -69,7 +69,7 @@ obj-$(CONFIG_XILINX_AXIEMAC) += xilinx_axi_emac.o obj-$(CONFIG_XILINX_EMACLITE) += xilinx_emaclite.o obj-$(CONFIG_XILINX_LL_TEMAC) += xilinx_ll_temac.o xilinx_ll_temac_mdio.o \ xilinx_ll_temac_fifo.o xilinx_ll_temac_sdma.o -obj-$(CONFIG_ZYNQ_GEM) += zynq_gem.o +obj-$(CONFIG_ZYNQ_GEM) += zynq_gem.o zynq_gem_mdio.o obj-$(CONFIG_FSL_MC_ENET) += fsl-mc/ obj-$(CONFIG_FSL_MC_ENET) += ldpaa_eth/ obj-$(CONFIG_FSL_MEMAC) += fm/memac_phy.o diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 3dec1ff..95d5a7d 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -27,6 +27,8 @@ #include <asm-generic/errno.h> #include <clk.h>
+#include "zynq_gem.h" + DECLARE_GLOBAL_DATA_PTR;
/* Bit/mask specification */ @@ -182,12 +184,13 @@ struct zynq_gem_priv { struct phy_device *phydev; int phy_of_handle; struct mii_dev *bus; + bool shared_mdio; #ifdef CONFIG_CLK_ZYNQMP struct clk clk; #endif };
-static inline int mdio_wait(struct zynq_gem_regs *regs) +inline int mdio_wait(struct zynq_gem_regs *regs) { u32 timeout = 20000;
@@ -206,11 +209,10 @@ static inline int mdio_wait(struct zynq_gem_regs *regs) return 0; }
-static u32 phy_setup_op(struct zynq_gem_priv *priv, u32 phy_addr, u32 regnum, +static u32 phy_setup_op(struct zynq_gem_regs *regs, u32 phy_addr, u32 regnum, u32 op, u16 *data) { u32 mgtcr; - struct zynq_gem_regs *regs = priv->iobase;
if (mdio_wait(regs)) return 1; @@ -232,12 +234,12 @@ static u32 phy_setup_op(struct zynq_gem_priv *priv, u32 phy_addr, u32 regnum, return 0; }
-static u32 phyread(struct zynq_gem_priv *priv, u32 phy_addr, +static u32 phyread(struct zynq_gem_regs *regs, u32 phy_addr, u32 regnum, u16 *val) { u32 ret;
- ret = phy_setup_op(priv, phy_addr, regnum, + ret = phy_setup_op(regs, phy_addr, regnum, ZYNQ_GEM_PHYMNTNC_OP_R_MASK, val);
if (!ret) @@ -247,13 +249,13 @@ static u32 phyread(struct zynq_gem_priv *priv, u32 phy_addr, return ret; }
-static u32 phywrite(struct zynq_gem_priv *priv, u32 phy_addr, +static u32 phywrite(struct zynq_gem_regs *regs, u32 phy_addr, u32 regnum, u16 data) { debug("%s: phy_addr %d, regnum 0x%x, data 0x%x\n", __func__, phy_addr, regnum, data);
- return phy_setup_op(priv, phy_addr, regnum, + return phy_setup_op(regs, phy_addr, regnum, ZYNQ_GEM_PHYMNTNC_OP_W_MASK, &data); }
@@ -264,7 +266,9 @@ static int phy_detection(struct udevice *dev) struct zynq_gem_priv *priv = dev->priv;
if (priv->phyaddr != -1) { - phyread(priv, priv->phyaddr, PHY_DETECT_REG, &phyreg); + phyreg = priv->bus->read(priv->bus, priv->phyaddr, 0, + PHY_DETECT_REG); + if ((phyreg != 0xFFFF) && ((phyreg & PHY_DETECT_MASK) == PHY_DETECT_MASK)) { /* Found a valid PHY address */ @@ -282,7 +286,8 @@ static int phy_detection(struct udevice *dev) if (priv->phyaddr == -1) { /* detect the PHY address */ for (i = 31; i >= 0; i--) { - phyread(priv, i, PHY_DETECT_REG, &phyreg); + phyreg = priv->bus->read(priv->bus, i, 0, + PHY_DETECT_REG); if ((phyreg != 0xFFFF) && ((phyreg & PHY_DETECT_MASK) == PHY_DETECT_MASK)) { /* Found a valid PHY address */ @@ -330,7 +335,6 @@ static int zynq_phy_init(struct udevice *dev) { int ret; struct zynq_gem_priv *priv = dev_get_priv(dev); - struct zynq_gem_regs *regs = priv->iobase; const u32 supported = SUPPORTED_10baseT_Half | SUPPORTED_10baseT_Full | SUPPORTED_100baseT_Half | @@ -338,9 +342,6 @@ static int zynq_phy_init(struct udevice *dev) SUPPORTED_1000baseT_Half | SUPPORTED_1000baseT_Full;
- /* Enable only MDIO bus */ - writel(ZYNQ_GEM_NWCTRL_MDEN_MASK, ®s->nwctrl); - if (priv->interface != PHY_INTERFACE_MODE_SGMII) { ret = phy_detection(dev); if (ret) { @@ -366,7 +367,7 @@ static int zynq_phy_init(struct udevice *dev)
static int zynq_gem_init(struct udevice *dev) { - u32 i, nwconfig; + u32 i, nwconfig, nwcontrol; int ret; unsigned long clk_rate = 0; struct zynq_gem_priv *priv = dev_get_priv(dev); @@ -379,7 +380,15 @@ static int zynq_gem_init(struct udevice *dev) writel(0xFFFFFFFF, ®s->idr);
/* Disable the receiver & transmitter */ - writel(0, ®s->nwctrl); + if (priv->shared_mdio) { + nwcontrol = readl(®s->nwctrl); + nwcontrol &= ~ZYNQ_GEM_NWCTRL_RXEN_MASK; + nwcontrol &= ~ZYNQ_GEM_NWCTRL_TXEN_MASK; + } else { + nwcontrol = 0; + } + writel(nwcontrol, ®s->nwctrl); + writel(0, ®s->txsr); writel(0, ®s->rxsr); writel(0, ®s->phymntnc); @@ -412,8 +421,10 @@ static int zynq_gem_init(struct udevice *dev) /* Setup for DMA Configuration register */ writel(ZYNQ_GEM_DMACR_INIT, ®s->dmacr);
- /* Setup for Network Control register, MDIO, Rx and Tx enable */ - setbits_le32(®s->nwctrl, ZYNQ_GEM_NWCTRL_MDEN_MASK); + if (!priv->shared_mdio) { + /* Enable MDIO */ + setbits_le32(®s->nwctrl, ZYNQ_GEM_NWCTRL_MDEN_MASK); + }
/* Disable the second priority queue */ dummy_tx_bd->addr = 0; @@ -613,25 +624,24 @@ static int zynq_gem_read_rom_mac(struct udevice *dev) return retval; }
-static int zynq_gem_miiphy_read(struct mii_dev *bus, int addr, - int devad, int reg) +int zynq_gem_miiphy_read(struct mii_dev *bus, int addr, int devad, int reg) { - struct zynq_gem_priv *priv = bus->priv; + struct zynq_gem_regs *regs = bus->priv; int ret; u16 val;
- ret = phyread(priv, addr, reg, &val); + ret = phyread(regs, addr, reg, &val); debug("%s 0x%x, 0x%x, 0x%x, 0x%x\n", __func__, addr, reg, val, ret); return val; }
-static int zynq_gem_miiphy_write(struct mii_dev *bus, int addr, int devad, - int reg, u16 value) +int zynq_gem_miiphy_write(struct mii_dev *bus, int addr, int devad, int reg, + u16 value) { - struct zynq_gem_priv *priv = bus->priv; + struct zynq_gem_regs *regs = bus->priv;
debug("%s 0x%x, 0x%x, 0x%x\n", __func__, addr, reg, value); - return phywrite(priv, addr, reg, value); + return phywrite(regs, addr, reg, value); }
static int zynq_gem_probe(struct udevice *dev) @@ -661,15 +671,18 @@ static int zynq_gem_probe(struct udevice *dev) } #endif
- priv->bus = mdio_alloc(); - priv->bus->read = zynq_gem_miiphy_read; - priv->bus->write = zynq_gem_miiphy_write; - priv->bus->priv = priv; - strcpy(priv->bus->name, "gem"); + if (!priv->shared_mdio) { + /* Initialize built-in MDIO */ + priv->bus = mdio_alloc(); + priv->bus->read = zynq_gem_miiphy_read; + priv->bus->write = zynq_gem_miiphy_write; + priv->bus->priv = priv->iobase; + strcpy(priv->bus->name, "gem");
- ret = mdio_register(priv->bus); - if (ret) - return ret; + ret = mdio_register(priv->bus); + if (ret) + return ret; + }
return zynq_phy_init(dev); } @@ -679,8 +692,13 @@ static int zynq_gem_remove(struct udevice *dev) struct zynq_gem_priv *priv = dev_get_priv(dev);
free(priv->phydev); - mdio_unregister(priv->bus); - mdio_free(priv->bus); + + if (priv->shared_mdio) { + mdio_unregister(priv->bus); + mdio_free(priv->bus); + + free(dev->parent_platdata); + }
return 0; } @@ -706,6 +724,7 @@ static int zynq_gem_ofdata_to_platdata(struct udevice *dev) /* Hardcode for now */ priv->emio = 0; priv->phyaddr = -1; + priv->shared_mdio = false;
priv->phy_of_handle = fdtdec_lookup_phandle(gd->fdt_blob, dev->of_offset, "phy-handle"); @@ -724,6 +743,11 @@ static int zynq_gem_ofdata_to_platdata(struct udevice *dev)
priv->emio = fdtdec_get_bool(gd->fdt_blob, dev->of_offset, "xlnx,emio");
+ /* Determine if we're connected to an external MDIO */ + priv->bus = dev_get_parent_platdata(dev); + if (priv->bus) + priv->shared_mdio = true; + printf("ZYNQ GEM: %lx, phyaddr %d, interface %s\n", (ulong)priv->iobase, priv->phyaddr, phy_string_for_interface(priv->interface));
diff --git a/drivers/net/zynq_gem.h b/drivers/net/zynq_gem.h new file mode 100644 index 0000000..a9bf175 --- /dev/null +++ b/drivers/net/zynq_gem.h @@ -0,0 +1,8 @@ +#ifndef ZYNQ_GEM_H +#define ZYNQ_GEM_H + +int zynq_gem_miiphy_read(struct mii_dev *bus, int addr, int devad, int reg); +int zynq_gem_miiphy_write(struct mii_dev *bus, int addr, int devad, int reg, + u16 value); + +#endif diff --git a/drivers/net/zynq_gem_mdio.c b/drivers/net/zynq_gem_mdio.c new file mode 100644 index 0000000..1e4b091 --- /dev/null +++ b/drivers/net/zynq_gem_mdio.c @@ -0,0 +1,95 @@ +/* + * Copyright (C) 2017 Enclustra GmbH + * Author: Tomasz Gorochowik tgorochowik@antmicro.com + * + * The idea for this driver was based on a Linux macb_mdio + * driver by Harini Katakam, Xilinx + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <dm.h> +#include <asm/io.h> +#include <miiphy.h> + +#include "zynq_gem.h" + +DECLARE_GLOBAL_DATA_PTR; + +#define ZYNQ_GEM_MDIO_NWCTRL_MDEN_MASK 0x00000010 /* Enable MDIO port */ + +static int zynq_gem_mdio_probe(struct udevice *dev) +{ + struct mii_dev *bus = dev_get_priv(dev); + u32 *nwctrl_reg = bus->priv; + + setbits_le32(nwctrl_reg, ZYNQ_GEM_MDIO_NWCTRL_MDEN_MASK); + + return 0; +} + +static int zynq_gem_mdio_remove(struct udevice *dev) +{ + struct mii_dev *bus = dev_get_priv(dev); + u32 *nwctrl_reg = bus->priv; + + clrbits_le32(nwctrl_reg, ZYNQ_GEM_MDIO_NWCTRL_MDEN_MASK); + + /* Unregister and free mdio bus */ + mdio_unregister(bus); + mdio_free(bus); + + return 0; +} + +static const struct udevice_id zynq_gem_mdio_ids[] = { + { .compatible = "cdns,zynqmp-gem-mdio" }, + { } +}; + +static int zynq_gem_mdio_ofdata_to_platdata(struct udevice *dev) +{ + struct mii_dev *bus; + + bus = mdio_alloc(); + if (!bus) + return -ENOMEM; + + sprintf(bus->name, "%s", "zynq_gem_mdio"); + bus->read = zynq_gem_miiphy_read; + bus->write = zynq_gem_miiphy_write; + + /* We use the fact that we only need a single register + * in this driver (network_control), the private data + * pointer holds its address directly + */ + bus->priv = (void *)dev_get_addr(dev); + dev->priv = bus; + + mdio_register(bus); + + return 0; +} + +static int zynq_gem_mdio_child_pre_probe(struct udevice *dev) +{ + struct udevice *pdev = dev_get_parent(dev); + struct mii_dev *bus = dev_get_priv(pdev); + + dev->parent_platdata = bus; + + return 0; +} + +U_BOOT_DRIVER(zynq_gem_mdio) = { + .name = "zynq_gem_mdio", + .id = UCLASS_SIMPLE_BUS, + .of_match = zynq_gem_mdio_ids, + .ofdata_to_platdata = zynq_gem_mdio_ofdata_to_platdata, + .probe = zynq_gem_mdio_probe, + .remove = zynq_gem_mdio_remove, + .priv_auto_alloc_size = sizeof(struct mii_dev *), + .platdata_auto_alloc_size = sizeof(struct mii_dev *), + .per_child_platdata_auto_alloc_size = sizeof(struct mii_dev *), + .child_pre_probe = zynq_gem_mdio_child_pre_probe, +}; -- libgit2 0.26.0
On Fri, Feb 16, 2018 at 8:24 PM, Joe Hershberger joe.hershberger@gmail.com wrote:
On Wed, Feb 7, 2018 at 1:46 AM, Michal Simek michal.simek@xilinx.com wrote:
On 6.2.2018 22:11, Joe Hershberger wrote:
On Mon, Feb 5, 2018 at 1:17 AM, Michal Simek michal.simek@xilinx.com
wrote:
Hi Joe,
On 2.2.2018 20:35, Joe Hershberger wrote:
Hi Michal,
On Thu, Feb 1, 2018 at 6:42 AM, Michal Simek michal.simek@xilinx.com
wrote:
Find out MDIO bus and enable MDIO access to it if this is done via different controller.
Signed-off-by: Michal Simek michal.simek@xilinx.com
Hi Joe,
this is the code I have hacked a year ago for ZynqMP where we can
have
configuration that 4 gems are enabled but they share the same MDIO
bus
which can be assigned to only gem. Normally recommendation is that
you
should assign it to IP which is required for boot and this is
suitable I
would say for almost everybody. But for testing purpose it will be good to support sharing mdio bus between others IPs. This hack is "enabling" this for others gem but
not
across different ethernet drivers.
Seems practical.
And my question is if there is any solution which can be used now for handling it. Or even this should work even now but we do something wrong.
I would envision some DM way to model the NIC, the MDIO bus, and the PHY separately. The big roadblock is that the DTS format is different for every NIC.
Is it really that different? There should be phy handle which is pointing to actual phy and that should be enough to recognize parents and IP responsible for MDIO bus.
The point is different drivers express the relationship with varying levels of implication, so there are a number of ways people describe it.
ok.
I am refreshing this topic based on communication with Tomasz Gorochowik when he wanted to separate mdio part but it is not
compatible
with solution used in Linux which is pattern we should follow.
We do the same thing on our 7020 products.
What exactly are you doing on 7020?
We are using a single MDIO interface to connect the PHYs for 2 GEMs.
And I expect you are using only one interface for boot right?
That's right. Currently we only have one defined in U-Boot, but of course use both from Linux.
-Joe
participants (4)
-
Joe Hershberger -
Joe Hershberger -
Michal Simek -
Tomasz Gorochowik