qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 6/8] i.MX: move FEC device to a register arra


From: Jean-Christophe DUBOIS
Subject: Re: [Qemu-devel] [PATCH v4 6/8] i.MX: move FEC device to a register array structure.
Date: Fri, 20 May 2016 23:25:46 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2

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 <address@hidden>
---

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;










reply via email to

[Prev in Thread] Current Thread [Next in Thread]