diff mbox

[v3,3/5] i.MX: move FEC device to a register array structure.

Message ID 68f630072da1364700f6957dc1ef14adf4db0326.1462736067.git.jcd@tribudubois.net
State New
Headers show

Commit Message

Jean-Christophe DUBOIS May 8, 2016, 7:41 p.m. UTC
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).

 hw/net/imx_fec.c         | 391 ++++++++++++++++++++++++++---------------------
 include/hw/net/imx_fec.h |  55 ++++---
 2 files changed, 252 insertions(+), 194 deletions(-)

Comments

Peter Maydell May 17, 2016, 12:47 p.m. UTC | #1
On 8 May 2016 at 20:41, Jean-Christophe Dubois <jcd@tribudubois.net> 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).
>
>  hw/net/imx_fec.c         | 391 ++++++++++++++++++++++++++---------------------
>  include/hw/net/imx_fec.h |  55 ++++---
>  2 files changed, 252 insertions(+), 194 deletions(-)

Thanks for splitting this patch out, it made it a lot easier
to review. I found a few places below which introduced a
change of behaviour but otherwise it looks good.

> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
> index e60e338..20e6524 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("interupt raised\n");
> +        qemu_set_irq(s->irq, 1);
> +    } else {
> +        FEC_PRINTF("interupt lowered\n");

"interrupt"

> +        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,23 @@ 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 = 0;
> -    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;

We used to reset this to 0. If this is a bug fix it should be
split out into its own patch.

> +    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;
>
>      /* We also reset the PHY */
>      phy_reset(s);
> @@ -354,183 +402,176 @@ 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) {
> +    case ENET_RDAR:
> +        value = ENET_RDAR_RDAR;
> +        s->regs[index] = value;

Writing to this now unconditionally results in a subsequent
read reading the RDAR bit as 1; it used to only do so if
imx_fec_enable_rx() was called and the right flag value was
read from the rx descriptor.

If this is a bug fix it should go in a separate patch.

> +        if ((s->regs[ENET_ECR] & FEC_EN) && !s->regs[ENET_RDAR]) {
>              imx_fec_enable_rx(s);
>          }
>          break;
> -    case 0x014: /* TDAR */
> -        if (s->ecr & FEC_EN) {
> +    case ENET_TDAR:
> +        /* TDAR bit is always writen to 1 whatever the value writen to reg */

"written"

> +        value = ENET_TDAR_TDAR;
> +        s->regs[index] = value;

This means that TDAR will subsequently read with the TDAR bit set to 1,
when it used to always read as zero. If this is a bug fix it should
be in its own patch.

> +        if (s->regs[ENET_ECR] & FEC_EN) {
>              imx_fec_do_tx(s);
>          }
>          break;
> -    case 0x024: /* ECR */
> -        s->ecr = value;
> +    case ENET_ECR:
>          if (value & FEC_RESET) {
> -            imx_fec_reset(DEVICE(s));
> +            value &= ~FEC_EN;
> +            s->regs[ENET_RDAR] = 0;
> +            s->regs[ENET_TDAR] = 0;
>          }
> -        if ((s->ecr & FEC_EN) == 0) {
> -            s->rx_enabled = 0;
> +        s->regs[index] = value;

This now results in a subsequent read getting a value with the
FEC_RESET bit set, which didn't happen in the previous code.
If this is a bug fix then it should be in a separate patch, but I
suspect it's a bug in this conversion patch.

> +        if ((s->regs[ENET_ECR] & FEC_EN) == 0) {
> +            s->regs[ENET_RDAR] = 0;
>          }
>          break;
> -    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));
> +    case ENET_MMFR:
> +        s->regs[index] = value;
> +        if (extract32(value, 29, 1)) {

Previous code was testing bit 28, not 29. If this is a bug fix
it should be in a separate patch.

> +            /* This a read operation */

"This is a".

> +            s->regs[ENET_MMFR] &= 0xffff0000;
> +            s->regs[ENET_MMFR] |= 0x0000ffff &
> +                                  do_phy_read(s, extract32(value, 18, 10));

Old code set all of mmfr based on the do_phy_read() call.
Old code used a bit field of size 9, not 10.

(Using deposit32 will be clearer than manual bit operations I think.)

>          } else {
> -            s->mmfr = do_phy_read(s, extract32(value, 18, 9));
> +            /* This a write operation */
> +            do_phy_write(s, extract32(value, 18, 10), extract32(value, 0, 16));

Old code used a size of 9, not 10.

>          }
>          /* 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 & 0x00000003;

This seems like an odd way to write "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 +582,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 +600,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 +618,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 +636,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 +659,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
>

thanks
-- PMM
diff mbox

Patch

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index e60e338..20e6524 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("interupt raised\n");
+        qemu_set_irq(s->irq, 1);
+    } else {
+        FEC_PRINTF("interupt 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,23 @@  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 = 0;
-    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;
 
     /* We also reset the PHY */
     phy_reset(s);
@@ -354,183 +402,176 @@  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) {
+    case ENET_RDAR:
+        value = ENET_RDAR_RDAR;
+        s->regs[index] = value;
+        if ((s->regs[ENET_ECR] & FEC_EN) && !s->regs[ENET_RDAR]) {
             imx_fec_enable_rx(s);
         }
         break;
-    case 0x014: /* TDAR */
-        if (s->ecr & FEC_EN) {
+    case ENET_TDAR:
+        /* TDAR bit is always writen to 1 whatever the value writen to reg */
+        value = ENET_TDAR_TDAR;
+        s->regs[index] = value;
+        if (s->regs[ENET_ECR] & FEC_EN) {
             imx_fec_do_tx(s);
         }
         break;
-    case 0x024: /* ECR */
-        s->ecr = value;
+    case ENET_ECR:
         if (value & FEC_RESET) {
-            imx_fec_reset(DEVICE(s));
+            value &= ~FEC_EN;
+            s->regs[ENET_RDAR] = 0;
+            s->regs[ENET_TDAR] = 0;
         }
-        if ((s->ecr & FEC_EN) == 0) {
-            s->rx_enabled = 0;
+        s->regs[index] = value;
+        if ((s->regs[ENET_ECR] & FEC_EN) == 0) {
+            s->regs[ENET_RDAR] = 0;
         }
         break;
-    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));
+    case ENET_MMFR:
+        s->regs[index] = value;
+        if (extract32(value, 29, 1)) {
+            /* This a read operation */
+            s->regs[ENET_MMFR] &= 0xffff0000;
+            s->regs[ENET_MMFR] |= 0x0000ffff &
+                                  do_phy_read(s, extract32(value, 18, 10));
         } else {
-            s->mmfr = do_phy_read(s, extract32(value, 18, 9));
+            /* 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 & 0x00000003;
         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 +582,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 +600,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 +618,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 +636,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 +659,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;