From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53047) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b3aH3-0007tb-N3 for qemu-devel@nongnu.org; Thu, 19 May 2016 22:34:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b3aGx-0001KN-UZ for qemu-devel@nongnu.org; Thu, 19 May 2016 22:34:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50502) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b3aGx-0001KH-ML for qemu-devel@nongnu.org; Thu, 19 May 2016 22:34:43 -0400 References: <573D3799.9090109@redhat.com> <573E0276.8040002@tribudubois.net> From: Jason Wang Message-ID: <573E77BE.4090609@redhat.com> Date: Fri, 20 May 2016 10:34:38 +0800 MIME-Version: 1.0 In-Reply-To: <573E0276.8040002@tribudubois.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 7/8] Add ENET/Gbps Ethernet support to FEC device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jean-Christophe DUBOIS , qemu-devel@nongnu.org, peter.maydell@linaro.org On 2016=E5=B9=B405=E6=9C=8820=E6=97=A5 02:14, Jean-Christophe DUBOIS wrot= e: > Le 19/05/2016 05:48, Jason Wang a =C3=A9crit : >> >> >> On 2016=E5=B9=B405=E6=9C=8819=E6=97=A5 06:23, Jean-Christophe Dubois w= rote: >>> 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 ENE= T >>> device to allow Linux to use it (on supported processors). >>> >>> Signed-off-by: Jean-Christophe Dubois >>> --- >>> >>> 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=20 >>> ++++++++++++++++++++++++++++++++++++++--------- >>> include/hw/net/imx_fec.h | 195 ++++++++++--- >>> 3 files changed, 745 insertions(+), 166 deletions(-) >> >> [...] >> >>> -static Property imx_fec_properties[] =3D { >>> +static Property imx_eth_properties[] =3D { >>> 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=20 >> for that? > > Well, there is a lot of common code between FEC and ENET because ENET=20 > is basically backward compatible with FEC. So most/all of the FEC code=20 > needs to go in the ENET device. > > I thought this way of doing thing was the best way to avoid=20 > duplicating things. You can still share almost all the codes. E.g you can have a look at=20 e1000.c which have 3 types of rather similar devices. Another question not relate to this patch, I saw version were bumped for=20 vmstate version. Is it better to keep the vmstate for FEC and using a=20 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 =3D DEVICE_CLASS(klass); >>> - dc->vmsd =3D &vmstate_imx_fec; >>> - dc->reset =3D imx_fec_reset; >>> - dc->props =3D imx_fec_properties; >>> - dc->realize =3D imx_fec_realize; >>> - dc->desc =3D "i.MX FEC Ethernet Controller"; >>> + dc->vmsd =3D &vmstate_imx_eth; >>> + dc->reset =3D imx_eth_reset; >>> + dc->props =3D imx_eth_properties; >>> + dc->realize =3D imx_eth_realize; >>> + dc->desc =3D "i.MX FEC/ENET Ethernet Controller"; >>> } >>> -static const TypeInfo imx_fec_info =3D { >>> - .name =3D TYPE_IMX_FEC, >>> - .parent =3D TYPE_SYS_BUS_DEVICE, >>> +static const TypeInfo imx_eth_info =3D { >>> + .name =3D TYPE_IMX_FEC, >>> + .parent =3D TYPE_SYS_BUS_DEVICE, >>> .instance_size =3D sizeof(IMXFECState), >>> - .class_init =3D imx_fec_class_init, >>> + .class_init =3D 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. >>> * >>> @@ -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=20 >> which may brings issue when backporint patches to -stable. Can we=20 >> just keep this, and all ENET_*** should be used after we're sure we=20 >> 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=20 > a legacy device these days (for ARM9 family). So it seems we are=20 > 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 |=20 >>> ENET_INT_BABT | \ >>> + ENET_INT_GRA | ENET_INT_TXF |=20 >>> ENET_INT_TXB | \ >>> + ENET_INT_RXF | ENET_INT_RXB |=20 >>> ENET_INT_MII | \ >>> + ENET_INT_EBERR | ENET_INT_LC |=20 >>> ENET_INT_RL | \ >>> + ENET_INT_UN | ENET_INT_PLR |=20 >>> 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 >> >> >