All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v4 0/8] Add Ethernet device for i.MX6 SOC
@ 2016-05-18 22:22 Jean-Christophe Dubois
  2016-05-18 22:22 ` [Qemu-devel] [PATCH v4 1/8] net: improve UDP/TCP checksum computation Jean-Christophe Dubois
                   ` (7 more replies)
  0 siblings, 8 replies; 22+ messages in thread
From: Jean-Christophe Dubois @ 2016-05-18 22:22 UTC (permalink / raw)
  To: qemu-devel, peter.maydell, jasowang; +Cc: Jean-Christophe Dubois

This patch series adds Gb ENET Ethernet device to the i.MX6 SOC.

The ENET device is an evolution of the FEC device present on the i.MX25 SOC
and is backward compatible with it.

Therefore the ENET support has been added to the actual Qemu FEC device (
rather than adding a new device).

The Patch has been tested by:
 * Booting linux on i.MX25 PDK board emulation and accessing internet
 * Booting linux on i.MX6 Sabrelite board emulation and accessing internet

Jean-Christophe Dubois (8):
  net: improve UDP/TCP checksum computation.
  net: handle optional VLAN header in checksum computation.
  i.MX: Fix FEC code for MDIO operation selection
  i.MX: Fix FEC code for MDIO address selection
  i.MX: Fix FEC code for ECR register reset value.
  i.MX: move FEC device to a register array structure.
  Add ENET/Gbps Ethernet support to FEC device
  Add ENET device to i.MX6 SOC.

 hw/arm/fsl-imx25.c        |   3 +
 hw/arm/fsl-imx6.c         |  17 +
 hw/net/imx_fec.c          | 995 ++++++++++++++++++++++++++++++++++------------
 include/hw/arm/fsl-imx6.h |   6 +-
 include/hw/net/imx_fec.h  | 248 +++++++++---
 net/checksum.c            | 121 ++++--
 6 files changed, 1060 insertions(+), 330 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH v4 1/8] net: improve UDP/TCP checksum computation.
  2016-05-18 22:22 [Qemu-devel] [PATCH v4 0/8] Add Ethernet device for i.MX6 SOC Jean-Christophe Dubois
@ 2016-05-18 22:22 ` Jean-Christophe Dubois
  2016-05-18 22:22 ` [Qemu-devel] [PATCH v4 2/8] net: handle optional VLAN header in " Jean-Christophe Dubois
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 22+ messages in thread
From: Jean-Christophe Dubois @ 2016-05-18 22:22 UTC (permalink / raw)
  To: qemu-devel, peter.maydell, jasowang; +Cc: Jean-Christophe Dubois

 * based on Eth, UDP, TCP struct present in eth.h instead of hardcoded
   indexes and sizes.
 * based on various macros present in eth.h.

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---

Changes since v1:
 * None

Changes since v2:
 * The patch was split in 2 parts: 
   - a rewrite of the TCP/UDB checksum function (this patch)
   - the addition of the support of the VLAN header (next patch).
  
Changes since v3:
 * None

 net/checksum.c | 94 +++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 67 insertions(+), 27 deletions(-)

diff --git a/net/checksum.c b/net/checksum.c
index d0fa424..f62b18a 100644
--- a/net/checksum.c
+++ b/net/checksum.c
@@ -18,9 +18,7 @@
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "net/checksum.h"
-
-#define PROTO_TCP  6
-#define PROTO_UDP 17
+#include "net/eth.h"
 
 uint32_t net_checksum_add_cont(int len, uint8_t *buf, int seq)
 {
@@ -57,40 +55,82 @@ uint16_t net_checksum_tcpudp(uint16_t length, uint16_t proto,
 
 void net_checksum_calculate(uint8_t *data, int length)
 {
-    int hlen, plen, proto, csum_offset;
-    uint16_t csum;
+    int ip_len;
+    struct ip_header *ip;
+
+    /*
+     * Note: We cannot assume "data" is aligned, so the all code uses
+     * some macros that take care of possible unaligned access for
+     * struct members (just in case).
+     */
 
     /* Ensure data has complete L2 & L3 headers. */
-    if (length < 14 + 20) {
+    if (length < (sizeof(struct eth_header) + sizeof(struct ip_header))) {
         return;
     }
 
-    if ((data[14] & 0xf0) != 0x40)
-	return; /* not IPv4 */
-    hlen  = (data[14] & 0x0f) * 4;
-    plen  = (data[16] << 8 | data[17]) - hlen;
-    proto = data[23];
-
-    switch (proto) {
-    case PROTO_TCP:
-	csum_offset = 16;
-	break;
-    case PROTO_UDP:
-	csum_offset = 6;
-	break;
-    default:
-	return;
+    ip = (struct ip_header *)(data + sizeof(struct eth_header));
+
+    if (IP_HEADER_VERSION(ip) != IP_HEADER_VERSION_4) {
+        return; /* not IPv4 */
     }
 
-    if (plen < csum_offset + 2 || 14 + hlen + plen > length) {
+    ip_len = lduw_be_p(&ip->ip_len);
+
+    /* Last, check that we have enough data for the all IP frame */
+    if (length < ip_len) {
         return;
     }
 
-    data[14+hlen+csum_offset]   = 0;
-    data[14+hlen+csum_offset+1] = 0;
-    csum = net_checksum_tcpudp(plen, proto, data+14+12, data+14+hlen);
-    data[14+hlen+csum_offset]   = csum >> 8;
-    data[14+hlen+csum_offset+1] = csum & 0xff;
+    ip_len -= IP_HDR_GET_LEN(ip);
+
+    switch (ip->ip_p) {
+    case IP_PROTO_TCP:
+    {
+        uint16_t csum;
+        tcp_header *tcp = (tcp_header *)(ip + 1);
+
+        if (ip_len < sizeof(tcp_header)) {
+            return;
+        }
+
+        /* Set csum to 0 */
+        stw_he_p(&tcp->th_sum, 0);
+
+        csum = net_checksum_tcpudp(ip_len, ip->ip_p,
+                                   (uint8_t *)&ip->ip_src,
+                                   (uint8_t *)tcp);
+
+        /* Store computed csum */
+        stw_be_p(&tcp->th_sum, csum);
+
+        break;
+    }
+    case IP_PROTO_UDP:
+    {
+        uint16_t csum;
+        udp_header *udp = (udp_header *)(ip + 1);
+
+        if (ip_len < sizeof(udp_header)) {
+            return;
+        }
+
+        /* Set csum to 0 */
+        stw_he_p(&udp->uh_sum, 0);
+
+        csum = net_checksum_tcpudp(ip_len, ip->ip_p,
+                                   (uint8_t *)&ip->ip_src,
+                                   (uint8_t *)udp);
+
+        /* Store computed csum */
+        stw_be_p(&udp->uh_sum, csum);
+
+        break;
+    }
+    default:
+        /* Can't handle any other protocol */
+        break;
+    }
 }
 
 uint32_t
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH v4 2/8] net: handle optional VLAN header in checksum computation.
  2016-05-18 22:22 [Qemu-devel] [PATCH v4 0/8] Add Ethernet device for i.MX6 SOC Jean-Christophe Dubois
  2016-05-18 22:22 ` [Qemu-devel] [PATCH v4 1/8] net: improve UDP/TCP checksum computation Jean-Christophe Dubois
@ 2016-05-18 22:22 ` Jean-Christophe Dubois
  2016-05-18 22:22 ` [Qemu-devel] [PATCH v4 3/8] i.MX: Fix FEC code for MDIO operation selection Jean-Christophe Dubois
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 22+ messages in thread
From: Jean-Christophe Dubois @ 2016-05-18 22:22 UTC (permalink / raw)
  To: qemu-devel, peter.maydell, jasowang; +Cc: Jean-Christophe Dubois

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---

Changes since v1:
 * Not present on v1
 
Changes since v2:
 * Not present on v2

Changes since v3:
 * local variable name change.

 net/checksum.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/net/checksum.c b/net/checksum.c
index f62b18a..62d3465 100644
--- a/net/checksum.c
+++ b/net/checksum.c
@@ -55,7 +55,7 @@ uint16_t net_checksum_tcpudp(uint16_t length, uint16_t proto,
 
 void net_checksum_calculate(uint8_t *data, int length)
 {
-    int ip_len;
+    int mac_hdr_len, ip_len;
     struct ip_header *ip;
 
     /*
@@ -64,12 +64,39 @@ void net_checksum_calculate(uint8_t *data, int length)
      * struct members (just in case).
      */
 
-    /* Ensure data has complete L2 & L3 headers. */
-    if (length < (sizeof(struct eth_header) + sizeof(struct ip_header))) {
+    /* Ensure we have at least an Eth header */
+    if (length < sizeof(struct eth_header)) {
         return;
     }
 
-    ip = (struct ip_header *)(data + sizeof(struct eth_header));
+    /* Handle the optionnal VLAN headers */
+    switch (lduw_be_p(&PKT_GET_ETH_HDR(data)->h_proto)) {
+    case ETH_P_VLAN:
+        mac_hdr_len = sizeof(struct eth_header) +
+                     sizeof(struct vlan_header);
+        break;
+    case ETH_P_DVLAN:
+        if (lduw_be_p(&PKT_GET_VLAN_HDR(data)->h_proto) == ETH_P_VLAN) {
+            mac_hdr_len = sizeof(struct eth_header) +
+                         2 * sizeof(struct vlan_header);
+        } else {
+            mac_hdr_len = sizeof(struct eth_header) +
+                         sizeof(struct vlan_header);
+        }
+        break;
+    default:
+        mac_hdr_len = sizeof(struct eth_header);
+        break;
+    }
+
+    length -= mac_hdr_len;
+
+    /* Now check we have an IP header (with an optionnal VLAN header) */
+    if (length < sizeof(struct ip_header)) {
+        return;
+    }
+
+    ip = (struct ip_header *)(data + mac_hdr_len);
 
     if (IP_HEADER_VERSION(ip) != IP_HEADER_VERSION_4) {
         return; /* not IPv4 */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH v4 3/8] i.MX: Fix FEC code for MDIO operation selection
  2016-05-18 22:22 [Qemu-devel] [PATCH v4 0/8] Add Ethernet device for i.MX6 SOC Jean-Christophe Dubois
  2016-05-18 22:22 ` [Qemu-devel] [PATCH v4 1/8] net: improve UDP/TCP checksum computation Jean-Christophe Dubois
  2016-05-18 22:22 ` [Qemu-devel] [PATCH v4 2/8] net: handle optional VLAN header in " Jean-Christophe Dubois
@ 2016-05-18 22:22 ` Jean-Christophe Dubois
  2016-05-18 22:22 ` [Qemu-devel] [PATCH v4 4/8] i.MX: Fix FEC code for MDIO address selection Jean-Christophe Dubois
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 22+ messages in thread
From: Jean-Christophe Dubois @ 2016-05-18 22:22 UTC (permalink / raw)
  To: qemu-devel, peter.maydell, jasowang; +Cc: Jean-Christophe Dubois

According to the FEC chapter of i.MX25 reference manual

When writing the MMFR register, bit 29 and 28 select the requested operation.
 * 10 means read operation with valid MII mgmt frame
 * 11 means read operation with non compliant MII mgmt frame
 * 01 means write operation with valid MII mgmt frame
 * 00 means write operation with non compliant MII mgmt frame

So while bit 28 does change beween read/write for valid MII mgmt frame, the
mening is inverted for non compliant MII mgmt frame.

Bit 29 on the other hand means read/write whatever the type of mgmt frame
involved.

So this patch change the operation selection from bit 28 to bit 29 as it is
more generic.

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---

Changes since v1: 
 * Not present on v1
 
Changes since v2:
 * Not present on v2

Changes since v3:
 * Not present on v3

 hw/net/imx_fec.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index e60e338..b3f5e4a 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -458,10 +458,10 @@ static void imx_fec_write(void *opaque, hwaddr addr,
     case 0x040: /* MMFR */
         /* store the value */
         s->mmfr = value;
-        if (extract32(value, 28, 1)) {
-            do_phy_write(s, extract32(value, 18, 9), extract32(value, 0, 16));
-        } else {
+        if (extract32(value, 29, 1)) {
             s->mmfr = do_phy_read(s, extract32(value, 18, 9));
+        } else {
+            do_phy_write(s, extract32(value, 18, 9), extract32(value, 0, 16));
         }
         /* raise the interrupt as the PHY operation is done */
         s->eir |= FEC_INT_MII;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH v4 4/8] i.MX: Fix FEC code for MDIO address selection
  2016-05-18 22:22 [Qemu-devel] [PATCH v4 0/8] Add Ethernet device for i.MX6 SOC Jean-Christophe Dubois
                   ` (2 preceding siblings ...)
  2016-05-18 22:22 ` [Qemu-devel] [PATCH v4 3/8] i.MX: Fix FEC code for MDIO operation selection Jean-Christophe Dubois
@ 2016-05-18 22:22 ` Jean-Christophe Dubois
  2016-05-18 22:23 ` [Qemu-devel] [PATCH v4 5/8] i.MX: Fix FEC code for ECR register reset value Jean-Christophe Dubois
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 22+ messages in thread
From: Jean-Christophe Dubois @ 2016-05-18 22:22 UTC (permalink / raw)
  To: qemu-devel, peter.maydell, jasowang; +Cc: Jean-Christophe Dubois

According to the FEC chapter of i.MX25 reference manual

When writing to MMFR register, the MDIO device and adress are selected by
bit 27 to 23 and bit 22 to 18 respectively. This is a total of 10 bits
that need to be used by the Phy chip/address decoding function.

This patch fixes the number of bits used from 9 to 10.

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---

Changes since v1:
 * Not present on v1
 
Changes since v2:
 * Not present on v2

Changes since v3:
 * Not present on v3

 hw/net/imx_fec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index b3f5e4a..a649027 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -459,9 +459,9 @@ static void imx_fec_write(void *opaque, hwaddr addr,
         /* store the value */
         s->mmfr = value;
         if (extract32(value, 29, 1)) {
-            s->mmfr = do_phy_read(s, extract32(value, 18, 9));
+            s->mmfr = do_phy_read(s, extract32(value, 18, 10));
         } else {
-            do_phy_write(s, extract32(value, 18, 9), extract32(value, 0, 16));
+            do_phy_write(s, extract32(value, 18, 10), extract32(value, 0, 16));
         }
         /* raise the interrupt as the PHY operation is done */
         s->eir |= FEC_INT_MII;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH v4 5/8] i.MX: Fix FEC code for ECR register reset value.
  2016-05-18 22:22 [Qemu-devel] [PATCH v4 0/8] Add Ethernet device for i.MX6 SOC Jean-Christophe Dubois
                   ` (3 preceding siblings ...)
  2016-05-18 22:22 ` [Qemu-devel] [PATCH v4 4/8] i.MX: Fix FEC code for MDIO address selection Jean-Christophe Dubois
@ 2016-05-18 22:23 ` Jean-Christophe Dubois
  2016-05-18 22:23 ` [Qemu-devel] [PATCH v4 6/8] i.MX: move FEC device to a register array structure Jean-Christophe Dubois
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 22+ messages in thread
From: Jean-Christophe Dubois @ 2016-05-18 22:23 UTC (permalink / raw)
  To: qemu-devel, peter.maydell, jasowang; +Cc: Jean-Christophe Dubois

According to the FEC chapter of i.MX25 reference manual ECR register is
initialized at 0xf0000000 at reset time.

We fix the value.

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---

Changes since v1:
 * Not present on v1

Changes since v2:
 * Not present on v2
 
Changes since v3:
 * Not present on v3

 hw/net/imx_fec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index a649027..87e3c87 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -338,7 +338,7 @@ static void imx_fec_reset(DeviceState *d)
     s->eir = 0;
     s->eimr = 0;
     s->rx_enabled = 0;
-    s->ecr = 0;
+    s->ecr = 0xf0000000;
     s->mscr = 0;
     s->mibc = 0xc0000000;
     s->rcr = 0x05ee0001;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH v4 6/8] i.MX: move FEC device to a register array structure.
  2016-05-18 22:22 [Qemu-devel] [PATCH v4 0/8] Add Ethernet device for i.MX6 SOC Jean-Christophe Dubois
                   ` (4 preceding siblings ...)
  2016-05-18 22:23 ` [Qemu-devel] [PATCH v4 5/8] i.MX: Fix FEC code for ECR register reset value Jean-Christophe Dubois
@ 2016-05-18 22:23 ` Jean-Christophe Dubois
  2016-05-19  3:28   ` Jason Wang
  2016-05-18 22:23 ` [Qemu-devel] [PATCH v4 7/8] Add ENET/Gbps Ethernet support to FEC device Jean-Christophe Dubois
  2016-05-18 22:23 ` [Qemu-devel] [PATCH v4 8/8] Add ENET device to i.MX6 SOC Jean-Christophe Dubois
  7 siblings, 1 reply; 22+ messages in thread
From: Jean-Christophe Dubois @ 2016-05-18 22:23 UTC (permalink / raw)
  To: qemu-devel, peter.maydell, jasowang; +Cc: Jean-Christophe Dubois

This is to prepare for the ENET Gb device of the i.MX6.

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---

Changes since v1: 
 * Not present on v1.

Changes since v2:
 * The patch was split in 2 parts:
   - a "port" to a reg array based device (this patch).
   - the addition of the Gb support (next patch).
 
Changes since v3:
 * Small fix patches were extracted from this patch (see previous 3 patches)
 * Reset logic through ECR was fixed.
 * TDAR/RDAR behavior was fixed.

 hw/net/imx_fec.c         | 396 ++++++++++++++++++++++++++---------------------
 include/hw/net/imx_fec.h |  55 ++++---
 2 files changed, 258 insertions(+), 193 deletions(-)

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index 87e3c87..565b4a3 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -52,30 +52,75 @@
         } \
     } while (0)
 
+static const char *imx_fec_reg_name(IMXFECState *s, uint32_t index)
+{
+    static char tmp[20];
+
+    switch (index) {
+    case ENET_EIR:
+        return "EIR";
+    case ENET_EIMR:
+        return "EIMR";
+    case ENET_RDAR:
+        return "RDAR";
+    case ENET_TDAR:
+        return "TDAR";
+    case ENET_ECR:
+        return "ECR";
+    case ENET_MMFR:
+        return "MMFR";
+    case ENET_MSCR:
+        return "MSCR";
+    case ENET_MIBC:
+        return "MIBC";
+    case ENET_RCR:
+        return "RCR";
+    case ENET_TCR:
+        return "TCR";
+    case ENET_PALR:
+        return "PALR";
+    case ENET_PAUR:
+        return "PAUR";
+    case ENET_OPD:
+        return "OPD";
+    case ENET_IAUR:
+        return "IAUR";
+    case ENET_IALR:
+        return "IALR";
+    case ENET_GAUR:
+        return "GAUR";
+    case ENET_GALR:
+        return "GALR";
+    case ENET_TFWR:
+        return "TFWR";
+    case ENET_RDSR:
+        return "RDSR";
+    case ENET_TDSR:
+        return "TDSR";
+    case ENET_MRBR:
+        return "MRBR";
+    case ENET_FRBR:
+        return "FRBR";
+    case ENET_FRSR:
+        return "FRSR";
+    case ENET_MIIGSK_CFGR:
+        return "MIIGSK_CFGR";
+    case ENET_MIIGSK_ENR:
+        return "MIIGSK_ENR";
+    default:
+        sprintf(tmp, "index %d", index);
+        return tmp;
+    }
+}
+
 static const VMStateDescription vmstate_imx_fec = {
     .name = TYPE_IMX_FEC,
-    .version_id = 1,
-    .minimum_version_id = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
     .fields = (VMStateField[]) {
-        VMSTATE_UINT32(irq_state, IMXFECState),
-        VMSTATE_UINT32(eir, IMXFECState),
-        VMSTATE_UINT32(eimr, IMXFECState),
-        VMSTATE_UINT32(rx_enabled, IMXFECState),
+        VMSTATE_UINT32_ARRAY(regs, IMXFECState, ENET_MAX),
         VMSTATE_UINT32(rx_descriptor, IMXFECState),
         VMSTATE_UINT32(tx_descriptor, IMXFECState),
-        VMSTATE_UINT32(ecr, IMXFECState),
-        VMSTATE_UINT32(mmfr, IMXFECState),
-        VMSTATE_UINT32(mscr, IMXFECState),
-        VMSTATE_UINT32(mibc, IMXFECState),
-        VMSTATE_UINT32(rcr, IMXFECState),
-        VMSTATE_UINT32(tcr, IMXFECState),
-        VMSTATE_UINT32(tfwr, IMXFECState),
-        VMSTATE_UINT32(frsr, IMXFECState),
-        VMSTATE_UINT32(erdsr, IMXFECState),
-        VMSTATE_UINT32(etdsr, IMXFECState),
-        VMSTATE_UINT32(emrbr, IMXFECState),
-        VMSTATE_UINT32(miigsk_cfgr, IMXFECState),
-        VMSTATE_UINT32(miigsk_enr, IMXFECState),
 
         VMSTATE_UINT32(phy_status, IMXFECState),
         VMSTATE_UINT32(phy_control, IMXFECState),
@@ -251,15 +296,13 @@ static void imx_fec_write_bd(IMXFECBufDesc *bd, dma_addr_t addr)
 
 static void imx_fec_update(IMXFECState *s)
 {
-    uint32_t active;
-    uint32_t changed;
-
-    active = s->eir & s->eimr;
-    changed = active ^ s->irq_state;
-    if (changed) {
-        qemu_set_irq(s->irq, active);
+    if (s->regs[ENET_EIR] & s->regs[ENET_EIMR]) {
+        FEC_PRINTF("interrupt raised\n");
+        qemu_set_irq(s->irq, 1);
+    } else {
+        FEC_PRINTF("interrupt lowered\n");
+        qemu_set_irq(s->irq, 0);
     }
-    s->irq_state = active;
 }
 
 static void imx_fec_do_tx(IMXFECState *s)
@@ -283,7 +326,7 @@ static void imx_fec_do_tx(IMXFECState *s)
         len = bd.length;
         if (frame_size + len > FEC_MAX_FRAME_SIZE) {
             len = FEC_MAX_FRAME_SIZE - frame_size;
-            s->eir |= FEC_INT_BABT;
+            s->regs[ENET_EIR] |= FEC_INT_BABT;
         }
         dma_memory_read(&address_space_memory, bd.data, ptr, len);
         ptr += len;
@@ -293,17 +336,17 @@ static void imx_fec_do_tx(IMXFECState *s)
             qemu_send_packet(qemu_get_queue(s->nic), frame, len);
             ptr = frame;
             frame_size = 0;
-            s->eir |= FEC_INT_TXF;
+            s->regs[ENET_EIR] |= FEC_INT_TXF;
         }
-        s->eir |= FEC_INT_TXB;
+        s->regs[ENET_EIR] |= FEC_INT_TXB;
         bd.flags &= ~FEC_BD_R;
         /* Write back the modified descriptor.  */
         imx_fec_write_bd(&bd, addr);
         /* Advance to the next descriptor.  */
         if ((bd.flags & FEC_BD_W) != 0) {
-            addr = s->etdsr;
+            addr = s->regs[ENET_TDSR];
         } else {
-            addr += 8;
+            addr += sizeof(bd);
         }
     }
 
@@ -315,7 +358,7 @@ static void imx_fec_do_tx(IMXFECState *s)
 static void imx_fec_enable_rx(IMXFECState *s)
 {
     IMXFECBufDesc bd;
-    uint32_t tmp;
+    bool tmp;
 
     imx_fec_read_bd(&bd, s->rx_descriptor);
 
@@ -323,11 +366,11 @@ static void imx_fec_enable_rx(IMXFECState *s)
 
     if (!tmp) {
         FEC_PRINTF("RX buffer full\n");
-    } else if (!s->rx_enabled) {
+    } else if (!s->regs[ENET_RDAR]) {
         qemu_flush_queued_packets(qemu_get_queue(s->nic));
     }
 
-    s->rx_enabled = tmp;
+    s->regs[ENET_RDAR] = tmp ? ENET_RDAR_RDAR : 0;
 }
 
 static void imx_fec_reset(DeviceState *d)
@@ -335,18 +378,26 @@ static void imx_fec_reset(DeviceState *d)
     IMXFECState *s = IMX_FEC(d);
 
     /* Reset the FEC */
-    s->eir = 0;
-    s->eimr = 0;
-    s->rx_enabled = 0;
-    s->ecr = 0xf0000000;
-    s->mscr = 0;
-    s->mibc = 0xc0000000;
-    s->rcr = 0x05ee0001;
-    s->tcr = 0;
-    s->tfwr = 0;
-    s->frsr = 0x500;
-    s->miigsk_cfgr = 0;
-    s->miigsk_enr = 0x6;
+    memset(s->regs, 0, sizeof(s->regs));
+    s->regs[ENET_ECR]   = 0xf0000000;
+    s->regs[ENET_MIBC]  = 0xc0000000;
+    s->regs[ENET_RCR]   = 0x05ee0001;
+    s->regs[ENET_OPD]   = 0x00010000;
+
+    s->regs[ENET_PALR]  = (s->conf.macaddr.a[0] << 24)
+                          | (s->conf.macaddr.a[1] << 16)
+                          | (s->conf.macaddr.a[2] << 8)
+                          | s->conf.macaddr.a[3];
+    s->regs[ENET_PAUR]  = (s->conf.macaddr.a[4] << 24)
+                          | (s->conf.macaddr.a[5] << 16)
+                          | 0x8808;
+
+    s->regs[ENET_FRBR]  = 0x00000600;
+    s->regs[ENET_FRSR]  = 0x00000500;
+    s->regs[ENET_MIIGSK_ENR]  = 0x00000006;
+
+    s->rx_descriptor = 0;
+    s->tx_descriptor = 0;
 
     /* We also reset the PHY */
     phy_reset(s);
@@ -354,183 +405,180 @@ static void imx_fec_reset(DeviceState *d)
 
 static uint64_t imx_fec_read(void *opaque, hwaddr addr, unsigned size)
 {
+    uint32_t value = 0;
     IMXFECState *s = IMX_FEC(opaque);
-
-    FEC_PRINTF("reading from @ 0x%" HWADDR_PRIx "\n", addr);
-
-    switch (addr & 0x3ff) {
-    case 0x004:
-        return s->eir;
-    case 0x008:
-        return s->eimr;
-    case 0x010:
-        return s->rx_enabled ? (1 << 24) : 0;   /* RDAR */
-    case 0x014:
-        return 0;   /* TDAR */
-    case 0x024:
-        return s->ecr;
-    case 0x040:
-        return s->mmfr;
-    case 0x044:
-        return s->mscr;
-    case 0x064:
-        return s->mibc; /* MIBC */
-    case 0x084:
-        return s->rcr;
-    case 0x0c4:
-        return s->tcr;
-    case 0x0e4:     /* PALR */
-        return (s->conf.macaddr.a[0] << 24)
-               | (s->conf.macaddr.a[1] << 16)
-               | (s->conf.macaddr.a[2] << 8)
-               | s->conf.macaddr.a[3];
-        break;
-    case 0x0e8:     /* PAUR */
-        return (s->conf.macaddr.a[4] << 24)
-               | (s->conf.macaddr.a[5] << 16)
-               | 0x8808;
-    case 0x0ec:
-        return 0x10000; /* OPD */
-    case 0x118:
-        return 0;
-    case 0x11c:
-        return 0;
-    case 0x120:
-        return 0;
-    case 0x124:
-        return 0;
-    case 0x144:
-        return s->tfwr;
-    case 0x14c:
-        return 0x600;
-    case 0x150:
-        return s->frsr;
-    case 0x180:
-        return s->erdsr;
-    case 0x184:
-        return s->etdsr;
-    case 0x188:
-        return s->emrbr;
-    case 0x300:
-        return s->miigsk_cfgr;
-    case 0x308:
-        return s->miigsk_enr;
+    uint32_t index = addr >> 2;
+
+    switch (index) {
+    case ENET_EIR:
+    case ENET_EIMR:
+    case ENET_RDAR:
+    case ENET_TDAR:
+    case ENET_ECR:
+    case ENET_MMFR:
+    case ENET_MSCR:
+    case ENET_MIBC:
+    case ENET_RCR:
+    case ENET_TCR:
+    case ENET_PALR:
+    case ENET_PAUR:
+    case ENET_OPD:
+    case ENET_IAUR:
+    case ENET_IALR:
+    case ENET_GAUR:
+    case ENET_GALR:
+    case ENET_TFWR:
+    case ENET_RDSR:
+    case ENET_TDSR:
+    case ENET_MRBR:
+    case ENET_FRBR:
+    case ENET_FRSR:
+    case ENET_MIIGSK_CFGR:
+    case ENET_MIIGSK_ENR:
+        value = s->regs[index];
+        break;
     default:
-        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad address at offset 0x%"
-                      HWADDR_PRIx "\n", TYPE_IMX_FEC, __func__, addr);
-        return 0;
+        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad register at offset 0x%"
+                      PRIx32 "\n", TYPE_IMX_FEC, __func__, index * 4);
+        break;
     }
+
+    FEC_PRINTF("reg[%s] => 0x%" PRIx32 "\n", imx_fec_reg_name(s, index),
+                                              value);
+
+    return value;
 }
 
 static void imx_fec_write(void *opaque, hwaddr addr,
                           uint64_t value, unsigned size)
 {
     IMXFECState *s = IMX_FEC(opaque);
+    uint32_t index = addr >> 2;
 
-    FEC_PRINTF("writing 0x%08x @ 0x%" HWADDR_PRIx "\n", (int)value, addr);
+    FEC_PRINTF("reg[%s] <= 0x%" PRIx32 "\n", imx_fec_reg_name(s, index),
+                                             (uint32_t)value);
 
-    switch (addr & 0x3ff) {
-    case 0x004: /* EIR */
-        s->eir &= ~value;
+    switch (index) {
+    case ENET_EIR:
+        s->regs[index] &= ~value;
         break;
-    case 0x008: /* EIMR */
-        s->eimr = value;
+    case ENET_EIMR:
+        s->regs[index] = value;
         break;
-    case 0x010: /* RDAR */
-        if ((s->ecr & FEC_EN) && !s->rx_enabled) {
-            imx_fec_enable_rx(s);
+    case ENET_RDAR:
+        if (s->regs[ENET_ECR] & FEC_EN) {
+            if (!s->regs[index]) {
+                s->regs[index] = ENET_RDAR_RDAR;
+                imx_fec_enable_rx(s);
+            }
+        } else {
+            s->regs[index] = 0;
         }
         break;
-    case 0x014: /* TDAR */
-        if (s->ecr & FEC_EN) {
+    case ENET_TDAR:
+        if (s->regs[ENET_ECR] & FEC_EN) {
+            s->regs[index] = ENET_TDAR_TDAR;
             imx_fec_do_tx(s);
         }
+        s->regs[index] = 0;
         break;
-    case 0x024: /* ECR */
-        s->ecr = value;
+    case ENET_ECR:
         if (value & FEC_RESET) {
-            imx_fec_reset(DEVICE(s));
+            return imx_fec_reset(DEVICE(s));
         }
-        if ((s->ecr & FEC_EN) == 0) {
-            s->rx_enabled = 0;
+        s->regs[index] = value;
+        if ((s->regs[index] & FEC_EN) == 0) {
+            s->regs[ENET_RDAR] = 0;
+            s->rx_descriptor = s->regs[ENET_RDSR];
+            s->regs[ENET_TDAR] = 0;
+            s->tx_descriptor = s->regs[ENET_TDSR];
         }
         break;
-    case 0x040: /* MMFR */
-        /* store the value */
-        s->mmfr = value;
+    case ENET_MMFR:
+        s->regs[index] = value;
         if (extract32(value, 29, 1)) {
-            s->mmfr = do_phy_read(s, extract32(value, 18, 10));
+            /* This is a read operation */
+            s->regs[ENET_MMFR] = deposit32(s->regs[ENET_MMFR], 0, 16,
+                                           do_phy_read(s,
+                                                       extract32(value,
+                                                                 18, 10)));
         } else {
+            /* This a write operation */
             do_phy_write(s, extract32(value, 18, 10), extract32(value, 0, 16));
         }
         /* raise the interrupt as the PHY operation is done */
-        s->eir |= FEC_INT_MII;
+        s->regs[ENET_EIR] |= FEC_INT_MII;
         break;
-    case 0x044: /* MSCR */
-        s->mscr = value & 0xfe;
+    case ENET_MSCR:
+        s->regs[index] = value & 0xfe;
         break;
-    case 0x064: /* MIBC */
+    case ENET_MIBC:
         /* TODO: Implement MIB.  */
-        s->mibc = (value & 0x80000000) ? 0xc0000000 : 0;
+        s->regs[index] = (value & 0x80000000) ? 0xc0000000 : 0;
         break;
-    case 0x084: /* RCR */
-        s->rcr = value & 0x07ff003f;
+    case ENET_RCR:
+        s->regs[index] = value & 0x07ff003f;
         /* TODO: Implement LOOP mode.  */
         break;
-    case 0x0c4: /* TCR */
+    case ENET_TCR:
         /* We transmit immediately, so raise GRA immediately.  */
-        s->tcr = value;
+        s->regs[index] = value;
         if (value & 1) {
-            s->eir |= FEC_INT_GRA;
+            s->regs[ENET_EIR] |= FEC_INT_GRA;
         }
         break;
-    case 0x0e4: /* PALR */
+    case ENET_PALR:
+        s->regs[index] = value;
         s->conf.macaddr.a[0] = value >> 24;
         s->conf.macaddr.a[1] = value >> 16;
         s->conf.macaddr.a[2] = value >> 8;
         s->conf.macaddr.a[3] = value;
         break;
-    case 0x0e8: /* PAUR */
+    case ENET_PAUR:
+        s->regs[index] = (value | 0x0000ffff) & 0xffff8808;
         s->conf.macaddr.a[4] = value >> 24;
         s->conf.macaddr.a[5] = value >> 16;
         break;
-    case 0x0ec: /* OPDR */
+    case ENET_OPD:
+        s->regs[index] = (value & 0x0000ffff) | 0x00010000;
         break;
-    case 0x118: /* IAUR */
-    case 0x11c: /* IALR */
-    case 0x120: /* GAUR */
-    case 0x124: /* GALR */
+    case ENET_IAUR:
+    case ENET_IALR:
+    case ENET_GAUR:
+    case ENET_GALR:
         /* TODO: implement MAC hash filtering.  */
         break;
-    case 0x144: /* TFWR */
-        s->tfwr = value & 3;
+    case ENET_TFWR:
+        s->regs[index] = value & 3;
         break;
-    case 0x14c: /* FRBR */
-        /* FRBR writes ignored.  */
+    case ENET_FRBR:
+        /* FRBR is read only */
+        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Register FRBR is read only\n",
+                      TYPE_IMX_FEC, __func__);
         break;
-    case 0x150: /* FRSR */
-        s->frsr = (value & 0x3fc) | 0x400;
+    case ENET_FRSR:
+        s->regs[index] = (value & 0x000003fc) | 0x00000400;
         break;
-    case 0x180: /* ERDSR */
-        s->erdsr = value & ~3;
-        s->rx_descriptor = s->erdsr;
+    case ENET_RDSR:
+        s->regs[index] = value & ~3;
+        s->rx_descriptor = s->regs[index];
         break;
-    case 0x184: /* ETDSR */
-        s->etdsr = value & ~3;
-        s->tx_descriptor = s->etdsr;
+    case ENET_TDSR:
+        s->regs[index] = value & ~3;
+        s->tx_descriptor = s->regs[index];
         break;
-    case 0x188: /* EMRBR */
-        s->emrbr = value & 0x7f0;
+    case ENET_MRBR:
+        s->regs[index] = value & 0x000007f0;
         break;
-    case 0x300: /* MIIGSK_CFGR */
-        s->miigsk_cfgr = value & 0x53;
+    case ENET_MIIGSK_CFGR:
+        s->regs[index] = value & 0x00000053;
         break;
-    case 0x308: /* MIIGSK_ENR */
-        s->miigsk_enr = (value & 0x2) ? 0x6 : 0;
+    case ENET_MIIGSK_ENR:
+        s->regs[index] = (value & 0x00000002) ? 0x00000006 : 0;
         break;
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad address at offset 0x%"
-                      HWADDR_PRIx "\n", TYPE_IMX_FEC, __func__, addr);
+                      PRIx32 "\n", TYPE_IMX_FEC, __func__, index * 4);
         break;
     }
 
@@ -541,7 +589,7 @@ static int imx_fec_can_receive(NetClientState *nc)
 {
     IMXFECState *s = IMX_FEC(qemu_get_nic_opaque(nc));
 
-    return s->rx_enabled;
+    return s->regs[ENET_RDAR] ? 1 : 0;
 }
 
 static ssize_t imx_fec_receive(NetClientState *nc, const uint8_t *buf,
@@ -559,7 +607,7 @@ static ssize_t imx_fec_receive(NetClientState *nc, const uint8_t *buf,
 
     FEC_PRINTF("len %d\n", (int)size);
 
-    if (!s->rx_enabled) {
+    if (!s->regs[ENET_RDAR]) {
         qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Unexpected packet\n",
                       TYPE_IMX_FEC, __func__);
         return 0;
@@ -577,7 +625,7 @@ static ssize_t imx_fec_receive(NetClientState *nc, const uint8_t *buf,
     }
 
     /* Frames larger than the user limit just set error flags.  */
-    if (size > (s->rcr >> 16)) {
+    if (size > (s->regs[ENET_RCR] >> 16)) {
         flags |= FEC_BD_LG;
     }
 
@@ -595,7 +643,7 @@ static ssize_t imx_fec_receive(NetClientState *nc, const uint8_t *buf,
                           TYPE_IMX_FEC, __func__);
             break;
         }
-        buf_len = (size <= s->emrbr) ? size : s->emrbr;
+        buf_len = (size <= s->regs[ENET_MRBR]) ? size : s->regs[ENET_MRBR];
         bd.length = buf_len;
         size -= buf_len;
 
@@ -618,16 +666,16 @@ static ssize_t imx_fec_receive(NetClientState *nc, const uint8_t *buf,
             /* Last buffer in frame.  */
             bd.flags |= flags | FEC_BD_L;
             FEC_PRINTF("rx frame flags %04x\n", bd.flags);
-            s->eir |= FEC_INT_RXF;
+            s->regs[ENET_EIR] |= FEC_INT_RXF;
         } else {
-            s->eir |= FEC_INT_RXB;
+            s->regs[ENET_EIR] |= FEC_INT_RXB;
         }
         imx_fec_write_bd(&bd, addr);
         /* Advance to the next descriptor.  */
         if ((bd.flags & FEC_BD_W) != 0) {
-            addr = s->erdsr;
+            addr = s->regs[ENET_RDSR];
         } else {
-            addr += 8;
+            addr += sizeof(bd);
         }
     }
     s->rx_descriptor = addr;
diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h
index cbf8650..709f8a0 100644
--- a/include/hw/net/imx_fec.h
+++ b/include/hw/net/imx_fec.h
@@ -30,6 +30,33 @@
 #include "hw/sysbus.h"
 #include "net/net.h"
 
+#define ENET_EIR               1
+#define ENET_EIMR              2
+#define ENET_RDAR              4
+#define ENET_TDAR              5
+#define ENET_ECR               9
+#define ENET_MMFR              16
+#define ENET_MSCR              17
+#define ENET_MIBC              25
+#define ENET_RCR               33
+#define ENET_TCR               49
+#define ENET_PALR              57
+#define ENET_PAUR              58
+#define ENET_OPD               59
+#define ENET_IAUR              70
+#define ENET_IALR              71
+#define ENET_GAUR              72
+#define ENET_GALR              73
+#define ENET_TFWR              81
+#define ENET_FRBR              83
+#define ENET_FRSR              84
+#define ENET_RDSR              96
+#define ENET_TDSR              97
+#define ENET_MRBR              98
+#define ENET_MIIGSK_CFGR       192
+#define ENET_MIIGSK_ENR        194
+#define ENET_MAX               400
+
 #define FEC_MAX_FRAME_SIZE 2032
 
 #define FEC_INT_HB      (1 << 31)
@@ -46,8 +73,14 @@
 #define FEC_INT_RL      (1 << 20)
 #define FEC_INT_UN      (1 << 19)
 
-#define FEC_EN      2
-#define FEC_RESET   1
+/* RDAR */
+#define ENET_RDAR_RDAR         (1 << 24)
+
+/* TDAR */
+#define ENET_TDAR_TDAR         (1 << 24)
+
+#define FEC_EN                 (1 << 1)
+#define FEC_RESET              (1 << 0)
 
 /* Buffer Descriptor.  */
 typedef struct {
@@ -83,25 +116,9 @@ typedef struct IMXFECState {
     qemu_irq irq;
     MemoryRegion iomem;
 
-    uint32_t irq_state;
-    uint32_t eir;
-    uint32_t eimr;
-    uint32_t rx_enabled;
+    uint32_t regs[ENET_MAX];
     uint32_t rx_descriptor;
     uint32_t tx_descriptor;
-    uint32_t ecr;
-    uint32_t mmfr;
-    uint32_t mscr;
-    uint32_t mibc;
-    uint32_t rcr;
-    uint32_t tcr;
-    uint32_t tfwr;
-    uint32_t frsr;
-    uint32_t erdsr;
-    uint32_t etdsr;
-    uint32_t emrbr;
-    uint32_t miigsk_cfgr;
-    uint32_t miigsk_enr;
 
     uint32_t phy_status;
     uint32_t phy_control;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH v4 7/8] Add ENET/Gbps Ethernet support to FEC device
  2016-05-18 22:22 [Qemu-devel] [PATCH v4 0/8] Add Ethernet device for i.MX6 SOC Jean-Christophe Dubois
                   ` (5 preceding siblings ...)
  2016-05-18 22:23 ` [Qemu-devel] [PATCH v4 6/8] i.MX: move FEC device to a register array structure Jean-Christophe Dubois
@ 2016-05-18 22:23 ` Jean-Christophe Dubois
  2016-05-19  3:48   ` Jason Wang
  2016-05-18 22:23 ` [Qemu-devel] [PATCH v4 8/8] Add ENET device to i.MX6 SOC Jean-Christophe Dubois
  7 siblings, 1 reply; 22+ messages in thread
From: Jean-Christophe Dubois @ 2016-05-18 22:23 UTC (permalink / raw)
  To: qemu-devel, peter.maydell, jasowang; +Cc: Jean-Christophe Dubois

The ENET device (present in i.MX6) is "derived" from FEC and backward
compatible with it.

This patch adds the necessary support of the added feature in the ENET
device to allow Linux to use it (on supported processors).

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---

Changes since v1:
 * Not present on v1
   
Changes since v2:
 * Not present on v2
 
Changes since v3:
 * Separate and fix the 2 supported interrupts

 hw/arm/fsl-imx25.c       |   3 +
 hw/net/imx_fec.c         | 713 ++++++++++++++++++++++++++++++++++++++---------
 include/hw/net/imx_fec.h | 195 ++++++++++---
 3 files changed, 745 insertions(+), 166 deletions(-)

diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
index 2f878b9..ddb2b22 100644
--- a/hw/arm/fsl-imx25.c
+++ b/hw/arm/fsl-imx25.c
@@ -191,6 +191,9 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp)
     }
 
     qdev_set_nic_properties(DEVICE(&s->fec), &nd_table[0]);
+
+    object_property_set_bool(OBJECT(&s->fec), true, "is-fec", &error_abort);
+
     object_property_set_bool(OBJECT(&s->fec), true, "realized", &err);
     if (err) {
         error_propagate(errp, err);
diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index 565b4a3..af5f6cb 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -24,6 +24,8 @@
 #include "qemu/osdep.h"
 #include "hw/net/imx_fec.h"
 #include "sysemu/dma.h"
+#include "net/checksum.h"
+#include "net/eth.h"
 
 /* For crc32 */
 #include <zlib.h>
@@ -52,10 +54,93 @@
         } \
     } while (0)
 
-static const char *imx_fec_reg_name(IMXFECState *s, uint32_t index)
+static const char *imx_default_reg_name(IMXFECState *s, uint32_t index)
 {
     static char tmp[20];
+    sprintf(tmp, "index %d", index);
+    return tmp;
+}
+
+static const char *imx_fec_reg_name(IMXFECState *s, uint32_t index)
+{
+    switch (index) {
+    case ENET_FRBR:
+        return "FRBR";
+    case ENET_FRSR:
+        return "FRSR";
+    case ENET_MIIGSK_CFGR:
+        return "MIIGSK_CFGR";
+    case ENET_MIIGSK_ENR:
+        return "MIIGSK_ENR";
+    default:
+        return imx_default_reg_name(s, index);
+    }
+}
 
+static const char *imx_enet_reg_name(IMXFECState *s, uint32_t index)
+{
+    switch (index) {
+    case ENET_RSFL:
+        return "RSFL";
+    case ENET_RSEM:
+        return "RSEM";
+    case ENET_RAEM:
+        return "RAEM";
+    case ENET_RAFL:
+        return "RAFL";
+    case ENET_TSEM:
+        return "TSEM";
+    case ENET_TAEM:
+        return "TAEM";
+    case ENET_TAFL:
+        return "TAFL";
+    case ENET_TIPG:
+        return "TIPG";
+    case ENET_FTRL:
+        return "FTRL";
+    case ENET_TACC:
+        return "TACC";
+    case ENET_RACC:
+        return "RACC";
+    case ENET_ATCR:
+        return "ATCR";
+    case ENET_ATVR:
+        return "ATVR";
+    case ENET_ATOFF:
+        return "ATOFF";
+    case ENET_ATPER:
+        return "ATPER";
+    case ENET_ATCOR:
+        return "ATCOR";
+    case ENET_ATINC:
+        return "ATINC";
+    case ENET_ATSTMP:
+        return "ATSTMP";
+    case ENET_TGSR:
+        return "TGSR";
+    case ENET_TCSR0:
+        return "TCSR0";
+    case ENET_TCCR0:
+        return "TCCR0";
+    case ENET_TCSR1:
+        return "TCSR1";
+    case ENET_TCCR1:
+        return "TCCR1";
+    case ENET_TCSR2:
+        return "TCSR2";
+    case ENET_TCCR2:
+        return "TCCR2";
+    case ENET_TCSR3:
+        return "TCSR3";
+    case ENET_TCCR3:
+        return "TCCR3";
+    default:
+        return imx_default_reg_name(s, index);
+    }
+}
+
+static const char *imx_eth_reg_name(IMXFECState *s, uint32_t index)
+{
     switch (index) {
     case ENET_EIR:
         return "EIR";
@@ -99,21 +184,16 @@ static const char *imx_fec_reg_name(IMXFECState *s, uint32_t index)
         return "TDSR";
     case ENET_MRBR:
         return "MRBR";
-    case ENET_FRBR:
-        return "FRBR";
-    case ENET_FRSR:
-        return "FRSR";
-    case ENET_MIIGSK_CFGR:
-        return "MIIGSK_CFGR";
-    case ENET_MIIGSK_ENR:
-        return "MIIGSK_ENR";
     default:
-        sprintf(tmp, "index %d", index);
-        return tmp;
+        if (s->is_fec) {
+            return imx_fec_reg_name(s, index);
+        } else {
+            return imx_enet_reg_name(s, index);
+        }
     }
 }
 
-static const VMStateDescription vmstate_imx_fec = {
+static const VMStateDescription vmstate_imx_eth = {
     .name = TYPE_IMX_FEC,
     .version_id = 2,
     .minimum_version_id = 2,
@@ -139,7 +219,7 @@ static const VMStateDescription vmstate_imx_fec = {
 #define PHY_INT_PARFAULT            (1 << 2)
 #define PHY_INT_AUTONEG_PAGE        (1 << 1)
 
-static void imx_fec_update(IMXFECState *s);
+static void imx_eth_update(IMXFECState *s);
 
 /*
  * The MII phy could raise a GPIO to the processor which in turn
@@ -149,7 +229,7 @@ static void imx_fec_update(IMXFECState *s);
  */
 static void phy_update_irq(IMXFECState *s)
 {
-    imx_fec_update(s);
+    imx_eth_update(s);
 }
 
 static void phy_update_link(IMXFECState *s)
@@ -168,7 +248,7 @@ static void phy_update_link(IMXFECState *s)
     phy_update_irq(s);
 }
 
-static void imx_fec_set_link(NetClientState *nc)
+static void imx_eth_set_link(NetClientState *nc)
 {
     phy_update_link(IMX_FEC(qemu_get_nic_opaque(nc)));
 }
@@ -294,21 +374,35 @@ static void imx_fec_write_bd(IMXFECBufDesc *bd, dma_addr_t addr)
     dma_memory_write(&address_space_memory, addr, bd, sizeof(*bd));
 }
 
-static void imx_fec_update(IMXFECState *s)
+static void imx_enet_read_bd(IMXENETBufDesc *bd, dma_addr_t addr)
 {
-    if (s->regs[ENET_EIR] & s->regs[ENET_EIMR]) {
-        FEC_PRINTF("interrupt raised\n");
-        qemu_set_irq(s->irq, 1);
+    dma_memory_read(&address_space_memory, addr, bd, sizeof(*bd));
+}
+
+static void imx_enet_write_bd(IMXENETBufDesc *bd, dma_addr_t addr)
+{
+    dma_memory_write(&address_space_memory, addr, bd, sizeof(*bd));
+}
+
+static void imx_eth_update(IMXFECState *s)
+{
+    if (s->regs[ENET_EIR] & s->regs[ENET_EIMR] & ENET_INT_TS_TIMER) {
+        qemu_set_irq(s->irq[1], 1);
     } else {
-        FEC_PRINTF("interrupt lowered\n");
-        qemu_set_irq(s->irq, 0);
+        qemu_set_irq(s->irq[1], 0);
+    }
+
+    if (s->regs[ENET_EIR] & s->regs[ENET_EIMR] & ENET_INT_MAC) {
+        qemu_set_irq(s->irq[0], 1);
+    } else {
+        qemu_set_irq(s->irq[0], 0);
     }
 }
 
 static void imx_fec_do_tx(IMXFECState *s)
 {
     int frame_size = 0;
-    uint8_t frame[FEC_MAX_FRAME_SIZE];
+    uint8_t frame[ENET_MAX_FRAME_SIZE];
     uint8_t *ptr = frame;
     uint32_t addr = s->tx_descriptor;
 
@@ -318,32 +412,34 @@ static void imx_fec_do_tx(IMXFECState *s)
 
         imx_fec_read_bd(&bd, addr);
         FEC_PRINTF("tx_bd %x flags %04x len %d data %08x\n",
-                   addr, bd.flags, bd.length, bd.data);
-        if ((bd.flags & FEC_BD_R) == 0) {
+                    addr, bd.flags, bd.length, bd.data);
+        if ((bd.flags & ENET_BD_R) == 0) {
             /* Run out of descriptors to transmit.  */
+            FEC_PRINTF("tx_bd ran out of descriptors to transmit\n");
             break;
         }
         len = bd.length;
-        if (frame_size + len > FEC_MAX_FRAME_SIZE) {
-            len = FEC_MAX_FRAME_SIZE - frame_size;
-            s->regs[ENET_EIR] |= FEC_INT_BABT;
+        if (frame_size + len > ENET_MAX_FRAME_SIZE) {
+            len = ENET_MAX_FRAME_SIZE - frame_size;
+            s->regs[ENET_EIR] |= ENET_INT_BABT;
         }
         dma_memory_read(&address_space_memory, bd.data, ptr, len);
         ptr += len;
         frame_size += len;
-        if (bd.flags & FEC_BD_L) {
+        if (bd.flags & ENET_BD_L) {
+            FEC_PRINTF("sending %d bytes frame\n", len);
             /* Last buffer in frame.  */
             qemu_send_packet(qemu_get_queue(s->nic), frame, len);
             ptr = frame;
             frame_size = 0;
-            s->regs[ENET_EIR] |= FEC_INT_TXF;
+            s->regs[ENET_EIR] |= ENET_INT_TXF;
         }
-        s->regs[ENET_EIR] |= FEC_INT_TXB;
-        bd.flags &= ~FEC_BD_R;
+        s->regs[ENET_EIR] |= ENET_INT_TXB;
+        bd.flags &= ~ENET_BD_R;
         /* Write back the modified descriptor.  */
         imx_fec_write_bd(&bd, addr);
         /* Advance to the next descriptor.  */
-        if ((bd.flags & FEC_BD_W) != 0) {
+        if ((bd.flags & ENET_BD_W) != 0) {
             addr = s->regs[ENET_TDSR];
         } else {
             addr += sizeof(bd);
@@ -352,17 +448,97 @@ static void imx_fec_do_tx(IMXFECState *s)
 
     s->tx_descriptor = addr;
 
-    imx_fec_update(s);
+    imx_eth_update(s);
 }
 
-static void imx_fec_enable_rx(IMXFECState *s)
+static void imx_enet_do_tx(IMXFECState *s)
+{
+    int frame_size = 0;
+    uint8_t frame[ENET_MAX_FRAME_SIZE];
+    uint8_t *ptr = frame;
+    uint32_t addr = s->tx_descriptor;
+
+    while (1) {
+        IMXENETBufDesc bd;
+        int len;
+
+        imx_enet_read_bd(&bd, addr);
+        FEC_PRINTF("tx_bd %x flags %04x len %d data %08x option %04x "
+                   "status %04x\n", addr, bd.flags, bd.length, bd.data,
+                   bd.option, bd.status);
+        if ((bd.flags & ENET_BD_R) == 0) {
+            /* Run out of descriptors to transmit.  */
+            break;
+        }
+        len = bd.length;
+        if (frame_size + len > ENET_MAX_FRAME_SIZE) {
+            len = ENET_MAX_FRAME_SIZE - frame_size;
+            s->regs[ENET_EIR] |= ENET_INT_BABT;
+        }
+        dma_memory_read(&address_space_memory, bd.data, ptr, len);
+        ptr += len;
+        frame_size += len;
+        if (bd.flags & ENET_BD_L) {
+            if (bd.option & ENET_BD_PINS) {
+                struct ip_header *ip_hd = PKT_GET_IP_HDR(frame);
+                if (IP_HEADER_VERSION(ip_hd) == 4) {
+                    net_checksum_calculate(frame, frame_size);
+                }
+            }
+            if (bd.option & ENET_BD_IINS) {
+                struct ip_header *ip_hd = PKT_GET_IP_HDR(frame);
+                /* We compute checksum only for IPv4 frames */
+                if (IP_HEADER_VERSION(ip_hd) == 4) {
+                    uint16_t csum;
+                    ip_hd->ip_sum = 0;
+                    csum = net_raw_checksum((uint8_t *)ip_hd, sizeof(*ip_hd));
+                    ip_hd->ip_sum = cpu_to_be16(csum);
+                }
+            }
+            /* Last buffer in frame.  */
+            qemu_send_packet(qemu_get_queue(s->nic), frame, len);
+            ptr = frame;
+            frame_size = 0;
+            if (bd.option & ENET_BD_TX_INT) {
+                s->regs[ENET_EIR] |= ENET_INT_TXF;
+            }
+        }
+        if (bd.option & ENET_BD_TX_INT) {
+            s->regs[ENET_EIR] |= ENET_INT_TXB;
+        }
+        bd.flags &= ~ENET_BD_R;
+        /* Write back the modified descriptor.  */
+        imx_enet_write_bd(&bd, addr);
+        /* Advance to the next descriptor.  */
+        if ((bd.flags & ENET_BD_W) != 0) {
+            addr = s->regs[ENET_TDSR];
+        } else {
+            addr += sizeof(bd);
+        }
+    }
+
+    s->tx_descriptor = addr;
+
+    imx_eth_update(s);
+}
+
+static void imx_eth_do_tx(IMXFECState *s)
+{
+    if (!s->is_fec && (s->regs[ENET_ECR] & ENET_ECR_EN1588)) {
+        imx_enet_do_tx(s);
+    } else {
+        imx_fec_do_tx(s);
+    }
+}
+
+static void imx_eth_enable_rx(IMXFECState *s)
 {
     IMXFECBufDesc bd;
     bool tmp;
 
     imx_fec_read_bd(&bd, s->rx_descriptor);
 
-    tmp = ((bd.flags & FEC_BD_E) != 0);
+    tmp = ((bd.flags & ENET_BD_E) != 0);
 
     if (!tmp) {
         FEC_PRINTF("RX buffer full\n");
@@ -373,11 +549,11 @@ static void imx_fec_enable_rx(IMXFECState *s)
     s->regs[ENET_RDAR] = tmp ? ENET_RDAR_RDAR : 0;
 }
 
-static void imx_fec_reset(DeviceState *d)
+static void imx_eth_reset(DeviceState *d)
 {
     IMXFECState *s = IMX_FEC(d);
 
-    /* Reset the FEC */
+    /* Reset the Device */
     memset(s->regs, 0, sizeof(s->regs));
     s->regs[ENET_ECR]   = 0xf0000000;
     s->regs[ENET_MIBC]  = 0xc0000000;
@@ -392,9 +568,19 @@ static void imx_fec_reset(DeviceState *d)
                           | (s->conf.macaddr.a[5] << 16)
                           | 0x8808;
 
-    s->regs[ENET_FRBR]  = 0x00000600;
-    s->regs[ENET_FRSR]  = 0x00000500;
-    s->regs[ENET_MIIGSK_ENR]  = 0x00000006;
+    if (s->is_fec) {
+        s->regs[ENET_FRBR]  = 0x00000600;
+        s->regs[ENET_FRSR]  = 0x00000500;
+        s->regs[ENET_MIIGSK_ENR]  = 0x00000006;
+    } else {
+        s->regs[ENET_RAEM]  = 0x00000004;
+        s->regs[ENET_RAFL]  = 0x00000004;
+        s->regs[ENET_TAEM]  = 0x00000004;
+        s->regs[ENET_TAFL]  = 0x00000008;
+        s->regs[ENET_TIPG]  = 0x0000000c;
+        s->regs[ENET_FTRL]  = 0x000007ff;
+        s->regs[ENET_ATPER] = 0x3b9aca00;
+    }
 
     s->rx_descriptor = 0;
     s->tx_descriptor = 0;
@@ -403,11 +589,67 @@ static void imx_fec_reset(DeviceState *d)
     phy_reset(s);
 }
 
-static uint64_t imx_fec_read(void *opaque, hwaddr addr, unsigned size)
+static uint32_t imx_default_read(IMXFECState *s, uint32_t index)
+{
+    qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad register at offset 0x%"
+                  PRIx32 "\n", TYPE_IMX_FEC, __func__, index * 4);
+    return 0;
+}
+
+static uint32_t imx_fec_read(IMXFECState *s, uint32_t index)
+{
+    switch (index) {
+    case ENET_FRBR:
+    case ENET_FRSR:
+    case ENET_MIIGSK_CFGR:
+    case ENET_MIIGSK_ENR:
+        return s->regs[index];
+    default:
+        return imx_default_read(s, index);
+    }
+}
+
+static uint32_t imx_enet_read(IMXFECState *s, uint32_t index)
+{
+    switch (index) {
+    case ENET_RSFL:
+    case ENET_RSEM:
+    case ENET_RAEM:
+    case ENET_RAFL:
+    case ENET_TSEM:
+    case ENET_TAEM:
+    case ENET_TAFL:
+    case ENET_TIPG:
+    case ENET_FTRL:
+    case ENET_TACC:
+    case ENET_RACC:
+    case ENET_ATCR:
+    case ENET_ATVR:
+    case ENET_ATOFF:
+    case ENET_ATPER:
+    case ENET_ATCOR:
+    case ENET_ATINC:
+    case ENET_ATSTMP:
+    case ENET_TGSR:
+    case ENET_TCSR0:
+    case ENET_TCCR0:
+    case ENET_TCSR1:
+    case ENET_TCCR1:
+    case ENET_TCSR2:
+    case ENET_TCCR2:
+    case ENET_TCSR3:
+    case ENET_TCCR3:
+        return s->regs[index];
+    default:
+        return imx_default_read(s, index);
+    }
+}
+
+static uint64_t imx_eth_read(void *opaque, hwaddr offset, unsigned size)
 {
     uint32_t value = 0;
     IMXFECState *s = IMX_FEC(opaque);
-    uint32_t index = addr >> 2;
+    uint32_t index = offset >> 2;
 
     switch (index) {
     case ENET_EIR:
@@ -431,32 +673,126 @@ static uint64_t imx_fec_read(void *opaque, hwaddr addr, unsigned size)
     case ENET_RDSR:
     case ENET_TDSR:
     case ENET_MRBR:
-    case ENET_FRBR:
-    case ENET_FRSR:
-    case ENET_MIIGSK_CFGR:
-    case ENET_MIIGSK_ENR:
         value = s->regs[index];
         break;
     default:
-        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad register at offset 0x%"
-                      PRIx32 "\n", TYPE_IMX_FEC, __func__, index * 4);
+        if (s->is_fec) {
+            value = imx_fec_read(s, index);
+        } else {
+            value = imx_enet_read(s, index);
+        }
         break;
     }
 
-    FEC_PRINTF("reg[%s] => 0x%" PRIx32 "\n", imx_fec_reg_name(s, index),
+    FEC_PRINTF("reg[%s] => 0x%" PRIx32 "\n", imx_eth_reg_name(s, index),
                                               value);
 
     return value;
 }
 
-static void imx_fec_write(void *opaque, hwaddr addr,
-                          uint64_t value, unsigned size)
+static void imx_default_write(IMXFECState *s, uint32_t index, uint32_t value)
+{
+    qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad address at offset 0x%"
+                  PRIx32 "\n", TYPE_IMX_FEC, __func__, index * 4);
+    return;
+}
+
+static void imx_fec_write(IMXFECState *s, uint32_t index, uint32_t value)
+{
+    switch (index) {
+    case ENET_FRBR:
+        /* FRBR is read only */
+        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Register FRBR is read only\n",
+                      TYPE_IMX_FEC, __func__);
+        break;
+    case ENET_FRSR:
+        s->regs[index] = (value & 0x000003fc) | 0x00000400;
+        break;
+    case ENET_MIIGSK_CFGR:
+        s->regs[index] = value & 0x00000053;
+        break;
+    case ENET_MIIGSK_ENR:
+        s->regs[index] = (value & 0x00000002) ? 0x00000006 : 0;
+        break;
+    default:
+        imx_default_write(s, index, value);
+        break;
+    }
+}
+
+static void imx_enet_write(IMXFECState *s, uint32_t index, uint32_t value)
+{
+    switch (index) {
+    case ENET_RSFL:
+    case ENET_RSEM:
+    case ENET_RAEM:
+    case ENET_RAFL:
+    case ENET_TSEM:
+    case ENET_TAEM:
+    case ENET_TAFL:
+        s->regs[index] = value & 0x000001ff;
+        break;
+    case ENET_TIPG:
+        s->regs[index] = value & 0x0000001f;
+        break;
+    case ENET_FTRL:
+        s->regs[index] = value & 0x00003fff;
+        break;
+    case ENET_TACC:
+        s->regs[index] = value & 0x00000019;
+        break;
+    case ENET_RACC:
+        s->regs[index] = value & 0x000000C7;
+        break;
+    case ENET_ATCR:
+        s->regs[index] = value & 0x00002a9d;
+        break;
+    case ENET_ATVR:
+    case ENET_ATOFF:
+    case ENET_ATPER:
+        s->regs[index] = value;
+        break;
+    case ENET_ATSTMP:
+        /* ATSTMP is read only */
+        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Register ATSTMP is read only\n",
+                      TYPE_IMX_FEC, __func__);
+        break;
+    case ENET_ATCOR:
+        s->regs[index] = value & 0x7fffffff;
+        break;
+    case ENET_ATINC:
+        s->regs[index] = value & 0x00007f7f;
+        break;
+    case ENET_TGSR:
+        /* implement clear timer flag */
+        value = value & 0x0000000f;
+        break;
+    case ENET_TCSR0:
+    case ENET_TCSR1:
+    case ENET_TCSR2:
+    case ENET_TCSR3:
+        value = value & 0x000000fd;
+        break;
+    case ENET_TCCR0:
+    case ENET_TCCR1:
+    case ENET_TCCR2:
+    case ENET_TCCR3:
+        s->regs[index] = value;
+        break;
+    default:
+        imx_default_write(s, index, value);
+        break;
+    }
+}
+
+static void imx_eth_write(void *opaque, hwaddr offset, uint64_t value,
+                           unsigned size)
 {
     IMXFECState *s = IMX_FEC(opaque);
-    uint32_t index = addr >> 2;
+    uint32_t index = offset >> 2;
 
-    FEC_PRINTF("reg[%s] <= 0x%" PRIx32 "\n", imx_fec_reg_name(s, index),
-                                             (uint32_t)value);
+    FEC_PRINTF("reg[%s] <= 0x%" PRIx32 "\n", imx_eth_reg_name(s, index),
+                (uint32_t)value);
 
     switch (index) {
     case ENET_EIR:
@@ -466,28 +802,28 @@ static void imx_fec_write(void *opaque, hwaddr addr,
         s->regs[index] = value;
         break;
     case ENET_RDAR:
-        if (s->regs[ENET_ECR] & FEC_EN) {
+        if (s->regs[ENET_ECR] & ENET_ECR_ETHEREN) {
             if (!s->regs[index]) {
                 s->regs[index] = ENET_RDAR_RDAR;
-                imx_fec_enable_rx(s);
+                imx_eth_enable_rx(s);
             }
         } else {
             s->regs[index] = 0;
         }
         break;
     case ENET_TDAR:
-        if (s->regs[ENET_ECR] & FEC_EN) {
+        if (s->regs[ENET_ECR] & ENET_ECR_ETHEREN) {
             s->regs[index] = ENET_TDAR_TDAR;
-            imx_fec_do_tx(s);
+            imx_eth_do_tx(s);
         }
         s->regs[index] = 0;
         break;
     case ENET_ECR:
-        if (value & FEC_RESET) {
-            return imx_fec_reset(DEVICE(s));
+        if (value & ENET_ECR_RESET) {
+            return imx_eth_reset(DEVICE(s));
         }
         s->regs[index] = value;
-        if ((s->regs[index] & FEC_EN) == 0) {
+        if ((s->regs[index] & ENET_ECR_ETHEREN) == 0) {
             s->regs[ENET_RDAR] = 0;
             s->rx_descriptor = s->regs[ENET_RDSR];
             s->regs[ENET_TDAR] = 0;
@@ -507,7 +843,7 @@ static void imx_fec_write(void *opaque, hwaddr addr,
             do_phy_write(s, extract32(value, 18, 10), extract32(value, 0, 16));
         }
         /* raise the interrupt as the PHY operation is done */
-        s->regs[ENET_EIR] |= FEC_INT_MII;
+        s->regs[ENET_EIR] |= ENET_INT_MII;
         break;
     case ENET_MSCR:
         s->regs[index] = value & 0xfe;
@@ -524,7 +860,7 @@ static void imx_fec_write(void *opaque, hwaddr addr,
         /* We transmit immediately, so raise GRA immediately.  */
         s->regs[index] = value;
         if (value & 1) {
-            s->regs[ENET_EIR] |= FEC_INT_GRA;
+            s->regs[ENET_EIR] |= ENET_INT_GRA;
         }
         break;
     case ENET_PALR:
@@ -549,46 +885,49 @@ static void imx_fec_write(void *opaque, hwaddr addr,
         /* TODO: implement MAC hash filtering.  */
         break;
     case ENET_TFWR:
-        s->regs[index] = value & 3;
-        break;
-    case ENET_FRBR:
-        /* FRBR is read only */
-        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Register FRBR is read only\n",
-                      TYPE_IMX_FEC, __func__);
-        break;
-    case ENET_FRSR:
-        s->regs[index] = (value & 0x000003fc) | 0x00000400;
+        if (s->is_fec) {
+            s->regs[index] = value & 0x3;
+        } else {
+            s->regs[index] = value & 0x13f;
+        }
         break;
     case ENET_RDSR:
-        s->regs[index] = value & ~3;
+        if (s->is_fec) {
+            s->regs[index] = value & ~3;
+        } else {
+            s->regs[index] = value & ~7;
+        }
         s->rx_descriptor = s->regs[index];
         break;
     case ENET_TDSR:
-        s->regs[index] = value & ~3;
+        if (s->is_fec) {
+            s->regs[index] = value & ~3;
+        } else {
+            s->regs[index] = value & ~7;
+        }
         s->tx_descriptor = s->regs[index];
         break;
     case ENET_MRBR:
-        s->regs[index] = value & 0x000007f0;
-        break;
-    case ENET_MIIGSK_CFGR:
-        s->regs[index] = value & 0x00000053;
-        break;
-    case ENET_MIIGSK_ENR:
-        s->regs[index] = (value & 0x00000002) ? 0x00000006 : 0;
+        s->regs[index] = value & 0x00003ff0;
         break;
     default:
-        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad address at offset 0x%"
-                      PRIx32 "\n", TYPE_IMX_FEC, __func__, index * 4);
-        break;
+        if (s->is_fec) {
+            imx_fec_write(s, index, value);
+        } else {
+            imx_enet_write(s, index, value);
+        }
+        return;
     }
 
-    imx_fec_update(s);
+    imx_eth_update(s);
 }
 
-static int imx_fec_can_receive(NetClientState *nc)
+static int imx_eth_can_receive(NetClientState *nc)
 {
     IMXFECState *s = IMX_FEC(qemu_get_nic_opaque(nc));
 
+    FEC_PRINTF("\n");
+
     return s->regs[ENET_RDAR] ? 1 : 0;
 }
 
@@ -618,21 +957,21 @@ static ssize_t imx_fec_receive(NetClientState *nc, const uint8_t *buf,
     crc = cpu_to_be32(crc32(~0, buf, size));
     crc_ptr = (uint8_t *) &crc;
 
-    /* Huge frames are truncted.  */
-    if (size > FEC_MAX_FRAME_SIZE) {
-        size = FEC_MAX_FRAME_SIZE;
-        flags |= FEC_BD_TR | FEC_BD_LG;
+    /* Huge frames are truncated.  */
+    if (size > ENET_MAX_FRAME_SIZE) {
+        size = ENET_MAX_FRAME_SIZE;
+        flags |= ENET_BD_TR | ENET_BD_LG;
     }
 
     /* Frames larger than the user limit just set error flags.  */
     if (size > (s->regs[ENET_RCR] >> 16)) {
-        flags |= FEC_BD_LG;
+        flags |= ENET_BD_LG;
     }
 
     addr = s->rx_descriptor;
     while (size > 0) {
         imx_fec_read_bd(&bd, addr);
-        if ((bd.flags & FEC_BD_E) == 0) {
+        if ((bd.flags & ENET_BD_E) == 0) {
             /* No descriptors available.  Bail out.  */
             /*
              * FIXME: This is wrong. We should probably either
@@ -661,99 +1000,211 @@ static ssize_t imx_fec_receive(NetClientState *nc, const uint8_t *buf,
                              crc_ptr, 4 - size);
             crc_ptr += 4 - size;
         }
-        bd.flags &= ~FEC_BD_E;
+        bd.flags &= ~ENET_BD_E;
         if (size == 0) {
             /* Last buffer in frame.  */
-            bd.flags |= flags | FEC_BD_L;
+            bd.flags |= flags | ENET_BD_L;
             FEC_PRINTF("rx frame flags %04x\n", bd.flags);
-            s->regs[ENET_EIR] |= FEC_INT_RXF;
+            s->regs[ENET_EIR] |= ENET_INT_RXF;
         } else {
-            s->regs[ENET_EIR] |= FEC_INT_RXB;
+            s->regs[ENET_EIR] |= ENET_INT_RXB;
         }
         imx_fec_write_bd(&bd, addr);
         /* Advance to the next descriptor.  */
-        if ((bd.flags & FEC_BD_W) != 0) {
+        if ((bd.flags & ENET_BD_W) != 0) {
             addr = s->regs[ENET_RDSR];
         } else {
             addr += sizeof(bd);
         }
     }
     s->rx_descriptor = addr;
-    imx_fec_enable_rx(s);
-    imx_fec_update(s);
+    imx_eth_enable_rx(s);
+    imx_eth_update(s);
     return len;
 }
 
-static const MemoryRegionOps imx_fec_ops = {
-    .read = imx_fec_read,
-    .write = imx_fec_write,
+static ssize_t imx_enet_receive(NetClientState *nc, const uint8_t *buf,
+                                size_t len)
+{
+    IMXFECState *s = IMX_FEC(qemu_get_nic_opaque(nc));
+    IMXENETBufDesc bd;
+    uint32_t flags = 0;
+    uint32_t addr;
+    uint32_t crc;
+    uint32_t buf_addr;
+    uint8_t *crc_ptr;
+    unsigned int buf_len;
+    size_t size = len;
+
+    FEC_PRINTF("len %d\n", (int)size);
+
+    if (!s->regs[ENET_RDAR]) {
+        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Unexpected packet\n",
+                      TYPE_IMX_FEC, __func__);
+        return 0;
+    }
+
+    /* 4 bytes for the CRC.  */
+    size += 4;
+    crc = cpu_to_be32(crc32(~0, buf, size));
+    crc_ptr = (uint8_t *) &crc;
+
+    /* Huge frames are truncted.  */
+    if (size > ENET_MAX_FRAME_SIZE) {
+        size = ENET_MAX_FRAME_SIZE;
+        flags |= ENET_BD_TR | ENET_BD_LG;
+    }
+
+    /* Frames larger than the user limit just set error flags.  */
+    if (size > (s->regs[ENET_RCR] >> 16)) {
+        flags |= ENET_BD_LG;
+    }
+
+    addr = s->rx_descriptor;
+    while (size > 0) {
+        imx_enet_read_bd(&bd, addr);
+        if ((bd.flags & ENET_BD_E) == 0) {
+            /* No descriptors available.  Bail out.  */
+            /*
+             * FIXME: This is wrong. We should probably either
+             * save the remainder for when more RX buffers are
+             * available, or flag an error.
+             */
+            qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Lost end of frame\n",
+                          TYPE_IMX_FEC, __func__);
+            break;
+        }
+        buf_len = (size <= s->regs[ENET_MRBR]) ? size : s->regs[ENET_MRBR];
+        bd.length = buf_len;
+        size -= buf_len;
+
+        FEC_PRINTF("rx_bd 0x%x length %d\n", addr, bd.length);
+
+        /* The last 4 bytes are the CRC.  */
+        if (size < 4) {
+            buf_len += size - 4;
+        }
+        buf_addr = bd.data;
+        dma_memory_write(&address_space_memory, buf_addr, buf, buf_len);
+        buf += buf_len;
+        if (size < 4) {
+            dma_memory_write(&address_space_memory, buf_addr + buf_len,
+                             crc_ptr, 4 - size);
+            crc_ptr += 4 - size;
+        }
+        bd.flags &= ~ENET_BD_E;
+        if (size == 0) {
+            /* Last buffer in frame.  */
+            bd.flags |= flags | ENET_BD_L;
+            FEC_PRINTF("rx frame flags %04x\n", bd.flags);
+            if (bd.option & ENET_BD_RX_INT) {
+                s->regs[ENET_EIR] |= ENET_INT_RXF;
+            }
+        } else {
+            if (bd.option & ENET_BD_RX_INT) {
+                s->regs[ENET_EIR] |= ENET_INT_RXB;
+            }
+        }
+        imx_enet_write_bd(&bd, addr);
+        /* Advance to the next descriptor.  */
+        if ((bd.flags & ENET_BD_W) != 0) {
+            addr = s->regs[ENET_RDSR];
+        } else {
+            addr += sizeof(bd);
+        }
+    }
+    s->rx_descriptor = addr;
+    imx_eth_enable_rx(s);
+    imx_eth_update(s);
+    return len;
+}
+
+static ssize_t imx_eth_receive(NetClientState *nc, const uint8_t *buf,
+                                size_t len)
+{
+    IMXFECState *s = IMX_FEC(qemu_get_nic_opaque(nc));
+
+    if (!s->is_fec && (s->regs[ENET_ECR] & ENET_ECR_EN1588)) {
+        return imx_enet_receive(nc, buf, len);
+    } else {
+        return imx_fec_receive(nc, buf, len);
+    }
+}
+
+static const MemoryRegionOps imx_eth_ops = {
+    .read                  = imx_eth_read,
+    .write                 = imx_eth_write,
     .valid.min_access_size = 4,
     .valid.max_access_size = 4,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness            = DEVICE_NATIVE_ENDIAN,
 };
 
-static void imx_fec_cleanup(NetClientState *nc)
+static void imx_eth_cleanup(NetClientState *nc)
 {
     IMXFECState *s = IMX_FEC(qemu_get_nic_opaque(nc));
 
     s->nic = NULL;
 }
 
-static NetClientInfo net_imx_fec_info = {
-    .type = NET_CLIENT_OPTIONS_KIND_NIC,
-    .size = sizeof(NICState),
-    .can_receive = imx_fec_can_receive,
-    .receive = imx_fec_receive,
-    .cleanup = imx_fec_cleanup,
-    .link_status_changed = imx_fec_set_link,
+static NetClientInfo imx_eth_net_info = {
+    .type                = NET_CLIENT_OPTIONS_KIND_NIC,
+    .size                = sizeof(NICState),
+    .can_receive         = imx_eth_can_receive,
+    .receive             = imx_eth_receive,
+    .cleanup             = imx_eth_cleanup,
+    .link_status_changed = imx_eth_set_link,
 };
 
 
-static void imx_fec_realize(DeviceState *dev, Error **errp)
+static void imx_eth_realize(DeviceState *dev, Error **errp)
 {
     IMXFECState *s = IMX_FEC(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 
-    memory_region_init_io(&s->iomem, OBJECT(dev), &imx_fec_ops, s,
+    memory_region_init_io(&s->iomem, OBJECT(dev), &imx_eth_ops, s,
                           TYPE_IMX_FEC, 0x400);
     sysbus_init_mmio(sbd, &s->iomem);
-    sysbus_init_irq(sbd, &s->irq);
+    sysbus_init_irq(sbd, &s->irq[0]);
+    sysbus_init_irq(sbd, &s->irq[1]);
+
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
 
     s->conf.peers.ncs[0] = nd_table[0].netdev;
 
-    s->nic = qemu_new_nic(&net_imx_fec_info, &s->conf,
-                          object_get_typename(OBJECT(dev)), DEVICE(dev)->id,
-                          s);
+    s->nic = qemu_new_nic(&imx_eth_net_info, &s->conf,
+                          object_get_typename(OBJECT(dev)),
+                          DEVICE(dev)->id, s);
+
     qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
 }
 
-static Property imx_fec_properties[] = {
+static Property imx_eth_properties[] = {
     DEFINE_NIC_PROPERTIES(IMXFECState, conf),
+    DEFINE_PROP_BOOL("is-fec", IMXFECState, is_fec, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void imx_fec_class_init(ObjectClass *klass, void *data)
+static void imx_eth_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    dc->vmsd = &vmstate_imx_fec;
-    dc->reset = imx_fec_reset;
-    dc->props = imx_fec_properties;
-    dc->realize = imx_fec_realize;
-    dc->desc = "i.MX FEC Ethernet Controller";
+    dc->vmsd    = &vmstate_imx_eth;
+    dc->reset   = imx_eth_reset;
+    dc->props   = imx_eth_properties;
+    dc->realize = imx_eth_realize;
+    dc->desc    = "i.MX FEC/ENET Ethernet Controller";
 }
 
-static const TypeInfo imx_fec_info = {
-    .name = TYPE_IMX_FEC,
-    .parent = TYPE_SYS_BUS_DEVICE,
+static const TypeInfo imx_eth_info = {
+    .name          = TYPE_IMX_FEC,
+    .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(IMXFECState),
-    .class_init = imx_fec_class_init,
+    .class_init    = imx_eth_class_init,
 };
 
-static void imx_fec_register_types(void)
+static void imx_eth_register_types(void)
 {
-    type_register_static(&imx_fec_info);
+    type_register_static(&imx_eth_info);
 }
 
-type_init(imx_fec_register_types)
+type_init(imx_eth_register_types)
diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h
index 709f8a0..a09b7d7 100644
--- a/include/hw/net/imx_fec.h
+++ b/include/hw/net/imx_fec.h
@@ -1,5 +1,5 @@
 /*
- * i.MX Fast Ethernet Controller emulation.
+ * i.MX FEC/ENET Ethernet Controller emulation.
  *
  * Copyright (c) 2013 Jean-Christophe Dubois. <jcd@tribudubois.net>
  *
@@ -53,25 +53,64 @@
 #define ENET_RDSR              96
 #define ENET_TDSR              97
 #define ENET_MRBR              98
+#define ENET_RSFL              100
+#define ENET_RSEM              101
+#define ENET_RAEM              102
+#define ENET_RAFL              103
+#define ENET_TSEM              104
+#define ENET_TAEM              105
+#define ENET_TAFL              106
+#define ENET_TIPG              107
+#define ENET_FTRL              108
+#define ENET_TACC              112
+#define ENET_RACC              113
 #define ENET_MIIGSK_CFGR       192
 #define ENET_MIIGSK_ENR        194
+#define ENET_ATCR              256
+#define ENET_ATVR              257
+#define ENET_ATOFF             258
+#define ENET_ATPER             259
+#define ENET_ATCOR             260
+#define ENET_ATINC             261
+#define ENET_ATSTMP            262
+#define ENET_TGSR              385
+#define ENET_TCSR0             386
+#define ENET_TCCR0             387
+#define ENET_TCSR1             388
+#define ENET_TCCR1             389
+#define ENET_TCSR2             390
+#define ENET_TCCR2             391
+#define ENET_TCSR3             392
+#define ENET_TCCR3             393
 #define ENET_MAX               400
 
-#define FEC_MAX_FRAME_SIZE 2032
-
-#define FEC_INT_HB      (1 << 31)
-#define FEC_INT_BABR    (1 << 30)
-#define FEC_INT_BABT    (1 << 29)
-#define FEC_INT_GRA     (1 << 28)
-#define FEC_INT_TXF     (1 << 27)
-#define FEC_INT_TXB     (1 << 26)
-#define FEC_INT_RXF     (1 << 25)
-#define FEC_INT_RXB     (1 << 24)
-#define FEC_INT_MII     (1 << 23)
-#define FEC_INT_EBERR   (1 << 22)
-#define FEC_INT_LC      (1 << 21)
-#define FEC_INT_RL      (1 << 20)
-#define FEC_INT_UN      (1 << 19)
+#define ENET_MAX_FRAME_SIZE    2032
+
+/* EIR and EIMR */
+#define ENET_INT_HB            (1 << 31)
+#define ENET_INT_BABR          (1 << 30)
+#define ENET_INT_BABT          (1 << 29)
+#define ENET_INT_GRA           (1 << 28)
+#define ENET_INT_TXF           (1 << 27)
+#define ENET_INT_TXB           (1 << 26)
+#define ENET_INT_RXF           (1 << 25)
+#define ENET_INT_RXB           (1 << 24)
+#define ENET_INT_MII           (1 << 23)
+#define ENET_INT_EBERR         (1 << 22)
+#define ENET_INT_LC            (1 << 21)
+#define ENET_INT_RL            (1 << 20)
+#define ENET_INT_UN            (1 << 19)
+#define ENET_INT_PLR           (1 << 18)
+#define ENET_INT_WAKEUP        (1 << 17)
+#define ENET_INT_TS_AVAIL      (1 << 16)
+#define ENET_INT_TS_TIMER      (1 << 15)
+
+#define ENET_INT_MAC           (ENET_INT_HB | ENET_INT_BABR | ENET_INT_BABT | \
+                                ENET_INT_GRA | ENET_INT_TXF | ENET_INT_TXB | \
+                                ENET_INT_RXF | ENET_INT_RXB | ENET_INT_MII | \
+                                ENET_INT_EBERR | ENET_INT_LC | ENET_INT_RL | \
+                                ENET_INT_UN | ENET_INT_PLR | ENET_INT_WAKEUP | \
+                                ENET_INT_TS_AVAIL)
 
 /* RDAR */
 #define ENET_RDAR_RDAR         (1 << 24)
@@ -79,8 +118,54 @@
 /* TDAR */
 #define ENET_TDAR_TDAR         (1 << 24)
 
-#define FEC_EN                 (1 << 1)
-#define FEC_RESET              (1 << 0)
+/* ECR */
+#define ENET_ECR_RESET         (1 << 0)
+#define ENET_ECR_ETHEREN       (1 << 1)
+#define ENET_ECR_MAGICEN       (1 << 2)
+#define ENET_ECR_SLEEP         (1 << 3)
+#define ENET_ECR_EN1588        (1 << 4)
+#define ENET_ECR_SPEED         (1 << 5)
+#define ENET_ECR_DBGEN         (1 << 6)
+#define ENET_ECR_STOPEN        (1 << 7)
+#define ENET_ECR_DSBWP         (1 << 8)
+
+/* MIBC */
+#define ENET_MIBC_MIB_DIS      (1 << 31)
+#define ENET_MIBC_MIB_IDLE     (1 << 30)
+#define ENET_MIBC_MIB_CLEAR    (1 << 29)
+
+/* RCR */
+#define ENET_RCR_LOOP          (1 << 0)
+#define ENET_RCR_DRT           (1 << 1)
+#define ENET_RCR_MII_MODE      (1 << 2)
+#define ENET_RCR_PROM          (1 << 3)
+#define ENET_RCR_BC_REJ        (1 << 4)
+#define ENET_RCR_FCE           (1 << 5)
+#define ENET_RCR_RGMII_EN      (1 << 6)
+#define ENET_RCR_RMII_MODE     (1 << 8)
+#define ENET_RCR_RMII_10T      (1 << 9)
+#define ENET_RCR_PADEN         (1 << 12)
+#define ENET_RCR_PAUFWD        (1 << 13)
+#define ENET_RCR_CRCFWD        (1 << 14)
+#define ENET_RCR_CFEN          (1 << 15)
+#define ENET_RCR_MAX_FL_SHIFT  (16)
+#define ENET_RCR_MAX_FL_LENGTH (14)
+#define ENET_RCR_NLC           (1 << 30)
+#define ENET_RCR_GRS           (1 << 31)
+
+/* TCR */
+#define ENET_TCR_GTS           (1 << 0)
+#define ENET_TCR_FDEN          (1 << 2)
+#define ENET_TCR_TFC_PAUSE     (1 << 3)
+#define ENET_TCR_RFC_PAUSE     (1 << 4)
+#define ENET_TCR_ADDSEL_SHIFT  (5)
+#define ENET_TCR_ADDSEL_LENGTH (3)
+#define ENET_TCR_CRCFWD        (1 << 9)
+
+/* RDSR */
+#define ENET_TWFR_TFWR_SHIFT   (0)
+#define ENET_TWFR_TFWR_LENGTH  (6)
+#define ENET_TWFR_STRFWD       (1 << 8)
 
 /* Buffer Descriptor.  */
 typedef struct {
@@ -89,22 +174,60 @@ typedef struct {
     uint32_t data;
 } IMXFECBufDesc;
 
-#define FEC_BD_R    (1 << 15)
-#define FEC_BD_E    (1 << 15)
-#define FEC_BD_O1   (1 << 14)
-#define FEC_BD_W    (1 << 13)
-#define FEC_BD_O2   (1 << 12)
-#define FEC_BD_L    (1 << 11)
-#define FEC_BD_TC   (1 << 10)
-#define FEC_BD_ABC  (1 << 9)
-#define FEC_BD_M    (1 << 8)
-#define FEC_BD_BC   (1 << 7)
-#define FEC_BD_MC   (1 << 6)
-#define FEC_BD_LG   (1 << 5)
-#define FEC_BD_NO   (1 << 4)
-#define FEC_BD_CR   (1 << 2)
-#define FEC_BD_OV   (1 << 1)
-#define FEC_BD_TR   (1 << 0)
+#define ENET_BD_R              (1 << 15)
+#define ENET_BD_E              (1 << 15)
+#define ENET_BD_O1             (1 << 14)
+#define ENET_BD_W              (1 << 13)
+#define ENET_BD_O2             (1 << 12)
+#define ENET_BD_L              (1 << 11)
+#define ENET_BD_TC             (1 << 10)
+#define ENET_BD_ABC            (1 << 9)
+#define ENET_BD_M              (1 << 8)
+#define ENET_BD_BC             (1 << 7)
+#define ENET_BD_MC             (1 << 6)
+#define ENET_BD_LG             (1 << 5)
+#define ENET_BD_NO             (1 << 4)
+#define ENET_BD_CR             (1 << 2)
+#define ENET_BD_OV             (1 << 1)
+#define ENET_BD_TR             (1 << 0)
+
+typedef struct {
+    uint16_t length;
+    uint16_t flags;
+    uint32_t data;
+    uint16_t status;
+    uint16_t option;
+    uint16_t checksum;
+    uint16_t head_proto;
+    uint32_t last_buffer;
+    uint32_t timestamp;
+    uint32_t reserved[2];
+} IMXENETBufDesc;
+
+#define ENET_BD_ME             (1 << 15)
+#define ENET_BD_TX_INT         (1 << 14)
+#define ENET_BD_TS             (1 << 13)
+#define ENET_BD_PINS           (1 << 12)
+#define ENET_BD_IINS           (1 << 11)
+#define ENET_BD_PE             (1 << 10)
+#define ENET_BD_CE             (1 << 9)
+#define ENET_BD_UC             (1 << 8)
+#define ENET_BD_RX_INT         (1 << 7)
+
+#define ENET_BD_TXE            (1 << 15)
+#define ENET_BD_UE             (1 << 13)
+#define ENET_BD_EE             (1 << 12)
+#define ENET_BD_FE             (1 << 11)
+#define ENET_BD_LCE            (1 << 10)
+#define ENET_BD_OE             (1 << 9)
+#define ENET_BD_TSE            (1 << 8)
+#define ENET_BD_ICE            (1 << 5)
+#define ENET_BD_PCR            (1 << 4)
+#define ENET_BD_VLAN           (1 << 2)
+#define ENET_BD_IPV6           (1 << 1)
+#define ENET_BD_FRAG           (1 << 0)
+
+#define ENET_BD_BDU            (1 << 31)
 
 typedef struct IMXFECState {
     /*< private >*/
@@ -113,7 +236,7 @@ typedef struct IMXFECState {
     /*< public >*/
     NICState *nic;
     NICConf conf;
-    qemu_irq irq;
+    qemu_irq irq[2];
     MemoryRegion iomem;
 
     uint32_t regs[ENET_MAX];
@@ -125,6 +248,8 @@ typedef struct IMXFECState {
     uint32_t phy_advertise;
     uint32_t phy_int;
     uint32_t phy_int_mask;
+
+    bool is_fec;
 } IMXFECState;
 
 #endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH v4 8/8] Add ENET device to i.MX6 SOC.
  2016-05-18 22:22 [Qemu-devel] [PATCH v4 0/8] Add Ethernet device for i.MX6 SOC Jean-Christophe Dubois
                   ` (6 preceding siblings ...)
  2016-05-18 22:23 ` [Qemu-devel] [PATCH v4 7/8] Add ENET/Gbps Ethernet support to FEC device Jean-Christophe Dubois
@ 2016-05-18 22:23 ` Jean-Christophe Dubois
  7 siblings, 0 replies; 22+ messages in thread
From: Jean-Christophe Dubois @ 2016-05-18 22:23 UTC (permalink / raw)
  To: qemu-devel, peter.maydell, jasowang; +Cc: Jean-Christophe Dubois

This adds the ENET device to the i.MX6 SOC.

This was tested by booting Linux on an Qemu i.MX6 instance and accessing
the internet from the linux guest.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---

Changes since v1:
 * Not present on v1
   
Changes since v2:
 * None
 
Changes since v3:
 * switch the 2 Eth interrupt numbers.

 hw/arm/fsl-imx6.c         | 17 +++++++++++++++++
 include/hw/arm/fsl-imx6.h |  6 ++++--
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
index a5331bf..c08222c 100644
--- a/hw/arm/fsl-imx6.c
+++ b/hw/arm/fsl-imx6.c
@@ -105,6 +105,10 @@ static void fsl_imx6_init(Object *obj)
         snprintf(name, NAME_SIZE, "spi%d", i + 1);
         object_property_add_child(obj, name, OBJECT(&s->spi[i]), NULL);
     }
+
+    object_initialize(&s->eth, sizeof(s->eth), TYPE_IMX_FEC);
+    qdev_set_parent_bus(DEVICE(&s->eth), sysbus_get_default());
+    object_property_add_child(obj, "eth", OBJECT(&s->eth), NULL);
 }
 
 static void fsl_imx6_realize(DeviceState *dev, Error **errp)
@@ -381,6 +385,19 @@ static void fsl_imx6_realize(DeviceState *dev, Error **errp)
                                             spi_table[i].irq));
     }
 
+    object_property_set_bool(OBJECT(&s->eth), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->eth), 0, FSL_IMX6_ENET_ADDR);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->eth), 0,
+                       qdev_get_gpio_in(DEVICE(&s->a9mpcore),
+                                        FSL_IMX6_ENET_MAC_IRQ));
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->eth), 1,
+                       qdev_get_gpio_in(DEVICE(&s->a9mpcore),
+                                        FSL_IMX6_ENET_MAC_1588_IRQ));
+
     /* ROM memory */
     memory_region_init_rom_device(&s->rom, NULL, NULL, NULL, "imx6.rom",
                                   FSL_IMX6_ROM_SIZE, &err);
diff --git a/include/hw/arm/fsl-imx6.h b/include/hw/arm/fsl-imx6.h
index d24aaee..fc1a438 100644
--- a/include/hw/arm/fsl-imx6.h
+++ b/include/hw/arm/fsl-imx6.h
@@ -28,6 +28,7 @@
 #include "hw/gpio/imx_gpio.h"
 #include "hw/sd/sdhci.h"
 #include "hw/ssi/imx_spi.h"
+#include "hw/net/imx_fec.h"
 #include "exec/memory.h"
 
 #define TYPE_FSL_IMX6 "fsl,imx6"
@@ -57,6 +58,7 @@ typedef struct FslIMX6State {
     IMXGPIOState   gpio[FSL_IMX6_NUM_GPIOS];
     SDHCIState     esdhc[FSL_IMX6_NUM_ESDHCS];
     IMXSPIState    spi[FSL_IMX6_NUM_ECSPIS];
+    IMXFECState    eth;
     MemoryRegion   rom;
     MemoryRegion   caam;
     MemoryRegion   ocram;
@@ -435,8 +437,8 @@ typedef struct FslIMX6State {
 #define FSL_IMX6_HDMI_MASTER_IRQ 115
 #define FSL_IMX6_HDMI_CEC_IRQ 116
 #define FSL_IMX6_MLB150_LOW_IRQ 117
-#define FSL_IMX6_ENET_MAC_IRQ 118
-#define FSL_IMX6_ENET_MAC_1588_IRQ 119
+#define FSL_IMX6_ENET_MAC_1588_IRQ 118
+#define FSL_IMX6_ENET_MAC_IRQ 119
 #define FSL_IMX6_PCIE1_IRQ 120
 #define FSL_IMX6_PCIE2_IRQ 121
 #define FSL_IMX6_PCIE3_IRQ 122
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH v4 6/8] i.MX: move FEC device to a register array structure.
  2016-05-18 22:23 ` [Qemu-devel] [PATCH v4 6/8] i.MX: move FEC device to a register array structure Jean-Christophe Dubois
@ 2016-05-19  3:28   ` Jason Wang
  2016-05-19  6:10     ` Jean-Christophe DUBOIS
  0 siblings, 1 reply; 22+ messages in thread
From: Jason Wang @ 2016-05-19  3:28 UTC (permalink / raw)
  To: Jean-Christophe Dubois, qemu-devel, peter.maydell



On 2016年05月19日 06:23, Jean-Christophe Dubois wrote:
> This is to prepare for the ENET Gb device of the i.MX6.
>
> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
> ---
>
> Changes since v1:
>   * Not present on v1.
>
> Changes since v2:
>   * The patch was split in 2 parts:
>     - a "port" to a reg array based device (this patch).
>     - the addition of the Gb support (next patch).
>   
> Changes since v3:
>   * Small fix patches were extracted from this patch (see previous 3 patches)
>   * Reset logic through ECR was fixed.
>   * TDAR/RDAR behavior was fixed.
>
>   hw/net/imx_fec.c         | 396 ++++++++++++++++++++++++++---------------------
>   include/hw/net/imx_fec.h |  55 ++++---
>   2 files changed, 258 insertions(+), 193 deletions(-)
>
> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
> index 87e3c87..565b4a3 100644
> --- a/hw/net/imx_fec.c
> +++ b/hw/net/imx_fec.c
> @@ -52,30 +52,75 @@
>           } \
>       } while (0)
>   
> +static const char *imx_fec_reg_name(IMXFECState *s, uint32_t index)
> +{
> +    static char tmp[20];
> +
> +    switch (index) {
> +    case ENET_EIR:
> +        return "EIR";
> +    case ENET_EIMR:
> +        return "EIMR";
> +    case ENET_RDAR:
> +        return "RDAR";
> +    case ENET_TDAR:
> +        return "TDAR";
> +    case ENET_ECR:
> +        return "ECR";
> +    case ENET_MMFR:
> +        return "MMFR";
> +    case ENET_MSCR:
> +        return "MSCR";
> +    case ENET_MIBC:
> +        return "MIBC";
> +    case ENET_RCR:
> +        return "RCR";
> +    case ENET_TCR:
> +        return "TCR";
> +    case ENET_PALR:
> +        return "PALR";
> +    case ENET_PAUR:
> +        return "PAUR";
> +    case ENET_OPD:
> +        return "OPD";
> +    case ENET_IAUR:
> +        return "IAUR";
> +    case ENET_IALR:
> +        return "IALR";
> +    case ENET_GAUR:
> +        return "GAUR";
> +    case ENET_GALR:
> +        return "GALR";
> +    case ENET_TFWR:
> +        return "TFWR";
> +    case ENET_RDSR:
> +        return "RDSR";
> +    case ENET_TDSR:
> +        return "TDSR";
> +    case ENET_MRBR:
> +        return "MRBR";
> +    case ENET_FRBR:
> +        return "FRBR";
> +    case ENET_FRSR:
> +        return "FRSR";
> +    case ENET_MIIGSK_CFGR:
> +        return "MIIGSK_CFGR";
> +    case ENET_MIIGSK_ENR:
> +        return "MIIGSK_ENR";
> +    default:
> +        sprintf(tmp, "index %d", index);
> +        return tmp;
> +    }
> +}
> +
>   static const VMStateDescription vmstate_imx_fec = {
>       .name = TYPE_IMX_FEC,
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>       .fields = (VMStateField[]) {
> -        VMSTATE_UINT32(irq_state, IMXFECState),
> -        VMSTATE_UINT32(eir, IMXFECState),
> -        VMSTATE_UINT32(eimr, IMXFECState),
> -        VMSTATE_UINT32(rx_enabled, IMXFECState),
> +        VMSTATE_UINT32_ARRAY(regs, IMXFECState, ENET_MAX),
>           VMSTATE_UINT32(rx_descriptor, IMXFECState),
>           VMSTATE_UINT32(tx_descriptor, IMXFECState),
> -        VMSTATE_UINT32(ecr, IMXFECState),
> -        VMSTATE_UINT32(mmfr, IMXFECState),
> -        VMSTATE_UINT32(mscr, IMXFECState),
> -        VMSTATE_UINT32(mibc, IMXFECState),
> -        VMSTATE_UINT32(rcr, IMXFECState),
> -        VMSTATE_UINT32(tcr, IMXFECState),
> -        VMSTATE_UINT32(tfwr, IMXFECState),
> -        VMSTATE_UINT32(frsr, IMXFECState),
> -        VMSTATE_UINT32(erdsr, IMXFECState),
> -        VMSTATE_UINT32(etdsr, IMXFECState),
> -        VMSTATE_UINT32(emrbr, IMXFECState),
> -        VMSTATE_UINT32(miigsk_cfgr, IMXFECState),
> -        VMSTATE_UINT32(miigsk_enr, IMXFECState),
>   
>           VMSTATE_UINT32(phy_status, IMXFECState),
>           VMSTATE_UINT32(phy_control, IMXFECState),
> @@ -251,15 +296,13 @@ static void imx_fec_write_bd(IMXFECBufDesc *bd, dma_addr_t addr)
>   
>   static void imx_fec_update(IMXFECState *s)
>   {
> -    uint32_t active;
> -    uint32_t changed;
> -
> -    active = s->eir & s->eimr;
> -    changed = active ^ s->irq_state;
> -    if (changed) {
> -        qemu_set_irq(s->irq, active);
> +    if (s->regs[ENET_EIR] & s->regs[ENET_EIMR]) {
> +        FEC_PRINTF("interrupt raised\n");
> +        qemu_set_irq(s->irq, 1);
> +    } else {
> +        FEC_PRINTF("interrupt lowered\n");
> +        qemu_set_irq(s->irq, 0);
>       }
> -    s->irq_state = active;
>   }
>   
>   static void imx_fec_do_tx(IMXFECState *s)
> @@ -283,7 +326,7 @@ static void imx_fec_do_tx(IMXFECState *s)
>           len = bd.length;
>           if (frame_size + len > FEC_MAX_FRAME_SIZE) {
>               len = FEC_MAX_FRAME_SIZE - frame_size;
> -            s->eir |= FEC_INT_BABT;
> +            s->regs[ENET_EIR] |= FEC_INT_BABT;
>           }
>           dma_memory_read(&address_space_memory, bd.data, ptr, len);
>           ptr += len;
> @@ -293,17 +336,17 @@ static void imx_fec_do_tx(IMXFECState *s)
>               qemu_send_packet(qemu_get_queue(s->nic), frame, len);
>               ptr = frame;
>               frame_size = 0;
> -            s->eir |= FEC_INT_TXF;
> +            s->regs[ENET_EIR] |= FEC_INT_TXF;
>           }
> -        s->eir |= FEC_INT_TXB;
> +        s->regs[ENET_EIR] |= FEC_INT_TXB;
>           bd.flags &= ~FEC_BD_R;
>           /* Write back the modified descriptor.  */
>           imx_fec_write_bd(&bd, addr);
>           /* Advance to the next descriptor.  */
>           if ((bd.flags & FEC_BD_W) != 0) {
> -            addr = s->etdsr;
> +            addr = s->regs[ENET_TDSR];
>           } else {
> -            addr += 8;
> +            addr += sizeof(bd);
>           }
>       }
>   
> @@ -315,7 +358,7 @@ static void imx_fec_do_tx(IMXFECState *s)
>   static void imx_fec_enable_rx(IMXFECState *s)
>   {
>       IMXFECBufDesc bd;
> -    uint32_t tmp;
> +    bool tmp;
>   
>       imx_fec_read_bd(&bd, s->rx_descriptor);
>   
> @@ -323,11 +366,11 @@ static void imx_fec_enable_rx(IMXFECState *s)
>   
>       if (!tmp) {
>           FEC_PRINTF("RX buffer full\n");
> -    } else if (!s->rx_enabled) {
> +    } else if (!s->regs[ENET_RDAR]) {
>           qemu_flush_queued_packets(qemu_get_queue(s->nic));
>       }
>   
> -    s->rx_enabled = tmp;
> +    s->regs[ENET_RDAR] = tmp ? ENET_RDAR_RDAR : 0;
>   }
>   
>   static void imx_fec_reset(DeviceState *d)
> @@ -335,18 +378,26 @@ static void imx_fec_reset(DeviceState *d)
>       IMXFECState *s = IMX_FEC(d);
>   
>       /* Reset the FEC */
> -    s->eir = 0;
> -    s->eimr = 0;
> -    s->rx_enabled = 0;
> -    s->ecr = 0xf0000000;
> -    s->mscr = 0;
> -    s->mibc = 0xc0000000;
> -    s->rcr = 0x05ee0001;
> -    s->tcr = 0;
> -    s->tfwr = 0;
> -    s->frsr = 0x500;
> -    s->miigsk_cfgr = 0;
> -    s->miigsk_enr = 0x6;
> +    memset(s->regs, 0, sizeof(s->regs));
> +    s->regs[ENET_ECR]   = 0xf0000000;
> +    s->regs[ENET_MIBC]  = 0xc0000000;
> +    s->regs[ENET_RCR]   = 0x05ee0001;
> +    s->regs[ENET_OPD]   = 0x00010000;
> +
> +    s->regs[ENET_PALR]  = (s->conf.macaddr.a[0] << 24)
> +                          | (s->conf.macaddr.a[1] << 16)
> +                          | (s->conf.macaddr.a[2] << 8)
> +                          | s->conf.macaddr.a[3];
> +    s->regs[ENET_PAUR]  = (s->conf.macaddr.a[4] << 24)
> +                          | (s->conf.macaddr.a[5] << 16)
> +                          | 0x8808;
> +
> +    s->regs[ENET_FRBR]  = 0x00000600;
> +    s->regs[ENET_FRSR]  = 0x00000500;
> +    s->regs[ENET_MIIGSK_ENR]  = 0x00000006;
> +
> +    s->rx_descriptor = 0;
> +    s->tx_descriptor = 0;
>   
>       /* We also reset the PHY */
>       phy_reset(s);
> @@ -354,183 +405,180 @@ static void imx_fec_reset(DeviceState *d)
>   
>   static uint64_t imx_fec_read(void *opaque, hwaddr addr, unsigned size)
>   {
> +    uint32_t value = 0;
>       IMXFECState *s = IMX_FEC(opaque);
> -
> -    FEC_PRINTF("reading from @ 0x%" HWADDR_PRIx "\n", addr);
> -
> -    switch (addr & 0x3ff) {
> -    case 0x004:
> -        return s->eir;
> -    case 0x008:
> -        return s->eimr;
> -    case 0x010:
> -        return s->rx_enabled ? (1 << 24) : 0;   /* RDAR */
> -    case 0x014:
> -        return 0;   /* TDAR */
> -    case 0x024:
> -        return s->ecr;
> -    case 0x040:
> -        return s->mmfr;
> -    case 0x044:
> -        return s->mscr;
> -    case 0x064:
> -        return s->mibc; /* MIBC */
> -    case 0x084:
> -        return s->rcr;
> -    case 0x0c4:
> -        return s->tcr;
> -    case 0x0e4:     /* PALR */
> -        return (s->conf.macaddr.a[0] << 24)
> -               | (s->conf.macaddr.a[1] << 16)
> -               | (s->conf.macaddr.a[2] << 8)
> -               | s->conf.macaddr.a[3];
> -        break;
> -    case 0x0e8:     /* PAUR */
> -        return (s->conf.macaddr.a[4] << 24)
> -               | (s->conf.macaddr.a[5] << 16)
> -               | 0x8808;
> -    case 0x0ec:
> -        return 0x10000; /* OPD */
> -    case 0x118:
> -        return 0;
> -    case 0x11c:
> -        return 0;
> -    case 0x120:
> -        return 0;
> -    case 0x124:
> -        return 0;
> -    case 0x144:
> -        return s->tfwr;
> -    case 0x14c:
> -        return 0x600;
> -    case 0x150:
> -        return s->frsr;
> -    case 0x180:
> -        return s->erdsr;
> -    case 0x184:
> -        return s->etdsr;
> -    case 0x188:
> -        return s->emrbr;
> -    case 0x300:
> -        return s->miigsk_cfgr;
> -    case 0x308:
> -        return s->miigsk_enr;
> +    uint32_t index = addr >> 2;
> +
> +    switch (index) {
> +    case ENET_EIR:
> +    case ENET_EIMR:
> +    case ENET_RDAR:
> +    case ENET_TDAR:
> +    case ENET_ECR:
> +    case ENET_MMFR:
> +    case ENET_MSCR:
> +    case ENET_MIBC:
> +    case ENET_RCR:
> +    case ENET_TCR:
> +    case ENET_PALR:
> +    case ENET_PAUR:
> +    case ENET_OPD:
> +    case ENET_IAUR:
> +    case ENET_IALR:
> +    case ENET_GAUR:
> +    case ENET_GALR:
> +    case ENET_TFWR:
> +    case ENET_RDSR:
> +    case ENET_TDSR:
> +    case ENET_MRBR:
> +    case ENET_FRBR:
> +    case ENET_FRSR:
> +    case ENET_MIIGSK_CFGR:
> +    case ENET_MIIGSK_ENR:
> +        value = s->regs[index];
> +        break;
>       default:
> -        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad address at offset 0x%"
> -                      HWADDR_PRIx "\n", TYPE_IMX_FEC, __func__, addr);
> -        return 0;
> +        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad register at offset 0x%"
> +                      PRIx32 "\n", TYPE_IMX_FEC, __func__, index * 4);
> +        break;
>       }
> +
> +    FEC_PRINTF("reg[%s] => 0x%" PRIx32 "\n", imx_fec_reg_name(s, index),
> +                                              value);
> +
> +    return value;
>   }
>   
>   static void imx_fec_write(void *opaque, hwaddr addr,
>                             uint64_t value, unsigned size)
>   {
>       IMXFECState *s = IMX_FEC(opaque);
> +    uint32_t index = addr >> 2;
>   
> -    FEC_PRINTF("writing 0x%08x @ 0x%" HWADDR_PRIx "\n", (int)value, addr);
> +    FEC_PRINTF("reg[%s] <= 0x%" PRIx32 "\n", imx_fec_reg_name(s, index),
> +                                             (uint32_t)value);
>   
> -    switch (addr & 0x3ff) {
> -    case 0x004: /* EIR */
> -        s->eir &= ~value;
> +    switch (index) {
> +    case ENET_EIR:
> +        s->regs[index] &= ~value;
>           break;
> -    case 0x008: /* EIMR */
> -        s->eimr = value;
> +    case ENET_EIMR:
> +        s->regs[index] = value;
>           break;
> -    case 0x010: /* RDAR */
> -        if ((s->ecr & FEC_EN) && !s->rx_enabled) {
> -            imx_fec_enable_rx(s);
> +    case ENET_RDAR:
> +        if (s->regs[ENET_ECR] & FEC_EN) {
> +            if (!s->regs[index]) {
> +                s->regs[index] = ENET_RDAR_RDAR;
> +                imx_fec_enable_rx(s);
> +            }
> +        } else {
> +            s->regs[index] = 0;
>           }
>           break;
> -    case 0x014: /* TDAR */
> -        if (s->ecr & FEC_EN) {
> +    case ENET_TDAR:
> +        if (s->regs[ENET_ECR] & FEC_EN) {
> +            s->regs[index] = ENET_TDAR_TDAR;
>               imx_fec_do_tx(s);
>           }
> +        s->regs[index] = 0;
>           break;
> -    case 0x024: /* ECR */
> -        s->ecr = value;
> +    case ENET_ECR:
>           if (value & FEC_RESET) {
> -            imx_fec_reset(DEVICE(s));
> +            return imx_fec_reset(DEVICE(s));
>           }
> -        if ((s->ecr & FEC_EN) == 0) {
> -            s->rx_enabled = 0;
> +        s->regs[index] = value;
> +        if ((s->regs[index] & FEC_EN) == 0) {
> +            s->regs[ENET_RDAR] = 0;
> +            s->rx_descriptor = s->regs[ENET_RDSR];
> +            s->regs[ENET_TDAR] = 0;
> +            s->tx_descriptor = s->regs[ENET_TDSR];

This seems like a new behavior, is this a bug fix? If yes, better split.

>           }
>           break;
> -    case 0x040: /* MMFR */
> -        /* store the value */
> -        s->mmfr = value;
> +    case ENET_MMFR:
> +        s->regs[index] = value;
>           if (extract32(value, 29, 1)) {
> -            s->mmfr = do_phy_read(s, extract32(value, 18, 10));
> +            /* This is a read operation */
> +            s->regs[ENET_MMFR] = deposit32(s->regs[ENET_MMFR], 0, 16,
> +                                           do_phy_read(s,
> +                                                       extract32(value,
> +                                                                 18, 10)));
>           } else {
> +            /* This a write operation */
>               do_phy_write(s, extract32(value, 18, 10), extract32(value, 0, 16));
>           }
>           /* raise the interrupt as the PHY operation is done */
> -        s->eir |= FEC_INT_MII;
> +        s->regs[ENET_EIR] |= FEC_INT_MII;
>           break;
> -    case 0x044: /* MSCR */
> -        s->mscr = value & 0xfe;
> +    case ENET_MSCR:
> +        s->regs[index] = value & 0xfe;
>           break;
> -    case 0x064: /* MIBC */
> +    case ENET_MIBC:
>           /* TODO: Implement MIB.  */
> -        s->mibc = (value & 0x80000000) ? 0xc0000000 : 0;
> +        s->regs[index] = (value & 0x80000000) ? 0xc0000000 : 0;
>           break;
> -    case 0x084: /* RCR */
> -        s->rcr = value & 0x07ff003f;
> +    case ENET_RCR:
> +        s->regs[index] = value & 0x07ff003f;
>           /* TODO: Implement LOOP mode.  */
>           break;
> -    case 0x0c4: /* TCR */
> +    case ENET_TCR:
>           /* We transmit immediately, so raise GRA immediately.  */
> -        s->tcr = value;
> +        s->regs[index] = value;
>           if (value & 1) {
> -            s->eir |= FEC_INT_GRA;
> +            s->regs[ENET_EIR] |= FEC_INT_GRA;
>           }
>           break;
> -    case 0x0e4: /* PALR */
> +    case ENET_PALR:
> +        s->regs[index] = value;
>           s->conf.macaddr.a[0] = value >> 24;
>           s->conf.macaddr.a[1] = value >> 16;
>           s->conf.macaddr.a[2] = value >> 8;
>           s->conf.macaddr.a[3] = value;

I believe we should use stl_be_p() here?

>           break;
> -    case 0x0e8: /* PAUR */
> +    case ENET_PAUR:
> +        s->regs[index] = (value | 0x0000ffff) & 0xffff8808;
>           s->conf.macaddr.a[4] = value >> 24;
>           s->conf.macaddr.a[5] = value >> 16;
>           break;
> -    case 0x0ec: /* OPDR */
> +    case ENET_OPD:
> +        s->regs[index] = (value & 0x0000ffff) | 0x00010000;
>           break;
> -    case 0x118: /* IAUR */
> -    case 0x11c: /* IALR */
> -    case 0x120: /* GAUR */
> -    case 0x124: /* GALR */
> +    case ENET_IAUR:
> +    case ENET_IALR:
> +    case ENET_GAUR:
> +    case ENET_GALR:
>           /* TODO: implement MAC hash filtering.  */
>           break;
> -    case 0x144: /* TFWR */
> -        s->tfwr = value & 3;
> +    case ENET_TFWR:
> +        s->regs[index] = value & 3;
>           break;
> -    case 0x14c: /* FRBR */
> -        /* FRBR writes ignored.  */
> +    case ENET_FRBR:
> +        /* FRBR is read only */
> +        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Register FRBR is read only\n",
> +                      TYPE_IMX_FEC, __func__);
>           break;
> -    case 0x150: /* FRSR */
> -        s->frsr = (value & 0x3fc) | 0x400;
> +    case ENET_FRSR:
> +        s->regs[index] = (value & 0x000003fc) | 0x00000400;
>           break;
> -    case 0x180: /* ERDSR */
> -        s->erdsr = value & ~3;
> -        s->rx_descriptor = s->erdsr;
> +    case ENET_RDSR:
> +        s->regs[index] = value & ~3;
> +        s->rx_descriptor = s->regs[index];
>           break;
> -    case 0x184: /* ETDSR */
> -        s->etdsr = value & ~3;
> -        s->tx_descriptor = s->etdsr;
> +    case ENET_TDSR:
> +        s->regs[index] = value & ~3;
> +        s->tx_descriptor = s->regs[index];
>           break;
> -    case 0x188: /* EMRBR */
> -        s->emrbr = value & 0x7f0;
> +    case ENET_MRBR:
> +        s->regs[index] = value & 0x000007f0;
>           break;
> -    case 0x300: /* MIIGSK_CFGR */
> -        s->miigsk_cfgr = value & 0x53;
> +    case ENET_MIIGSK_CFGR:
> +        s->regs[index] = value & 0x00000053;
>           break;
> -    case 0x308: /* MIIGSK_ENR */
> -        s->miigsk_enr = (value & 0x2) ? 0x6 : 0;
> +    case ENET_MIIGSK_ENR:
> +        s->regs[index] = (value & 0x00000002) ? 0x00000006 : 0;
>           break;
>       default:
>           qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad address at offset 0x%"
> -                      HWADDR_PRIx "\n", TYPE_IMX_FEC, __func__, addr);
> +                      PRIx32 "\n", TYPE_IMX_FEC, __func__, index * 4);
>           break;
>       }
>   
> @@ -541,7 +589,7 @@ static int imx_fec_can_receive(NetClientState *nc)
>   {
>       IMXFECState *s = IMX_FEC(qemu_get_nic_opaque(nc));
>   
> -    return s->rx_enabled;
> +    return s->regs[ENET_RDAR] ? 1 : 0;
>   }
>   
>   static ssize_t imx_fec_receive(NetClientState *nc, const uint8_t *buf,
> @@ -559,7 +607,7 @@ static ssize_t imx_fec_receive(NetClientState *nc, const uint8_t *buf,
>   
>       FEC_PRINTF("len %d\n", (int)size);
>   
> -    if (!s->rx_enabled) {
> +    if (!s->regs[ENET_RDAR]) {
>           qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Unexpected packet\n",
>                         TYPE_IMX_FEC, __func__);
>           return 0;
> @@ -577,7 +625,7 @@ static ssize_t imx_fec_receive(NetClientState *nc, const uint8_t *buf,
>       }
>   
>       /* Frames larger than the user limit just set error flags.  */
> -    if (size > (s->rcr >> 16)) {
> +    if (size > (s->regs[ENET_RCR] >> 16)) {
>           flags |= FEC_BD_LG;
>       }
>   
> @@ -595,7 +643,7 @@ static ssize_t imx_fec_receive(NetClientState *nc, const uint8_t *buf,
>                             TYPE_IMX_FEC, __func__);
>               break;
>           }
> -        buf_len = (size <= s->emrbr) ? size : s->emrbr;
> +        buf_len = (size <= s->regs[ENET_MRBR]) ? size : s->regs[ENET_MRBR];
>           bd.length = buf_len;
>           size -= buf_len;
>   
> @@ -618,16 +666,16 @@ static ssize_t imx_fec_receive(NetClientState *nc, const uint8_t *buf,
>               /* Last buffer in frame.  */
>               bd.flags |= flags | FEC_BD_L;
>               FEC_PRINTF("rx frame flags %04x\n", bd.flags);
> -            s->eir |= FEC_INT_RXF;
> +            s->regs[ENET_EIR] |= FEC_INT_RXF;
>           } else {
> -            s->eir |= FEC_INT_RXB;
> +            s->regs[ENET_EIR] |= FEC_INT_RXB;
>           }
>           imx_fec_write_bd(&bd, addr);
>           /* Advance to the next descriptor.  */
>           if ((bd.flags & FEC_BD_W) != 0) {
> -            addr = s->erdsr;
> +            addr = s->regs[ENET_RDSR];
>           } else {
> -            addr += 8;
> +            addr += sizeof(bd);
>           }
>       }
>       s->rx_descriptor = addr;
> diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h
> index cbf8650..709f8a0 100644
> --- a/include/hw/net/imx_fec.h
> +++ b/include/hw/net/imx_fec.h
> @@ -30,6 +30,33 @@
>   #include "hw/sysbus.h"
>   #include "net/net.h"
>   
> +#define ENET_EIR               1
> +#define ENET_EIMR              2
> +#define ENET_RDAR              4
> +#define ENET_TDAR              5
> +#define ENET_ECR               9
> +#define ENET_MMFR              16
> +#define ENET_MSCR              17
> +#define ENET_MIBC              25
> +#define ENET_RCR               33
> +#define ENET_TCR               49
> +#define ENET_PALR              57
> +#define ENET_PAUR              58
> +#define ENET_OPD               59
> +#define ENET_IAUR              70
> +#define ENET_IALR              71
> +#define ENET_GAUR              72
> +#define ENET_GALR              73
> +#define ENET_TFWR              81
> +#define ENET_FRBR              83
> +#define ENET_FRSR              84
> +#define ENET_RDSR              96
> +#define ENET_TDSR              97
> +#define ENET_MRBR              98
> +#define ENET_MIIGSK_CFGR       192
> +#define ENET_MIIGSK_ENR        194
> +#define ENET_MAX               400

Is this an arbitrary value or there's a plan to add more register whose 
index is greater than 192?

> +
>   #define FEC_MAX_FRAME_SIZE 2032
>   
>   #define FEC_INT_HB      (1 << 31)
> @@ -46,8 +73,14 @@
>   #define FEC_INT_RL      (1 << 20)
>   #define FEC_INT_UN      (1 << 19)
>   
> -#define FEC_EN      2
> -#define FEC_RESET   1
> +/* RDAR */
> +#define ENET_RDAR_RDAR         (1 << 24)
> +
> +/* TDAR */
> +#define ENET_TDAR_TDAR         (1 << 24)
> +
> +#define FEC_EN                 (1 << 1)
> +#define FEC_RESET              (1 << 0)
>   
>   /* Buffer Descriptor.  */
>   typedef struct {
> @@ -83,25 +116,9 @@ typedef struct IMXFECState {
>       qemu_irq irq;
>       MemoryRegion iomem;
>   
> -    uint32_t irq_state;
> -    uint32_t eir;
> -    uint32_t eimr;
> -    uint32_t rx_enabled;
> +    uint32_t regs[ENET_MAX];
>       uint32_t rx_descriptor;
>       uint32_t tx_descriptor;
> -    uint32_t ecr;
> -    uint32_t mmfr;
> -    uint32_t mscr;
> -    uint32_t mibc;
> -    uint32_t rcr;
> -    uint32_t tcr;
> -    uint32_t tfwr;
> -    uint32_t frsr;
> -    uint32_t erdsr;
> -    uint32_t etdsr;
> -    uint32_t emrbr;
> -    uint32_t miigsk_cfgr;
> -    uint32_t miigsk_enr;
>   
>       uint32_t phy_status;
>       uint32_t phy_control;

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH v4 7/8] Add ENET/Gbps Ethernet support to FEC device
  2016-05-18 22:23 ` [Qemu-devel] [PATCH v4 7/8] Add ENET/Gbps Ethernet support to FEC device Jean-Christophe Dubois
@ 2016-05-19  3:48   ` Jason Wang
  2016-05-19 18:14     ` Jean-Christophe DUBOIS
  0 siblings, 1 reply; 22+ messages in thread
From: Jason Wang @ 2016-05-19  3:48 UTC (permalink / raw)
  To: Jean-Christophe Dubois, qemu-devel, peter.maydell



On 2016年05月19日 06:23, Jean-Christophe Dubois wrote:
> The ENET device (present in i.MX6) is "derived" from FEC and backward
> compatible with it.
>
> This patch adds the necessary support of the added feature in the ENET
> device to allow Linux to use it (on supported processors).
>
> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
> ---
>
> Changes since v1:
>   * Not present on v1
>     
> Changes since v2:
>   * Not present on v2
>   
> Changes since v3:
>   * Separate and fix the 2 supported interrupts
>
>   hw/arm/fsl-imx25.c       |   3 +
>   hw/net/imx_fec.c         | 713 ++++++++++++++++++++++++++++++++++++++---------
>   include/hw/net/imx_fec.h | 195 ++++++++++---
>   3 files changed, 745 insertions(+), 166 deletions(-)

[...]

>   
> -static Property imx_fec_properties[] = {
> +static Property imx_eth_properties[] = {
>       DEFINE_NIC_PROPERTIES(IMXFECState, conf),
> +    DEFINE_PROP_BOOL("is-fec", IMXFECState, is_fec, false),
>       DEFINE_PROP_END_OF_LIST(),
>   };

It's ok to decide with "is-fec", but is it better to use a new type for 
that?

>   
> -static void imx_fec_class_init(ObjectClass *klass, void *data)
> +static void imx_eth_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
>   
> -    dc->vmsd = &vmstate_imx_fec;
> -    dc->reset = imx_fec_reset;
> -    dc->props = imx_fec_properties;
> -    dc->realize = imx_fec_realize;
> -    dc->desc = "i.MX FEC Ethernet Controller";
> +    dc->vmsd    = &vmstate_imx_eth;
> +    dc->reset   = imx_eth_reset;
> +    dc->props   = imx_eth_properties;
> +    dc->realize = imx_eth_realize;
> +    dc->desc    = "i.MX FEC/ENET Ethernet Controller";
>   }
>   
> -static const TypeInfo imx_fec_info = {
> -    .name = TYPE_IMX_FEC,
> -    .parent = TYPE_SYS_BUS_DEVICE,
> +static const TypeInfo imx_eth_info = {
> +    .name          = TYPE_IMX_FEC,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
>       .instance_size = sizeof(IMXFECState),
> -    .class_init = imx_fec_class_init,
> +    .class_init    = imx_eth_class_init,
>   };
>   
> -static void imx_fec_register_types(void)
> +static void imx_eth_register_types(void)
>   {
> -    type_register_static(&imx_fec_info);
> +    type_register_static(&imx_eth_info);
>   }
>   
> -type_init(imx_fec_register_types)
> +type_init(imx_eth_register_types)
> diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h
> index 709f8a0..a09b7d7 100644
> --- a/include/hw/net/imx_fec.h
> +++ b/include/hw/net/imx_fec.h
> @@ -1,5 +1,5 @@
>   /*
> - * i.MX Fast Ethernet Controller emulation.
> + * i.MX FEC/ENET Ethernet Controller emulation.
>    *
>    * Copyright (c) 2013 Jean-Christophe Dubois. <jcd@tribudubois.net>
>    *
> @@ -53,25 +53,64 @@
>   #define ENET_RDSR              96
>   #define ENET_TDSR              97
>   #define ENET_MRBR              98
> +#define ENET_RSFL              100
> +#define ENET_RSEM              101
> +#define ENET_RAEM              102
> +#define ENET_RAFL              103
> +#define ENET_TSEM              104
> +#define ENET_TAEM              105
> +#define ENET_TAFL              106
> +#define ENET_TIPG              107
> +#define ENET_FTRL              108
> +#define ENET_TACC              112
> +#define ENET_RACC              113
>   #define ENET_MIIGSK_CFGR       192
>   #define ENET_MIIGSK_ENR        194
> +#define ENET_ATCR              256
> +#define ENET_ATVR              257
> +#define ENET_ATOFF             258
> +#define ENET_ATPER             259
> +#define ENET_ATCOR             260
> +#define ENET_ATINC             261
> +#define ENET_ATSTMP            262
> +#define ENET_TGSR              385
> +#define ENET_TCSR0             386
> +#define ENET_TCCR0             387
> +#define ENET_TCSR1             388
> +#define ENET_TCCR1             389
> +#define ENET_TCSR2             390
> +#define ENET_TCCR2             391
> +#define ENET_TCSR3             392
> +#define ENET_TCCR3             393
>   #define ENET_MAX               400
>   
> -#define FEC_MAX_FRAME_SIZE 2032
> -
> -#define FEC_INT_HB      (1 << 31)
> -#define FEC_INT_BABR    (1 << 30)
> -#define FEC_INT_BABT    (1 << 29)
> -#define FEC_INT_GRA     (1 << 28)
> -#define FEC_INT_TXF     (1 << 27)
> -#define FEC_INT_TXB     (1 << 26)
> -#define FEC_INT_RXF     (1 << 25)
> -#define FEC_INT_RXB     (1 << 24)
> -#define FEC_INT_MII     (1 << 23)
> -#define FEC_INT_EBERR   (1 << 22)
> -#define FEC_INT_LC      (1 << 21)
> -#define FEC_INT_RL      (1 << 20)
> -#define FEC_INT_UN      (1 << 19)
> +#define ENET_MAX_FRAME_SIZE    2032
> +
> +/* EIR and EIMR */
> +#define ENET_INT_HB            (1 << 31)
> +#define ENET_INT_BABR          (1 << 30)
> +#define ENET_INT_BABT          (1 << 29)
> +#define ENET_INT_GRA           (1 << 28)
> +#define ENET_INT_TXF           (1 << 27)
> +#define ENET_INT_TXB           (1 << 26)
> +#define ENET_INT_RXF           (1 << 25)
> +#define ENET_INT_RXB           (1 << 24)
> +#define ENET_INT_MII           (1 << 23)
> +#define ENET_INT_EBERR         (1 << 22)
> +#define ENET_INT_LC            (1 << 21)
> +#define ENET_INT_RL            (1 << 20)
> +#define ENET_INT_UN            (1 << 19)

The above renaming seems cause lots of unnecessary changes above which 
may brings issue when backporint patches to -stable. Can we just keep 
this, and all ENET_*** should be used after we're sure we are ENET?

> +#define ENET_INT_PLR           (1 << 18)
> +#define ENET_INT_WAKEUP        (1 << 17)
> +#define ENET_INT_TS_AVAIL      (1 << 16)
> +#define ENET_INT_TS_TIMER      (1 << 15)
> +
> +#define ENET_INT_MAC           (ENET_INT_HB | ENET_INT_BABR | ENET_INT_BABT | \
> +                                ENET_INT_GRA | ENET_INT_TXF | ENET_INT_TXB | \
> +                                ENET_INT_RXF | ENET_INT_RXB | ENET_INT_MII | \
> +                                ENET_INT_EBERR | ENET_INT_LC | ENET_INT_RL | \
> +                                ENET_INT_UN | ENET_INT_PLR | ENET_INT_WAKEUP | \
> +                                ENET_INT_TS_AVAIL)
>   
>   /* RDAR */
>   #define ENET_RDAR_RDAR         (1 << 24)
> @@ -79,8 +118,54 @@
>   /* TDAR */
>   #define ENET_TDAR_TDAR         (1 << 24)
>   
> -#define FEC_EN                 (1 << 1)
> -#define FEC_RESET              (1 << 0)
> +/* ECR */
> +#define ENET_ECR_RESET         (1 << 0)
> +#define ENET_ECR_ETHEREN       (1 << 1)
> +#define ENET_ECR_MAGICEN       (1 << 2)
> +#define ENET_ECR_SLEEP         (1 << 3)
> +#define ENET_ECR_EN1588        (1 << 4)
> +#define ENET_ECR_SPEED         (1 << 5)
> +#define ENET_ECR_DBGEN         (1 << 6)
> +#define ENET_ECR_STOPEN        (1 << 7)
> +#define ENET_ECR_DSBWP         (1 << 8)
> +
> +/* MIBC */
> +#define ENET_MIBC_MIB_DIS      (1 << 31)
> +#define ENET_MIBC_MIB_IDLE     (1 << 30)
> +#define ENET_MIBC_MIB_CLEAR    (1 << 29)
> +
> +/* RCR */
> +#define ENET_RCR_LOOP          (1 << 0)
> +#define ENET_RCR_DRT           (1 << 1)
> +#define ENET_RCR_MII_MODE      (1 << 2)
> +#define ENET_RCR_PROM          (1 << 3)
> +#define ENET_RCR_BC_REJ        (1 << 4)
> +#define ENET_RCR_FCE           (1 << 5)
> +#define ENET_RCR_RGMII_EN      (1 << 6)
> +#define ENET_RCR_RMII_MODE     (1 << 8)
> +#define ENET_RCR_RMII_10T      (1 << 9)
> +#define ENET_RCR_PADEN         (1 << 12)
> +#define ENET_RCR_PAUFWD        (1 << 13)
> +#define ENET_RCR_CRCFWD        (1 << 14)
> +#define ENET_RCR_CFEN          (1 << 15)
> +#define ENET_RCR_MAX_FL_SHIFT  (16)
> +#define ENET_RCR_MAX_FL_LENGTH (14)
> +#define ENET_RCR_NLC           (1 << 30)
> +#define ENET_RCR_GRS           (1 << 31)
> +
> +/* TCR */
> +#define ENET_TCR_GTS           (1 << 0)
> +#define ENET_TCR_FDEN          (1 << 2)
> +#define ENET_TCR_TFC_PAUSE     (1 << 3)
> +#define ENET_TCR_RFC_PAUSE     (1 << 4)
> +#define ENET_TCR_ADDSEL_SHIFT  (5)
> +#define ENET_TCR_ADDSEL_LENGTH (3)
> +#define ENET_TCR_CRCFWD        (1 << 9)
> +
> +/* RDSR */
> +#define ENET_TWFR_TFWR_SHIFT   (0)
> +#define ENET_TWFR_TFWR_LENGTH  (6)
> +#define ENET_TWFR_STRFWD       (1 << 8)
>   
>   /* Buffer Descriptor.  */
>   typedef struct {
> @@ -89,22 +174,60 @@ typedef struct {
>       uint32_t data;
>   } IMXFECBufDesc;
>   
> -#define FEC_BD_R    (1 << 15)
> -#define FEC_BD_E    (1 << 15)
> -#define FEC_BD_O1   (1 << 14)
> -#define FEC_BD_W    (1 << 13)
> -#define FEC_BD_O2   (1 << 12)
> -#define FEC_BD_L    (1 << 11)
> -#define FEC_BD_TC   (1 << 10)
> -#define FEC_BD_ABC  (1 << 9)
> -#define FEC_BD_M    (1 << 8)
> -#define FEC_BD_BC   (1 << 7)
> -#define FEC_BD_MC   (1 << 6)
> -#define FEC_BD_LG   (1 << 5)
> -#define FEC_BD_NO   (1 << 4)
> -#define FEC_BD_CR   (1 << 2)
> -#define FEC_BD_OV   (1 << 1)
> -#define FEC_BD_TR   (1 << 0)
> +#define ENET_BD_R              (1 << 15)
> +#define ENET_BD_E              (1 << 15)
> +#define ENET_BD_O1             (1 << 14)
> +#define ENET_BD_W              (1 << 13)
> +#define ENET_BD_O2             (1 << 12)
> +#define ENET_BD_L              (1 << 11)
> +#define ENET_BD_TC             (1 << 10)
> +#define ENET_BD_ABC            (1 << 9)
> +#define ENET_BD_M              (1 << 8)
> +#define ENET_BD_BC             (1 << 7)
> +#define ENET_BD_MC             (1 << 6)
> +#define ENET_BD_LG             (1 << 5)
> +#define ENET_BD_NO             (1 << 4)
> +#define ENET_BD_CR             (1 << 2)
> +#define ENET_BD_OV             (1 << 1)
> +#define ENET_BD_TR             (1 << 0)
> +
> +typedef struct {
> +    uint16_t length;
> +    uint16_t flags;
> +    uint32_t data;
> +    uint16_t status;
> +    uint16_t option;
> +    uint16_t checksum;
> +    uint16_t head_proto;
> +    uint32_t last_buffer;
> +    uint32_t timestamp;
> +    uint32_t reserved[2];
> +} IMXENETBufDesc;
> +
> +#define ENET_BD_ME             (1 << 15)
> +#define ENET_BD_TX_INT         (1 << 14)
> +#define ENET_BD_TS             (1 << 13)
> +#define ENET_BD_PINS           (1 << 12)
> +#define ENET_BD_IINS           (1 << 11)
> +#define ENET_BD_PE             (1 << 10)
> +#define ENET_BD_CE             (1 << 9)
> +#define ENET_BD_UC             (1 << 8)
> +#define ENET_BD_RX_INT         (1 << 7)
> +
> +#define ENET_BD_TXE            (1 << 15)
> +#define ENET_BD_UE             (1 << 13)
> +#define ENET_BD_EE             (1 << 12)
> +#define ENET_BD_FE             (1 << 11)
> +#define ENET_BD_LCE            (1 << 10)
> +#define ENET_BD_OE             (1 << 9)
> +#define ENET_BD_TSE            (1 << 8)
> +#define ENET_BD_ICE            (1 << 5)
> +#define ENET_BD_PCR            (1 << 4)
> +#define ENET_BD_VLAN           (1 << 2)
> +#define ENET_BD_IPV6           (1 << 1)
> +#define ENET_BD_FRAG           (1 << 0)
> +
> +#define ENET_BD_BDU            (1 << 31)
>   
>   typedef struct IMXFECState {
>       /*< private >*/
> @@ -113,7 +236,7 @@ typedef struct IMXFECState {
>       /*< public >*/
>       NICState *nic;
>       NICConf conf;
> -    qemu_irq irq;
> +    qemu_irq irq[2];
>       MemoryRegion iomem;
>   
>       uint32_t regs[ENET_MAX];
> @@ -125,6 +248,8 @@ typedef struct IMXFECState {
>       uint32_t phy_advertise;
>       uint32_t phy_int;
>       uint32_t phy_int_mask;
> +
> +    bool is_fec;
>   } IMXFECState;
>   
>   #endif

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH v4 6/8] i.MX: move FEC device to a register array structure.
  2016-05-19  3:28   ` Jason Wang
@ 2016-05-19  6:10     ` Jean-Christophe DUBOIS
  2016-05-20  2:26       ` Jason Wang
  0 siblings, 1 reply; 22+ messages in thread
From: Jean-Christophe DUBOIS @ 2016-05-19  6:10 UTC (permalink / raw)
  To: Jason Wang, qemu-devel, peter.maydell

Le 19/05/2016 05:28, Jason Wang a écrit :
>
>
> On 2016年05月19日 06:23, Jean-Christophe Dubois wrote:
>> This is to prepare for the ENET Gb device of the i.MX6.
>>
>> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
>> ---
>>
>> Changes since v1:
>>   * Not present on v1.
>>
>> Changes since v2:
>>   * The patch was split in 2 parts:
>>     - a "port" to a reg array based device (this patch).
>>     - the addition of the Gb support (next patch).
>>   Changes since v3:
>>   * Small fix patches were extracted from this patch (see previous 3 
>> patches)
>>   * Reset logic through ECR was fixed.
>>   * TDAR/RDAR behavior was fixed.
>>
>>   hw/net/imx_fec.c         | 396 
>> ++++++++++++++++++++++++++---------------------
>>   include/hw/net/imx_fec.h |  55 ++++---
>>   2 files changed, 258 insertions(+), 193 deletions(-)
>>
>> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
>> index 87e3c87..565b4a3 100644
>> --- a/hw/net/imx_fec.c
>> +++ b/hw/net/imx_fec.c
>> @@ -52,30 +52,75 @@
>>           } \
>>       } while (0)
>>   +static const char *imx_fec_reg_name(IMXFECState *s, uint32_t index)
>> +{
>> +    static char tmp[20];
>> +
>> +    switch (index) {
>> +    case ENET_EIR:
>> +        return "EIR";
>> +    case ENET_EIMR:
>> +        return "EIMR";
>> +    case ENET_RDAR:
>> +        return "RDAR";
>> +    case ENET_TDAR:
>> +        return "TDAR";
>> +    case ENET_ECR:
>> +        return "ECR";
>> +    case ENET_MMFR:
>> +        return "MMFR";
>> +    case ENET_MSCR:
>> +        return "MSCR";
>> +    case ENET_MIBC:
>> +        return "MIBC";
>> +    case ENET_RCR:
>> +        return "RCR";
>> +    case ENET_TCR:
>> +        return "TCR";
>> +    case ENET_PALR:
>> +        return "PALR";
>> +    case ENET_PAUR:
>> +        return "PAUR";
>> +    case ENET_OPD:
>> +        return "OPD";
>> +    case ENET_IAUR:
>> +        return "IAUR";
>> +    case ENET_IALR:
>> +        return "IALR";
>> +    case ENET_GAUR:
>> +        return "GAUR";
>> +    case ENET_GALR:
>> +        return "GALR";
>> +    case ENET_TFWR:
>> +        return "TFWR";
>> +    case ENET_RDSR:
>> +        return "RDSR";
>> +    case ENET_TDSR:
>> +        return "TDSR";
>> +    case ENET_MRBR:
>> +        return "MRBR";
>> +    case ENET_FRBR:
>> +        return "FRBR";
>> +    case ENET_FRSR:
>> +        return "FRSR";
>> +    case ENET_MIIGSK_CFGR:
>> +        return "MIIGSK_CFGR";
>> +    case ENET_MIIGSK_ENR:
>> +        return "MIIGSK_ENR";
>> +    default:
>> +        sprintf(tmp, "index %d", index);
>> +        return tmp;
>> +    }
>> +}
>> +
>>   static const VMStateDescription vmstate_imx_fec = {
>>       .name = TYPE_IMX_FEC,
>> -    .version_id = 1,
>> -    .minimum_version_id = 1,
>> +    .version_id = 2,
>> +    .minimum_version_id = 2,
>>       .fields = (VMStateField[]) {
>> -        VMSTATE_UINT32(irq_state, IMXFECState),
>> -        VMSTATE_UINT32(eir, IMXFECState),
>> -        VMSTATE_UINT32(eimr, IMXFECState),
>> -        VMSTATE_UINT32(rx_enabled, IMXFECState),
>> +        VMSTATE_UINT32_ARRAY(regs, IMXFECState, ENET_MAX),
>>           VMSTATE_UINT32(rx_descriptor, IMXFECState),
>>           VMSTATE_UINT32(tx_descriptor, IMXFECState),
>> -        VMSTATE_UINT32(ecr, IMXFECState),
>> -        VMSTATE_UINT32(mmfr, IMXFECState),
>> -        VMSTATE_UINT32(mscr, IMXFECState),
>> -        VMSTATE_UINT32(mibc, IMXFECState),
>> -        VMSTATE_UINT32(rcr, IMXFECState),
>> -        VMSTATE_UINT32(tcr, IMXFECState),
>> -        VMSTATE_UINT32(tfwr, IMXFECState),
>> -        VMSTATE_UINT32(frsr, IMXFECState),
>> -        VMSTATE_UINT32(erdsr, IMXFECState),
>> -        VMSTATE_UINT32(etdsr, IMXFECState),
>> -        VMSTATE_UINT32(emrbr, IMXFECState),
>> -        VMSTATE_UINT32(miigsk_cfgr, IMXFECState),
>> -        VMSTATE_UINT32(miigsk_enr, IMXFECState),
>>             VMSTATE_UINT32(phy_status, IMXFECState),
>>           VMSTATE_UINT32(phy_control, IMXFECState),
>> @@ -251,15 +296,13 @@ static void imx_fec_write_bd(IMXFECBufDesc *bd, 
>> dma_addr_t addr)
>>     static void imx_fec_update(IMXFECState *s)
>>   {
>> -    uint32_t active;
>> -    uint32_t changed;
>> -
>> -    active = s->eir & s->eimr;
>> -    changed = active ^ s->irq_state;
>> -    if (changed) {
>> -        qemu_set_irq(s->irq, active);
>> +    if (s->regs[ENET_EIR] & s->regs[ENET_EIMR]) {
>> +        FEC_PRINTF("interrupt raised\n");
>> +        qemu_set_irq(s->irq, 1);
>> +    } else {
>> +        FEC_PRINTF("interrupt lowered\n");
>> +        qemu_set_irq(s->irq, 0);
>>       }
>> -    s->irq_state = active;
>>   }
>>     static void imx_fec_do_tx(IMXFECState *s)
>> @@ -283,7 +326,7 @@ static void imx_fec_do_tx(IMXFECState *s)
>>           len = bd.length;
>>           if (frame_size + len > FEC_MAX_FRAME_SIZE) {
>>               len = FEC_MAX_FRAME_SIZE - frame_size;
>> -            s->eir |= FEC_INT_BABT;
>> +            s->regs[ENET_EIR] |= FEC_INT_BABT;
>>           }
>>           dma_memory_read(&address_space_memory, bd.data, ptr, len);
>>           ptr += len;
>> @@ -293,17 +336,17 @@ static void imx_fec_do_tx(IMXFECState *s)
>>               qemu_send_packet(qemu_get_queue(s->nic), frame, len);
>>               ptr = frame;
>>               frame_size = 0;
>> -            s->eir |= FEC_INT_TXF;
>> +            s->regs[ENET_EIR] |= FEC_INT_TXF;
>>           }
>> -        s->eir |= FEC_INT_TXB;
>> +        s->regs[ENET_EIR] |= FEC_INT_TXB;
>>           bd.flags &= ~FEC_BD_R;
>>           /* Write back the modified descriptor.  */
>>           imx_fec_write_bd(&bd, addr);
>>           /* Advance to the next descriptor.  */
>>           if ((bd.flags & FEC_BD_W) != 0) {
>> -            addr = s->etdsr;
>> +            addr = s->regs[ENET_TDSR];
>>           } else {
>> -            addr += 8;
>> +            addr += sizeof(bd);
>>           }
>>       }
>>   @@ -315,7 +358,7 @@ static void imx_fec_do_tx(IMXFECState *s)
>>   static void imx_fec_enable_rx(IMXFECState *s)
>>   {
>>       IMXFECBufDesc bd;
>> -    uint32_t tmp;
>> +    bool tmp;
>>         imx_fec_read_bd(&bd, s->rx_descriptor);
>>   @@ -323,11 +366,11 @@ static void imx_fec_enable_rx(IMXFECState *s)
>>         if (!tmp) {
>>           FEC_PRINTF("RX buffer full\n");
>> -    } else if (!s->rx_enabled) {
>> +    } else if (!s->regs[ENET_RDAR]) {
>>           qemu_flush_queued_packets(qemu_get_queue(s->nic));
>>       }
>>   -    s->rx_enabled = tmp;
>> +    s->regs[ENET_RDAR] = tmp ? ENET_RDAR_RDAR : 0;
>>   }
>>     static void imx_fec_reset(DeviceState *d)
>> @@ -335,18 +378,26 @@ static void imx_fec_reset(DeviceState *d)
>>       IMXFECState *s = IMX_FEC(d);
>>         /* Reset the FEC */
>> -    s->eir = 0;
>> -    s->eimr = 0;
>> -    s->rx_enabled = 0;
>> -    s->ecr = 0xf0000000;
>> -    s->mscr = 0;
>> -    s->mibc = 0xc0000000;
>> -    s->rcr = 0x05ee0001;
>> -    s->tcr = 0;
>> -    s->tfwr = 0;
>> -    s->frsr = 0x500;
>> -    s->miigsk_cfgr = 0;
>> -    s->miigsk_enr = 0x6;
>> +    memset(s->regs, 0, sizeof(s->regs));
>> +    s->regs[ENET_ECR]   = 0xf0000000;
>> +    s->regs[ENET_MIBC]  = 0xc0000000;
>> +    s->regs[ENET_RCR]   = 0x05ee0001;
>> +    s->regs[ENET_OPD]   = 0x00010000;
>> +
>> +    s->regs[ENET_PALR]  = (s->conf.macaddr.a[0] << 24)
>> +                          | (s->conf.macaddr.a[1] << 16)
>> +                          | (s->conf.macaddr.a[2] << 8)
>> +                          | s->conf.macaddr.a[3];
>> +    s->regs[ENET_PAUR]  = (s->conf.macaddr.a[4] << 24)
>> +                          | (s->conf.macaddr.a[5] << 16)
>> +                          | 0x8808;
>> +
>> +    s->regs[ENET_FRBR]  = 0x00000600;
>> +    s->regs[ENET_FRSR]  = 0x00000500;
>> +    s->regs[ENET_MIIGSK_ENR]  = 0x00000006;
>> +
>> +    s->rx_descriptor = 0;
>> +    s->tx_descriptor = 0;
>>         /* We also reset the PHY */
>>       phy_reset(s);
>> @@ -354,183 +405,180 @@ static void imx_fec_reset(DeviceState *d)
>>     static uint64_t imx_fec_read(void *opaque, hwaddr addr, unsigned 
>> size)
>>   {
>> +    uint32_t value = 0;
>>       IMXFECState *s = IMX_FEC(opaque);
>> -
>> -    FEC_PRINTF("reading from @ 0x%" HWADDR_PRIx "\n", addr);
>> -
>> -    switch (addr & 0x3ff) {
>> -    case 0x004:
>> -        return s->eir;
>> -    case 0x008:
>> -        return s->eimr;
>> -    case 0x010:
>> -        return s->rx_enabled ? (1 << 24) : 0;   /* RDAR */
>> -    case 0x014:
>> -        return 0;   /* TDAR */
>> -    case 0x024:
>> -        return s->ecr;
>> -    case 0x040:
>> -        return s->mmfr;
>> -    case 0x044:
>> -        return s->mscr;
>> -    case 0x064:
>> -        return s->mibc; /* MIBC */
>> -    case 0x084:
>> -        return s->rcr;
>> -    case 0x0c4:
>> -        return s->tcr;
>> -    case 0x0e4:     /* PALR */
>> -        return (s->conf.macaddr.a[0] << 24)
>> -               | (s->conf.macaddr.a[1] << 16)
>> -               | (s->conf.macaddr.a[2] << 8)
>> -               | s->conf.macaddr.a[3];
>> -        break;
>> -    case 0x0e8:     /* PAUR */
>> -        return (s->conf.macaddr.a[4] << 24)
>> -               | (s->conf.macaddr.a[5] << 16)
>> -               | 0x8808;
>> -    case 0x0ec:
>> -        return 0x10000; /* OPD */
>> -    case 0x118:
>> -        return 0;
>> -    case 0x11c:
>> -        return 0;
>> -    case 0x120:
>> -        return 0;
>> -    case 0x124:
>> -        return 0;
>> -    case 0x144:
>> -        return s->tfwr;
>> -    case 0x14c:
>> -        return 0x600;
>> -    case 0x150:
>> -        return s->frsr;
>> -    case 0x180:
>> -        return s->erdsr;
>> -    case 0x184:
>> -        return s->etdsr;
>> -    case 0x188:
>> -        return s->emrbr;
>> -    case 0x300:
>> -        return s->miigsk_cfgr;
>> -    case 0x308:
>> -        return s->miigsk_enr;
>> +    uint32_t index = addr >> 2;
>> +
>> +    switch (index) {
>> +    case ENET_EIR:
>> +    case ENET_EIMR:
>> +    case ENET_RDAR:
>> +    case ENET_TDAR:
>> +    case ENET_ECR:
>> +    case ENET_MMFR:
>> +    case ENET_MSCR:
>> +    case ENET_MIBC:
>> +    case ENET_RCR:
>> +    case ENET_TCR:
>> +    case ENET_PALR:
>> +    case ENET_PAUR:
>> +    case ENET_OPD:
>> +    case ENET_IAUR:
>> +    case ENET_IALR:
>> +    case ENET_GAUR:
>> +    case ENET_GALR:
>> +    case ENET_TFWR:
>> +    case ENET_RDSR:
>> +    case ENET_TDSR:
>> +    case ENET_MRBR:
>> +    case ENET_FRBR:
>> +    case ENET_FRSR:
>> +    case ENET_MIIGSK_CFGR:
>> +    case ENET_MIIGSK_ENR:
>> +        value = s->regs[index];
>> +        break;
>>       default:
>> -        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad address at 
>> offset 0x%"
>> -                      HWADDR_PRIx "\n", TYPE_IMX_FEC, __func__, addr);
>> -        return 0;
>> +        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad register at 
>> offset 0x%"
>> +                      PRIx32 "\n", TYPE_IMX_FEC, __func__, index * 4);
>> +        break;
>>       }
>> +
>> +    FEC_PRINTF("reg[%s] => 0x%" PRIx32 "\n", imx_fec_reg_name(s, 
>> index),
>> +                                              value);
>> +
>> +    return value;
>>   }
>>     static void imx_fec_write(void *opaque, hwaddr addr,
>>                             uint64_t value, unsigned size)
>>   {
>>       IMXFECState *s = IMX_FEC(opaque);
>> +    uint32_t index = addr >> 2;
>>   -    FEC_PRINTF("writing 0x%08x @ 0x%" HWADDR_PRIx "\n", 
>> (int)value, addr);
>> +    FEC_PRINTF("reg[%s] <= 0x%" PRIx32 "\n", imx_fec_reg_name(s, 
>> index),
>> +                                             (uint32_t)value);
>>   -    switch (addr & 0x3ff) {
>> -    case 0x004: /* EIR */
>> -        s->eir &= ~value;
>> +    switch (index) {
>> +    case ENET_EIR:
>> +        s->regs[index] &= ~value;
>>           break;
>> -    case 0x008: /* EIMR */
>> -        s->eimr = value;
>> +    case ENET_EIMR:
>> +        s->regs[index] = value;
>>           break;
>> -    case 0x010: /* RDAR */
>> -        if ((s->ecr & FEC_EN) && !s->rx_enabled) {
>> -            imx_fec_enable_rx(s);
>> +    case ENET_RDAR:
>> +        if (s->regs[ENET_ECR] & FEC_EN) {
>> +            if (!s->regs[index]) {
>> +                s->regs[index] = ENET_RDAR_RDAR;
>> +                imx_fec_enable_rx(s);
>> +            }
>> +        } else {
>> +            s->regs[index] = 0;
>>           }
>>           break;
>> -    case 0x014: /* TDAR */
>> -        if (s->ecr & FEC_EN) {
>> +    case ENET_TDAR:
>> +        if (s->regs[ENET_ECR] & FEC_EN) {
>> +            s->regs[index] = ENET_TDAR_TDAR;
>>               imx_fec_do_tx(s);
>>           }
>> +        s->regs[index] = 0;
>>           break;
>> -    case 0x024: /* ECR */
>> -        s->ecr = value;
>> +    case ENET_ECR:
>>           if (value & FEC_RESET) {
>> -            imx_fec_reset(DEVICE(s));
>> +            return imx_fec_reset(DEVICE(s));
>>           }
>> -        if ((s->ecr & FEC_EN) == 0) {
>> -            s->rx_enabled = 0;
>> +        s->regs[index] = value;
>> +        if ((s->regs[index] & FEC_EN) == 0) {
>> +            s->regs[ENET_RDAR] = 0;
>> +            s->rx_descriptor = s->regs[ENET_RDSR];
>> +            s->regs[ENET_TDAR] = 0;
>> +            s->tx_descriptor = s->regs[ENET_TDSR];
>
> This seems like a new behavior, is this a bug fix? If yes, better split.

It is a more correct behavior I think. Note however that our guest OSes 
(in particular Linux) do not trigger this bit. So it is mostly 
untested/unused.

>
>>           }
>>           break;
>> -    case 0x040: /* MMFR */
>> -        /* store the value */
>> -        s->mmfr = value;
>> +    case ENET_MMFR:
>> +        s->regs[index] = value;
>>           if (extract32(value, 29, 1)) {
>> -            s->mmfr = do_phy_read(s, extract32(value, 18, 10));
>> +            /* This is a read operation */
>> +            s->regs[ENET_MMFR] = deposit32(s->regs[ENET_MMFR], 0, 16,
>> +                                           do_phy_read(s,
>> + extract32(value,
>> + 18, 10)));
>>           } else {
>> +            /* This a write operation */
>>               do_phy_write(s, extract32(value, 18, 10), 
>> extract32(value, 0, 16));
>>           }
>>           /* raise the interrupt as the PHY operation is done */
>> -        s->eir |= FEC_INT_MII;
>> +        s->regs[ENET_EIR] |= FEC_INT_MII;
>>           break;
>> -    case 0x044: /* MSCR */
>> -        s->mscr = value & 0xfe;
>> +    case ENET_MSCR:
>> +        s->regs[index] = value & 0xfe;
>>           break;
>> -    case 0x064: /* MIBC */
>> +    case ENET_MIBC:
>>           /* TODO: Implement MIB.  */
>> -        s->mibc = (value & 0x80000000) ? 0xc0000000 : 0;
>> +        s->regs[index] = (value & 0x80000000) ? 0xc0000000 : 0;
>>           break;
>> -    case 0x084: /* RCR */
>> -        s->rcr = value & 0x07ff003f;
>> +    case ENET_RCR:
>> +        s->regs[index] = value & 0x07ff003f;
>>           /* TODO: Implement LOOP mode.  */
>>           break;
>> -    case 0x0c4: /* TCR */
>> +    case ENET_TCR:
>>           /* We transmit immediately, so raise GRA immediately. */
>> -        s->tcr = value;
>> +        s->regs[index] = value;
>>           if (value & 1) {
>> -            s->eir |= FEC_INT_GRA;
>> +            s->regs[ENET_EIR] |= FEC_INT_GRA;
>>           }
>>           break;
>> -    case 0x0e4: /* PALR */
>> +    case ENET_PALR:
>> +        s->regs[index] = value;
>>           s->conf.macaddr.a[0] = value >> 24;
>>           s->conf.macaddr.a[1] = value >> 16;
>>           s->conf.macaddr.a[2] = value >> 8;
>>           s->conf.macaddr.a[3] = value;
>
> I believe we should use stl_be_p() here?

I didn't change this bit ...

>
>>           break;
>> -    case 0x0e8: /* PAUR */
>> +    case ENET_PAUR:
>> +        s->regs[index] = (value | 0x0000ffff) & 0xffff8808;
>>           s->conf.macaddr.a[4] = value >> 24;
>>           s->conf.macaddr.a[5] = value >> 16;
>>           break;
>> -    case 0x0ec: /* OPDR */
>> +    case ENET_OPD:
>> +        s->regs[index] = (value & 0x0000ffff) | 0x00010000;
>>           break;
>> -    case 0x118: /* IAUR */
>> -    case 0x11c: /* IALR */
>> -    case 0x120: /* GAUR */
>> -    case 0x124: /* GALR */
>> +    case ENET_IAUR:
>> +    case ENET_IALR:
>> +    case ENET_GAUR:
>> +    case ENET_GALR:
>>           /* TODO: implement MAC hash filtering.  */
>>           break;
>> -    case 0x144: /* TFWR */
>> -        s->tfwr = value & 3;
>> +    case ENET_TFWR:
>> +        s->regs[index] = value & 3;
>>           break;
>> -    case 0x14c: /* FRBR */
>> -        /* FRBR writes ignored.  */
>> +    case ENET_FRBR:
>> +        /* FRBR is read only */
>> +        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Register FRBR is 
>> read only\n",
>> +                      TYPE_IMX_FEC, __func__);
>>           break;
>> -    case 0x150: /* FRSR */
>> -        s->frsr = (value & 0x3fc) | 0x400;
>> +    case ENET_FRSR:
>> +        s->regs[index] = (value & 0x000003fc) | 0x00000400;
>>           break;
>> -    case 0x180: /* ERDSR */
>> -        s->erdsr = value & ~3;
>> -        s->rx_descriptor = s->erdsr;
>> +    case ENET_RDSR:
>> +        s->regs[index] = value & ~3;
>> +        s->rx_descriptor = s->regs[index];
>>           break;
>> -    case 0x184: /* ETDSR */
>> -        s->etdsr = value & ~3;
>> -        s->tx_descriptor = s->etdsr;
>> +    case ENET_TDSR:
>> +        s->regs[index] = value & ~3;
>> +        s->tx_descriptor = s->regs[index];
>>           break;
>> -    case 0x188: /* EMRBR */
>> -        s->emrbr = value & 0x7f0;
>> +    case ENET_MRBR:
>> +        s->regs[index] = value & 0x000007f0;
>>           break;
>> -    case 0x300: /* MIIGSK_CFGR */
>> -        s->miigsk_cfgr = value & 0x53;
>> +    case ENET_MIIGSK_CFGR:
>> +        s->regs[index] = value & 0x00000053;
>>           break;
>> -    case 0x308: /* MIIGSK_ENR */
>> -        s->miigsk_enr = (value & 0x2) ? 0x6 : 0;
>> +    case ENET_MIIGSK_ENR:
>> +        s->regs[index] = (value & 0x00000002) ? 0x00000006 : 0;
>>           break;
>>       default:
>>           qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad address at 
>> offset 0x%"
>> -                      HWADDR_PRIx "\n", TYPE_IMX_FEC, __func__, addr);
>> +                      PRIx32 "\n", TYPE_IMX_FEC, __func__, index * 4);
>>           break;
>>       }
>>   @@ -541,7 +589,7 @@ static int imx_fec_can_receive(NetClientState *nc)
>>   {
>>       IMXFECState *s = IMX_FEC(qemu_get_nic_opaque(nc));
>>   -    return s->rx_enabled;
>> +    return s->regs[ENET_RDAR] ? 1 : 0;
>>   }
>>     static ssize_t imx_fec_receive(NetClientState *nc, const uint8_t 
>> *buf,
>> @@ -559,7 +607,7 @@ static ssize_t imx_fec_receive(NetClientState 
>> *nc, const uint8_t *buf,
>>         FEC_PRINTF("len %d\n", (int)size);
>>   -    if (!s->rx_enabled) {
>> +    if (!s->regs[ENET_RDAR]) {
>>           qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Unexpected packet\n",
>>                         TYPE_IMX_FEC, __func__);
>>           return 0;
>> @@ -577,7 +625,7 @@ static ssize_t imx_fec_receive(NetClientState 
>> *nc, const uint8_t *buf,
>>       }
>>         /* Frames larger than the user limit just set error flags.  */
>> -    if (size > (s->rcr >> 16)) {
>> +    if (size > (s->regs[ENET_RCR] >> 16)) {
>>           flags |= FEC_BD_LG;
>>       }
>>   @@ -595,7 +643,7 @@ static ssize_t imx_fec_receive(NetClientState 
>> *nc, const uint8_t *buf,
>>                             TYPE_IMX_FEC, __func__);
>>               break;
>>           }
>> -        buf_len = (size <= s->emrbr) ? size : s->emrbr;
>> +        buf_len = (size <= s->regs[ENET_MRBR]) ? size : 
>> s->regs[ENET_MRBR];
>>           bd.length = buf_len;
>>           size -= buf_len;
>>   @@ -618,16 +666,16 @@ static ssize_t imx_fec_receive(NetClientState 
>> *nc, const uint8_t *buf,
>>               /* Last buffer in frame.  */
>>               bd.flags |= flags | FEC_BD_L;
>>               FEC_PRINTF("rx frame flags %04x\n", bd.flags);
>> -            s->eir |= FEC_INT_RXF;
>> +            s->regs[ENET_EIR] |= FEC_INT_RXF;
>>           } else {
>> -            s->eir |= FEC_INT_RXB;
>> +            s->regs[ENET_EIR] |= FEC_INT_RXB;
>>           }
>>           imx_fec_write_bd(&bd, addr);
>>           /* Advance to the next descriptor.  */
>>           if ((bd.flags & FEC_BD_W) != 0) {
>> -            addr = s->erdsr;
>> +            addr = s->regs[ENET_RDSR];
>>           } else {
>> -            addr += 8;
>> +            addr += sizeof(bd);
>>           }
>>       }
>>       s->rx_descriptor = addr;
>> diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h
>> index cbf8650..709f8a0 100644
>> --- a/include/hw/net/imx_fec.h
>> +++ b/include/hw/net/imx_fec.h
>> @@ -30,6 +30,33 @@
>>   #include "hw/sysbus.h"
>>   #include "net/net.h"
>>   +#define ENET_EIR               1
>> +#define ENET_EIMR              2
>> +#define ENET_RDAR              4
>> +#define ENET_TDAR              5
>> +#define ENET_ECR               9
>> +#define ENET_MMFR              16
>> +#define ENET_MSCR              17
>> +#define ENET_MIBC              25
>> +#define ENET_RCR               33
>> +#define ENET_TCR               49
>> +#define ENET_PALR              57
>> +#define ENET_PAUR              58
>> +#define ENET_OPD               59
>> +#define ENET_IAUR              70
>> +#define ENET_IALR              71
>> +#define ENET_GAUR              72
>> +#define ENET_GALR              73
>> +#define ENET_TFWR              81
>> +#define ENET_FRBR              83
>> +#define ENET_FRSR              84
>> +#define ENET_RDSR              96
>> +#define ENET_TDSR              97
>> +#define ENET_MRBR              98
>> +#define ENET_MIIGSK_CFGR       192
>> +#define ENET_MIIGSK_ENR        194
>> +#define ENET_MAX               400
>
> Is this an arbitrary value or there's a plan to add more register 
> whose index is greater than 192?

More registers are coming with the ENET device.

>
>> +
>>   #define FEC_MAX_FRAME_SIZE 2032
>>     #define FEC_INT_HB      (1 << 31)
>> @@ -46,8 +73,14 @@
>>   #define FEC_INT_RL      (1 << 20)
>>   #define FEC_INT_UN      (1 << 19)
>>   -#define FEC_EN      2
>> -#define FEC_RESET   1
>> +/* RDAR */
>> +#define ENET_RDAR_RDAR         (1 << 24)
>> +
>> +/* TDAR */
>> +#define ENET_TDAR_TDAR         (1 << 24)
>> +
>> +#define FEC_EN                 (1 << 1)
>> +#define FEC_RESET              (1 << 0)
>>     /* Buffer Descriptor.  */
>>   typedef struct {
>> @@ -83,25 +116,9 @@ typedef struct IMXFECState {
>>       qemu_irq irq;
>>       MemoryRegion iomem;
>>   -    uint32_t irq_state;
>> -    uint32_t eir;
>> -    uint32_t eimr;
>> -    uint32_t rx_enabled;
>> +    uint32_t regs[ENET_MAX];
>>       uint32_t rx_descriptor;
>>       uint32_t tx_descriptor;
>> -    uint32_t ecr;
>> -    uint32_t mmfr;
>> -    uint32_t mscr;
>> -    uint32_t mibc;
>> -    uint32_t rcr;
>> -    uint32_t tcr;
>> -    uint32_t tfwr;
>> -    uint32_t frsr;
>> -    uint32_t erdsr;
>> -    uint32_t etdsr;
>> -    uint32_t emrbr;
>> -    uint32_t miigsk_cfgr;
>> -    uint32_t miigsk_enr;
>>         uint32_t phy_status;
>>       uint32_t phy_control;
>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH v4 7/8] Add ENET/Gbps Ethernet support to FEC device
  2016-05-19  3:48   ` Jason Wang
@ 2016-05-19 18:14     ` Jean-Christophe DUBOIS
  2016-05-19 18:37       ` Peter Maydell
  2016-05-20  2:34       ` Jason Wang
  0 siblings, 2 replies; 22+ messages in thread
From: Jean-Christophe DUBOIS @ 2016-05-19 18:14 UTC (permalink / raw)
  To: Jason Wang, qemu-devel, peter.maydell

Le 19/05/2016 05:48, Jason Wang a écrit :
>
>
> On 2016年05月19日 06:23, Jean-Christophe Dubois wrote:
>> The ENET device (present in i.MX6) is "derived" from FEC and backward
>> compatible with it.
>>
>> This patch adds the necessary support of the added feature in the ENET
>> device to allow Linux to use it (on supported processors).
>>
>> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
>> ---
>>
>> Changes since v1:
>>   * Not present on v1
>>     Changes since v2:
>>   * Not present on v2
>>   Changes since v3:
>>   * Separate and fix the 2 supported interrupts
>>
>>   hw/arm/fsl-imx25.c       |   3 +
>>   hw/net/imx_fec.c         | 713 
>> ++++++++++++++++++++++++++++++++++++++---------
>>   include/hw/net/imx_fec.h | 195 ++++++++++---
>>   3 files changed, 745 insertions(+), 166 deletions(-)
>
> [...]
>
>>   -static Property imx_fec_properties[] = {
>> +static Property imx_eth_properties[] = {
>>       DEFINE_NIC_PROPERTIES(IMXFECState, conf),
>> +    DEFINE_PROP_BOOL("is-fec", IMXFECState, is_fec, false),
>>       DEFINE_PROP_END_OF_LIST(),
>>   };
>
> It's ok to decide with "is-fec", but is it better to use a new type 
> for that?

Well, there is a lot of common code between FEC and ENET because ENET is 
basically backward compatible with FEC. So most/all of the FEC code 
needs to go in the ENET device.

I thought this way of doing thing was the best way to avoid duplicating 
things.

>
>>   -static void imx_fec_class_init(ObjectClass *klass, void *data)
>> +static void imx_eth_class_init(ObjectClass *klass, void *data)
>>   {
>>       DeviceClass *dc = DEVICE_CLASS(klass);
>>   -    dc->vmsd = &vmstate_imx_fec;
>> -    dc->reset = imx_fec_reset;
>> -    dc->props = imx_fec_properties;
>> -    dc->realize = imx_fec_realize;
>> -    dc->desc = "i.MX FEC Ethernet Controller";
>> +    dc->vmsd    = &vmstate_imx_eth;
>> +    dc->reset   = imx_eth_reset;
>> +    dc->props   = imx_eth_properties;
>> +    dc->realize = imx_eth_realize;
>> +    dc->desc    = "i.MX FEC/ENET Ethernet Controller";
>>   }
>>   -static const TypeInfo imx_fec_info = {
>> -    .name = TYPE_IMX_FEC,
>> -    .parent = TYPE_SYS_BUS_DEVICE,
>> +static const TypeInfo imx_eth_info = {
>> +    .name          = TYPE_IMX_FEC,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>>       .instance_size = sizeof(IMXFECState),
>> -    .class_init = imx_fec_class_init,
>> +    .class_init    = imx_eth_class_init,
>>   };
>>   -static void imx_fec_register_types(void)
>> +static void imx_eth_register_types(void)
>>   {
>> -    type_register_static(&imx_fec_info);
>> +    type_register_static(&imx_eth_info);
>>   }
>>   -type_init(imx_fec_register_types)
>> +type_init(imx_eth_register_types)
>> diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h
>> index 709f8a0..a09b7d7 100644
>> --- a/include/hw/net/imx_fec.h
>> +++ b/include/hw/net/imx_fec.h
>> @@ -1,5 +1,5 @@
>>   /*
>> - * i.MX Fast Ethernet Controller emulation.
>> + * i.MX FEC/ENET Ethernet Controller emulation.
>>    *
>>    * Copyright (c) 2013 Jean-Christophe Dubois. <jcd@tribudubois.net>
>>    *
>> @@ -53,25 +53,64 @@
>>   #define ENET_RDSR              96
>>   #define ENET_TDSR              97
>>   #define ENET_MRBR              98
>> +#define ENET_RSFL              100
>> +#define ENET_RSEM              101
>> +#define ENET_RAEM              102
>> +#define ENET_RAFL              103
>> +#define ENET_TSEM              104
>> +#define ENET_TAEM              105
>> +#define ENET_TAFL              106
>> +#define ENET_TIPG              107
>> +#define ENET_FTRL              108
>> +#define ENET_TACC              112
>> +#define ENET_RACC              113
>>   #define ENET_MIIGSK_CFGR       192
>>   #define ENET_MIIGSK_ENR        194
>> +#define ENET_ATCR              256
>> +#define ENET_ATVR              257
>> +#define ENET_ATOFF             258
>> +#define ENET_ATPER             259
>> +#define ENET_ATCOR             260
>> +#define ENET_ATINC             261
>> +#define ENET_ATSTMP            262
>> +#define ENET_TGSR              385
>> +#define ENET_TCSR0             386
>> +#define ENET_TCCR0             387
>> +#define ENET_TCSR1             388
>> +#define ENET_TCCR1             389
>> +#define ENET_TCSR2             390
>> +#define ENET_TCCR2             391
>> +#define ENET_TCSR3             392
>> +#define ENET_TCCR3             393
>>   #define ENET_MAX               400
>>   -#define FEC_MAX_FRAME_SIZE 2032
>> -
>> -#define FEC_INT_HB      (1 << 31)
>> -#define FEC_INT_BABR    (1 << 30)
>> -#define FEC_INT_BABT    (1 << 29)
>> -#define FEC_INT_GRA     (1 << 28)
>> -#define FEC_INT_TXF     (1 << 27)
>> -#define FEC_INT_TXB     (1 << 26)
>> -#define FEC_INT_RXF     (1 << 25)
>> -#define FEC_INT_RXB     (1 << 24)
>> -#define FEC_INT_MII     (1 << 23)
>> -#define FEC_INT_EBERR   (1 << 22)
>> -#define FEC_INT_LC      (1 << 21)
>> -#define FEC_INT_RL      (1 << 20)
>> -#define FEC_INT_UN      (1 << 19)
>> +#define ENET_MAX_FRAME_SIZE    2032
>> +
>> +/* EIR and EIMR */
>> +#define ENET_INT_HB            (1 << 31)
>> +#define ENET_INT_BABR          (1 << 30)
>> +#define ENET_INT_BABT          (1 << 29)
>> +#define ENET_INT_GRA           (1 << 28)
>> +#define ENET_INT_TXF           (1 << 27)
>> +#define ENET_INT_TXB           (1 << 26)
>> +#define ENET_INT_RXF           (1 << 25)
>> +#define ENET_INT_RXB           (1 << 24)
>> +#define ENET_INT_MII           (1 << 23)
>> +#define ENET_INT_EBERR         (1 << 22)
>> +#define ENET_INT_LC            (1 << 21)
>> +#define ENET_INT_RL            (1 << 20)
>> +#define ENET_INT_UN            (1 << 19)
>
> The above renaming seems cause lots of unnecessary changes above which 
> may brings issue when backporint patches to -stable. Can we just keep 
> this, and all ENET_*** should be used after we're sure we are ENET?

What do you mean by "after we're sure we are ENET"?

ENET is the evolution of FEC to support Gb interface. FEC is more like a 
legacy device these days (for ARM9 family). So it seems we are better 
supporting ENET as default going forward.

>
>> +#define ENET_INT_PLR           (1 << 18)
>> +#define ENET_INT_WAKEUP        (1 << 17)
>> +#define ENET_INT_TS_AVAIL      (1 << 16)
>> +#define ENET_INT_TS_TIMER      (1 << 15)
>> +
>> +#define ENET_INT_MAC           (ENET_INT_HB | ENET_INT_BABR | 
>> ENET_INT_BABT | \
>> +                                ENET_INT_GRA | ENET_INT_TXF | 
>> ENET_INT_TXB | \
>> +                                ENET_INT_RXF | ENET_INT_RXB | 
>> ENET_INT_MII | \
>> +                                ENET_INT_EBERR | ENET_INT_LC | 
>> ENET_INT_RL | \
>> +                                ENET_INT_UN | ENET_INT_PLR | 
>> ENET_INT_WAKEUP | \
>> +                                ENET_INT_TS_AVAIL)
>>     /* RDAR */
>>   #define ENET_RDAR_RDAR         (1 << 24)
>> @@ -79,8 +118,54 @@
>>   /* TDAR */
>>   #define ENET_TDAR_TDAR         (1 << 24)
>>   -#define FEC_EN                 (1 << 1)
>> -#define FEC_RESET              (1 << 0)
>> +/* ECR */
>> +#define ENET_ECR_RESET         (1 << 0)
>> +#define ENET_ECR_ETHEREN       (1 << 1)
>> +#define ENET_ECR_MAGICEN       (1 << 2)
>> +#define ENET_ECR_SLEEP         (1 << 3)
>> +#define ENET_ECR_EN1588        (1 << 4)
>> +#define ENET_ECR_SPEED         (1 << 5)
>> +#define ENET_ECR_DBGEN         (1 << 6)
>> +#define ENET_ECR_STOPEN        (1 << 7)
>> +#define ENET_ECR_DSBWP         (1 << 8)
>> +
>> +/* MIBC */
>> +#define ENET_MIBC_MIB_DIS      (1 << 31)
>> +#define ENET_MIBC_MIB_IDLE     (1 << 30)
>> +#define ENET_MIBC_MIB_CLEAR    (1 << 29)
>> +
>> +/* RCR */
>> +#define ENET_RCR_LOOP          (1 << 0)
>> +#define ENET_RCR_DRT           (1 << 1)
>> +#define ENET_RCR_MII_MODE      (1 << 2)
>> +#define ENET_RCR_PROM          (1 << 3)
>> +#define ENET_RCR_BC_REJ        (1 << 4)
>> +#define ENET_RCR_FCE           (1 << 5)
>> +#define ENET_RCR_RGMII_EN      (1 << 6)
>> +#define ENET_RCR_RMII_MODE     (1 << 8)
>> +#define ENET_RCR_RMII_10T      (1 << 9)
>> +#define ENET_RCR_PADEN         (1 << 12)
>> +#define ENET_RCR_PAUFWD        (1 << 13)
>> +#define ENET_RCR_CRCFWD        (1 << 14)
>> +#define ENET_RCR_CFEN          (1 << 15)
>> +#define ENET_RCR_MAX_FL_SHIFT  (16)
>> +#define ENET_RCR_MAX_FL_LENGTH (14)
>> +#define ENET_RCR_NLC           (1 << 30)
>> +#define ENET_RCR_GRS           (1 << 31)
>> +
>> +/* TCR */
>> +#define ENET_TCR_GTS           (1 << 0)
>> +#define ENET_TCR_FDEN          (1 << 2)
>> +#define ENET_TCR_TFC_PAUSE     (1 << 3)
>> +#define ENET_TCR_RFC_PAUSE     (1 << 4)
>> +#define ENET_TCR_ADDSEL_SHIFT  (5)
>> +#define ENET_TCR_ADDSEL_LENGTH (3)
>> +#define ENET_TCR_CRCFWD        (1 << 9)
>> +
>> +/* RDSR */
>> +#define ENET_TWFR_TFWR_SHIFT   (0)
>> +#define ENET_TWFR_TFWR_LENGTH  (6)
>> +#define ENET_TWFR_STRFWD       (1 << 8)
>>     /* Buffer Descriptor.  */
>>   typedef struct {
>> @@ -89,22 +174,60 @@ typedef struct {
>>       uint32_t data;
>>   } IMXFECBufDesc;
>>   -#define FEC_BD_R    (1 << 15)
>> -#define FEC_BD_E    (1 << 15)
>> -#define FEC_BD_O1   (1 << 14)
>> -#define FEC_BD_W    (1 << 13)
>> -#define FEC_BD_O2   (1 << 12)
>> -#define FEC_BD_L    (1 << 11)
>> -#define FEC_BD_TC   (1 << 10)
>> -#define FEC_BD_ABC  (1 << 9)
>> -#define FEC_BD_M    (1 << 8)
>> -#define FEC_BD_BC   (1 << 7)
>> -#define FEC_BD_MC   (1 << 6)
>> -#define FEC_BD_LG   (1 << 5)
>> -#define FEC_BD_NO   (1 << 4)
>> -#define FEC_BD_CR   (1 << 2)
>> -#define FEC_BD_OV   (1 << 1)
>> -#define FEC_BD_TR   (1 << 0)
>> +#define ENET_BD_R              (1 << 15)
>> +#define ENET_BD_E              (1 << 15)
>> +#define ENET_BD_O1             (1 << 14)
>> +#define ENET_BD_W              (1 << 13)
>> +#define ENET_BD_O2             (1 << 12)
>> +#define ENET_BD_L              (1 << 11)
>> +#define ENET_BD_TC             (1 << 10)
>> +#define ENET_BD_ABC            (1 << 9)
>> +#define ENET_BD_M              (1 << 8)
>> +#define ENET_BD_BC             (1 << 7)
>> +#define ENET_BD_MC             (1 << 6)
>> +#define ENET_BD_LG             (1 << 5)
>> +#define ENET_BD_NO             (1 << 4)
>> +#define ENET_BD_CR             (1 << 2)
>> +#define ENET_BD_OV             (1 << 1)
>> +#define ENET_BD_TR             (1 << 0)
>> +
>> +typedef struct {
>> +    uint16_t length;
>> +    uint16_t flags;
>> +    uint32_t data;
>> +    uint16_t status;
>> +    uint16_t option;
>> +    uint16_t checksum;
>> +    uint16_t head_proto;
>> +    uint32_t last_buffer;
>> +    uint32_t timestamp;
>> +    uint32_t reserved[2];
>> +} IMXENETBufDesc;
>> +
>> +#define ENET_BD_ME             (1 << 15)
>> +#define ENET_BD_TX_INT         (1 << 14)
>> +#define ENET_BD_TS             (1 << 13)
>> +#define ENET_BD_PINS           (1 << 12)
>> +#define ENET_BD_IINS           (1 << 11)
>> +#define ENET_BD_PE             (1 << 10)
>> +#define ENET_BD_CE             (1 << 9)
>> +#define ENET_BD_UC             (1 << 8)
>> +#define ENET_BD_RX_INT         (1 << 7)
>> +
>> +#define ENET_BD_TXE            (1 << 15)
>> +#define ENET_BD_UE             (1 << 13)
>> +#define ENET_BD_EE             (1 << 12)
>> +#define ENET_BD_FE             (1 << 11)
>> +#define ENET_BD_LCE            (1 << 10)
>> +#define ENET_BD_OE             (1 << 9)
>> +#define ENET_BD_TSE            (1 << 8)
>> +#define ENET_BD_ICE            (1 << 5)
>> +#define ENET_BD_PCR            (1 << 4)
>> +#define ENET_BD_VLAN           (1 << 2)
>> +#define ENET_BD_IPV6           (1 << 1)
>> +#define ENET_BD_FRAG           (1 << 0)
>> +
>> +#define ENET_BD_BDU            (1 << 31)
>>     typedef struct IMXFECState {
>>       /*< private >*/
>> @@ -113,7 +236,7 @@ typedef struct IMXFECState {
>>       /*< public >*/
>>       NICState *nic;
>>       NICConf conf;
>> -    qemu_irq irq;
>> +    qemu_irq irq[2];
>>       MemoryRegion iomem;
>>         uint32_t regs[ENET_MAX];
>> @@ -125,6 +248,8 @@ typedef struct IMXFECState {
>>       uint32_t phy_advertise;
>>       uint32_t phy_int;
>>       uint32_t phy_int_mask;
>> +
>> +    bool is_fec;
>>   } IMXFECState;
>>     #endif
>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH v4 7/8] Add ENET/Gbps Ethernet support to FEC device
  2016-05-19 18:14     ` Jean-Christophe DUBOIS
@ 2016-05-19 18:37       ` Peter Maydell
  2016-05-20 21:24         ` Jean-Christophe DUBOIS
  2016-05-20  2:34       ` Jason Wang
  1 sibling, 1 reply; 22+ messages in thread
From: Peter Maydell @ 2016-05-19 18:37 UTC (permalink / raw)
  To: Jean-Christophe DUBOIS; +Cc: Jason Wang, QEMU Developers

On 19 May 2016 at 19:14, Jean-Christophe DUBOIS <jcd@tribudubois.net> wrote:
> Le 19/05/2016 05:48, Jason Wang a écrit :
>> On 2016年05月19日 06:23, Jean-Christophe Dubois wrote:
>>
>> It's ok to decide with "is-fec", but is it better to use a new type for
>> that?
>
>
> Well, there is a lot of common code between FEC and ENET because ENET is
> basically backward compatible with FEC. So most/all of the FEC code needs to
> go in the ENET device.
>
> I thought this way of doing thing was the best way to avoid duplicating
> things.

Right, but usually we have different device types, even if under
the hood it's the same code using a bool to decide what to do
(see for instance hw/display/pl110.c, which provides 3 devices
which are all pretty similar). This means that the users of your
device don't need to care that the FEC is an ENET device with
a flag to say "be like an FEC device", they just instantiate
the kind of device they want.

>> The above renaming seems cause lots of unnecessary changes above which may
>> brings issue when backporint patches to -stable. Can we just keep this, and
>> all ENET_*** should be used after we're sure we are ENET?
>
>
> What do you mean by "after we're sure we are ENET"?
>
> ENET is the evolution of FEC to support Gb interface. FEC is more like a
> legacy device these days (for ARM9 family). So it seems we are better
> supporting ENET as default going forward.

I don't mind if we rename the functions, #defines, etc, but if
we do can we have a patch which does that renaming and only that
renaming, so it's easy to review?

thanks
-- PMM

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH v4 6/8] i.MX: move FEC device to a register array structure.
  2016-05-19  6:10     ` Jean-Christophe DUBOIS
@ 2016-05-20  2:26       ` Jason Wang
  2016-05-20 21:25         ` Jean-Christophe DUBOIS
  0 siblings, 1 reply; 22+ messages in thread
From: Jason Wang @ 2016-05-20  2:26 UTC (permalink / raw)
  To: Jean-Christophe DUBOIS
  Cc: qemu-devel, peter.maydell@linaro.org >> Peter Maydell



On 2016年05月19日 14:10, Jean-Christophe DUBOIS wrote:
> Le 19/05/2016 05:28, Jason Wang a écrit :
>>
>>
>> On 2016年05月19日 06:23, Jean-Christophe Dubois wrote:
>>> This is to prepare for the ENET Gb device of the i.MX6.
>>>
>>> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
>>> ---
>>>
>>> Changes since v1:
>>>   * Not present on v1.
>>>
>>> Changes since v2:
>>>   * The patch was split in 2 parts:
>>>     - a "port" to a reg array based device (this patch).
>>>     - the addition of the Gb support (next patch).
>>>   Changes since v3:
>>>   * Small fix patches were extracted from this patch (see previous 3 
>>> patches)
>>>   * Reset logic through ECR was fixed.
>>>   * TDAR/RDAR behavior was fixed.
>>>
>>>   hw/net/imx_fec.c         | 396 
>>> ++++++++++++++++++++++++++---------------------
>>>   include/hw/net/imx_fec.h |  55 ++++---
>>>   2 files changed, 258 insertions(+), 193 deletions(-)
>>>
>>>

[...]

>>> -    case 0x014: /* TDAR */
>>> -        if (s->ecr & FEC_EN) {
>>> +    case ENET_TDAR:
>>> +        if (s->regs[ENET_ECR] & FEC_EN) {
>>> +            s->regs[index] = ENET_TDAR_TDAR;
>>>               imx_fec_do_tx(s);
>>>           }
>>> +        s->regs[index] = 0;
>>>           break;
>>> -    case 0x024: /* ECR */
>>> -        s->ecr = value;
>>> +    case ENET_ECR:
>>>           if (value & FEC_RESET) {
>>> -            imx_fec_reset(DEVICE(s));
>>> +            return imx_fec_reset(DEVICE(s));
>>>           }
>>> -        if ((s->ecr & FEC_EN) == 0) {
>>> -            s->rx_enabled = 0;
>>> +        s->regs[index] = value;
>>> +        if ((s->regs[index] & FEC_EN) == 0) {
>>> +            s->regs[ENET_RDAR] = 0;
>>> +            s->rx_descriptor = s->regs[ENET_RDSR];
>>> +            s->regs[ENET_TDAR] = 0;
>>> +            s->tx_descriptor = s->regs[ENET_TDSR];
>>
>> This seems like a new behavior, is this a bug fix? If yes, better split.
>
> It is a more correct behavior I think. Note however that our guest 
> OSes (in particular Linux) do not trigger this bit. So it is mostly 
> untested/unused.
>

Is this the real hardware or documented behavior? If yes, need a 
separate patch for this. If not, we'd better not add this.

>>
>>>           }
>>>           break;
>>> -    case 0x040: /* MMFR */
>>> -        /* store the value */
>>> -        s->mmfr = value;
>>> +    case ENET_MMFR:
>>> +        s->regs[index] = value;
>>>           if (extract32(value, 29, 1)) {
>>> -            s->mmfr = do_phy_read(s, extract32(value, 18, 10));
>>> +            /* This is a read operation */
>>> +            s->regs[ENET_MMFR] = deposit32(s->regs[ENET_MMFR], 0, 16,
>>> +                                           do_phy_read(s,
>>> + extract32(value,
>>> + 18, 10)));
>>>           } else {
>>> +            /* This a write operation */
>>>               do_phy_write(s, extract32(value, 18, 10), 
>>> extract32(value, 0, 16));
>>>           }
>>>           /* raise the interrupt as the PHY operation is done */
>>> -        s->eir |= FEC_INT_MII;
>>> +        s->regs[ENET_EIR] |= FEC_INT_MII;
>>>           break;
>>> -    case 0x044: /* MSCR */
>>> -        s->mscr = value & 0xfe;
>>> +    case ENET_MSCR:
>>> +        s->regs[index] = value & 0xfe;
>>>           break;
>>> -    case 0x064: /* MIBC */
>>> +    case ENET_MIBC:
>>>           /* TODO: Implement MIB.  */
>>> -        s->mibc = (value & 0x80000000) ? 0xc0000000 : 0;
>>> +        s->regs[index] = (value & 0x80000000) ? 0xc0000000 : 0;
>>>           break;
>>> -    case 0x084: /* RCR */
>>> -        s->rcr = value & 0x07ff003f;
>>> +    case ENET_RCR:
>>> +        s->regs[index] = value & 0x07ff003f;
>>>           /* TODO: Implement LOOP mode.  */
>>>           break;
>>> -    case 0x0c4: /* TCR */
>>> +    case ENET_TCR:
>>>           /* We transmit immediately, so raise GRA immediately. */
>>> -        s->tcr = value;
>>> +        s->regs[index] = value;
>>>           if (value & 1) {
>>> -            s->eir |= FEC_INT_GRA;
>>> +            s->regs[ENET_EIR] |= FEC_INT_GRA;
>>>           }
>>>           break;
>>> -    case 0x0e4: /* PALR */
>>> +    case ENET_PALR:
>>> +        s->regs[index] = value;
>>>           s->conf.macaddr.a[0] = value >> 24;
>>>           s->conf.macaddr.a[1] = value >> 16;
>>>           s->conf.macaddr.a[2] = value >> 8;
>>>           s->conf.macaddr.a[3] = value;
>>
>> I believe we should use stl_be_p() here?
>
> I didn't change this bit ...
>

Sorry, you are right. I misread the patch.

>>
>>>           break;
>>> -    case 0x0e8: /* PAUR */
>>> +    case ENET_PAUR:
>>> +        s->regs[index] = (value | 0x0000ffff) & 0xffff8808;
>>>           s->conf.macaddr.a[4] = value >> 24;
>>>           s->conf.macaddr.a[5] = value >> 16;
>>>           break;
>>> -    case 0x0ec: /* OPDR */

[...]

>>> diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h
>>> index cbf8650..709f8a0 100644
>>> --- a/include/hw/net/imx_fec.h
>>> +++ b/include/hw/net/imx_fec.h
>>> @@ -30,6 +30,33 @@
>>>   #include "hw/sysbus.h"
>>>   #include "net/net.h"
>>>   +#define ENET_EIR               1
>>> +#define ENET_EIMR              2
>>> +#define ENET_RDAR              4
>>> +#define ENET_TDAR              5
>>> +#define ENET_ECR               9
>>> +#define ENET_MMFR              16
>>> +#define ENET_MSCR              17
>>> +#define ENET_MIBC              25
>>> +#define ENET_RCR               33
>>> +#define ENET_TCR               49
>>> +#define ENET_PALR              57
>>> +#define ENET_PAUR              58
>>> +#define ENET_OPD               59
>>> +#define ENET_IAUR              70
>>> +#define ENET_IALR              71
>>> +#define ENET_GAUR              72
>>> +#define ENET_GALR              73
>>> +#define ENET_TFWR              81
>>> +#define ENET_FRBR              83
>>> +#define ENET_FRSR              84
>>> +#define ENET_RDSR              96
>>> +#define ENET_TDSR              97
>>> +#define ENET_MRBR              98
>>> +#define ENET_MIIGSK_CFGR       192
>>> +#define ENET_MIIGSK_ENR        194
>>> +#define ENET_MAX               400
>>
>> Is this an arbitrary value or there's a plan to add more register 
>> whose index is greater than 192?
>
> More registers are coming with the ENET device.
>

Right, I see.

Thanks

>>
>>> +
>>>   #define FEC_MAX_FRAME_SIZE 2032
>>>     #define FEC_INT_HB      (1 << 31)
>>> @@ -46,8 +73,14 @@
>>>   #define FEC_INT_RL      (1 << 20)
>>>   #define FEC_INT_UN      (1 << 19)
>>>   -#define FEC_EN      2
>>> -#define FEC_RESET   1
>>> +/* RDAR */
>>> +#define ENET_RDAR_RDAR         (1 << 24)
>>> +
>>> +/* TDAR */
>>> +#define ENET_TDAR_TDAR         (1 << 24)
>>> +
>>> +#define FEC_EN                 (1 << 1)
>>> +#define FEC_RESET              (1 << 0)
>>>     /* Buffer Descriptor.  */
>>>   typedef struct {
>>> @@ -83,25 +116,9 @@ typedef struct IMXFECState {
>>>       qemu_irq irq;
>>>       MemoryRegion iomem;
>>>   -    uint32_t irq_state;
>>> -    uint32_t eir;
>>> -    uint32_t eimr;
>>> -    uint32_t rx_enabled;
>>> +    uint32_t regs[ENET_MAX];
>>>       uint32_t rx_descriptor;
>>>       uint32_t tx_descriptor;
>>> -    uint32_t ecr;
>>> -    uint32_t mmfr;
>>> -    uint32_t mscr;
>>> -    uint32_t mibc;
>>> -    uint32_t rcr;
>>> -    uint32_t tcr;
>>> -    uint32_t tfwr;
>>> -    uint32_t frsr;
>>> -    uint32_t erdsr;
>>> -    uint32_t etdsr;
>>> -    uint32_t emrbr;
>>> -    uint32_t miigsk_cfgr;
>>> -    uint32_t miigsk_enr;
>>>         uint32_t phy_status;
>>>       uint32_t phy_control;
>>
>>
>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH v4 7/8] Add ENET/Gbps Ethernet support to FEC device
  2016-05-19 18:14     ` Jean-Christophe DUBOIS
  2016-05-19 18:37       ` Peter Maydell
@ 2016-05-20  2:34       ` Jason Wang
  2016-05-20 21:23         ` Jean-Christophe DUBOIS
  1 sibling, 1 reply; 22+ messages in thread
From: Jason Wang @ 2016-05-20  2:34 UTC (permalink / raw)
  To: Jean-Christophe DUBOIS, qemu-devel, peter.maydell



On 2016年05月20日 02:14, Jean-Christophe DUBOIS wrote:
> Le 19/05/2016 05:48, Jason Wang a écrit :
>>
>>
>> On 2016年05月19日 06:23, Jean-Christophe Dubois wrote:
>>> The ENET device (present in i.MX6) is "derived" from FEC and backward
>>> compatible with it.
>>>
>>> This patch adds the necessary support of the added feature in the ENET
>>> device to allow Linux to use it (on supported processors).
>>>
>>> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
>>> ---
>>>
>>> Changes since v1:
>>>   * Not present on v1
>>>     Changes since v2:
>>>   * Not present on v2
>>>   Changes since v3:
>>>   * Separate and fix the 2 supported interrupts
>>>
>>>   hw/arm/fsl-imx25.c       |   3 +
>>>   hw/net/imx_fec.c         | 713 
>>> ++++++++++++++++++++++++++++++++++++++---------
>>>   include/hw/net/imx_fec.h | 195 ++++++++++---
>>>   3 files changed, 745 insertions(+), 166 deletions(-)
>>
>> [...]
>>
>>>   -static Property imx_fec_properties[] = {
>>> +static Property imx_eth_properties[] = {
>>>       DEFINE_NIC_PROPERTIES(IMXFECState, conf),
>>> +    DEFINE_PROP_BOOL("is-fec", IMXFECState, is_fec, false),
>>>       DEFINE_PROP_END_OF_LIST(),
>>>   };
>>
>> It's ok to decide with "is-fec", but is it better to use a new type 
>> for that?
>
> Well, there is a lot of common code between FEC and ENET because ENET 
> is basically backward compatible with FEC. So most/all of the FEC code 
> needs to go in the ENET device.
>
> I thought this way of doing thing was the best way to avoid 
> duplicating things.

You can still share almost all the codes. E.g you can have a look at 
e1000.c which have 3 types of rather similar devices.

Another question not relate to this patch, I saw version were bumped for 
vmstate version. Is it better to keep the vmstate for FEC and using a 
new vmstate for ENET (register array).

>
>>
>>>   -static void imx_fec_class_init(ObjectClass *klass, void *data)
>>> +static void imx_eth_class_init(ObjectClass *klass, void *data)
>>>   {
>>>       DeviceClass *dc = DEVICE_CLASS(klass);
>>>   -    dc->vmsd = &vmstate_imx_fec;
>>> -    dc->reset = imx_fec_reset;
>>> -    dc->props = imx_fec_properties;
>>> -    dc->realize = imx_fec_realize;
>>> -    dc->desc = "i.MX FEC Ethernet Controller";
>>> +    dc->vmsd    = &vmstate_imx_eth;
>>> +    dc->reset   = imx_eth_reset;
>>> +    dc->props   = imx_eth_properties;
>>> +    dc->realize = imx_eth_realize;
>>> +    dc->desc    = "i.MX FEC/ENET Ethernet Controller";
>>>   }
>>>   -static const TypeInfo imx_fec_info = {
>>> -    .name = TYPE_IMX_FEC,
>>> -    .parent = TYPE_SYS_BUS_DEVICE,
>>> +static const TypeInfo imx_eth_info = {
>>> +    .name          = TYPE_IMX_FEC,
>>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>>>       .instance_size = sizeof(IMXFECState),
>>> -    .class_init = imx_fec_class_init,
>>> +    .class_init    = imx_eth_class_init,
>>>   };
>>>   -static void imx_fec_register_types(void)
>>> +static void imx_eth_register_types(void)
>>>   {
>>> -    type_register_static(&imx_fec_info);
>>> +    type_register_static(&imx_eth_info);
>>>   }
>>>   -type_init(imx_fec_register_types)
>>> +type_init(imx_eth_register_types)
>>> diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h
>>> index 709f8a0..a09b7d7 100644
>>> --- a/include/hw/net/imx_fec.h
>>> +++ b/include/hw/net/imx_fec.h
>>> @@ -1,5 +1,5 @@
>>>   /*
>>> - * i.MX Fast Ethernet Controller emulation.
>>> + * i.MX FEC/ENET Ethernet Controller emulation.
>>>    *
>>>    * Copyright (c) 2013 Jean-Christophe Dubois. <jcd@tribudubois.net>
>>>    *
>>> @@ -53,25 +53,64 @@
>>>   #define ENET_RDSR              96
>>>   #define ENET_TDSR              97
>>>   #define ENET_MRBR              98
>>> +#define ENET_RSFL              100
>>> +#define ENET_RSEM              101
>>> +#define ENET_RAEM              102
>>> +#define ENET_RAFL              103
>>> +#define ENET_TSEM              104
>>> +#define ENET_TAEM              105
>>> +#define ENET_TAFL              106
>>> +#define ENET_TIPG              107
>>> +#define ENET_FTRL              108
>>> +#define ENET_TACC              112
>>> +#define ENET_RACC              113
>>>   #define ENET_MIIGSK_CFGR       192
>>>   #define ENET_MIIGSK_ENR        194
>>> +#define ENET_ATCR              256
>>> +#define ENET_ATVR              257
>>> +#define ENET_ATOFF             258
>>> +#define ENET_ATPER             259
>>> +#define ENET_ATCOR             260
>>> +#define ENET_ATINC             261
>>> +#define ENET_ATSTMP            262
>>> +#define ENET_TGSR              385
>>> +#define ENET_TCSR0             386
>>> +#define ENET_TCCR0             387
>>> +#define ENET_TCSR1             388
>>> +#define ENET_TCCR1             389
>>> +#define ENET_TCSR2             390
>>> +#define ENET_TCCR2             391
>>> +#define ENET_TCSR3             392
>>> +#define ENET_TCCR3             393
>>>   #define ENET_MAX               400
>>>   -#define FEC_MAX_FRAME_SIZE 2032
>>> -
>>> -#define FEC_INT_HB      (1 << 31)
>>> -#define FEC_INT_BABR    (1 << 30)
>>> -#define FEC_INT_BABT    (1 << 29)
>>> -#define FEC_INT_GRA     (1 << 28)
>>> -#define FEC_INT_TXF     (1 << 27)
>>> -#define FEC_INT_TXB     (1 << 26)
>>> -#define FEC_INT_RXF     (1 << 25)
>>> -#define FEC_INT_RXB     (1 << 24)
>>> -#define FEC_INT_MII     (1 << 23)
>>> -#define FEC_INT_EBERR   (1 << 22)
>>> -#define FEC_INT_LC      (1 << 21)
>>> -#define FEC_INT_RL      (1 << 20)
>>> -#define FEC_INT_UN      (1 << 19)
>>> +#define ENET_MAX_FRAME_SIZE    2032
>>> +
>>> +/* EIR and EIMR */
>>> +#define ENET_INT_HB            (1 << 31)
>>> +#define ENET_INT_BABR          (1 << 30)
>>> +#define ENET_INT_BABT          (1 << 29)
>>> +#define ENET_INT_GRA           (1 << 28)
>>> +#define ENET_INT_TXF           (1 << 27)
>>> +#define ENET_INT_TXB           (1 << 26)
>>> +#define ENET_INT_RXF           (1 << 25)
>>> +#define ENET_INT_RXB           (1 << 24)
>>> +#define ENET_INT_MII           (1 << 23)
>>> +#define ENET_INT_EBERR         (1 << 22)
>>> +#define ENET_INT_LC            (1 << 21)
>>> +#define ENET_INT_RL            (1 << 20)
>>> +#define ENET_INT_UN            (1 << 19)
>>
>> The above renaming seems cause lots of unnecessary changes above 
>> which may brings issue when backporint patches to -stable. Can we 
>> just keep this, and all ENET_*** should be used after we're sure we 
>> are ENET?
>
> What do you mean by "after we're sure we are ENET"?

I mean "is_fec" is false here.

>
> ENET is the evolution of FEC to support Gb interface. FEC is more like 
> a legacy device these days (for ARM9 family). So it seems we are 
> better supporting ENET as default going forward.

Since Peter does not care about this, we're ok here.

Thanks

>
>>
>>> +#define ENET_INT_PLR           (1 << 18)
>>> +#define ENET_INT_WAKEUP        (1 << 17)
>>> +#define ENET_INT_TS_AVAIL      (1 << 16)
>>> +#define ENET_INT_TS_TIMER      (1 << 15)
>>> +
>>> +#define ENET_INT_MAC           (ENET_INT_HB | ENET_INT_BABR | 
>>> ENET_INT_BABT | \
>>> +                                ENET_INT_GRA | ENET_INT_TXF | 
>>> ENET_INT_TXB | \
>>> +                                ENET_INT_RXF | ENET_INT_RXB | 
>>> ENET_INT_MII | \
>>> +                                ENET_INT_EBERR | ENET_INT_LC | 
>>> ENET_INT_RL | \
>>> +                                ENET_INT_UN | ENET_INT_PLR | 
>>> ENET_INT_WAKEUP | \
>>> +                                ENET_INT_TS_AVAIL)
>>>     /* RDAR */
>>>   #define ENET_RDAR_RDAR         (1 << 24)
>>> @@ -79,8 +118,54 @@
>>>   /* TDAR */
>>>   #define ENET_TDAR_TDAR         (1 << 24)
>>>   -#define FEC_EN                 (1 << 1)
>>> -#define FEC_RESET              (1 << 0)
>>> +/* ECR */
>>> +#define ENET_ECR_RESET         (1 << 0)
>>> +#define ENET_ECR_ETHEREN       (1 << 1)
>>> +#define ENET_ECR_MAGICEN       (1 << 2)
>>> +#define ENET_ECR_SLEEP         (1 << 3)
>>> +#define ENET_ECR_EN1588        (1 << 4)
>>> +#define ENET_ECR_SPEED         (1 << 5)
>>> +#define ENET_ECR_DBGEN         (1 << 6)
>>> +#define ENET_ECR_STOPEN        (1 << 7)
>>> +#define ENET_ECR_DSBWP         (1 << 8)
>>> +
>>> +/* MIBC */
>>> +#define ENET_MIBC_MIB_DIS      (1 << 31)
>>> +#define ENET_MIBC_MIB_IDLE     (1 << 30)
>>> +#define ENET_MIBC_MIB_CLEAR    (1 << 29)
>>> +
>>> +/* RCR */
>>> +#define ENET_RCR_LOOP          (1 << 0)
>>> +#define ENET_RCR_DRT           (1 << 1)
>>> +#define ENET_RCR_MII_MODE      (1 << 2)
>>> +#define ENET_RCR_PROM          (1 << 3)
>>> +#define ENET_RCR_BC_REJ        (1 << 4)
>>> +#define ENET_RCR_FCE           (1 << 5)
>>> +#define ENET_RCR_RGMII_EN      (1 << 6)
>>> +#define ENET_RCR_RMII_MODE     (1 << 8)
>>> +#define ENET_RCR_RMII_10T      (1 << 9)
>>> +#define ENET_RCR_PADEN         (1 << 12)
>>> +#define ENET_RCR_PAUFWD        (1 << 13)
>>> +#define ENET_RCR_CRCFWD        (1 << 14)
>>> +#define ENET_RCR_CFEN          (1 << 15)
>>> +#define ENET_RCR_MAX_FL_SHIFT  (16)
>>> +#define ENET_RCR_MAX_FL_LENGTH (14)
>>> +#define ENET_RCR_NLC           (1 << 30)
>>> +#define ENET_RCR_GRS           (1 << 31)
>>> +
>>> +/* TCR */
>>> +#define ENET_TCR_GTS           (1 << 0)
>>> +#define ENET_TCR_FDEN          (1 << 2)
>>> +#define ENET_TCR_TFC_PAUSE     (1 << 3)
>>> +#define ENET_TCR_RFC_PAUSE     (1 << 4)
>>> +#define ENET_TCR_ADDSEL_SHIFT  (5)
>>> +#define ENET_TCR_ADDSEL_LENGTH (3)
>>> +#define ENET_TCR_CRCFWD        (1 << 9)
>>> +
>>> +/* RDSR */
>>> +#define ENET_TWFR_TFWR_SHIFT   (0)
>>> +#define ENET_TWFR_TFWR_LENGTH  (6)
>>> +#define ENET_TWFR_STRFWD       (1 << 8)
>>>     /* Buffer Descriptor.  */
>>>   typedef struct {
>>> @@ -89,22 +174,60 @@ typedef struct {
>>>       uint32_t data;
>>>   } IMXFECBufDesc;
>>>   -#define FEC_BD_R    (1 << 15)
>>> -#define FEC_BD_E    (1 << 15)
>>> -#define FEC_BD_O1   (1 << 14)
>>> -#define FEC_BD_W    (1 << 13)
>>> -#define FEC_BD_O2   (1 << 12)
>>> -#define FEC_BD_L    (1 << 11)
>>> -#define FEC_BD_TC   (1 << 10)
>>> -#define FEC_BD_ABC  (1 << 9)
>>> -#define FEC_BD_M    (1 << 8)
>>> -#define FEC_BD_BC   (1 << 7)
>>> -#define FEC_BD_MC   (1 << 6)
>>> -#define FEC_BD_LG   (1 << 5)
>>> -#define FEC_BD_NO   (1 << 4)
>>> -#define FEC_BD_CR   (1 << 2)
>>> -#define FEC_BD_OV   (1 << 1)
>>> -#define FEC_BD_TR   (1 << 0)
>>> +#define ENET_BD_R              (1 << 15)
>>> +#define ENET_BD_E              (1 << 15)
>>> +#define ENET_BD_O1             (1 << 14)
>>> +#define ENET_BD_W              (1 << 13)
>>> +#define ENET_BD_O2             (1 << 12)
>>> +#define ENET_BD_L              (1 << 11)
>>> +#define ENET_BD_TC             (1 << 10)
>>> +#define ENET_BD_ABC            (1 << 9)
>>> +#define ENET_BD_M              (1 << 8)
>>> +#define ENET_BD_BC             (1 << 7)
>>> +#define ENET_BD_MC             (1 << 6)
>>> +#define ENET_BD_LG             (1 << 5)
>>> +#define ENET_BD_NO             (1 << 4)
>>> +#define ENET_BD_CR             (1 << 2)
>>> +#define ENET_BD_OV             (1 << 1)
>>> +#define ENET_BD_TR             (1 << 0)
>>> +
>>> +typedef struct {
>>> +    uint16_t length;
>>> +    uint16_t flags;
>>> +    uint32_t data;
>>> +    uint16_t status;
>>> +    uint16_t option;
>>> +    uint16_t checksum;
>>> +    uint16_t head_proto;
>>> +    uint32_t last_buffer;
>>> +    uint32_t timestamp;
>>> +    uint32_t reserved[2];
>>> +} IMXENETBufDesc;
>>> +
>>> +#define ENET_BD_ME             (1 << 15)
>>> +#define ENET_BD_TX_INT         (1 << 14)
>>> +#define ENET_BD_TS             (1 << 13)
>>> +#define ENET_BD_PINS           (1 << 12)
>>> +#define ENET_BD_IINS           (1 << 11)
>>> +#define ENET_BD_PE             (1 << 10)
>>> +#define ENET_BD_CE             (1 << 9)
>>> +#define ENET_BD_UC             (1 << 8)
>>> +#define ENET_BD_RX_INT         (1 << 7)
>>> +
>>> +#define ENET_BD_TXE            (1 << 15)
>>> +#define ENET_BD_UE             (1 << 13)
>>> +#define ENET_BD_EE             (1 << 12)
>>> +#define ENET_BD_FE             (1 << 11)
>>> +#define ENET_BD_LCE            (1 << 10)
>>> +#define ENET_BD_OE             (1 << 9)
>>> +#define ENET_BD_TSE            (1 << 8)
>>> +#define ENET_BD_ICE            (1 << 5)
>>> +#define ENET_BD_PCR            (1 << 4)
>>> +#define ENET_BD_VLAN           (1 << 2)
>>> +#define ENET_BD_IPV6           (1 << 1)
>>> +#define ENET_BD_FRAG           (1 << 0)
>>> +
>>> +#define ENET_BD_BDU            (1 << 31)
>>>     typedef struct IMXFECState {
>>>       /*< private >*/
>>> @@ -113,7 +236,7 @@ typedef struct IMXFECState {
>>>       /*< public >*/
>>>       NICState *nic;
>>>       NICConf conf;
>>> -    qemu_irq irq;
>>> +    qemu_irq irq[2];
>>>       MemoryRegion iomem;
>>>         uint32_t regs[ENET_MAX];
>>> @@ -125,6 +248,8 @@ typedef struct IMXFECState {
>>>       uint32_t phy_advertise;
>>>       uint32_t phy_int;
>>>       uint32_t phy_int_mask;
>>> +
>>> +    bool is_fec;
>>>   } IMXFECState;
>>>     #endif
>>
>>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH v4 7/8] Add ENET/Gbps Ethernet support to FEC device
  2016-05-20  2:34       ` Jason Wang
@ 2016-05-20 21:23         ` Jean-Christophe DUBOIS
  2016-05-24  2:16           ` Jason Wang
  0 siblings, 1 reply; 22+ messages in thread
From: Jean-Christophe DUBOIS @ 2016-05-20 21:23 UTC (permalink / raw)
  To: Jason Wang, qemu-devel, peter.maydell

Le 20/05/2016 04:34, Jason Wang a écrit :
>
>
> On 2016年05月20日 02:14, Jean-Christophe DUBOIS wrote:
>> Le 19/05/2016 05:48, Jason Wang a écrit :
>>>
>>>
>>> On 2016年05月19日 06:23, Jean-Christophe Dubois wrote:
>>>> The ENET device (present in i.MX6) is "derived" from FEC and backward
>>>> compatible with it.
>>>>
>>>> This patch adds the necessary support of the added feature in the ENET
>>>> device to allow Linux to use it (on supported processors).
>>>>
>>>> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
>>>> ---
>>>>
>>>> Changes since v1:
>>>>   * Not present on v1
>>>>     Changes since v2:
>>>>   * Not present on v2
>>>>   Changes since v3:
>>>>   * Separate and fix the 2 supported interrupts
>>>>
>>>>   hw/arm/fsl-imx25.c       |   3 +
>>>>   hw/net/imx_fec.c         | 713 
>>>> ++++++++++++++++++++++++++++++++++++++---------
>>>>   include/hw/net/imx_fec.h | 195 ++++++++++---
>>>>   3 files changed, 745 insertions(+), 166 deletions(-)
>>>
>>> [...]
>>>
>>>>   -static Property imx_fec_properties[] = {
>>>> +static Property imx_eth_properties[] = {
>>>>       DEFINE_NIC_PROPERTIES(IMXFECState, conf),
>>>> +    DEFINE_PROP_BOOL("is-fec", IMXFECState, is_fec, false),
>>>>       DEFINE_PROP_END_OF_LIST(),
>>>>   };
>>>
>>> It's ok to decide with "is-fec", but is it better to use a new type 
>>> for that?
>>
>> Well, there is a lot of common code between FEC and ENET because ENET 
>> is basically backward compatible with FEC. So most/all of the FEC 
>> code needs to go in the ENET device.
>>
>> I thought this way of doing thing was the best way to avoid 
>> duplicating things.
>
> You can still share almost all the codes. E.g you can have a look at 
> e1000.c which have 3 types of rather similar devices.

OK, I'll change it in next patch to match the pl110 model as proposed by 
Peter. No more additional property...

>
> Another question not relate to this patch, I saw version were bumped 
> for vmstate version. Is it better to keep the vmstate for FEC and 
> using a new vmstate for ENET (register array).

Well, as I moved the FEC to register array, the VMState structure has 
changed and therefore I believe the vmstate version needs to be bumped.

Am I wrong?

>
>>
>>>
>>>>   -static void imx_fec_class_init(ObjectClass *klass, void *data)
>>>> +static void imx_eth_class_init(ObjectClass *klass, void *data)
>>>>   {
>>>>       DeviceClass *dc = DEVICE_CLASS(klass);
>>>>   -    dc->vmsd = &vmstate_imx_fec;
>>>> -    dc->reset = imx_fec_reset;
>>>> -    dc->props = imx_fec_properties;
>>>> -    dc->realize = imx_fec_realize;
>>>> -    dc->desc = "i.MX FEC Ethernet Controller";
>>>> +    dc->vmsd    = &vmstate_imx_eth;
>>>> +    dc->reset   = imx_eth_reset;
>>>> +    dc->props   = imx_eth_properties;
>>>> +    dc->realize = imx_eth_realize;
>>>> +    dc->desc    = "i.MX FEC/ENET Ethernet Controller";
>>>>   }
>>>>   -static const TypeInfo imx_fec_info = {
>>>> -    .name = TYPE_IMX_FEC,
>>>> -    .parent = TYPE_SYS_BUS_DEVICE,
>>>> +static const TypeInfo imx_eth_info = {
>>>> +    .name          = TYPE_IMX_FEC,
>>>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>>>>       .instance_size = sizeof(IMXFECState),
>>>> -    .class_init = imx_fec_class_init,
>>>> +    .class_init    = imx_eth_class_init,
>>>>   };
>>>>   -static void imx_fec_register_types(void)
>>>> +static void imx_eth_register_types(void)
>>>>   {
>>>> -    type_register_static(&imx_fec_info);
>>>> +    type_register_static(&imx_eth_info);
>>>>   }
>>>>   -type_init(imx_fec_register_types)
>>>> +type_init(imx_eth_register_types)
>>>> diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h
>>>> index 709f8a0..a09b7d7 100644
>>>> --- a/include/hw/net/imx_fec.h
>>>> +++ b/include/hw/net/imx_fec.h
>>>> @@ -1,5 +1,5 @@
>>>>   /*
>>>> - * i.MX Fast Ethernet Controller emulation.
>>>> + * i.MX FEC/ENET Ethernet Controller emulation.
>>>>    *
>>>>    * Copyright (c) 2013 Jean-Christophe Dubois. <jcd@tribudubois.net>
>>>>    *
>>>> @@ -53,25 +53,64 @@
>>>>   #define ENET_RDSR              96
>>>>   #define ENET_TDSR              97
>>>>   #define ENET_MRBR              98
>>>> +#define ENET_RSFL              100
>>>> +#define ENET_RSEM              101
>>>> +#define ENET_RAEM              102
>>>> +#define ENET_RAFL              103
>>>> +#define ENET_TSEM              104
>>>> +#define ENET_TAEM              105
>>>> +#define ENET_TAFL              106
>>>> +#define ENET_TIPG              107
>>>> +#define ENET_FTRL              108
>>>> +#define ENET_TACC              112
>>>> +#define ENET_RACC              113
>>>>   #define ENET_MIIGSK_CFGR       192
>>>>   #define ENET_MIIGSK_ENR        194
>>>> +#define ENET_ATCR              256
>>>> +#define ENET_ATVR              257
>>>> +#define ENET_ATOFF             258
>>>> +#define ENET_ATPER             259
>>>> +#define ENET_ATCOR             260
>>>> +#define ENET_ATINC             261
>>>> +#define ENET_ATSTMP            262
>>>> +#define ENET_TGSR              385
>>>> +#define ENET_TCSR0             386
>>>> +#define ENET_TCCR0             387
>>>> +#define ENET_TCSR1             388
>>>> +#define ENET_TCCR1             389
>>>> +#define ENET_TCSR2             390
>>>> +#define ENET_TCCR2             391
>>>> +#define ENET_TCSR3             392
>>>> +#define ENET_TCCR3             393
>>>>   #define ENET_MAX               400
>>>>   -#define FEC_MAX_FRAME_SIZE 2032
>>>> -
>>>> -#define FEC_INT_HB      (1 << 31)
>>>> -#define FEC_INT_BABR    (1 << 30)
>>>> -#define FEC_INT_BABT    (1 << 29)
>>>> -#define FEC_INT_GRA     (1 << 28)
>>>> -#define FEC_INT_TXF     (1 << 27)
>>>> -#define FEC_INT_TXB     (1 << 26)
>>>> -#define FEC_INT_RXF     (1 << 25)
>>>> -#define FEC_INT_RXB     (1 << 24)
>>>> -#define FEC_INT_MII     (1 << 23)
>>>> -#define FEC_INT_EBERR   (1 << 22)
>>>> -#define FEC_INT_LC      (1 << 21)
>>>> -#define FEC_INT_RL      (1 << 20)
>>>> -#define FEC_INT_UN      (1 << 19)
>>>> +#define ENET_MAX_FRAME_SIZE    2032
>>>> +
>>>> +/* EIR and EIMR */
>>>> +#define ENET_INT_HB            (1 << 31)
>>>> +#define ENET_INT_BABR          (1 << 30)
>>>> +#define ENET_INT_BABT          (1 << 29)
>>>> +#define ENET_INT_GRA           (1 << 28)
>>>> +#define ENET_INT_TXF           (1 << 27)
>>>> +#define ENET_INT_TXB           (1 << 26)
>>>> +#define ENET_INT_RXF           (1 << 25)
>>>> +#define ENET_INT_RXB           (1 << 24)
>>>> +#define ENET_INT_MII           (1 << 23)
>>>> +#define ENET_INT_EBERR         (1 << 22)
>>>> +#define ENET_INT_LC            (1 << 21)
>>>> +#define ENET_INT_RL            (1 << 20)
>>>> +#define ENET_INT_UN            (1 << 19)
>>>
>>> The above renaming seems cause lots of unnecessary changes above 
>>> which may brings issue when backporint patches to -stable. Can we 
>>> just keep this, and all ENET_*** should be used after we're sure we 
>>> are ENET?
>>
>> What do you mean by "after we're sure we are ENET"?
>
> I mean "is_fec" is false here.
>
>>
>> ENET is the evolution of FEC to support Gb interface. FEC is more 
>> like a legacy device these days (for ARM9 family). So it seems we are 
>> better supporting ENET as default going forward.
>
> Since Peter does not care about this, we're ok here.

I'll make a renaming patch ...

>
> Thanks
>
>>
>>>
>>>> +#define ENET_INT_PLR           (1 << 18)
>>>> +#define ENET_INT_WAKEUP        (1 << 17)
>>>> +#define ENET_INT_TS_AVAIL      (1 << 16)
>>>> +#define ENET_INT_TS_TIMER      (1 << 15)
>>>> +
>>>> +#define ENET_INT_MAC           (ENET_INT_HB | ENET_INT_BABR | 
>>>> ENET_INT_BABT | \
>>>> +                                ENET_INT_GRA | ENET_INT_TXF | 
>>>> ENET_INT_TXB | \
>>>> +                                ENET_INT_RXF | ENET_INT_RXB | 
>>>> ENET_INT_MII | \
>>>> +                                ENET_INT_EBERR | ENET_INT_LC | 
>>>> ENET_INT_RL | \
>>>> +                                ENET_INT_UN | ENET_INT_PLR | 
>>>> ENET_INT_WAKEUP | \
>>>> +                                ENET_INT_TS_AVAIL)
>>>>     /* RDAR */
>>>>   #define ENET_RDAR_RDAR         (1 << 24)
>>>> @@ -79,8 +118,54 @@
>>>>   /* TDAR */
>>>>   #define ENET_TDAR_TDAR         (1 << 24)
>>>>   -#define FEC_EN                 (1 << 1)
>>>> -#define FEC_RESET              (1 << 0)
>>>> +/* ECR */
>>>> +#define ENET_ECR_RESET         (1 << 0)
>>>> +#define ENET_ECR_ETHEREN       (1 << 1)
>>>> +#define ENET_ECR_MAGICEN       (1 << 2)
>>>> +#define ENET_ECR_SLEEP         (1 << 3)
>>>> +#define ENET_ECR_EN1588        (1 << 4)
>>>> +#define ENET_ECR_SPEED         (1 << 5)
>>>> +#define ENET_ECR_DBGEN         (1 << 6)
>>>> +#define ENET_ECR_STOPEN        (1 << 7)
>>>> +#define ENET_ECR_DSBWP         (1 << 8)
>>>> +
>>>> +/* MIBC */
>>>> +#define ENET_MIBC_MIB_DIS      (1 << 31)
>>>> +#define ENET_MIBC_MIB_IDLE     (1 << 30)
>>>> +#define ENET_MIBC_MIB_CLEAR    (1 << 29)
>>>> +
>>>> +/* RCR */
>>>> +#define ENET_RCR_LOOP          (1 << 0)
>>>> +#define ENET_RCR_DRT           (1 << 1)
>>>> +#define ENET_RCR_MII_MODE      (1 << 2)
>>>> +#define ENET_RCR_PROM          (1 << 3)
>>>> +#define ENET_RCR_BC_REJ        (1 << 4)
>>>> +#define ENET_RCR_FCE           (1 << 5)
>>>> +#define ENET_RCR_RGMII_EN      (1 << 6)
>>>> +#define ENET_RCR_RMII_MODE     (1 << 8)
>>>> +#define ENET_RCR_RMII_10T      (1 << 9)
>>>> +#define ENET_RCR_PADEN         (1 << 12)
>>>> +#define ENET_RCR_PAUFWD        (1 << 13)
>>>> +#define ENET_RCR_CRCFWD        (1 << 14)
>>>> +#define ENET_RCR_CFEN          (1 << 15)
>>>> +#define ENET_RCR_MAX_FL_SHIFT  (16)
>>>> +#define ENET_RCR_MAX_FL_LENGTH (14)
>>>> +#define ENET_RCR_NLC           (1 << 30)
>>>> +#define ENET_RCR_GRS           (1 << 31)
>>>> +
>>>> +/* TCR */
>>>> +#define ENET_TCR_GTS           (1 << 0)
>>>> +#define ENET_TCR_FDEN          (1 << 2)
>>>> +#define ENET_TCR_TFC_PAUSE     (1 << 3)
>>>> +#define ENET_TCR_RFC_PAUSE     (1 << 4)
>>>> +#define ENET_TCR_ADDSEL_SHIFT  (5)
>>>> +#define ENET_TCR_ADDSEL_LENGTH (3)
>>>> +#define ENET_TCR_CRCFWD        (1 << 9)
>>>> +
>>>> +/* RDSR */
>>>> +#define ENET_TWFR_TFWR_SHIFT   (0)
>>>> +#define ENET_TWFR_TFWR_LENGTH  (6)
>>>> +#define ENET_TWFR_STRFWD       (1 << 8)
>>>>     /* Buffer Descriptor.  */
>>>>   typedef struct {
>>>> @@ -89,22 +174,60 @@ typedef struct {
>>>>       uint32_t data;
>>>>   } IMXFECBufDesc;
>>>>   -#define FEC_BD_R    (1 << 15)
>>>> -#define FEC_BD_E    (1 << 15)
>>>> -#define FEC_BD_O1   (1 << 14)
>>>> -#define FEC_BD_W    (1 << 13)
>>>> -#define FEC_BD_O2   (1 << 12)
>>>> -#define FEC_BD_L    (1 << 11)
>>>> -#define FEC_BD_TC   (1 << 10)
>>>> -#define FEC_BD_ABC  (1 << 9)
>>>> -#define FEC_BD_M    (1 << 8)
>>>> -#define FEC_BD_BC   (1 << 7)
>>>> -#define FEC_BD_MC   (1 << 6)
>>>> -#define FEC_BD_LG   (1 << 5)
>>>> -#define FEC_BD_NO   (1 << 4)
>>>> -#define FEC_BD_CR   (1 << 2)
>>>> -#define FEC_BD_OV   (1 << 1)
>>>> -#define FEC_BD_TR   (1 << 0)
>>>> +#define ENET_BD_R              (1 << 15)
>>>> +#define ENET_BD_E              (1 << 15)
>>>> +#define ENET_BD_O1             (1 << 14)
>>>> +#define ENET_BD_W              (1 << 13)
>>>> +#define ENET_BD_O2             (1 << 12)
>>>> +#define ENET_BD_L              (1 << 11)
>>>> +#define ENET_BD_TC             (1 << 10)
>>>> +#define ENET_BD_ABC            (1 << 9)
>>>> +#define ENET_BD_M              (1 << 8)
>>>> +#define ENET_BD_BC             (1 << 7)
>>>> +#define ENET_BD_MC             (1 << 6)
>>>> +#define ENET_BD_LG             (1 << 5)
>>>> +#define ENET_BD_NO             (1 << 4)
>>>> +#define ENET_BD_CR             (1 << 2)
>>>> +#define ENET_BD_OV             (1 << 1)
>>>> +#define ENET_BD_TR             (1 << 0)
>>>> +
>>>> +typedef struct {
>>>> +    uint16_t length;
>>>> +    uint16_t flags;
>>>> +    uint32_t data;
>>>> +    uint16_t status;
>>>> +    uint16_t option;
>>>> +    uint16_t checksum;
>>>> +    uint16_t head_proto;
>>>> +    uint32_t last_buffer;
>>>> +    uint32_t timestamp;
>>>> +    uint32_t reserved[2];
>>>> +} IMXENETBufDesc;
>>>> +
>>>> +#define ENET_BD_ME             (1 << 15)
>>>> +#define ENET_BD_TX_INT         (1 << 14)
>>>> +#define ENET_BD_TS             (1 << 13)
>>>> +#define ENET_BD_PINS           (1 << 12)
>>>> +#define ENET_BD_IINS           (1 << 11)
>>>> +#define ENET_BD_PE             (1 << 10)
>>>> +#define ENET_BD_CE             (1 << 9)
>>>> +#define ENET_BD_UC             (1 << 8)
>>>> +#define ENET_BD_RX_INT         (1 << 7)
>>>> +
>>>> +#define ENET_BD_TXE            (1 << 15)
>>>> +#define ENET_BD_UE             (1 << 13)
>>>> +#define ENET_BD_EE             (1 << 12)
>>>> +#define ENET_BD_FE             (1 << 11)
>>>> +#define ENET_BD_LCE            (1 << 10)
>>>> +#define ENET_BD_OE             (1 << 9)
>>>> +#define ENET_BD_TSE            (1 << 8)
>>>> +#define ENET_BD_ICE            (1 << 5)
>>>> +#define ENET_BD_PCR            (1 << 4)
>>>> +#define ENET_BD_VLAN           (1 << 2)
>>>> +#define ENET_BD_IPV6           (1 << 1)
>>>> +#define ENET_BD_FRAG           (1 << 0)
>>>> +
>>>> +#define ENET_BD_BDU            (1 << 31)
>>>>     typedef struct IMXFECState {
>>>>       /*< private >*/
>>>> @@ -113,7 +236,7 @@ typedef struct IMXFECState {
>>>>       /*< public >*/
>>>>       NICState *nic;
>>>>       NICConf conf;
>>>> -    qemu_irq irq;
>>>> +    qemu_irq irq[2];
>>>>       MemoryRegion iomem;
>>>>         uint32_t regs[ENET_MAX];
>>>> @@ -125,6 +248,8 @@ typedef struct IMXFECState {
>>>>       uint32_t phy_advertise;
>>>>       uint32_t phy_int;
>>>>       uint32_t phy_int_mask;
>>>> +
>>>> +    bool is_fec;
>>>>   } IMXFECState;
>>>>     #endif
>>>
>>>
>>
>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH v4 7/8] Add ENET/Gbps Ethernet support to FEC device
  2016-05-19 18:37       ` Peter Maydell
@ 2016-05-20 21:24         ` Jean-Christophe DUBOIS
  0 siblings, 0 replies; 22+ messages in thread
From: Jean-Christophe DUBOIS @ 2016-05-20 21:24 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Jason Wang, QEMU Developers

Le 19/05/2016 20:37, Peter Maydell a écrit :
> On 19 May 2016 at 19:14, Jean-Christophe DUBOIS <jcd@tribudubois.net> wrote:
>> Le 19/05/2016 05:48, Jason Wang a écrit :
>>> On 2016年05月19日 06:23, Jean-Christophe Dubois wrote:
>>>
>>> It's ok to decide with "is-fec", but is it better to use a new type for
>>> that?
>>
>> Well, there is a lot of common code between FEC and ENET because ENET is
>> basically backward compatible with FEC. So most/all of the FEC code needs to
>> go in the ENET device.
>>
>> I thought this way of doing thing was the best way to avoid duplicating
>> things.
> Right, but usually we have different device types, even if under
> the hood it's the same code using a bool to decide what to do
> (see for instance hw/display/pl110.c, which provides 3 devices
> which are all pretty similar). This means that the users of your
> device don't need to care that the FEC is an ENET device with
> a flag to say "be like an FEC device", they just instantiate
> the kind of device they want.

OK, I'll do that.

>
>>> The above renaming seems cause lots of unnecessary changes above which may
>>> brings issue when backporint patches to -stable. Can we just keep this, and
>>> all ENET_*** should be used after we're sure we are ENET?
>>
>> What do you mean by "after we're sure we are ENET"?
>>
>> ENET is the evolution of FEC to support Gb interface. FEC is more like a
>> legacy device these days (for ARM9 family). So it seems we are better
>> supporting ENET as default going forward.
> I don't mind if we rename the functions, #defines, etc, but if
> we do can we have a patch which does that renaming and only that
> renaming, so it's easy to review?

OK, I'll add a patch for renaming.

>
> thanks
> -- PMM
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH v4 6/8] i.MX: move FEC device to a register array structure.
  2016-05-20  2:26       ` Jason Wang
@ 2016-05-20 21:25         ` Jean-Christophe DUBOIS
  0 siblings, 0 replies; 22+ messages in thread
From: Jean-Christophe DUBOIS @ 2016-05-20 21:25 UTC (permalink / raw)
  To: Jason Wang; +Cc: qemu-devel, peter.maydell@linaro.org >> Peter Maydell

Le 20/05/2016 04:26, Jason Wang a écrit :
>
>
> On 2016年05月19日 14:10, Jean-Christophe DUBOIS wrote:
>> Le 19/05/2016 05:28, Jason Wang a écrit :
>>>
>>>
>>> On 2016年05月19日 06:23, Jean-Christophe Dubois wrote:
>>>> This is to prepare for the ENET Gb device of the i.MX6.
>>>>
>>>> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
>>>> ---
>>>>
>>>> Changes since v1:
>>>>   * Not present on v1.
>>>>
>>>> Changes since v2:
>>>>   * The patch was split in 2 parts:
>>>>     - a "port" to a reg array based device (this patch).
>>>>     - the addition of the Gb support (next patch).
>>>>   Changes since v3:
>>>>   * Small fix patches were extracted from this patch (see previous 
>>>> 3 patches)
>>>>   * Reset logic through ECR was fixed.
>>>>   * TDAR/RDAR behavior was fixed.
>>>>
>>>>   hw/net/imx_fec.c         | 396 
>>>> ++++++++++++++++++++++++++---------------------
>>>>   include/hw/net/imx_fec.h |  55 ++++---
>>>>   2 files changed, 258 insertions(+), 193 deletions(-)
>>>>
>>>>
>
> [...]
>
>>>> -    case 0x014: /* TDAR */
>>>> -        if (s->ecr & FEC_EN) {
>>>> +    case ENET_TDAR:
>>>> +        if (s->regs[ENET_ECR] & FEC_EN) {
>>>> +            s->regs[index] = ENET_TDAR_TDAR;
>>>>               imx_fec_do_tx(s);
>>>>           }
>>>> +        s->regs[index] = 0;
>>>>           break;
>>>> -    case 0x024: /* ECR */
>>>> -        s->ecr = value;
>>>> +    case ENET_ECR:
>>>>           if (value & FEC_RESET) {
>>>> -            imx_fec_reset(DEVICE(s));
>>>> +            return imx_fec_reset(DEVICE(s));
>>>>           }
>>>> -        if ((s->ecr & FEC_EN) == 0) {
>>>> -            s->rx_enabled = 0;
>>>> +        s->regs[index] = value;
>>>> +        if ((s->regs[index] & FEC_EN) == 0) {
>>>> +            s->regs[ENET_RDAR] = 0;
>>>> +            s->rx_descriptor = s->regs[ENET_RDSR];
>>>> +            s->regs[ENET_TDAR] = 0;
>>>> +            s->tx_descriptor = s->regs[ENET_TDSR];
>>>
>>> This seems like a new behavior, is this a bug fix? If yes, better 
>>> split.
>>
>> It is a more correct behavior I think. Note however that our guest 
>> OSes (in particular Linux) do not trigger this bit. So it is mostly 
>> untested/unused.
>>
>
> Is this the real hardware or documented behavior? If yes, need a 
> separate patch for this. If not, we'd better not add this.

I'll add a patch.

>
>>>
>>>>           }
>>>>           break;
>>>> -    case 0x040: /* MMFR */
>>>> -        /* store the value */
>>>> -        s->mmfr = value;
>>>> +    case ENET_MMFR:
>>>> +        s->regs[index] = value;
>>>>           if (extract32(value, 29, 1)) {
>>>> -            s->mmfr = do_phy_read(s, extract32(value, 18, 10));
>>>> +            /* This is a read operation */
>>>> +            s->regs[ENET_MMFR] = deposit32(s->regs[ENET_MMFR], 0, 16,
>>>> +                                           do_phy_read(s,
>>>> + extract32(value,
>>>> + 18, 10)));
>>>>           } else {
>>>> +            /* This a write operation */
>>>>               do_phy_write(s, extract32(value, 18, 10), 
>>>> extract32(value, 0, 16));
>>>>           }
>>>>           /* raise the interrupt as the PHY operation is done */
>>>> -        s->eir |= FEC_INT_MII;
>>>> +        s->regs[ENET_EIR] |= FEC_INT_MII;
>>>>           break;
>>>> -    case 0x044: /* MSCR */
>>>> -        s->mscr = value & 0xfe;
>>>> +    case ENET_MSCR:
>>>> +        s->regs[index] = value & 0xfe;
>>>>           break;
>>>> -    case 0x064: /* MIBC */
>>>> +    case ENET_MIBC:
>>>>           /* TODO: Implement MIB.  */
>>>> -        s->mibc = (value & 0x80000000) ? 0xc0000000 : 0;
>>>> +        s->regs[index] = (value & 0x80000000) ? 0xc0000000 : 0;
>>>>           break;
>>>> -    case 0x084: /* RCR */
>>>> -        s->rcr = value & 0x07ff003f;
>>>> +    case ENET_RCR:
>>>> +        s->regs[index] = value & 0x07ff003f;
>>>>           /* TODO: Implement LOOP mode.  */
>>>>           break;
>>>> -    case 0x0c4: /* TCR */
>>>> +    case ENET_TCR:
>>>>           /* We transmit immediately, so raise GRA immediately. */
>>>> -        s->tcr = value;
>>>> +        s->regs[index] = value;
>>>>           if (value & 1) {
>>>> -            s->eir |= FEC_INT_GRA;
>>>> +            s->regs[ENET_EIR] |= FEC_INT_GRA;
>>>>           }
>>>>           break;
>>>> -    case 0x0e4: /* PALR */
>>>> +    case ENET_PALR:
>>>> +        s->regs[index] = value;
>>>>           s->conf.macaddr.a[0] = value >> 24;
>>>>           s->conf.macaddr.a[1] = value >> 16;
>>>>           s->conf.macaddr.a[2] = value >> 8;
>>>>           s->conf.macaddr.a[3] = value;
>>>
>>> I believe we should use stl_be_p() here?
>>
>> I didn't change this bit ...
>>
>
> Sorry, you are right. I misread the patch.
>
>>>
>>>>           break;
>>>> -    case 0x0e8: /* PAUR */
>>>> +    case ENET_PAUR:
>>>> +        s->regs[index] = (value | 0x0000ffff) & 0xffff8808;
>>>>           s->conf.macaddr.a[4] = value >> 24;
>>>>           s->conf.macaddr.a[5] = value >> 16;
>>>>           break;
>>>> -    case 0x0ec: /* OPDR */
>
> [...]
>
>>>> diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h
>>>> index cbf8650..709f8a0 100644
>>>> --- a/include/hw/net/imx_fec.h
>>>> +++ b/include/hw/net/imx_fec.h
>>>> @@ -30,6 +30,33 @@
>>>>   #include "hw/sysbus.h"
>>>>   #include "net/net.h"
>>>>   +#define ENET_EIR               1
>>>> +#define ENET_EIMR              2
>>>> +#define ENET_RDAR              4
>>>> +#define ENET_TDAR              5
>>>> +#define ENET_ECR               9
>>>> +#define ENET_MMFR              16
>>>> +#define ENET_MSCR              17
>>>> +#define ENET_MIBC              25
>>>> +#define ENET_RCR               33
>>>> +#define ENET_TCR               49
>>>> +#define ENET_PALR              57
>>>> +#define ENET_PAUR              58
>>>> +#define ENET_OPD               59
>>>> +#define ENET_IAUR              70
>>>> +#define ENET_IALR              71
>>>> +#define ENET_GAUR              72
>>>> +#define ENET_GALR              73
>>>> +#define ENET_TFWR              81
>>>> +#define ENET_FRBR              83
>>>> +#define ENET_FRSR              84
>>>> +#define ENET_RDSR              96
>>>> +#define ENET_TDSR              97
>>>> +#define ENET_MRBR              98
>>>> +#define ENET_MIIGSK_CFGR       192
>>>> +#define ENET_MIIGSK_ENR        194
>>>> +#define ENET_MAX               400
>>>
>>> Is this an arbitrary value or there's a plan to add more register 
>>> whose index is greater than 192?
>>
>> More registers are coming with the ENET device.
>>
>
> Right, I see.
>
> Thanks
>
>>>
>>>> +
>>>>   #define FEC_MAX_FRAME_SIZE 2032
>>>>     #define FEC_INT_HB      (1 << 31)
>>>> @@ -46,8 +73,14 @@
>>>>   #define FEC_INT_RL      (1 << 20)
>>>>   #define FEC_INT_UN      (1 << 19)
>>>>   -#define FEC_EN      2
>>>> -#define FEC_RESET   1
>>>> +/* RDAR */
>>>> +#define ENET_RDAR_RDAR         (1 << 24)
>>>> +
>>>> +/* TDAR */
>>>> +#define ENET_TDAR_TDAR         (1 << 24)
>>>> +
>>>> +#define FEC_EN                 (1 << 1)
>>>> +#define FEC_RESET              (1 << 0)
>>>>     /* Buffer Descriptor.  */
>>>>   typedef struct {
>>>> @@ -83,25 +116,9 @@ typedef struct IMXFECState {
>>>>       qemu_irq irq;
>>>>       MemoryRegion iomem;
>>>>   -    uint32_t irq_state;
>>>> -    uint32_t eir;
>>>> -    uint32_t eimr;
>>>> -    uint32_t rx_enabled;
>>>> +    uint32_t regs[ENET_MAX];
>>>>       uint32_t rx_descriptor;
>>>>       uint32_t tx_descriptor;
>>>> -    uint32_t ecr;
>>>> -    uint32_t mmfr;
>>>> -    uint32_t mscr;
>>>> -    uint32_t mibc;
>>>> -    uint32_t rcr;
>>>> -    uint32_t tcr;
>>>> -    uint32_t tfwr;
>>>> -    uint32_t frsr;
>>>> -    uint32_t erdsr;
>>>> -    uint32_t etdsr;
>>>> -    uint32_t emrbr;
>>>> -    uint32_t miigsk_cfgr;
>>>> -    uint32_t miigsk_enr;
>>>>         uint32_t phy_status;
>>>>       uint32_t phy_control;
>>>
>>>
>>
>>
>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH v4 7/8] Add ENET/Gbps Ethernet support to FEC device
  2016-05-20 21:23         ` Jean-Christophe DUBOIS
@ 2016-05-24  2:16           ` Jason Wang
  2016-05-24 19:33             ` Jean-Christophe DUBOIS
  0 siblings, 1 reply; 22+ messages in thread
From: Jason Wang @ 2016-05-24  2:16 UTC (permalink / raw)
  To: Jean-Christophe DUBOIS, qemu-devel, peter.maydell



On 2016年05月21日 05:23, Jean-Christophe DUBOIS wrote:
> Le 20/05/2016 04:34, Jason Wang a écrit :
>>
>>
>> On 2016年05月20日 02:14, Jean-Christophe DUBOIS wrote:
>>> Le 19/05/2016 05:48, Jason Wang a écrit :
>>>>
>>>>
>>>> On 2016年05月19日 06:23, Jean-Christophe Dubois wrote:
>>>>> The ENET device (present in i.MX6) is "derived" from FEC and backward
>>>>> compatible with it.
>>>>>
>>>>> This patch adds the necessary support of the added feature in the 
>>>>> ENET
>>>>> device to allow Linux to use it (on supported processors).
>>>>>
>>>>> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
>>>>> ---
>>>>>
>>>>> Changes since v1:
>>>>>   * Not present on v1
>>>>>     Changes since v2:
>>>>>   * Not present on v2
>>>>>   Changes since v3:
>>>>>   * Separate and fix the 2 supported interrupts
>>>>>
>>>>>   hw/arm/fsl-imx25.c       |   3 +
>>>>>   hw/net/imx_fec.c         | 713 
>>>>> ++++++++++++++++++++++++++++++++++++++---------
>>>>>   include/hw/net/imx_fec.h | 195 ++++++++++---
>>>>>   3 files changed, 745 insertions(+), 166 deletions(-)
>>>>
>>>> [...]
>>>>
>>>>>   -static Property imx_fec_properties[] = {
>>>>> +static Property imx_eth_properties[] = {
>>>>>       DEFINE_NIC_PROPERTIES(IMXFECState, conf),
>>>>> +    DEFINE_PROP_BOOL("is-fec", IMXFECState, is_fec, false),
>>>>>       DEFINE_PROP_END_OF_LIST(),
>>>>>   };
>>>>
>>>> It's ok to decide with "is-fec", but is it better to use a new type 
>>>> for that?
>>>
>>> Well, there is a lot of common code between FEC and ENET because 
>>> ENET is basically backward compatible with FEC. So most/all of the 
>>> FEC code needs to go in the ENET device.
>>>
>>> I thought this way of doing thing was the best way to avoid 
>>> duplicating things.
>>
>> You can still share almost all the codes. E.g you can have a look at 
>> e1000.c which have 3 types of rather similar devices.
>
> OK, I'll change it in next patch to match the pl110 model as proposed 
> by Peter. No more additional property...
>
>>
>> Another question not relate to this patch, I saw version were bumped 
>> for vmstate version. Is it better to keep the vmstate for FEC and 
>> using a new vmstate for ENET (register array).
>
> Well, as I moved the FEC to register array, the VMState structure has 
> changed and therefore I believe the vmstate version needs to be bumped.
>
> Am I wrong? 

You're right. But migration to old version does not work in this way, 
even if there aren't any changes in FEC.

I'm not sure the policy for arm, but we used to do lots of works in the 
past to make migration to older version works.

You can achieve this by using two different vmstates, and keep the FEC's 
state (by just use parts of array).

Perter, any thoughts on this? Should we try to keep the migration 
compatibility here?

Thanks

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH v4 7/8] Add ENET/Gbps Ethernet support to FEC device
  2016-05-24  2:16           ` Jason Wang
@ 2016-05-24 19:33             ` Jean-Christophe DUBOIS
  2016-05-25  8:14               ` Jason Wang
  0 siblings, 1 reply; 22+ messages in thread
From: Jean-Christophe DUBOIS @ 2016-05-24 19:33 UTC (permalink / raw)
  To: Jason Wang, qemu-devel, peter.maydell

Le 24/05/2016 04:16, Jason Wang a écrit :
>
>
> On 2016年05月21日 05:23, Jean-Christophe DUBOIS wrote:
>> Le 20/05/2016 04:34, Jason Wang a écrit :
>>>
>>>
>>> On 2016年05月20日 02:14, Jean-Christophe DUBOIS wrote:
>>>> Le 19/05/2016 05:48, Jason Wang a écrit :
>>>>>
>>>>>
>>>>> On 2016年05月19日 06:23, Jean-Christophe Dubois wrote:
>>>>>> The ENET device (present in i.MX6) is "derived" from FEC and 
>>>>>> backward
>>>>>> compatible with it.
>>>>>>
>>>>>> This patch adds the necessary support of the added feature in the 
>>>>>> ENET
>>>>>> device to allow Linux to use it (on supported processors).
>>>>>>
>>>>>> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
>>>>>> ---
>>>>>>
>>>>>> Changes since v1:
>>>>>>   * Not present on v1
>>>>>>     Changes since v2:
>>>>>>   * Not present on v2
>>>>>>   Changes since v3:
>>>>>>   * Separate and fix the 2 supported interrupts
>>>>>>
>>>>>>   hw/arm/fsl-imx25.c       |   3 +
>>>>>>   hw/net/imx_fec.c         | 713 
>>>>>> ++++++++++++++++++++++++++++++++++++++---------
>>>>>>   include/hw/net/imx_fec.h | 195 ++++++++++---
>>>>>>   3 files changed, 745 insertions(+), 166 deletions(-)
>>>>>
>>>>> [...]
>>>>>
>>>>>>   -static Property imx_fec_properties[] = {
>>>>>> +static Property imx_eth_properties[] = {
>>>>>>       DEFINE_NIC_PROPERTIES(IMXFECState, conf),
>>>>>> +    DEFINE_PROP_BOOL("is-fec", IMXFECState, is_fec, false),
>>>>>>       DEFINE_PROP_END_OF_LIST(),
>>>>>>   };
>>>>>
>>>>> It's ok to decide with "is-fec", but is it better to use a new 
>>>>> type for that?
>>>>
>>>> Well, there is a lot of common code between FEC and ENET because 
>>>> ENET is basically backward compatible with FEC. So most/all of the 
>>>> FEC code needs to go in the ENET device.
>>>>
>>>> I thought this way of doing thing was the best way to avoid 
>>>> duplicating things.
>>>
>>> You can still share almost all the codes. E.g you can have a look at 
>>> e1000.c which have 3 types of rather similar devices.
>>
>> OK, I'll change it in next patch to match the pl110 model as proposed 
>> by Peter. No more additional property...
>>
>>>
>>> Another question not relate to this patch, I saw version were bumped 
>>> for vmstate version. Is it better to keep the vmstate for FEC and 
>>> using a new vmstate for ENET (register array).
>>
>> Well, as I moved the FEC to register array, the VMState structure has 
>> changed and therefore I believe the vmstate version needs to be bumped.
>>
>> Am I wrong? 
>
> You're right. But migration to old version does not work in this way, 
> even if there aren't any changes in FEC.
>
> I'm not sure the policy for arm, but we used to do lots of works in 
> the past to make migration to older version works.
>
> You can achieve this by using two different vmstates, and keep the 
> FEC's state (by just use parts of array).
>
> Perter, any thoughts on this? Should we try to keep the migration 
> compatibility here?

I was thinking the migration feature was to migrate old VMSTATE (from 
previous version of Qemu) to the present (or more modern) version.

I certainly understand the intent and it does make a lot of sense when 
you are migrating live guest image like for example when using KVM 
and/or Xen for x86.

Now I am not sure it is worth the trouble in this case. FEC is only used 
by the i.MX25 PDK board (for now) and I would be surprised if anybody 
had build a guest image using this platform and that they have the need 
to migrate this guest from one Qemu version to another.

I am not sure KVM/Xen for ARM is mainstream enough yet and in any case 
it would be surprising they used i.MX25 PDK as an emulated guest. 
Hopefully it might change someday ...

I might be wrong and you could have another opinion ...

JC

>
> Thanks
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH v4 7/8] Add ENET/Gbps Ethernet support to FEC device
  2016-05-24 19:33             ` Jean-Christophe DUBOIS
@ 2016-05-25  8:14               ` Jason Wang
  0 siblings, 0 replies; 22+ messages in thread
From: Jason Wang @ 2016-05-25  8:14 UTC (permalink / raw)
  To: Jean-Christophe DUBOIS, qemu-devel, peter.maydell



On 2016年05月25日 03:33, Jean-Christophe DUBOIS wrote:
> Le 24/05/2016 04:16, Jason Wang a écrit :
>>
>>
>> On 2016年05月21日 05:23, Jean-Christophe DUBOIS wrote:
>>> Le 20/05/2016 04:34, Jason Wang a écrit :
>>>>
>>>>
>>>> On 2016年05月20日 02:14, Jean-Christophe DUBOIS wrote:
>>>>> Le 19/05/2016 05:48, Jason Wang a écrit :
>>>>>>
>>>>>>
>>>>>> On 2016年05月19日 06:23, Jean-Christophe Dubois wrote:
>>>>>>> The ENET device (present in i.MX6) is "derived" from FEC and 
>>>>>>> backward
>>>>>>> compatible with it.
>>>>>>>
>>>>>>> This patch adds the necessary support of the added feature in 
>>>>>>> the ENET
>>>>>>> device to allow Linux to use it (on supported processors).
>>>>>>>
>>>>>>> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
>>>>>>> ---
>>>>>>>
>>>>>>> Changes since v1:
>>>>>>>   * Not present on v1
>>>>>>>     Changes since v2:
>>>>>>>   * Not present on v2
>>>>>>>   Changes since v3:
>>>>>>>   * Separate and fix the 2 supported interrupts
>>>>>>>
>>>>>>>   hw/arm/fsl-imx25.c       |   3 +
>>>>>>>   hw/net/imx_fec.c         | 713 
>>>>>>> ++++++++++++++++++++++++++++++++++++++---------
>>>>>>>   include/hw/net/imx_fec.h | 195 ++++++++++---
>>>>>>>   3 files changed, 745 insertions(+), 166 deletions(-)
>>>>>>
>>>>>> [...]
>>>>>>
>>>>>>> -static Property imx_fec_properties[] = {
>>>>>>> +static Property imx_eth_properties[] = {
>>>>>>>       DEFINE_NIC_PROPERTIES(IMXFECState, conf),
>>>>>>> +    DEFINE_PROP_BOOL("is-fec", IMXFECState, is_fec, false),
>>>>>>>       DEFINE_PROP_END_OF_LIST(),
>>>>>>>   };
>>>>>>
>>>>>> It's ok to decide with "is-fec", but is it better to use a new 
>>>>>> type for that?
>>>>>
>>>>> Well, there is a lot of common code between FEC and ENET because 
>>>>> ENET is basically backward compatible with FEC. So most/all of the 
>>>>> FEC code needs to go in the ENET device.
>>>>>
>>>>> I thought this way of doing thing was the best way to avoid 
>>>>> duplicating things.
>>>>
>>>> You can still share almost all the codes. E.g you can have a look 
>>>> at e1000.c which have 3 types of rather similar devices.
>>>
>>> OK, I'll change it in next patch to match the pl110 model as 
>>> proposed by Peter. No more additional property...
>>>
>>>>
>>>> Another question not relate to this patch, I saw version were 
>>>> bumped for vmstate version. Is it better to keep the vmstate for 
>>>> FEC and using a new vmstate for ENET (register array).
>>>
>>> Well, as I moved the FEC to register array, the VMState structure 
>>> has changed and therefore I believe the vmstate version needs to be 
>>> bumped.
>>>
>>> Am I wrong? 
>>
>> You're right. But migration to old version does not work in this way, 
>> even if there aren't any changes in FEC.
>>
>> I'm not sure the policy for arm, but we used to do lots of works in 
>> the past to make migration to older version works.
>>
>> You can achieve this by using two different vmstates, and keep the 
>> FEC's state (by just use parts of array).
>>
>> Perter, any thoughts on this? Should we try to keep the migration 
>> compatibility here?
>
> I was thinking the migration feature was to migrate old VMSTATE (from 
> previous version of Qemu) to the present (or more modern) version.
>
> I certainly understand the intent and it does make a lot of sense when 
> you are migrating live guest image like for example when using KVM 
> and/or Xen for x86.
>
> Now I am not sure it is worth the trouble in this case. FEC is only 
> used by the i.MX25 PDK board (for now) and I would be surprised if 
> anybody had build a guest image using this platform and that they have 
> the need to migrate this guest from one Qemu version to another.
>
> I am not sure KVM/Xen for ARM is mainstream enough yet and in any case 
> it would be surprising they used i.MX25 PDK as an emulated guest. 
> Hopefully it might change someday ...
>
> I might be wrong and you could have another opinion ...
>
> JC 

I get the points, then I think there's no need for doing the 
compatibility stuffs.

Thanks

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2016-05-25  8:14 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-18 22:22 [Qemu-devel] [PATCH v4 0/8] Add Ethernet device for i.MX6 SOC Jean-Christophe Dubois
2016-05-18 22:22 ` [Qemu-devel] [PATCH v4 1/8] net: improve UDP/TCP checksum computation Jean-Christophe Dubois
2016-05-18 22:22 ` [Qemu-devel] [PATCH v4 2/8] net: handle optional VLAN header in " Jean-Christophe Dubois
2016-05-18 22:22 ` [Qemu-devel] [PATCH v4 3/8] i.MX: Fix FEC code for MDIO operation selection Jean-Christophe Dubois
2016-05-18 22:22 ` [Qemu-devel] [PATCH v4 4/8] i.MX: Fix FEC code for MDIO address selection Jean-Christophe Dubois
2016-05-18 22:23 ` [Qemu-devel] [PATCH v4 5/8] i.MX: Fix FEC code for ECR register reset value Jean-Christophe Dubois
2016-05-18 22:23 ` [Qemu-devel] [PATCH v4 6/8] i.MX: move FEC device to a register array structure Jean-Christophe Dubois
2016-05-19  3:28   ` Jason Wang
2016-05-19  6:10     ` Jean-Christophe DUBOIS
2016-05-20  2:26       ` Jason Wang
2016-05-20 21:25         ` Jean-Christophe DUBOIS
2016-05-18 22:23 ` [Qemu-devel] [PATCH v4 7/8] Add ENET/Gbps Ethernet support to FEC device Jean-Christophe Dubois
2016-05-19  3:48   ` Jason Wang
2016-05-19 18:14     ` Jean-Christophe DUBOIS
2016-05-19 18:37       ` Peter Maydell
2016-05-20 21:24         ` Jean-Christophe DUBOIS
2016-05-20  2:34       ` Jason Wang
2016-05-20 21:23         ` Jean-Christophe DUBOIS
2016-05-24  2:16           ` Jason Wang
2016-05-24 19:33             ` Jean-Christophe DUBOIS
2016-05-25  8:14               ` Jason Wang
2016-05-18 22:23 ` [Qemu-devel] [PATCH v4 8/8] Add ENET device to i.MX6 SOC Jean-Christophe Dubois

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.