[Qemu-arm] [PATCH v5 0/7] Generalize MDIO framework

[Qemu-arm] [PATCH v5 0/7] Generalize MDIO framework

[Philippe Mathieu-Daudé]

Hi,

I have a follow up series using multiples PHY on the MDIO bus based on this
series.

Regards,

Phil.

Grant's previous work:
http://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg00257.html

"There is more work to be done, particularly in moving to the common GPIO api,
 but that work can be done as a follow on patch series."

Grant Likely (7):
  hw/mdio: Generalize etraxfs MDIO bitbanging emulation
  hw/mdio: Add PHY register definition
  hw/mdio: Generalize phy initialization routine
  hw/mdio: Mask out read-only bits.
  hw/mdio: Refactor bitbanging state machine
  hw/mdio: Add VMState support
  hw/mdio: Use bitbang core for smc91c111 network device

 include/hw/net/mdio.h   | 124 +++++++++++++++++++++
 hw/net/etraxfs_eth.c    | 291 +-----------------------------------------------
 hw/net/mdio.c           | 280 ++++++++++++++++++++++++++++++++++++++++++++++
 hw/net/smc91c111.c      |  27 ++++-
 hw/net/xilinx_axienet.c | 189 +------------------------------
 hw/net/Makefile.objs    |   2 +
 6 files changed, 438 insertions(+), 475 deletions(-)
 create mode 100644 include/hw/net/mdio.h
 create mode 100644 hw/net/mdio.c

-- 
2.14.1

[Qemu-devel] [PATCH v5 1/7] hw/mdio: Generalize etraxfs MDIO bitbanging emulation

[Philippe Mathieu-Daudé]

From: Grant Likely <grant.likely@arm.com>

The etraxfs and Xilinx axienet Ethernet models implement quite a nice
MDIO core that supports both bitbanging and direct register access. This
change factors the common code out into a separate file. There are no
functional changes here, just movement of code.

The etraxfs and axienet are slightly different. The etraxfs version
includes the bitbang state processing, but the axienet version has a
minor enhancement for read/write of phy registers without using bitbang
state variables.  This patch generalizes the etraxfs version, with the
axienet change backported in.

Signed-off-by: Grant Likely <grant.likely@arm.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
[PMD: rebased with a minor checkpatch fix]
---
 include/hw/net/mdio.h   |  76 +++++++++++++
 hw/net/etraxfs_eth.c    | 278 +-----------------------------------------------
 hw/net/mdio.c           | 262 +++++++++++++++++++++++++++++++++++++++++++++
 hw/net/xilinx_axienet.c | 187 +-------------------------------
 hw/net/Makefile.objs    |   2 +
 5 files changed, 344 insertions(+), 461 deletions(-)
 create mode 100644 include/hw/net/mdio.h
 create mode 100644 hw/net/mdio.c

diff --git a/include/hw/net/mdio.h b/include/hw/net/mdio.h
new file mode 100644
index 0000000000..ac36aed3c3
--- /dev/null
+++ b/include/hw/net/mdio.h
@@ -0,0 +1,76 @@
+#ifndef BITBANG_MDIO_H
+#define BITBANG_MDIO_H
+
+/*
+ * QEMU Bitbang Ethernet MDIO bus & PHY controllers.
+ *
+ * Copyright (c) 2008 Edgar E. Iglesias, Axis Communications AB.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+/* PHY Advertisement control register */
+#define PHY_ADVERTISE_10HALF    0x0020  /* Try for 10mbps half-duplex  */
+#define PHY_ADVERTISE_10FULL    0x0040  /* Try for 10mbps full-duplex  */
+#define PHY_ADVERTISE_100HALF   0x0080  /* Try for 100mbps half-duplex */
+#define PHY_ADVERTISE_100FULL   0x0100  /* Try for 100mbps full-duplex */
+
+struct qemu_phy {
+    uint32_t regs[32];
+
+    int link;
+
+    unsigned int (*read)(struct qemu_phy *phy, unsigned int req);
+    void (*write)(struct qemu_phy *phy, unsigned int req, unsigned int data);
+};
+
+struct qemu_mdio {
+    /* bus. */
+    int mdc;
+    int mdio;
+
+    /* decoder.  */
+    enum {
+        PREAMBLE,
+        SOF,
+        OPC,
+        ADDR,
+        REQ,
+        TURNAROUND,
+        DATA
+    } state;
+    unsigned int drive;
+
+    unsigned int cnt;
+    unsigned int addr;
+    unsigned int opc;
+    unsigned int req;
+    unsigned int data;
+
+    struct qemu_phy *devs[32];
+};
+
+void tdk_init(struct qemu_phy *phy);
+void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy,
+                 unsigned int addr);
+uint16_t mdio_read_req(struct qemu_mdio *bus, uint8_t addr, uint8_t req);
+void mdio_write_req(struct qemu_mdio *bus, uint8_t addr, uint8_t req, uint16_t data);
+void mdio_cycle(struct qemu_mdio *bus);
+
+#endif
diff --git a/hw/net/etraxfs_eth.c b/hw/net/etraxfs_eth.c
index 013c8d0a41..f8d8f8441d 100644
--- a/hw/net/etraxfs_eth.c
+++ b/hw/net/etraxfs_eth.c
@@ -26,287 +26,11 @@
 #include "hw/sysbus.h"
 #include "net/net.h"
 #include "hw/cris/etraxfs.h"
+#include "hw/net/mdio.h"
 #include "qemu/error-report.h"
 
 #define D(x)
 
-/* Advertisement control register. */
-#define ADVERTISE_10HALF        0x0020  /* Try for 10mbps half-duplex  */
-#define ADVERTISE_10FULL        0x0040  /* Try for 10mbps full-duplex  */
-#define ADVERTISE_100HALF       0x0080  /* Try for 100mbps half-duplex */
-#define ADVERTISE_100FULL       0x0100  /* Try for 100mbps full-duplex */
-
-/*
- * The MDIO extensions in the TDK PHY model were reversed engineered from the
- * linux driver (PHYID and Diagnostics reg).
- * TODO: Add friendly names for the register nums.
- */
-struct qemu_phy
-{
-    uint32_t regs[32];
-
-    int link;
-
-    unsigned int (*read)(struct qemu_phy *phy, unsigned int req);
-    void (*write)(struct qemu_phy *phy, unsigned int req, unsigned int data);
-};
-
-static unsigned int tdk_read(struct qemu_phy *phy, unsigned int req)
-{
-    int regnum;
-    unsigned r = 0;
-
-    regnum = req & 0x1f;
-
-    switch (regnum) {
-    case 1:
-        if (!phy->link) {
-            break;
-        }
-        /* MR1.     */
-        /* Speeds and modes.  */
-        r |= (1 << 13) | (1 << 14);
-        r |= (1 << 11) | (1 << 12);
-        r |= (1 << 5); /* Autoneg complete.  */
-        r |= (1 << 3); /* Autoneg able.     */
-        r |= (1 << 2); /* link.     */
-        break;
-    case 5:
-        /* Link partner ability.
-           We are kind; always agree with whatever best mode
-           the guest advertises.  */
-        r = 1 << 14; /* Success.  */
-        /* Copy advertised modes.  */
-        r |= phy->regs[4] & (15 << 5);
-        /* Autoneg support.  */
-        r |= 1;
-        break;
-    case 18:
-    {
-        /* Diagnostics reg.  */
-        int duplex = 0;
-        int speed_100 = 0;
-
-        if (!phy->link) {
-            break;
-        }
-
-        /* Are we advertising 100 half or 100 duplex ? */
-        speed_100 = !!(phy->regs[4] & ADVERTISE_100HALF);
-        speed_100 |= !!(phy->regs[4] & ADVERTISE_100FULL);
-
-        /* Are we advertising 10 duplex or 100 duplex ? */
-        duplex = !!(phy->regs[4] & ADVERTISE_100FULL);
-        duplex |= !!(phy->regs[4] & ADVERTISE_10FULL);
-        r = (speed_100 << 10) | (duplex << 11);
-    }
-    break;
-
-    default:
-        r = phy->regs[regnum];
-        break;
-    }
-    D(printf("\n%s %x = reg[%d]\n", __func__, r, regnum));
-    return r;
-}
-
-static void
-tdk_write(struct qemu_phy *phy, unsigned int req, unsigned int data)
-{
-    int regnum;
-
-    regnum = req & 0x1f;
-    D(printf("%s reg[%d] = %x\n", __func__, regnum, data));
-    switch (regnum) {
-    default:
-        phy->regs[regnum] = data;
-        break;
-    }
-}
-
-static void
-tdk_init(struct qemu_phy *phy)
-{
-    phy->regs[0] = 0x3100;
-    /* PHY Id.  */
-    phy->regs[2] = 0x0300;
-    phy->regs[3] = 0xe400;
-    /* Autonegotiation advertisement reg.  */
-    phy->regs[4] = 0x01E1;
-    phy->link = 1;
-
-    phy->read = tdk_read;
-    phy->write = tdk_write;
-}
-
-struct qemu_mdio
-{
-    /* bus.     */
-    int mdc;
-    int mdio;
-
-    /* decoder.  */
-    enum {
-        PREAMBLE,
-        SOF,
-        OPC,
-        ADDR,
-        REQ,
-        TURNAROUND,
-        DATA
-    } state;
-    unsigned int drive;
-
-    unsigned int cnt;
-    unsigned int addr;
-    unsigned int opc;
-    unsigned int req;
-    unsigned int data;
-
-    struct qemu_phy *devs[32];
-};
-
-static void
-mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy, unsigned int addr)
-{
-    bus->devs[addr & 0x1f] = phy;
-}
-
-#ifdef USE_THIS_DEAD_CODE
-static void
-mdio_detach(struct qemu_mdio *bus, struct qemu_phy *phy, unsigned int addr)
-{
-    bus->devs[addr & 0x1f] = NULL;
-}
-#endif
-
-static void mdio_read_req(struct qemu_mdio *bus)
-{
-    struct qemu_phy *phy;
-
-    phy = bus->devs[bus->addr];
-    if (phy && phy->read) {
-        bus->data = phy->read(phy, bus->req);
-    } else {
-        bus->data = 0xffff;
-    }
-}
-
-static void mdio_write_req(struct qemu_mdio *bus)
-{
-    struct qemu_phy *phy;
-
-    phy = bus->devs[bus->addr];
-    if (phy && phy->write) {
-        phy->write(phy, bus->req, bus->data);
-    }
-}
-
-static void mdio_cycle(struct qemu_mdio *bus)
-{
-    bus->cnt++;
-
-    D(printf("mdc=%d mdio=%d state=%d cnt=%d drv=%d\n",
-        bus->mdc, bus->mdio, bus->state, bus->cnt, bus->drive));
-#if 0
-    if (bus->mdc) {
-        printf("%d", bus->mdio);
-    }
-#endif
-    switch (bus->state) {
-    case PREAMBLE:
-        if (bus->mdc) {
-            if (bus->cnt >= (32 * 2) && !bus->mdio) {
-                bus->cnt = 0;
-                bus->state = SOF;
-                bus->data = 0;
-            }
-        }
-        break;
-    case SOF:
-        if (bus->mdc) {
-            if (bus->mdio != 1) {
-                printf("WARNING: no SOF\n");
-            }
-            if (bus->cnt == 1*2) {
-                bus->cnt = 0;
-                bus->opc = 0;
-                bus->state = OPC;
-            }
-        }
-        break;
-    case OPC:
-        if (bus->mdc) {
-            bus->opc <<= 1;
-            bus->opc |= bus->mdio & 1;
-            if (bus->cnt == 2*2) {
-                bus->cnt = 0;
-                bus->addr = 0;
-                bus->state = ADDR;
-            }
-        }
-        break;
-    case ADDR:
-        if (bus->mdc) {
-            bus->addr <<= 1;
-            bus->addr |= bus->mdio & 1;
-
-            if (bus->cnt == 5*2) {
-                bus->cnt = 0;
-                bus->req = 0;
-                bus->state = REQ;
-            }
-        }
-        break;
-    case REQ:
-        if (bus->mdc) {
-            bus->req <<= 1;
-            bus->req |= bus->mdio & 1;
-            if (bus->cnt == 5*2) {
-                bus->cnt = 0;
-                bus->state = TURNAROUND;
-            }
-        }
-        break;
-    case TURNAROUND:
-        if (bus->mdc && bus->cnt == 2*2) {
-            bus->mdio = 0;
-            bus->cnt = 0;
-
-            if (bus->opc == 2) {
-                bus->drive = 1;
-                mdio_read_req(bus);
-                bus->mdio = bus->data & 1;
-            }
-            bus->state = DATA;
-        }
-        break;
-    case DATA:
-        if (!bus->mdc) {
-            if (bus->drive) {
-                bus->mdio = !!(bus->data & (1 << 15));
-                bus->data <<= 1;
-            }
-        } else {
-            if (!bus->drive) {
-                bus->data <<= 1;
-                bus->data |= bus->mdio;
-            }
-            if (bus->cnt == 16 * 2) {
-                bus->cnt = 0;
-                bus->state = PREAMBLE;
-                if (!bus->drive) {
-                    mdio_write_req(bus);
-                }
-                bus->drive = 0;
-            }
-        }
-        break;
-    default:
-        break;
-    }
-}
-
 /* ETRAX-FS Ethernet MAC block starts here.  */
 
 #define RW_MA0_LO      0x00
diff --git a/hw/net/mdio.c b/hw/net/mdio.c
new file mode 100644
index 0000000000..3763fcc8af
--- /dev/null
+++ b/hw/net/mdio.c
@@ -0,0 +1,262 @@
+/*
+ * QEMU Ethernet MDIO bus & PHY models
+ *
+ * Copyright (c) 2008 Edgar E. Iglesias, Axis Communications AB.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * This is a generic MDIO implementation.
+ *
+ * TODO:
+ * - Split PHYs out as separate device models so they can be defined and
+ *   instantiated separately from the MDIO bus.
+ * - Split out bitbang state machine into a separate model. Mostly this consists
+ *   of the mdio_cycle() routine and the bitbang state data in struct qemu_mdio
+ * - Use the GPIO interface for driving bitbang
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qemu/log.h"
+#include "hw/net/mdio.h"
+
+#define D(x)
+
+/*
+ * The MDIO extensions in the TDK PHY model were reversed engineered from the
+ * linux driver (PHYID and Diagnostics reg).
+ * TODO: Add friendly names for the register nums.
+ */
+static unsigned int tdk_read(struct qemu_phy *phy, unsigned int req)
+{
+    int regnum;
+    unsigned r = 0;
+
+    regnum = req & 0x1f;
+
+    switch (regnum) {
+    case 1:
+        if (!phy->link) {
+            break;
+        }
+        /* MR1.     */
+        /* Speeds and modes.  */
+        r |= (1 << 13) | (1 << 14);
+        r |= (1 << 11) | (1 << 12);
+        r |= (1 << 5); /* Autoneg complete.  */
+        r |= (1 << 3); /* Autoneg able.     */
+        r |= (1 << 2); /* link.     */
+        r |= (1 << 1); /* link.     */
+        break;
+    case 5:
+        /* Link partner ability.
+           We are kind; always agree with whatever best mode
+           the guest advertises.  */
+        r = 1 << 14; /* Success.  */
+        /* Copy advertised modes.  */
+        r |= phy->regs[4] & (15 << 5);
+        /* Autoneg support.  */
+        r |= 1;
+        break;
+    case 17:
+        /* Marvel PHY on many xilinx boards. */
+        r = 0x8000; /* 1000Mb */
+        break;
+    case 18:
+    {
+        /* Diagnostics reg.  */
+        int duplex = 0;
+        int speed_100 = 0;
+
+        if (!phy->link) {
+            break;
+        }
+
+        /* Are we advertising 100 half or 100 duplex ? */
+        speed_100 = !!(phy->regs[4] & PHY_ADVERTISE_100HALF);
+        speed_100 |= !!(phy->regs[4] & PHY_ADVERTISE_100FULL);
+
+        /* Are we advertising 10 duplex or 100 duplex ? */
+        duplex = !!(phy->regs[4] & PHY_ADVERTISE_100FULL);
+        duplex |= !!(phy->regs[4] & PHY_ADVERTISE_10FULL);
+        r = (speed_100 << 10) | (duplex << 11);
+    }
+    break;
+
+    default:
+        r = phy->regs[regnum];
+        break;
+    }
+    D(printf("\n%s %x = reg[%d]\n", __func__, r, regnum));
+    return r;
+}
+
+static void tdk_write(struct qemu_phy *phy, unsigned int req, unsigned int data)
+{
+    int regnum;
+
+    regnum = req & 0x1f;
+    D(printf("%s reg[%d] = %x\n", __func__, regnum, data));
+    switch (regnum) {
+    default:
+        phy->regs[regnum] = data;
+        break;
+    }
+}
+
+void tdk_init(struct qemu_phy *phy)
+{
+    phy->regs[0] = 0x3100;
+    /* PHY Id. */
+    phy->regs[2] = 0x0300;
+    phy->regs[3] = 0xe400;
+    /* Autonegotiation advertisement reg. */
+    phy->regs[4] = 0x01e1;
+    phy->link = 1;
+
+    phy->read = tdk_read;
+    phy->write = tdk_write;
+}
+
+void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy, unsigned int addr)
+{
+    bus->devs[addr & 0x1f] = phy;
+}
+
+uint16_t mdio_read_req(struct qemu_mdio *bus, uint8_t addr, uint8_t req)
+{
+    struct qemu_phy *phy;
+
+    phy = bus->devs[bus->addr];
+    if (phy && phy->read) {
+        return phy->read(phy, req);
+    }
+    return 0xffff;
+}
+
+void mdio_write_req(struct qemu_mdio *bus, uint8_t addr, uint8_t req,
+                    uint16_t data)
+{
+    struct qemu_phy *phy;
+
+    phy = bus->devs[bus->addr];
+    if (phy && phy->write) {
+        phy->write(phy, req, data);
+    }
+}
+
+void mdio_cycle(struct qemu_mdio *bus)
+{
+    bus->cnt++;
+
+    D(printf("mdc=%d mdio=%d state=%d cnt=%d drv=%d\n",
+             bus->mdc, bus->mdio, bus->state, bus->cnt, bus->drive));
+    switch (bus->state) {
+    case PREAMBLE:
+        if (bus->mdc) {
+            if (bus->cnt >= (32 * 2) && !bus->mdio) {
+                bus->cnt = 0;
+                bus->state = SOF;
+                bus->data = 0;
+            }
+        }
+        break;
+    case SOF:
+        if (bus->mdc) {
+            if (bus->mdio != 1) {
+                printf("WARNING: no SOF\n");
+            }
+            if (bus->cnt == 1 * 2) {
+                bus->cnt = 0;
+                bus->opc = 0;
+                bus->state = OPC;
+            }
+        }
+        break;
+    case OPC:
+        if (bus->mdc) {
+            bus->opc <<= 1;
+            bus->opc |= bus->mdio & 1;
+            if (bus->cnt == 2 * 2) {
+                bus->cnt = 0;
+                bus->addr = 0;
+                bus->state = ADDR;
+            }
+        }
+        break;
+    case ADDR:
+        if (bus->mdc) {
+            bus->addr <<= 1;
+            bus->addr |= bus->mdio & 1;
+
+            if (bus->cnt == 5 * 2) {
+                bus->cnt = 0;
+                bus->req = 0;
+                bus->state = REQ;
+            }
+        }
+        break;
+    case REQ:
+        if (bus->mdc) {
+            bus->req <<= 1;
+            bus->req |= bus->mdio & 1;
+            if (bus->cnt == 5 * 2) {
+                bus->cnt = 0;
+                bus->state = TURNAROUND;
+            }
+        }
+        break;
+    case TURNAROUND:
+        if (bus->mdc && bus->cnt == 2 * 2) {
+            bus->mdio = 0;
+            bus->cnt = 0;
+
+            if (bus->opc == 2) {
+                bus->drive = 1;
+                bus->data = mdio_read_req(bus, bus->addr, bus->req);
+                bus->mdio = bus->data & 1;
+            }
+            bus->state = DATA;
+        }
+        break;
+    case DATA:
+        if (!bus->mdc) {
+            if (bus->drive) {
+                bus->mdio = !!(bus->data & (1 << 15));
+                bus->data <<= 1;
+            }
+        } else {
+            if (!bus->drive) {
+                bus->data <<= 1;
+                bus->data |= bus->mdio;
+            }
+            if (bus->cnt == 16 * 2) {
+                bus->cnt = 0;
+                bus->state = PREAMBLE;
+                if (!bus->drive) {
+                    mdio_write_req(bus, bus->addr, bus->req, bus->data);
+                }
+                bus->drive = 0;
+            }
+        }
+        break;
+    default:
+        break;
+    }
+}
diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index d4c2c89dc1..1e859fdaae 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -26,13 +26,12 @@
 #include "hw/sysbus.h"
 #include "qapi/error.h"
 #include "qemu/log.h"
+#include "hw/net/mdio.h"
 #include "net/net.h"
 #include "net/checksum.h"
 
 #include "hw/stream.h"
 
-#define DPHY(x)
-
 #define TYPE_XILINX_AXI_ENET "xlnx.axi-ethernet"
 #define TYPE_XILINX_AXI_ENET_DATA_STREAM "xilinx-axienet-data-stream"
 #define TYPE_XILINX_AXI_ENET_CONTROL_STREAM "xilinx-axienet-control-stream"
@@ -48,189 +47,9 @@
      OBJECT_CHECK(XilinxAXIEnetStreamSlave, (obj),\
      TYPE_XILINX_AXI_ENET_CONTROL_STREAM)
 
-/* Advertisement control register. */
-#define ADVERTISE_10HALF        0x0020  /* Try for 10mbps half-duplex  */
-#define ADVERTISE_10FULL        0x0040  /* Try for 10mbps full-duplex  */
-#define ADVERTISE_100HALF       0x0080  /* Try for 100mbps half-duplex */
-#define ADVERTISE_100FULL       0x0100  /* Try for 100mbps full-duplex */
-
 #define CONTROL_PAYLOAD_WORDS 5
 #define CONTROL_PAYLOAD_SIZE (CONTROL_PAYLOAD_WORDS * (sizeof(uint32_t)))
 
-struct PHY {
-    uint32_t regs[32];
-
-    int link;
-
-    unsigned int (*read)(struct PHY *phy, unsigned int req);
-    void (*write)(struct PHY *phy, unsigned int req,
-                  unsigned int data);
-};
-
-static unsigned int tdk_read(struct PHY *phy, unsigned int req)
-{
-    int regnum;
-    unsigned r = 0;
-
-    regnum = req & 0x1f;
-
-    switch (regnum) {
-        case 1:
-            if (!phy->link) {
-                break;
-            }
-            /* MR1.  */
-            /* Speeds and modes.  */
-            r |= (1 << 13) | (1 << 14);
-            r |= (1 << 11) | (1 << 12);
-            r |= (1 << 5); /* Autoneg complete.  */
-            r |= (1 << 3); /* Autoneg able.  */
-            r |= (1 << 2); /* link.  */
-            r |= (1 << 1); /* link.  */
-            break;
-        case 5:
-            /* Link partner ability.
-               We are kind; always agree with whatever best mode
-               the guest advertises.  */
-            r = 1 << 14; /* Success.  */
-            /* Copy advertised modes.  */
-            r |= phy->regs[4] & (15 << 5);
-            /* Autoneg support.  */
-            r |= 1;
-            break;
-        case 17:
-            /* Marvell PHY on many xilinx boards.  */
-            r = 0x8000; /* 1000Mb  */
-            break;
-        case 18:
-            {
-                /* Diagnostics reg.  */
-                int duplex = 0;
-                int speed_100 = 0;
-
-                if (!phy->link) {
-                    break;
-                }
-
-                /* Are we advertising 100 half or 100 duplex ? */
-                speed_100 = !!(phy->regs[4] & ADVERTISE_100HALF);
-                speed_100 |= !!(phy->regs[4] & ADVERTISE_100FULL);
-
-                /* Are we advertising 10 duplex or 100 duplex ? */
-                duplex = !!(phy->regs[4] & ADVERTISE_100FULL);
-                duplex |= !!(phy->regs[4] & ADVERTISE_10FULL);
-                r = (speed_100 << 10) | (duplex << 11);
-            }
-            break;
-
-        default:
-            r = phy->regs[regnum];
-            break;
-    }
-    DPHY(qemu_log("\n%s %x = reg[%d]\n", __func__, r, regnum));
-    return r;
-}
-
-static void
-tdk_write(struct PHY *phy, unsigned int req, unsigned int data)
-{
-    int regnum;
-
-    regnum = req & 0x1f;
-    DPHY(qemu_log("%s reg[%d] = %x\n", __func__, regnum, data));
-    switch (regnum) {
-        default:
-            phy->regs[regnum] = data;
-            break;
-    }
-
-    /* Unconditionally clear regs[BMCR][BMCR_RESET] */
-    phy->regs[0] &= ~0x8000;
-}
-
-static void
-tdk_init(struct PHY *phy)
-{
-    phy->regs[0] = 0x3100;
-    /* PHY Id.  */
-    phy->regs[2] = 0x0300;
-    phy->regs[3] = 0xe400;
-    /* Autonegotiation advertisement reg.  */
-    phy->regs[4] = 0x01E1;
-    phy->link = 1;
-
-    phy->read = tdk_read;
-    phy->write = tdk_write;
-}
-
-struct MDIOBus {
-    /* bus.  */
-    int mdc;
-    int mdio;
-
-    /* decoder.  */
-    enum {
-        PREAMBLE,
-        SOF,
-        OPC,
-        ADDR,
-        REQ,
-        TURNAROUND,
-        DATA
-    } state;
-    unsigned int drive;
-
-    unsigned int cnt;
-    unsigned int addr;
-    unsigned int opc;
-    unsigned int req;
-    unsigned int data;
-
-    struct PHY *devs[32];
-};
-
-static void
-mdio_attach(struct MDIOBus *bus, struct PHY *phy, unsigned int addr)
-{
-    bus->devs[addr & 0x1f] = phy;
-}
-
-#ifdef USE_THIS_DEAD_CODE
-static void
-mdio_detach(struct MDIOBus *bus, struct PHY *phy, unsigned int addr)
-{
-    bus->devs[addr & 0x1f] = NULL;
-}
-#endif
-
-static uint16_t mdio_read_req(struct MDIOBus *bus, unsigned int addr,
-                  unsigned int reg)
-{
-    struct PHY *phy;
-    uint16_t data;
-
-    phy = bus->devs[addr];
-    if (phy && phy->read) {
-        data = phy->read(phy, reg);
-    } else {
-        data = 0xffff;
-    }
-    DPHY(qemu_log("%s addr=%d reg=%d data=%x\n", __func__, addr, reg, data));
-    return data;
-}
-
-static void mdio_write_req(struct MDIOBus *bus, unsigned int addr,
-               unsigned int reg, uint16_t data)
-{
-    struct PHY *phy;
-
-    DPHY(qemu_log("%s addr=%d reg=%d data=%x\n", __func__, addr, reg, data));
-    phy = bus->devs[addr];
-    if (phy && phy->write) {
-        phy->write(phy, reg, data);
-    }
-}
-
 #define DENET(x)
 
 #define R_RAF      (0x000 / 4)
@@ -322,8 +141,8 @@ enum {
 
 /* Indirect registers.  */
 struct TEMAC  {
-    struct MDIOBus mdio_bus;
-    struct PHY phy;
+    struct qemu_mdio mdio_bus;
+    struct qemu_phy phy;
 
     void *parent;
 };
diff --git a/hw/net/Makefile.objs b/hw/net/Makefile.objs
index 4171af0b5d..a020963d10 100644
--- a/hw/net/Makefile.objs
+++ b/hw/net/Makefile.objs
@@ -30,6 +30,8 @@ common-obj-$(CONFIG_SUNHME) += sunhme.o
 common-obj-$(CONFIG_FTGMAC100) += ftgmac100.o
 common-obj-$(CONFIG_SUNGEM) += sungem.o
 
+common-obj-y += mdio.o
+
 obj-$(CONFIG_ETRAXFS) += etraxfs_eth.o
 obj-$(CONFIG_COLDFIRE) += mcf_fec.o
 obj-$(CONFIG_MILKYMIST) += milkymist-minimac2.o
-- 
2.14.1

Re: [Qemu-arm] [Qemu-devel] [PATCH v5 1/7] hw/mdio: Generalize etraxfs MDIO bitbanging emulation

[Alistair Francis]

On Fri, Sep 22, 2017 at 10:13 AM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> From: Grant Likely <grant.likely@arm.com>
>
> The etraxfs and Xilinx axienet Ethernet models implement quite a nice
> MDIO core that supports both bitbanging and direct register access. This
> change factors the common code out into a separate file. There are no
> functional changes here, just movement of code.
>
> The etraxfs and axienet are slightly different. The etraxfs version
> includes the bitbang state processing, but the axienet version has a
> minor enhancement for read/write of phy registers without using bitbang
> state variables.  This patch generalizes the etraxfs version, with the
> axienet change backported in.
>
> Signed-off-by: Grant Likely <grant.likely@arm.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> [PMD: rebased with a minor checkpatch fix]
> ---
>  include/hw/net/mdio.h   |  76 +++++++++++++
>  hw/net/etraxfs_eth.c    | 278 +-----------------------------------------------
>  hw/net/mdio.c           | 262 +++++++++++++++++++++++++++++++++++++++++++++
>  hw/net/xilinx_axienet.c | 187 +-------------------------------
>  hw/net/Makefile.objs    |   2 +
>  5 files changed, 344 insertions(+), 461 deletions(-)
>  create mode 100644 include/hw/net/mdio.h
>  create mode 100644 hw/net/mdio.c
>
> diff --git a/include/hw/net/mdio.h b/include/hw/net/mdio.h
> new file mode 100644
> index 0000000000..ac36aed3c3
> --- /dev/null
> +++ b/include/hw/net/mdio.h
> @@ -0,0 +1,76 @@
> +#ifndef BITBANG_MDIO_H
> +#define BITBANG_MDIO_H
> +
> +/*
> + * QEMU Bitbang Ethernet MDIO bus & PHY controllers.
> + *
> + * Copyright (c) 2008 Edgar E. Iglesias, Axis Communications AB.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +/* PHY Advertisement control register */
> +#define PHY_ADVERTISE_10HALF    0x0020  /* Try for 10mbps half-duplex  */
> +#define PHY_ADVERTISE_10FULL    0x0040  /* Try for 10mbps full-duplex  */
> +#define PHY_ADVERTISE_100HALF   0x0080  /* Try for 100mbps half-duplex */
> +#define PHY_ADVERTISE_100FULL   0x0100  /* Try for 100mbps full-duplex */
> +
> +struct qemu_phy {
> +    uint32_t regs[32];
> +
> +    int link;
> +
> +    unsigned int (*read)(struct qemu_phy *phy, unsigned int req);
> +    void (*write)(struct qemu_phy *phy, unsigned int req, unsigned int data);
> +};
> +
> +struct qemu_mdio {
> +    /* bus. */
> +    int mdc;
> +    int mdio;
> +
> +    /* decoder.  */
> +    enum {
> +        PREAMBLE,
> +        SOF,
> +        OPC,
> +        ADDR,
> +        REQ,
> +        TURNAROUND,
> +        DATA
> +    } state;
> +    unsigned int drive;
> +
> +    unsigned int cnt;
> +    unsigned int addr;
> +    unsigned int opc;
> +    unsigned int req;
> +    unsigned int data;
> +
> +    struct qemu_phy *devs[32];
> +};
> +
> +void tdk_init(struct qemu_phy *phy);
> +void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy,
> +                 unsigned int addr);
> +uint16_t mdio_read_req(struct qemu_mdio *bus, uint8_t addr, uint8_t req);
> +void mdio_write_req(struct qemu_mdio *bus, uint8_t addr, uint8_t req, uint16_t data);
> +void mdio_cycle(struct qemu_mdio *bus);
> +
> +#endif
> diff --git a/hw/net/etraxfs_eth.c b/hw/net/etraxfs_eth.c
> index 013c8d0a41..f8d8f8441d 100644
> --- a/hw/net/etraxfs_eth.c
> +++ b/hw/net/etraxfs_eth.c
> @@ -26,287 +26,11 @@
>  #include "hw/sysbus.h"
>  #include "net/net.h"
>  #include "hw/cris/etraxfs.h"
> +#include "hw/net/mdio.h"
>  #include "qemu/error-report.h"
>
>  #define D(x)
>
> -/* Advertisement control register. */
> -#define ADVERTISE_10HALF        0x0020  /* Try for 10mbps half-duplex  */
> -#define ADVERTISE_10FULL        0x0040  /* Try for 10mbps full-duplex  */
> -#define ADVERTISE_100HALF       0x0080  /* Try for 100mbps half-duplex */
> -#define ADVERTISE_100FULL       0x0100  /* Try for 100mbps full-duplex */
> -
> -/*
> - * The MDIO extensions in the TDK PHY model were reversed engineered from the
> - * linux driver (PHYID and Diagnostics reg).
> - * TODO: Add friendly names for the register nums.
> - */
> -struct qemu_phy
> -{
> -    uint32_t regs[32];
> -
> -    int link;
> -
> -    unsigned int (*read)(struct qemu_phy *phy, unsigned int req);
> -    void (*write)(struct qemu_phy *phy, unsigned int req, unsigned int data);
> -};
> -
> -static unsigned int tdk_read(struct qemu_phy *phy, unsigned int req)
> -{
> -    int regnum;
> -    unsigned r = 0;
> -
> -    regnum = req & 0x1f;
> -
> -    switch (regnum) {
> -    case 1:
> -        if (!phy->link) {
> -            break;
> -        }
> -        /* MR1.     */
> -        /* Speeds and modes.  */
> -        r |= (1 << 13) | (1 << 14);
> -        r |= (1 << 11) | (1 << 12);
> -        r |= (1 << 5); /* Autoneg complete.  */
> -        r |= (1 << 3); /* Autoneg able.     */
> -        r |= (1 << 2); /* link.     */
> -        break;
> -    case 5:
> -        /* Link partner ability.
> -           We are kind; always agree with whatever best mode
> -           the guest advertises.  */
> -        r = 1 << 14; /* Success.  */
> -        /* Copy advertised modes.  */
> -        r |= phy->regs[4] & (15 << 5);
> -        /* Autoneg support.  */
> -        r |= 1;
> -        break;
> -    case 18:
> -    {
> -        /* Diagnostics reg.  */
> -        int duplex = 0;
> -        int speed_100 = 0;
> -
> -        if (!phy->link) {
> -            break;
> -        }
> -
> -        /* Are we advertising 100 half or 100 duplex ? */
> -        speed_100 = !!(phy->regs[4] & ADVERTISE_100HALF);
> -        speed_100 |= !!(phy->regs[4] & ADVERTISE_100FULL);
> -
> -        /* Are we advertising 10 duplex or 100 duplex ? */
> -        duplex = !!(phy->regs[4] & ADVERTISE_100FULL);
> -        duplex |= !!(phy->regs[4] & ADVERTISE_10FULL);
> -        r = (speed_100 << 10) | (duplex << 11);
> -    }
> -    break;
> -
> -    default:
> -        r = phy->regs[regnum];
> -        break;
> -    }
> -    D(printf("\n%s %x = reg[%d]\n", __func__, r, regnum));
> -    return r;
> -}
> -
> -static void
> -tdk_write(struct qemu_phy *phy, unsigned int req, unsigned int data)
> -{
> -    int regnum;
> -
> -    regnum = req & 0x1f;
> -    D(printf("%s reg[%d] = %x\n", __func__, regnum, data));
> -    switch (regnum) {
> -    default:
> -        phy->regs[regnum] = data;
> -        break;
> -    }
> -}
> -
> -static void
> -tdk_init(struct qemu_phy *phy)
> -{
> -    phy->regs[0] = 0x3100;
> -    /* PHY Id.  */
> -    phy->regs[2] = 0x0300;
> -    phy->regs[3] = 0xe400;
> -    /* Autonegotiation advertisement reg.  */
> -    phy->regs[4] = 0x01E1;
> -    phy->link = 1;
> -
> -    phy->read = tdk_read;
> -    phy->write = tdk_write;
> -}
> -
> -struct qemu_mdio
> -{
> -    /* bus.     */
> -    int mdc;
> -    int mdio;
> -
> -    /* decoder.  */
> -    enum {
> -        PREAMBLE,
> -        SOF,
> -        OPC,
> -        ADDR,
> -        REQ,
> -        TURNAROUND,
> -        DATA
> -    } state;
> -    unsigned int drive;
> -
> -    unsigned int cnt;
> -    unsigned int addr;
> -    unsigned int opc;
> -    unsigned int req;
> -    unsigned int data;
> -
> -    struct qemu_phy *devs[32];
> -};
> -
> -static void
> -mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy, unsigned int addr)
> -{
> -    bus->devs[addr & 0x1f] = phy;
> -}
> -
> -#ifdef USE_THIS_DEAD_CODE
> -static void
> -mdio_detach(struct qemu_mdio *bus, struct qemu_phy *phy, unsigned int addr)
> -{
> -    bus->devs[addr & 0x1f] = NULL;
> -}
> -#endif
> -
> -static void mdio_read_req(struct qemu_mdio *bus)
> -{
> -    struct qemu_phy *phy;
> -
> -    phy = bus->devs[bus->addr];
> -    if (phy && phy->read) {
> -        bus->data = phy->read(phy, bus->req);
> -    } else {
> -        bus->data = 0xffff;
> -    }
> -}
> -
> -static void mdio_write_req(struct qemu_mdio *bus)
> -{
> -    struct qemu_phy *phy;
> -
> -    phy = bus->devs[bus->addr];
> -    if (phy && phy->write) {
> -        phy->write(phy, bus->req, bus->data);
> -    }
> -}
> -
> -static void mdio_cycle(struct qemu_mdio *bus)
> -{
> -    bus->cnt++;
> -
> -    D(printf("mdc=%d mdio=%d state=%d cnt=%d drv=%d\n",
> -        bus->mdc, bus->mdio, bus->state, bus->cnt, bus->drive));
> -#if 0
> -    if (bus->mdc) {
> -        printf("%d", bus->mdio);
> -    }
> -#endif
> -    switch (bus->state) {
> -    case PREAMBLE:
> -        if (bus->mdc) {
> -            if (bus->cnt >= (32 * 2) && !bus->mdio) {
> -                bus->cnt = 0;
> -                bus->state = SOF;
> -                bus->data = 0;
> -            }
> -        }
> -        break;
> -    case SOF:
> -        if (bus->mdc) {
> -            if (bus->mdio != 1) {
> -                printf("WARNING: no SOF\n");
> -            }
> -            if (bus->cnt == 1*2) {
> -                bus->cnt = 0;
> -                bus->opc = 0;
> -                bus->state = OPC;
> -            }
> -        }
> -        break;
> -    case OPC:
> -        if (bus->mdc) {
> -            bus->opc <<= 1;
> -            bus->opc |= bus->mdio & 1;
> -            if (bus->cnt == 2*2) {
> -                bus->cnt = 0;
> -                bus->addr = 0;
> -                bus->state = ADDR;
> -            }
> -        }
> -        break;
> -    case ADDR:
> -        if (bus->mdc) {
> -            bus->addr <<= 1;
> -            bus->addr |= bus->mdio & 1;
> -
> -            if (bus->cnt == 5*2) {
> -                bus->cnt = 0;
> -                bus->req = 0;
> -                bus->state = REQ;
> -            }
> -        }
> -        break;
> -    case REQ:
> -        if (bus->mdc) {
> -            bus->req <<= 1;
> -            bus->req |= bus->mdio & 1;
> -            if (bus->cnt == 5*2) {
> -                bus->cnt = 0;
> -                bus->state = TURNAROUND;
> -            }
> -        }
> -        break;
> -    case TURNAROUND:
> -        if (bus->mdc && bus->cnt == 2*2) {
> -            bus->mdio = 0;
> -            bus->cnt = 0;
> -
> -            if (bus->opc == 2) {
> -                bus->drive = 1;
> -                mdio_read_req(bus);
> -                bus->mdio = bus->data & 1;
> -            }
> -            bus->state = DATA;
> -        }
> -        break;
> -    case DATA:
> -        if (!bus->mdc) {
> -            if (bus->drive) {
> -                bus->mdio = !!(bus->data & (1 << 15));
> -                bus->data <<= 1;
> -            }
> -        } else {
> -            if (!bus->drive) {
> -                bus->data <<= 1;
> -                bus->data |= bus->mdio;
> -            }
> -            if (bus->cnt == 16 * 2) {
> -                bus->cnt = 0;
> -                bus->state = PREAMBLE;
> -                if (!bus->drive) {
> -                    mdio_write_req(bus);
> -                }
> -                bus->drive = 0;
> -            }
> -        }
> -        break;
> -    default:
> -        break;
> -    }
> -}
> -
>  /* ETRAX-FS Ethernet MAC block starts here.  */
>
>  #define RW_MA0_LO      0x00
> diff --git a/hw/net/mdio.c b/hw/net/mdio.c
> new file mode 100644
> index 0000000000..3763fcc8af
> --- /dev/null
> +++ b/hw/net/mdio.c
> @@ -0,0 +1,262 @@
> +/*
> + * QEMU Ethernet MDIO bus & PHY models
> + *
> + * Copyright (c) 2008 Edgar E. Iglesias, Axis Communications AB.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + *
> + * This is a generic MDIO implementation.
> + *
> + * TODO:
> + * - Split PHYs out as separate device models so they can be defined and
> + *   instantiated separately from the MDIO bus.
> + * - Split out bitbang state machine into a separate model. Mostly this consists
> + *   of the mdio_cycle() routine and the bitbang state data in struct qemu_mdio
> + * - Use the GPIO interface for driving bitbang
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "qemu/log.h"
> +#include "hw/net/mdio.h"
> +
> +#define D(x)

This should be updated to the new way to handle QEMU logging.

Otherwise this patch looks fine to me.

Alistair

> +
> +/*
> + * The MDIO extensions in the TDK PHY model were reversed engineered from the
> + * linux driver (PHYID and Diagnostics reg).
> + * TODO: Add friendly names for the register nums.
> + */
> +static unsigned int tdk_read(struct qemu_phy *phy, unsigned int req)
> +{
> +    int regnum;
> +    unsigned r = 0;
> +
> +    regnum = req & 0x1f;
> +
> +    switch (regnum) {
> +    case 1:
> +        if (!phy->link) {
> +            break;
> +        }
> +        /* MR1.     */
> +        /* Speeds and modes.  */
> +        r |= (1 << 13) | (1 << 14);
> +        r |= (1 << 11) | (1 << 12);
> +        r |= (1 << 5); /* Autoneg complete.  */
> +        r |= (1 << 3); /* Autoneg able.     */
> +        r |= (1 << 2); /* link.     */
> +        r |= (1 << 1); /* link.     */
> +        break;
> +    case 5:
> +        /* Link partner ability.
> +           We are kind; always agree with whatever best mode
> +           the guest advertises.  */
> +        r = 1 << 14; /* Success.  */
> +        /* Copy advertised modes.  */
> +        r |= phy->regs[4] & (15 << 5);
> +        /* Autoneg support.  */
> +        r |= 1;
> +        break;
> +    case 17:
> +        /* Marvel PHY on many xilinx boards. */
> +        r = 0x8000; /* 1000Mb */
> +        break;
> +    case 18:
> +    {
> +        /* Diagnostics reg.  */
> +        int duplex = 0;
> +        int speed_100 = 0;
> +
> +        if (!phy->link) {
> +            break;
> +        }
> +
> +        /* Are we advertising 100 half or 100 duplex ? */
> +        speed_100 = !!(phy->regs[4] & PHY_ADVERTISE_100HALF);
> +        speed_100 |= !!(phy->regs[4] & PHY_ADVERTISE_100FULL);
> +
> +        /* Are we advertising 10 duplex or 100 duplex ? */
> +        duplex = !!(phy->regs[4] & PHY_ADVERTISE_100FULL);
> +        duplex |= !!(phy->regs[4] & PHY_ADVERTISE_10FULL);
> +        r = (speed_100 << 10) | (duplex << 11);
> +    }
> +    break;
> +
> +    default:
> +        r = phy->regs[regnum];
> +        break;
> +    }
> +    D(printf("\n%s %x = reg[%d]\n", __func__, r, regnum));
> +    return r;
> +}
> +
> +static void tdk_write(struct qemu_phy *phy, unsigned int req, unsigned int data)
> +{
> +    int regnum;
> +
> +    regnum = req & 0x1f;
> +    D(printf("%s reg[%d] = %x\n", __func__, regnum, data));
> +    switch (regnum) {
> +    default:
> +        phy->regs[regnum] = data;
> +        break;
> +    }
> +}
> +
> +void tdk_init(struct qemu_phy *phy)
> +{
> +    phy->regs[0] = 0x3100;
> +    /* PHY Id. */
> +    phy->regs[2] = 0x0300;
> +    phy->regs[3] = 0xe400;
> +    /* Autonegotiation advertisement reg. */
> +    phy->regs[4] = 0x01e1;
> +    phy->link = 1;
> +
> +    phy->read = tdk_read;
> +    phy->write = tdk_write;
> +}
> +
> +void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy, unsigned int addr)
> +{
> +    bus->devs[addr & 0x1f] = phy;
> +}
> +
> +uint16_t mdio_read_req(struct qemu_mdio *bus, uint8_t addr, uint8_t req)
> +{
> +    struct qemu_phy *phy;
> +
> +    phy = bus->devs[bus->addr];
> +    if (phy && phy->read) {
> +        return phy->read(phy, req);
> +    }
> +    return 0xffff;
> +}
> +
> +void mdio_write_req(struct qemu_mdio *bus, uint8_t addr, uint8_t req,
> +                    uint16_t data)
> +{
> +    struct qemu_phy *phy;
> +
> +    phy = bus->devs[bus->addr];
> +    if (phy && phy->write) {
> +        phy->write(phy, req, data);
> +    }
> +}
> +
> +void mdio_cycle(struct qemu_mdio *bus)
> +{
> +    bus->cnt++;
> +
> +    D(printf("mdc=%d mdio=%d state=%d cnt=%d drv=%d\n",
> +             bus->mdc, bus->mdio, bus->state, bus->cnt, bus->drive));
> +    switch (bus->state) {
> +    case PREAMBLE:
> +        if (bus->mdc) {
> +            if (bus->cnt >= (32 * 2) && !bus->mdio) {
> +                bus->cnt = 0;
> +                bus->state = SOF;
> +                bus->data = 0;
> +            }
> +        }
> +        break;
> +    case SOF:
> +        if (bus->mdc) {
> +            if (bus->mdio != 1) {
> +                printf("WARNING: no SOF\n");
> +            }
> +            if (bus->cnt == 1 * 2) {
> +                bus->cnt = 0;
> +                bus->opc = 0;
> +                bus->state = OPC;
> +            }
> +        }
> +        break;
> +    case OPC:
> +        if (bus->mdc) {
> +            bus->opc <<= 1;
> +            bus->opc |= bus->mdio & 1;
> +            if (bus->cnt == 2 * 2) {
> +                bus->cnt = 0;
> +                bus->addr = 0;
> +                bus->state = ADDR;
> +            }
> +        }
> +        break;
> +    case ADDR:
> +        if (bus->mdc) {
> +            bus->addr <<= 1;
> +            bus->addr |= bus->mdio & 1;
> +
> +            if (bus->cnt == 5 * 2) {
> +                bus->cnt = 0;
> +                bus->req = 0;
> +                bus->state = REQ;
> +            }
> +        }
> +        break;
> +    case REQ:
> +        if (bus->mdc) {
> +            bus->req <<= 1;
> +            bus->req |= bus->mdio & 1;
> +            if (bus->cnt == 5 * 2) {
> +                bus->cnt = 0;
> +                bus->state = TURNAROUND;
> +            }
> +        }
> +        break;
> +    case TURNAROUND:
> +        if (bus->mdc && bus->cnt == 2 * 2) {
> +            bus->mdio = 0;
> +            bus->cnt = 0;
> +
> +            if (bus->opc == 2) {
> +                bus->drive = 1;
> +                bus->data = mdio_read_req(bus, bus->addr, bus->req);
> +                bus->mdio = bus->data & 1;
> +            }
> +            bus->state = DATA;
> +        }
> +        break;
> +    case DATA:
> +        if (!bus->mdc) {
> +            if (bus->drive) {
> +                bus->mdio = !!(bus->data & (1 << 15));
> +                bus->data <<= 1;
> +            }
> +        } else {
> +            if (!bus->drive) {
> +                bus->data <<= 1;
> +                bus->data |= bus->mdio;
> +            }
> +            if (bus->cnt == 16 * 2) {
> +                bus->cnt = 0;
> +                bus->state = PREAMBLE;
> +                if (!bus->drive) {
> +                    mdio_write_req(bus, bus->addr, bus->req, bus->data);
> +                }
> +                bus->drive = 0;
> +            }
> +        }
> +        break;
> +    default:
> +        break;
> +    }
> +}
> diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
> index d4c2c89dc1..1e859fdaae 100644
> --- a/hw/net/xilinx_axienet.c
> +++ b/hw/net/xilinx_axienet.c
> @@ -26,13 +26,12 @@
>  #include "hw/sysbus.h"
>  #include "qapi/error.h"
>  #include "qemu/log.h"
> +#include "hw/net/mdio.h"
>  #include "net/net.h"
>  #include "net/checksum.h"
>
>  #include "hw/stream.h"
>
> -#define DPHY(x)
> -
>  #define TYPE_XILINX_AXI_ENET "xlnx.axi-ethernet"
>  #define TYPE_XILINX_AXI_ENET_DATA_STREAM "xilinx-axienet-data-stream"
>  #define TYPE_XILINX_AXI_ENET_CONTROL_STREAM "xilinx-axienet-control-stream"
> @@ -48,189 +47,9 @@
>       OBJECT_CHECK(XilinxAXIEnetStreamSlave, (obj),\
>       TYPE_XILINX_AXI_ENET_CONTROL_STREAM)
>
> -/* Advertisement control register. */
> -#define ADVERTISE_10HALF        0x0020  /* Try for 10mbps half-duplex  */
> -#define ADVERTISE_10FULL        0x0040  /* Try for 10mbps full-duplex  */
> -#define ADVERTISE_100HALF       0x0080  /* Try for 100mbps half-duplex */
> -#define ADVERTISE_100FULL       0x0100  /* Try for 100mbps full-duplex */
> -
>  #define CONTROL_PAYLOAD_WORDS 5
>  #define CONTROL_PAYLOAD_SIZE (CONTROL_PAYLOAD_WORDS * (sizeof(uint32_t)))
>
> -struct PHY {
> -    uint32_t regs[32];
> -
> -    int link;
> -
> -    unsigned int (*read)(struct PHY *phy, unsigned int req);
> -    void (*write)(struct PHY *phy, unsigned int req,
> -                  unsigned int data);
> -};
> -
> -static unsigned int tdk_read(struct PHY *phy, unsigned int req)
> -{
> -    int regnum;
> -    unsigned r = 0;
> -
> -    regnum = req & 0x1f;
> -
> -    switch (regnum) {
> -        case 1:
> -            if (!phy->link) {
> -                break;
> -            }
> -            /* MR1.  */
> -            /* Speeds and modes.  */
> -            r |= (1 << 13) | (1 << 14);
> -            r |= (1 << 11) | (1 << 12);
> -            r |= (1 << 5); /* Autoneg complete.  */
> -            r |= (1 << 3); /* Autoneg able.  */
> -            r |= (1 << 2); /* link.  */
> -            r |= (1 << 1); /* link.  */
> -            break;
> -        case 5:
> -            /* Link partner ability.
> -               We are kind; always agree with whatever best mode
> -               the guest advertises.  */
> -            r = 1 << 14; /* Success.  */
> -            /* Copy advertised modes.  */
> -            r |= phy->regs[4] & (15 << 5);
> -            /* Autoneg support.  */
> -            r |= 1;
> -            break;
> -        case 17:
> -            /* Marvell PHY on many xilinx boards.  */
> -            r = 0x8000; /* 1000Mb  */
> -            break;
> -        case 18:
> -            {
> -                /* Diagnostics reg.  */
> -                int duplex = 0;
> -                int speed_100 = 0;
> -
> -                if (!phy->link) {
> -                    break;
> -                }
> -
> -                /* Are we advertising 100 half or 100 duplex ? */
> -                speed_100 = !!(phy->regs[4] & ADVERTISE_100HALF);
> -                speed_100 |= !!(phy->regs[4] & ADVERTISE_100FULL);
> -
> -                /* Are we advertising 10 duplex or 100 duplex ? */
> -                duplex = !!(phy->regs[4] & ADVERTISE_100FULL);
> -                duplex |= !!(phy->regs[4] & ADVERTISE_10FULL);
> -                r = (speed_100 << 10) | (duplex << 11);
> -            }
> -            break;
> -
> -        default:
> -            r = phy->regs[regnum];
> -            break;
> -    }
> -    DPHY(qemu_log("\n%s %x = reg[%d]\n", __func__, r, regnum));
> -    return r;
> -}
> -
> -static void
> -tdk_write(struct PHY *phy, unsigned int req, unsigned int data)
> -{
> -    int regnum;
> -
> -    regnum = req & 0x1f;
> -    DPHY(qemu_log("%s reg[%d] = %x\n", __func__, regnum, data));
> -    switch (regnum) {
> -        default:
> -            phy->regs[regnum] = data;
> -            break;
> -    }
> -
> -    /* Unconditionally clear regs[BMCR][BMCR_RESET] */
> -    phy->regs[0] &= ~0x8000;
> -}
> -
> -static void
> -tdk_init(struct PHY *phy)
> -{
> -    phy->regs[0] = 0x3100;
> -    /* PHY Id.  */
> -    phy->regs[2] = 0x0300;
> -    phy->regs[3] = 0xe400;
> -    /* Autonegotiation advertisement reg.  */
> -    phy->regs[4] = 0x01E1;
> -    phy->link = 1;
> -
> -    phy->read = tdk_read;
> -    phy->write = tdk_write;
> -}
> -
> -struct MDIOBus {
> -    /* bus.  */
> -    int mdc;
> -    int mdio;
> -
> -    /* decoder.  */
> -    enum {
> -        PREAMBLE,
> -        SOF,
> -        OPC,
> -        ADDR,
> -        REQ,
> -        TURNAROUND,
> -        DATA
> -    } state;
> -    unsigned int drive;
> -
> -    unsigned int cnt;
> -    unsigned int addr;
> -    unsigned int opc;
> -    unsigned int req;
> -    unsigned int data;
> -
> -    struct PHY *devs[32];
> -};
> -
> -static void
> -mdio_attach(struct MDIOBus *bus, struct PHY *phy, unsigned int addr)
> -{
> -    bus->devs[addr & 0x1f] = phy;
> -}
> -
> -#ifdef USE_THIS_DEAD_CODE
> -static void
> -mdio_detach(struct MDIOBus *bus, struct PHY *phy, unsigned int addr)
> -{
> -    bus->devs[addr & 0x1f] = NULL;
> -}
> -#endif
> -
> -static uint16_t mdio_read_req(struct MDIOBus *bus, unsigned int addr,
> -                  unsigned int reg)
> -{
> -    struct PHY *phy;
> -    uint16_t data;
> -
> -    phy = bus->devs[addr];
> -    if (phy && phy->read) {
> -        data = phy->read(phy, reg);
> -    } else {
> -        data = 0xffff;
> -    }
> -    DPHY(qemu_log("%s addr=%d reg=%d data=%x\n", __func__, addr, reg, data));
> -    return data;
> -}
> -
> -static void mdio_write_req(struct MDIOBus *bus, unsigned int addr,
> -               unsigned int reg, uint16_t data)
> -{
> -    struct PHY *phy;
> -
> -    DPHY(qemu_log("%s addr=%d reg=%d data=%x\n", __func__, addr, reg, data));
> -    phy = bus->devs[addr];
> -    if (phy && phy->write) {
> -        phy->write(phy, reg, data);
> -    }
> -}
> -
>  #define DENET(x)
>
>  #define R_RAF      (0x000 / 4)
> @@ -322,8 +141,8 @@ enum {
>
>  /* Indirect registers.  */
>  struct TEMAC  {
> -    struct MDIOBus mdio_bus;
> -    struct PHY phy;
> +    struct qemu_mdio mdio_bus;
> +    struct qemu_phy phy;
>
>      void *parent;
>  };
> diff --git a/hw/net/Makefile.objs b/hw/net/Makefile.objs
> index 4171af0b5d..a020963d10 100644
> --- a/hw/net/Makefile.objs
> +++ b/hw/net/Makefile.objs
> @@ -30,6 +30,8 @@ common-obj-$(CONFIG_SUNHME) += sunhme.o
>  common-obj-$(CONFIG_FTGMAC100) += ftgmac100.o
>  common-obj-$(CONFIG_SUNGEM) += sungem.o
>
> +common-obj-y += mdio.o
> +
>  obj-$(CONFIG_ETRAXFS) += etraxfs_eth.o
>  obj-$(CONFIG_COLDFIRE) += mcf_fec.o
>  obj-$(CONFIG_MILKYMIST) += milkymist-minimac2.o
> --
> 2.14.1
>
>

[Qemu-devel] [PATCH v5 3/7] hw/mdio: Generalize phy initialization routine

[Philippe Mathieu-Daudé]

From: Grant Likely <grant.likely@arm.com>

There really isn't anything tdk-specific about tdk_init() other than the
phy id registers. The function should instead be generalized for any
phy, at least as far as the ID registers are concerned. For the most
part the read/write behaviour should be very similar across PHYs.

This patch renames tdk_{read,write,init}() to mdio_phy_*() so it can be
used for any PHY.

More work definitely needs to be done here to make it easy to override
the default behaviour for specific PHYs, but this at least is a
reasonable start.

Signed-off-by: Grant Likely <grant.likely@arm.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
[PMD: just rebased]
---
 include/hw/net/mdio.h   |  2 +-
 hw/net/etraxfs_eth.c    |  2 +-
 hw/net/mdio.c           | 14 +++++++-------
 hw/net/xilinx_axienet.c |  2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/hw/net/mdio.h b/include/hw/net/mdio.h
index 7ffa4389b9..b3b4f497c0 100644
--- a/include/hw/net/mdio.h
+++ b/include/hw/net/mdio.h
@@ -86,7 +86,7 @@ struct qemu_mdio {
     struct qemu_phy *devs[32];
 };
 
-void tdk_init(struct qemu_phy *phy);
+void mdio_phy_init(struct qemu_phy *phy, uint16_t id1, uint16_t id2);
 void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy,
                  unsigned int addr);
 uint16_t mdio_read_req(struct qemu_mdio *bus, uint8_t addr, uint8_t req);
diff --git a/hw/net/etraxfs_eth.c b/hw/net/etraxfs_eth.c
index f8d8f8441d..4c5415771f 100644
--- a/hw/net/etraxfs_eth.c
+++ b/hw/net/etraxfs_eth.c
@@ -333,7 +333,7 @@ static int fs_eth_init(SysBusDevice *sbd)
     qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
 
 
-    tdk_init(&s->phy);
+    mdio_phy_init(&s->phy, 0x0300, 0xe400);
     mdio_attach(&s->mdio_bus, &s->phy, s->phyaddr);
     return 0;
 }
diff --git a/hw/net/mdio.c b/hw/net/mdio.c
index 3d70d99077..33bfbb4623 100644
--- a/hw/net/mdio.c
+++ b/hw/net/mdio.c
@@ -43,7 +43,7 @@
  * linux driver (PHYID and Diagnostics reg).
  * TODO: Add friendly names for the register nums.
  */
-static unsigned int tdk_read(struct qemu_phy *phy, unsigned int req)
+static unsigned int mdio_phy_read(struct qemu_phy *phy, unsigned int req)
 {
     int regnum;
     unsigned r = 0;
@@ -107,7 +107,7 @@ static unsigned int tdk_read(struct qemu_phy *phy, unsigned int req)
     return r;
 }
 
-static void tdk_write(struct qemu_phy *phy, unsigned int req, unsigned int data)
+static void mdio_phy_write(struct qemu_phy *phy, unsigned int req, unsigned int data)
 {
     int regnum;
 
@@ -120,18 +120,18 @@ static void tdk_write(struct qemu_phy *phy, unsigned int req, unsigned int data)
     }
 }
 
-void tdk_init(struct qemu_phy *phy)
+void mdio_phy_init(struct qemu_phy *phy, uint16_t id1, uint16_t id2)
 {
     phy->regs[PHY_CTRL] = 0x3100;
     /* PHY Id. */
-    phy->regs[PHY_ID1] = 0x0300;
-    phy->regs[PHY_ID2] = 0xe400;
+    phy->regs[PHY_ID1] = id1;
+    phy->regs[PHY_ID2] = id2;
     /* Autonegotiation advertisement reg. */
     phy->regs[PHY_AUTONEG_ADV] = 0x01e1;
     phy->link = 1;
 
-    phy->read = tdk_read;
-    phy->write = tdk_write;
+    phy->read = mdio_phy_read;
+    phy->write = mdio_phy_write;
 }
 
 void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy, unsigned int addr)
diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index 1e859fdaae..408cd6e675 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -791,7 +791,7 @@ static void xilinx_enet_realize(DeviceState *dev, Error **errp)
                           object_get_typename(OBJECT(dev)), dev->id, s);
     qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
 
-    tdk_init(&s->TEMAC.phy);
+    mdio_phy_init(&s->TEMAC.phy, 0x0300, 0xe400);
     mdio_attach(&s->TEMAC.mdio_bus, &s->TEMAC.phy, s->c_phyaddr);
 
     s->TEMAC.parent = s;
-- 
2.14.1

Re: [Qemu-arm] [Qemu-devel] [PATCH v5 3/7] hw/mdio: Generalize phy initialization routine

[Alistair Francis]

On Fri, Sep 22, 2017 at 10:13 AM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> From: Grant Likely <grant.likely@arm.com>
>
> There really isn't anything tdk-specific about tdk_init() other than the
> phy id registers. The function should instead be generalized for any
> phy, at least as far as the ID registers are concerned. For the most
> part the read/write behaviour should be very similar across PHYs.
>
> This patch renames tdk_{read,write,init}() to mdio_phy_*() so it can be
> used for any PHY.
>
> More work definitely needs to be done here to make it easy to override
> the default behaviour for specific PHYs, but this at least is a
> reasonable start.
>
> Signed-off-by: Grant Likely <grant.likely@arm.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> [PMD: just rebased]
> ---
>  include/hw/net/mdio.h   |  2 +-
>  hw/net/etraxfs_eth.c    |  2 +-
>  hw/net/mdio.c           | 14 +++++++-------
>  hw/net/xilinx_axienet.c |  2 +-
>  4 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/include/hw/net/mdio.h b/include/hw/net/mdio.h
> index 7ffa4389b9..b3b4f497c0 100644
> --- a/include/hw/net/mdio.h
> +++ b/include/hw/net/mdio.h
> @@ -86,7 +86,7 @@ struct qemu_mdio {
>      struct qemu_phy *devs[32];
>  };
>
> -void tdk_init(struct qemu_phy *phy);
> +void mdio_phy_init(struct qemu_phy *phy, uint16_t id1, uint16_t id2);
>  void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy,
>                   unsigned int addr);
>  uint16_t mdio_read_req(struct qemu_mdio *bus, uint8_t addr, uint8_t req);
> diff --git a/hw/net/etraxfs_eth.c b/hw/net/etraxfs_eth.c
> index f8d8f8441d..4c5415771f 100644
> --- a/hw/net/etraxfs_eth.c
> +++ b/hw/net/etraxfs_eth.c
> @@ -333,7 +333,7 @@ static int fs_eth_init(SysBusDevice *sbd)
>      qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
>
>
> -    tdk_init(&s->phy);
> +    mdio_phy_init(&s->phy, 0x0300, 0xe400);
>      mdio_attach(&s->mdio_bus, &s->phy, s->phyaddr);
>      return 0;
>  }
> diff --git a/hw/net/mdio.c b/hw/net/mdio.c
> index 3d70d99077..33bfbb4623 100644
> --- a/hw/net/mdio.c
> +++ b/hw/net/mdio.c
> @@ -43,7 +43,7 @@
>   * linux driver (PHYID and Diagnostics reg).
>   * TODO: Add friendly names for the register nums.
>   */
> -static unsigned int tdk_read(struct qemu_phy *phy, unsigned int req)
> +static unsigned int mdio_phy_read(struct qemu_phy *phy, unsigned int req)
>  {
>      int regnum;
>      unsigned r = 0;
> @@ -107,7 +107,7 @@ static unsigned int tdk_read(struct qemu_phy *phy, unsigned int req)
>      return r;
>  }
>
> -static void tdk_write(struct qemu_phy *phy, unsigned int req, unsigned int data)
> +static void mdio_phy_write(struct qemu_phy *phy, unsigned int req, unsigned int data)
>  {
>      int regnum;
>
> @@ -120,18 +120,18 @@ static void tdk_write(struct qemu_phy *phy, unsigned int req, unsigned int data)
>      }
>  }
>
> -void tdk_init(struct qemu_phy *phy)
> +void mdio_phy_init(struct qemu_phy *phy, uint16_t id1, uint16_t id2)
>  {
>      phy->regs[PHY_CTRL] = 0x3100;
>      /* PHY Id. */
> -    phy->regs[PHY_ID1] = 0x0300;
> -    phy->regs[PHY_ID2] = 0xe400;
> +    phy->regs[PHY_ID1] = id1;
> +    phy->regs[PHY_ID2] = id2;

These should be set by QEMU properties instead of values to the init() function.

Alistair

>      /* Autonegotiation advertisement reg. */
>      phy->regs[PHY_AUTONEG_ADV] = 0x01e1;
>      phy->link = 1;
>
> -    phy->read = tdk_read;
> -    phy->write = tdk_write;
> +    phy->read = mdio_phy_read;
> +    phy->write = mdio_phy_write;
>  }
>
>  void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy, unsigned int addr)
> diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
> index 1e859fdaae..408cd6e675 100644
> --- a/hw/net/xilinx_axienet.c
> +++ b/hw/net/xilinx_axienet.c
> @@ -791,7 +791,7 @@ static void xilinx_enet_realize(DeviceState *dev, Error **errp)
>                            object_get_typename(OBJECT(dev)), dev->id, s);
>      qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
>
> -    tdk_init(&s->TEMAC.phy);
> +    mdio_phy_init(&s->TEMAC.phy, 0x0300, 0xe400);
>      mdio_attach(&s->TEMAC.mdio_bus, &s->TEMAC.phy, s->c_phyaddr);
>
>      s->TEMAC.parent = s;
> --
> 2.14.1
>
>

Re: [Qemu-arm] [Qemu-devel] [PATCH v5 3/7] hw/mdio: Generalize phy initialization routine

[Philippe Mathieu-Daudé]

Hi Alistair,

On 02/27/2018 07:33 PM, Alistair Francis wrote:
> On Fri, Sep 22, 2017 at 10:13 AM, Philippe Mathieu-Daudé
> <f4bug@amsat.org> wrote:
>> From: Grant Likely <grant.likely@arm.com>
>>
>> There really isn't anything tdk-specific about tdk_init() other than the
>> phy id registers. The function should instead be generalized for any
>> phy, at least as far as the ID registers are concerned. For the most
>> part the read/write behaviour should be very similar across PHYs.
>>
>> This patch renames tdk_{read,write,init}() to mdio_phy_*() so it can be
>> used for any PHY.
>>
>> More work definitely needs to be done here to make it easy to override
>> the default behaviour for specific PHYs, but this at least is a
>> reasonable start.
>>
>> Signed-off-by: Grant Likely <grant.likely@arm.com>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> [PMD: just rebased]
>> ---
>>  include/hw/net/mdio.h   |  2 +-
>>  hw/net/etraxfs_eth.c    |  2 +-
>>  hw/net/mdio.c           | 14 +++++++-------
>>  hw/net/xilinx_axienet.c |  2 +-
>>  4 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/hw/net/mdio.h b/include/hw/net/mdio.h
>> index 7ffa4389b9..b3b4f497c0 100644
>> --- a/include/hw/net/mdio.h
>> +++ b/include/hw/net/mdio.h
>> @@ -86,7 +86,7 @@ struct qemu_mdio {
>>      struct qemu_phy *devs[32];
>>  };
>>
>> -void tdk_init(struct qemu_phy *phy);
>> +void mdio_phy_init(struct qemu_phy *phy, uint16_t id1, uint16_t id2);
>>  void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy,
>>                   unsigned int addr);
>>  uint16_t mdio_read_req(struct qemu_mdio *bus, uint8_t addr, uint8_t req);
>> diff --git a/hw/net/etraxfs_eth.c b/hw/net/etraxfs_eth.c
>> index f8d8f8441d..4c5415771f 100644
>> --- a/hw/net/etraxfs_eth.c
>> +++ b/hw/net/etraxfs_eth.c
>> @@ -333,7 +333,7 @@ static int fs_eth_init(SysBusDevice *sbd)
>>      qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
>>
>>
>> -    tdk_init(&s->phy);
>> +    mdio_phy_init(&s->phy, 0x0300, 0xe400);
>>      mdio_attach(&s->mdio_bus, &s->phy, s->phyaddr);
>>      return 0;
>>  }
>> diff --git a/hw/net/mdio.c b/hw/net/mdio.c
>> index 3d70d99077..33bfbb4623 100644
>> --- a/hw/net/mdio.c
>> +++ b/hw/net/mdio.c
>> @@ -43,7 +43,7 @@
>>   * linux driver (PHYID and Diagnostics reg).
>>   * TODO: Add friendly names for the register nums.
>>   */
>> -static unsigned int tdk_read(struct qemu_phy *phy, unsigned int req)
>> +static unsigned int mdio_phy_read(struct qemu_phy *phy, unsigned int req)
>>  {
>>      int regnum;
>>      unsigned r = 0;
>> @@ -107,7 +107,7 @@ static unsigned int tdk_read(struct qemu_phy *phy, unsigned int req)
>>      return r;
>>  }
>>
>> -static void tdk_write(struct qemu_phy *phy, unsigned int req, unsigned int data)
>> +static void mdio_phy_write(struct qemu_phy *phy, unsigned int req, unsigned int data)
>>  {
>>      int regnum;
>>
>> @@ -120,18 +120,18 @@ static void tdk_write(struct qemu_phy *phy, unsigned int req, unsigned int data)
>>      }
>>  }
>>
>> -void tdk_init(struct qemu_phy *phy)
>> +void mdio_phy_init(struct qemu_phy *phy, uint16_t id1, uint16_t id2)
>>  {
>>      phy->regs[PHY_CTRL] = 0x3100;
>>      /* PHY Id. */
>> -    phy->regs[PHY_ID1] = 0x0300;
>> -    phy->regs[PHY_ID2] = 0xe400;
>> +    phy->regs[PHY_ID1] = id1;
>> +    phy->regs[PHY_ID2] = id2;
> 
> These should be set by QEMU properties instead of values to the init() function.

I agree, but this is not (yet) a QOM'ified device.
I believe if I start to QOM'ify it this series will finish in 1 more
year. This is however a nice cleanup so far, and the QOM device can come
in a follow up series.

> 
> Alistair
> 
>>      /* Autonegotiation advertisement reg. */
>>      phy->regs[PHY_AUTONEG_ADV] = 0x01e1;
>>      phy->link = 1;
>>
>> -    phy->read = tdk_read;
>> -    phy->write = tdk_write;
>> +    phy->read = mdio_phy_read;
>> +    phy->write = mdio_phy_write;
>>  }
>>
>>  void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy, unsigned int addr)
>> diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
>> index 1e859fdaae..408cd6e675 100644
>> --- a/hw/net/xilinx_axienet.c
>> +++ b/hw/net/xilinx_axienet.c
>> @@ -791,7 +791,7 @@ static void xilinx_enet_realize(DeviceState *dev, Error **errp)
>>                            object_get_typename(OBJECT(dev)), dev->id, s);
>>      qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
>>
>> -    tdk_init(&s->TEMAC.phy);
>> +    mdio_phy_init(&s->TEMAC.phy, 0x0300, 0xe400);
>>      mdio_attach(&s->TEMAC.mdio_bus, &s->TEMAC.phy, s->c_phyaddr);
>>
>>      s->TEMAC.parent = s;
>> --
>> 2.14.1
>>
>>

Re: [Qemu-arm] [Qemu-devel] [PATCH v5 0/7] Generalize MDIO framework

[Alistair Francis]

On Fri, Sep 22, 2017 at 10:13 AM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> Hi,
>
> I have a follow up series using multiples PHY on the MDIO bus based on this
> series.

I haven't looked at this yet, but it looks interesting.

We actually have this concept merged in our tree and it works pretty
well. You can see it here:
https://github.com/Xilinx/qemu/tree/master/hw/mdio

Thanks,
Alistair

>
> Regards,
>
> Phil.
>
> Grant's previous work:
> http://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg00257.html
>
> "There is more work to be done, particularly in moving to the common GPIO api,
>  but that work can be done as a follow on patch series."
>
> Grant Likely (7):
>   hw/mdio: Generalize etraxfs MDIO bitbanging emulation
>   hw/mdio: Add PHY register definition
>   hw/mdio: Generalize phy initialization routine
>   hw/mdio: Mask out read-only bits.
>   hw/mdio: Refactor bitbanging state machine
>   hw/mdio: Add VMState support
>   hw/mdio: Use bitbang core for smc91c111 network device
>
>  include/hw/net/mdio.h   | 124 +++++++++++++++++++++
>  hw/net/etraxfs_eth.c    | 291 +-----------------------------------------------
>  hw/net/mdio.c           | 280 ++++++++++++++++++++++++++++++++++++++++++++++
>  hw/net/smc91c111.c      |  27 ++++-
>  hw/net/xilinx_axienet.c | 189 +------------------------------
>  hw/net/Makefile.objs    |   2 +
>  6 files changed, 438 insertions(+), 475 deletions(-)
>  create mode 100644 include/hw/net/mdio.h
>  create mode 100644 hw/net/mdio.c
>
> --
> 2.14.1
>
>

Re: [Qemu-devel] [PATCH v5 0/7] Generalize MDIO framework

[Edgar E. Iglesias]

On Fri, Sep 22, 2017 at 02:13:16PM -0300, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> I have a follow up series using multiples PHY on the MDIO bus based on this
> series.

Hi Philippe!

I think this is a good improvement compared to todays state.
It may make sense to have the generic mdio bus functions in mdio.c
and specific phy models in separate files, thoughts?

Cheers,
Edgar


> 
> Regards,
> 
> Phil.
> 
> Grant's previous work:
> http://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg00257.html
> 
> "There is more work to be done, particularly in moving to the common GPIO api,
>  but that work can be done as a follow on patch series."
> 
> Grant Likely (7):
>   hw/mdio: Generalize etraxfs MDIO bitbanging emulation
>   hw/mdio: Add PHY register definition
>   hw/mdio: Generalize phy initialization routine
>   hw/mdio: Mask out read-only bits.
>   hw/mdio: Refactor bitbanging state machine
>   hw/mdio: Add VMState support
>   hw/mdio: Use bitbang core for smc91c111 network device
> 
>  include/hw/net/mdio.h   | 124 +++++++++++++++++++++
>  hw/net/etraxfs_eth.c    | 291 +-----------------------------------------------
>  hw/net/mdio.c           | 280 ++++++++++++++++++++++++++++++++++++++++++++++
>  hw/net/smc91c111.c      |  27 ++++-
>  hw/net/xilinx_axienet.c | 189 +------------------------------
>  hw/net/Makefile.objs    |   2 +
>  6 files changed, 438 insertions(+), 475 deletions(-)
>  create mode 100644 include/hw/net/mdio.h
>  create mode 100644 hw/net/mdio.c
> 
> -- 
> 2.14.1
> 

Re: [Qemu-arm] [PATCH v5 0/7] Generalize MDIO framework

[Philippe Mathieu-Daudé]

Hi Edgar,

On 10/09/2017 10:21 AM, Edgar E. Iglesias wrote:
> On Fri, Sep 22, 2017 at 02:13:16PM -0300, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> I have a follow up series using multiples PHY on the MDIO bus based on this
>> series.
> 
> Hi Philippe!
> 
> I think this is a good improvement compared to todays state.
> It may make sense to have the generic mdio bus functions in mdio.c
> and specific phy models in separate files, thoughts?

I'm sorry I missed your mail and noticed it today since this thread got
awakened with Alistair's reviews.

I'll raise this series priority in my TODO and respin with Grant correct
S-o-b, Alistair comments addressed and split MDIO/PHY as you suggested.

After spending 2 months with the SD bus, I now fill more confident to
rework the MDIO bus and think of unit testing and tracing.

Regards,

Phil.

>>
>> Grant's previous work:
>> http://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg00257.html
>>
>> "There is more work to be done, particularly in moving to the common GPIO api,
>>  but that work can be done as a follow on patch series."
>>
>> Grant Likely (7):
>>   hw/mdio: Generalize etraxfs MDIO bitbanging emulation
>>   hw/mdio: Add PHY register definition
>>   hw/mdio: Generalize phy initialization routine
>>   hw/mdio: Mask out read-only bits.
>>   hw/mdio: Refactor bitbanging state machine
>>   hw/mdio: Add VMState support
>>   hw/mdio: Use bitbang core for smc91c111 network device
>>
>>  include/hw/net/mdio.h   | 124 +++++++++++++++++++++
>>  hw/net/etraxfs_eth.c    | 291 +-----------------------------------------------
>>  hw/net/mdio.c           | 280 ++++++++++++++++++++++++++++++++++++++++++++++
>>  hw/net/smc91c111.c      |  27 ++++-
>>  hw/net/xilinx_axienet.c | 189 +------------------------------
>>  hw/net/Makefile.objs    |   2 +
>>  6 files changed, 438 insertions(+), 475 deletions(-)
>>  create mode 100644 include/hw/net/mdio.h
>>  create mode 100644 hw/net/mdio.c
>>
>> -- 
>> 2.14.1
>>