qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v3 3/5] i.MX: move FEC device to a register arra


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH v3 3/5] i.MX: move FEC device to a register array structure.
Date: Tue, 17 May 2016 13:47:03 +0100

On 8 May 2016 at 20:41, Jean-Christophe Dubois <address@hidden> 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).
>
>  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



reply via email to

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