|
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 400Is 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;
[Prev in Thread] | Current Thread | [Next in Thread] |