Re: [Qemu-devel] [PATCH v4 7/8] Add ENET/Gbps Ethernet support to FEC device

[Jason Wang <jasowang@redhat.com>]

On 2016年05月19日 06:23, Jean-Christophe Dubois wrote:
> 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 ENET
> device to allow Linux to use it (on supported processors).
>
> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
> ---
>
> 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 ++++++++++++++++++++++++++++++++++++++---------
>   include/hw/net/imx_fec.h | 195 ++++++++++---
>   3 files changed, 745 insertions(+), 166 deletions(-)

[...]

>   
> -static Property imx_fec_properties[] = {
> +static Property imx_eth_properties[] = {
>       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 for 
that?

>   
> -static void imx_fec_class_init(ObjectClass *klass, void *data)
> +static void imx_eth_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
>   
> -    dc->vmsd = &vmstate_imx_fec;
> -    dc->reset = imx_fec_reset;
> -    dc->props = imx_fec_properties;
> -    dc->realize = imx_fec_realize;
> -    dc->desc = "i.MX FEC Ethernet Controller";
> +    dc->vmsd    = &vmstate_imx_eth;
> +    dc->reset   = imx_eth_reset;
> +    dc->props   = imx_eth_properties;
> +    dc->realize = imx_eth_realize;
> +    dc->desc    = "i.MX FEC/ENET Ethernet Controller";
>   }
>   
> -static const TypeInfo imx_fec_info = {
> -    .name = TYPE_IMX_FEC,
> -    .parent = TYPE_SYS_BUS_DEVICE,
> +static const TypeInfo imx_eth_info = {
> +    .name          = TYPE_IMX_FEC,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
>       .instance_size = sizeof(IMXFECState),
> -    .class_init = imx_fec_class_init,
> +    .class_init    = 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. <jcd@tribudubois.net>
>    *
> @@ -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 which 
may brings issue when backporint patches to -stable. Can we just keep 
this, and all ENET_*** should be used after we're sure we are ENET?

> +#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 | ENET_INT_BABT | \
> +                                ENET_INT_GRA | ENET_INT_TXF | ENET_INT_TXB | \
> +                                ENET_INT_RXF | ENET_INT_RXB | ENET_INT_MII | \
> +                                ENET_INT_EBERR | ENET_INT_LC | ENET_INT_RL | \
> +                                ENET_INT_UN | ENET_INT_PLR | 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