On Thu, Jan 02, 2014 at 08:25:10PM +1000, Peter Crosthwaite wrote: > > +#undef AW_EMAC_DEBUG > > + > > +#ifdef AW_EMAC_DEBUG > > +#define debug(...) \ > > + do { \ > > + fprintf(stderr, "allwinner_emac : %s: ", __func__); \ > > + fprintf(stderr, ## __VA_ARGS__); \ > > + } while (0) > > +#else > > +#define debug(...) do {} while (0) > > +#endif > > For debug macros, its better to use a regular if (DEBUG_SYMBOL) so the > body of the macro always gets compile tested. We have had incidences > where major change patterns break stripped debug instrumentation > because the code is compiled out.
Ok. > > > + > > +static void mii_set_link(AwEmacMii *mii, bool link_ok) > > +{ > > + if (link_ok) { > > + mii->bmsr |= MII_BMSR_LINK_ST; > > + mii->anlpar |= MII_ANAR_TXFD | MII_ANAR_TX | MII_ANAR_10FD | > > + MII_ANAR_10 | MII_ANAR_CSMACD; > > + } else { > > + mii->bmsr &= ~MII_BMSR_LINK_ST; > > + mii->anlpar = MII_ANAR_TX; > > + } > > + mii->link_ok = link_ok; > > +} > > + > > +static void mii_reset(AwEmacMii *mii) > > +{ > > + mii->bmcr = MII_BMCR_FD | MII_BMCR_AUTOEN | MII_BMCR_SPEED; > > + mii->bmsr = MII_BMSR_100TX_FD | MII_BMSR_100TX_HD | MII_BMSR_10T_FD | > > + MII_BMSR_10T_HD | MII_BMSR_MFPS | MII_BMSR_AUTONEG; > > + mii->anar = MII_ANAR_TXFD | MII_ANAR_TX | MII_ANAR_10FD | MII_ANAR_10 | > > + MII_ANAR_CSMACD; > > + mii->anlpar = MII_ANAR_TX; > > + mii_set_link(mii, mii->link_ok); > > +} > > + > > +static uint16_t mii_read(AwEmacMii *mii, uint8_t phy_addr, uint8_t reg) > > +{ > > + uint16_t ret = 0xffff; > > + > > + if (phy_addr == BOARD_PHY_ADDRESS) { > > + switch (reg) { > > + case MII_BMCR: > > + ret = mii->bmcr; > > + break; > > + case MII_BMSR: > > + ret = mii->bmsr; > > + break; > > + case MII_PHYID1: > > + ret = RTL8201CP_PHYID1; > > + break; > > + case MII_PHYID2: > > + ret = RTL8201CP_PHYID2; > > + break; > > + case MII_ANAR: > > + ret = mii->anar; > > + break; > > + case MII_ANLPAR: > > + ret = mii->anlpar; > > + break; > > + default: > > + debug("unknown mii register %x\n", reg); > > + ret = 0; > > + } > > + } > > + return ret; > > +} > > + > > +static void mii_write(AwEmacMii *mii, uint8_t phy_addr, uint8_t reg, > > + uint16_t value) > > +{ > > + if (phy_addr == BOARD_PHY_ADDRESS) { > > + switch (reg) { > > + case MII_BMCR: > > + if (value & MII_BMCR_RESET) { > > + mii_reset(mii); > > + } else { > > + mii->bmcr = value; > > + } > > + break; > > + case MII_BMSR: > > + case MII_PHYID1: > > + case MII_PHYID2: > > + case MII_ANLPAR: > > + qemu_log_mask(LOG_GUEST_ERROR, "%s: write to mii register > > %x\n", > > + __func__, reg); > > + break; > > + case MII_ANAR: > > + mii->anar = value; > > + break; > > + default: > > + debug("unknown mii register %x\n", reg); > > + } > > + } > > +} > > + > > +static void aw_emac_update_irq(AwEmacState *s) > > +{ > > + qemu_set_irq(s->irq, (s->int_sta & s->int_ctl) != 0); > > +} > > + > > +static int aw_emac_can_receive(NetClientState *nc) > > +{ > > + AwEmacState *s = qemu_get_nic_opaque(nc); > > + > > + return (s->ctl & EMAC_CTL_RX_EN) && (s->num_rx < MAX_RX); > > If you return false from a can_recieve(), you need to explictly flush > queued packets (qemu_flush_queued_packets()) when the blocking > condition is lifted, heres a good example a bugfix patch addressing > this issue for another mac: > > http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg02255.html Ok. > > +} > > + > > +static ssize_t aw_emac_receive(NetClientState *nc, const uint8_t *buf, > > + size_t size) > > +{ > > + AwEmacState *s = qemu_get_nic_opaque(nc); > > + uint32_t *fifo; > > + uint32_t crc; > > + char *dest; > > + > > + if (s->num_rx >= MAX_RX) { > > + debug("rx queue full - packed dropped\n"); > > + return -1; > > + } > > + > > + if (size + RX_HDR_SIZE > FIFO_SIZE) { > > + debug("packet too big\n"); > > + return -1; > > + } > > + > > + fifo = s->rx_fifos[(s->first_rx + s->num_rx) % MAX_RX]; > > + dest = (char *)&fifo[2]; > > + s->num_rx++; > > + > > + memcpy(dest, buf, size); > > + > > + /* Fill to minimum frame length */ > > + if (size < 60) { > > + memset(dest + size, 0, 60 - size); > > + size = 60; > > + } > > + > > + /* Append FCS */ > > + crc = crc32(~0, buf, size); > > + memcpy(dest + size, &crc, 4); > > + > > + fifo[0] = EMAC_UNDOCUMENTED_MAGIC; > > + fifo[1] = EMAC_RX_HEADER(size + 4, EMAC_RX_IO_DATA_STATUS_OK); > > + > > + /* Set rx interrupt flag */ > > + s->int_sta |= EMAC_INT_RX; > > + aw_emac_update_irq(s); > > + > > + return size; > > +} > > + > > +static void aw_emac_cleanup(NetClientState *nc) > > +{ > > + AwEmacState *s = qemu_get_nic_opaque(nc); > > + > > + s->nic = NULL; > > +} > > + > > +static void aw_emac_reset(AwEmacState *s) > > +{ > > + s->ctl = 0; > > + s->tx_mode = 0; > > + s->int_ctl = 0; > > + s->int_sta = 0; > > + s->tx_channel = 0; > > + s->first_rx = 0; > > + s->num_rx = 0; > > + s->rx_offset = 0; > > + s->phy_target = 0; > > +} > > + > > +static uint64_t aw_emac_read(void *opaque, hwaddr offset, unsigned size) > > +{ > > + AwEmacState *s = opaque; > > + uint64_t ret; > > + uint32_t *rx_fifo; > > + > > + switch (offset) { > > + case EMAC_CTL_REG: > > + ret = s->ctl; > > + break; > > + case EMAC_TX_MODE_REG: > > + ret = s->tx_mode; > > + break; > > + case EMAC_TX_INS_REG: > > + ret = s->tx_channel; > > + break; > > + case EMAC_RX_CTL_REG: > > + ret = s->rx_ctl; > > + break; > > + case EMAC_RX_IO_DATA_REG: > > + if (!s->num_rx) { > > + ret = 0; > > + break; > > + } > > + rx_fifo = s->rx_fifos[s->first_rx]; > > + > > + if (s->rx_offset >= FIFO_SIZE || > > + s->rx_offset >= EMAC_RX_IO_DATA_LEN(rx_fifo[1]) + RX_HDR_SIZE) > > { > > + /* This should never happen */ > > Why? If its impossible via your implementation logic then you should > assert. Its a misbehaving guest then you should > qemu_log_mask(LOG_GUEST_ERROR In this case it should be impossible due to the implementation of the model, so I will add an assertion. > > + debug("RX fifo overrun\n"); > > + s->first_rx = (s->first_rx + 1) % MAX_RX; > > + s->num_rx--; > > + s->rx_offset = 0; > > + ret = 0; > > + break; > > + } > > + > > + ret = rx_fifo[s->rx_offset >> 2]; > > + s->rx_offset += 4; > > + > > + if (s->rx_offset >= EMAC_RX_IO_DATA_LEN(rx_fifo[1]) + RX_HDR_SIZE) > > { > > + s->first_rx = (s->first_rx + 1) % MAX_RX; > > + s->num_rx--; > > + s->rx_offset = 0; > > + } > > + break; > > + case EMAC_RX_FBC_REG: > > + ret = s->num_rx; > > + break; > > + case EMAC_INT_CTL_REG: > > + ret = s->int_ctl; > > + break; > > + case EMAC_INT_STA_REG: > > + ret = s->int_sta; > > + break; > > + case EMAC_MAC_MRDD_REG: > > + ret = mii_read(&s->mii, PHY_TARGET_ADDR(s->phy_target), > > + PHY_TARGET_REG(s->phy_target)); > > + break; > > + default: > > + debug("unknown offset %08x\n", (unsigned int)offset); > > qemu_log_mask(LOG_UNIMP > > I'm thinking you should UNIMP rather than the usual GUEST_ERROR as you > have no specs to work on (same problem as the recent Digic series). I agree. > > > + ret = 0; > > + } > > + > > + return ret; > > +} > > + > > +static void aw_emac_write(void *opaque, hwaddr offset, uint64_t value, > > + unsigned size) > > +{ > > + AwEmacState *s = opaque; > > + AwEmacTxFifo *fifo; > > + int chan; > > + > > + switch (offset) { > > + case EMAC_CTL_REG: > > + if (value & EMAC_CTL_RESET) { > > + debug("reset\n"); > > + aw_emac_reset(s); > > + value &= ~EMAC_CTL_RESET; > > + } > > + s->ctl = value; > > This is one example of a place where you may need to flush queued packets. Ok. > > + break; > > + case EMAC_TX_MODE_REG: > > + s->tx_mode = value; > > + break; > > + case EMAC_TX_CTL0_REG: > > + case EMAC_TX_CTL1_REG: > > + chan = (offset == EMAC_TX_CTL0_REG ? 0 : 1); > > + if ((value & 1) && (s->ctl & EMAC_CTL_TX_EN)) { > > + qemu_send_packet(qemu_get_queue(s->nic), > > + (uint8_t *)s->tx_fifos[chan].data, > > + s->tx_fifos[chan].length); > > + > > + /* Raise TX interrupt */ > > + s->int_sta |= EMAC_INT_TX_CHAN(chan); > > + s->tx_fifos[chan].offset = 0; > > + aw_emac_update_irq(s); > > + } > > + break; > > + case EMAC_TX_INS_REG: > > + s->tx_channel = value < NUM_CHAN ? value : 0; > > + break; > > + case EMAC_TX_PL0_REG: > > + case EMAC_TX_PL1_REG: > > + chan = (offset == EMAC_TX_PL0_REG ? 0 : 1); > > + if (value > FIFO_SIZE) { > > + debug("invalid TX frame length %d\n", (int)value); > > assert or GUEST_ERROR - any debug errory type messages should be one > or the other depending on root cause. If caused by bad sw GUEST_ERROR. > If a bug in this device model code, assert(false). Ok, in this case it's a guest error. > > > + value = FIFO_SIZE; > > + } > > + s->tx_fifos[chan].length = value; > > + break; > > + case EMAC_TX_IO_DATA_REG: > > + fifo = &s->tx_fifos[s->tx_channel]; > > + if (fifo->offset >= FIFO_SIZE) { > > + debug("TX frame data overruns fifo (%d)\n", fifo->offset); > > + break; > > + } > > + fifo->data[fifo->offset >> 2] = value; > > + fifo->offset += 4; > > + break; > > + case EMAC_RX_CTL_REG: > > + s->rx_ctl = value; > > + break; > > + case EMAC_RX_FBC_REG: > > + if (value == 0) { > > + s->num_rx = 0; > > + } > > + break; > > + case EMAC_INT_CTL_REG: > > + s->int_ctl = value; > > + break; > > + case EMAC_INT_STA_REG: > > + s->int_sta &= ~value; > > + break; > > + case EMAC_MAC_MADR_REG: > > + s->phy_target = value; > > + break; > > + case EMAC_MAC_MWTD_REG: > > + mii_write(&s->mii, PHY_TARGET_ADDR(s->phy_target), > > + PHY_TARGET_REG(s->phy_target), value); > > + break; > > + default: > > + debug("unknown offset %08x\n", (unsigned int)offset); > > LOG_UNIMP > > > + } > > +} > > + > > +static void aw_emac_set_link(NetClientState *nc) > > +{ > > + AwEmacState *s = qemu_get_nic_opaque(nc); > > + > > + mii_set_link(&s->mii, !nc->link_down); > > +} > > + > > +static const MemoryRegionOps aw_emac_mem_ops = { > > + .read = aw_emac_read, > > + .write = aw_emac_write, > > + .endianness = DEVICE_NATIVE_ENDIAN, > > Does your linux driver ever do sub-word accesses? If not you could set > the appropriate restrictions to access size/alignment here for > defensiveness. No, all accesses have word size and are aligned; I will add a restriction on the size. > > > +}; > > + > > +static NetClientInfo net_aw_emac_info = { > > + .type = NET_CLIENT_OPTIONS_KIND_NIC, > > + .size = sizeof(NICState), > > + .can_receive = aw_emac_can_receive, > > + .receive = aw_emac_receive, > > + .cleanup = aw_emac_cleanup, > > + .link_status_changed = aw_emac_set_link, > > +}; > > + > > +static void aw_emac_init(Object *obj) > > +{ > > + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); > > + AwEmacState *s = AW_EMAC(obj); > > + > > + memory_region_init_io(&s->iomem, OBJECT(s), &aw_emac_mem_ops, s, > > + "aw_emac", 0x1000); > > + sysbus_init_mmio(sbd, &s->iomem); > > + sysbus_init_irq(sbd, &s->irq); > > +} > > + > > +static void aw_emac_realize(DeviceState *dev, Error **errp) > > +{ > > + AwEmacState *s = AW_EMAC(dev); > > + > > + qemu_macaddr_default_if_unset(&s->conf.macaddr); > > + > > + s->nic = qemu_new_nic(&net_aw_emac_info, &s->conf, > > + object_get_typename(OBJECT(dev)), dev->id, s); > > + qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a); > > + > > + aw_emac_reset(s); > > + aw_emac_set_link(qemu_get_queue(s->nic)); > > + mii_reset(&s->mii); > > +} > > + > > +static Property aw_emac_properties[] = { > > + DEFINE_NIC_PROPERTIES(AwEmacState, conf), > > + DEFINE_PROP_END_OF_LIST(), > > +}; > > + > > +static const VMStateDescription vmstate_mii = { > > + .name = "allwinner_emac_mii", > > + .version_id = 1, > > + .minimum_version_id = 1, > > + .fields = (VMStateField[]) { > > + VMSTATE_UINT16(bmcr, AwEmacMii), > > + VMSTATE_UINT16(bmsr, AwEmacMii), > > + VMSTATE_UINT16(anar, AwEmacMii), > > + VMSTATE_UINT16(anlpar, AwEmacMii), > > + VMSTATE_BOOL(link_ok, AwEmacMii), > > + VMSTATE_END_OF_LIST() > > + } > > +}; > > + > > +static const VMStateDescription vmstate_tx_fifo = { > > + .name = "allwinner_emac_tx_fifo", > > + .version_id = 1, > > + .minimum_version_id = 1, > > + .fields = (VMStateField[]) { > > + VMSTATE_UINT32_ARRAY(data, AwEmacTxFifo, FIFO_SIZE >> 2), > > + VMSTATE_INT32(length, AwEmacTxFifo), > > + VMSTATE_INT32(offset, AwEmacTxFifo), > > + VMSTATE_END_OF_LIST() > > + } > > +}; > > This might warrant a dup of fifo8 as fifo32 if you want to clean this > up. I think I have a patch handy that might help, will send tmrw. > Check util/fifo8.c for fifo8 example. Yes, probably AwEmacTxFifo can be replaced with Fifo32. > > + > > +static const VMStateDescription vmstate_aw_emac = { > > + .name = "allwinner_emac", > > + .version_id = 1, > > + .minimum_version_id = 1, > > + .fields = (VMStateField[]) { > > + VMSTATE_STRUCT(mii, AwEmacState, 1, vmstate_mii, AwEmacMii), > > + VMSTATE_UINT32(ctl, AwEmacState), > > + VMSTATE_UINT32(tx_mode, AwEmacState), > > + VMSTATE_UINT32(rx_ctl, AwEmacState), > > + VMSTATE_UINT32(int_ctl, AwEmacState), > > + VMSTATE_UINT32(int_sta, AwEmacState), > > + VMSTATE_UINT32(phy_target, AwEmacState), > > + VMSTATE_INT32(tx_channel, AwEmacState), > > + VMSTATE_STRUCT_ARRAY(tx_fifos, AwEmacState, > > + NUM_CHAN, 1, vmstate_tx_fifo, AwEmacTxFifo), > > + VMSTATE_BUFFER_UNSAFE(tx_fifos, AwEmacState, 1, MAX_RX * > > FIFO_SIZE), > > + VMSTATE_INT32(first_rx, AwEmacState), > > + VMSTATE_INT32(num_rx, AwEmacState), > > + VMSTATE_INT32(rx_offset, AwEmacState), > > + VMSTATE_END_OF_LIST() > > + } > > +}; > > + > > +static void aw_emac_class_init(ObjectClass *klass, void *data) > > +{ > > + DeviceClass *dc = DEVICE_CLASS(klass); > > + > > + dc->realize = aw_emac_realize; > > + dc->props = aw_emac_properties; > > + dc->vmsd = &vmstate_aw_emac; > > +} > > + > > +static const TypeInfo aw_emac_info = { > > + .name = TYPE_AW_EMAC, > > + .parent = TYPE_SYS_BUS_DEVICE, > > + .instance_size = sizeof(AwEmacState), > > + .instance_init = aw_emac_init, > > + .class_init = aw_emac_class_init, > > +}; > > + > > +static void aw_emac_register_types(void) > > +{ > > + type_register_static(&aw_emac_info); > > +} > > + > > +type_init(aw_emac_register_types) > > diff --git a/include/hw/net/allwinner_emac.h > > b/include/hw/net/allwinner_emac.h > > new file mode 100644 > > index 0000000..f9f9682 > > --- /dev/null > > +++ b/include/hw/net/allwinner_emac.h > > @@ -0,0 +1,204 @@ > > +/* > > + * Emulation of Allwinner EMAC Fast Ethernet controller and > > + * Realtek RTL8201CP PHY > > + * > > + * Copyright (C) 2013 Beniamino Galvani <b.galv...@gmail.com> > > + * > > + * Allwinner EMAC register definitions from Linux kernel are: > > + * Copyright 2012 Stefan Roese <s...@denx.de> > > + * Copyright 2013 Maxime Ripard <maxime.rip...@free-electrons.com> > > + * Copyright 1997 Sten Wang > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + */ > > +#ifndef AW_EMAC_H > > +#define AW_EMAC_H > > + > > +#include "net/net.h" > > + > > +#define TYPE_AW_EMAC "allwinner_emac" > > +#define AW_EMAC(obj) OBJECT_CHECK(AwEmacState, (obj), TYPE_AW_EMAC) > > + > > +/* > > + * Allwinner EMAC register list > > + */ > > +#define EMAC_CTL_REG 0x00 > > +#define EMAC_TX_MODE_REG 0x04 > > +#define EMAC_TX_FLOW_REG 0x08 > > +#define EMAC_TX_CTL0_REG 0x0C > > +#define EMAC_TX_CTL1_REG 0x10 > > +#define EMAC_TX_INS_REG 0x14 > > +#define EMAC_TX_PL0_REG 0x18 > > +#define EMAC_TX_PL1_REG 0x1C > > +#define EMAC_TX_STA_REG 0x20 > > +#define EMAC_TX_IO_DATA_REG 0x24 > > +#define EMAC_TX_IO_DATA1_REG 0x28 > > +#define EMAC_TX_TSVL0_REG 0x2C > > +#define EMAC_TX_TSVH0_REG 0x30 > > +#define EMAC_TX_TSVL1_REG 0x34 > > +#define EMAC_TX_TSVH1_REG 0x38 > > +#define EMAC_RX_CTL_REG 0x3C > > +#define EMAC_RX_HASH0_REG 0x40 > > +#define EMAC_RX_HASH1_REG 0x44 > > +#define EMAC_RX_STA_REG 0x48 > > +#define EMAC_RX_IO_DATA_REG 0x4C > > +#define EMAC_RX_FBC_REG 0x50 > > +#define EMAC_INT_CTL_REG 0x54 > > +#define EMAC_INT_STA_REG 0x58 > > +#define EMAC_MAC_CTL0_REG 0x5C > > +#define EMAC_MAC_CTL1_REG 0x60 > > +#define EMAC_MAC_IPGT_REG 0x64 > > +#define EMAC_MAC_IPGR_REG 0x68 > > +#define EMAC_MAC_CLRT_REG 0x6C > > +#define EMAC_MAC_MAXF_REG 0x70 > > +#define EMAC_MAC_SUPP_REG 0x74 > > +#define EMAC_MAC_TEST_REG 0x78 > > +#define EMAC_MAC_MCFG_REG 0x7C > > +#define EMAC_MAC_MCMD_REG 0x80 > > +#define EMAC_MAC_MADR_REG 0x84 > > +#define EMAC_MAC_MWTD_REG 0x88 > > +#define EMAC_MAC_MRDD_REG 0x8C > > +#define EMAC_MAC_MIND_REG 0x90 > > +#define EMAC_MAC_SSRR_REG 0x94 > > +#define EMAC_MAC_A0_REG 0x98 > > +#define EMAC_MAC_A1_REG 0x9C > > +#define EMAC_MAC_A2_REG 0xA0 > > +#define EMAC_SAFX_L_REG0 0xA4 > > +#define EMAC_SAFX_H_REG0 0xA8 > > +#define EMAC_SAFX_L_REG1 0xAC > > +#define EMAC_SAFX_H_REG1 0xB0 > > +#define EMAC_SAFX_L_REG2 0xB4 > > +#define EMAC_SAFX_H_REG2 0xB8 > > +#define EMAC_SAFX_L_REG3 0xBC > > +#define EMAC_SAFX_H_REG3 0xC0 > > + > > +/* CTL register fields */ > > +#define EMAC_CTL_RESET (1 << 0) > > +#define EMAC_CTL_TX_EN (1 << 1) > > +#define EMAC_CTL_RX_EN (1 << 2) > > + > > +/* TX MODE register fields */ > > +#define EMAC_TX_MODE_ABORTED_FRAME_EN (1 << 0) > > +#define EMAC_TX_MODE_DMA_EN (1 << 1) > > + > > +/* RX CTL register fields */ > > +#define EMAC_RX_CTL_AUTO_DRQ_EN (1 << 1) > > +#define EMAC_RX_CTL_DMA_EN (1 << 2) > > +#define EMAC_RX_CTL_PASS_ALL_EN (1 << 4) > > +#define EMAC_RX_CTL_PASS_CTL_EN (1 << 5) > > +#define EMAC_RX_CTL_PASS_CRC_ERR_EN (1 << 6) > > +#define EMAC_RX_CTL_PASS_LEN_ERR_EN (1 << 7) > > +#define EMAC_RX_CTL_PASS_LEN_OOR_EN (1 << 8) > > +#define EMAC_RX_CTL_ACCEPT_UNICAST_EN (1 << 16) > > +#define EMAC_RX_CTL_DA_FILTER_EN (1 << 17) > > +#define EMAC_RX_CTL_ACCEPT_MULTICAST_EN (1 << 20) > > +#define EMAC_RX_CTL_HASH_FILTER_EN (1 << 21) > > +#define EMAC_RX_CTL_ACCEPT_BROADCAST_EN (1 << 22) > > +#define EMAC_RX_CTL_SA_FILTER_EN (1 << 24) > > +#define EMAC_RX_CTL_SA_FILTER_INVERT_EN (1 << 25) > > + > > +/* RX IO DATA register fields */ > > +#define EMAC_RX_IO_DATA_LEN(x) (x & 0xffff) > > +#define EMAC_RX_IO_DATA_STATUS(x) ((x >> 16) & 0xffff) > > use extract32 rather than >> & logic. Although I find the macrofied > extractors a bit wierd. Usually only a shift and width are macroified > and the extraction process is done inline. Ok. > > > +#define EMAC_RX_HEADER(len, status) (((len) & 0xffff) | ((status) << > > 16)) > > +#define EMAC_RX_IO_DATA_STATUS_CRC_ERR (1 << 4) > > +#define EMAC_RX_IO_DATA_STATUS_LEN_ERR (3 << 5) > > +#define EMAC_RX_IO_DATA_STATUS_OK (1 << 7) > > +#define EMAC_UNDOCUMENTED_MAGIC 0x0143414d /* header for RX > > frames */ > > + > > +/* PHY registers */ > > +#define MII_BMCR 0 > > +#define MII_BMSR 1 > > +#define MII_PHYID1 2 > > +#define MII_PHYID2 3 > > +#define MII_ANAR 4 > > +#define MII_ANLPAR 5 > > + > > +/* PHY registers fields */ > > +#define MII_BMCR_RESET (1 << 15) > > +#define MII_BMCR_LOOPBACK (1 << 14) > > +#define MII_BMCR_SPEED (1 << 13) > > +#define MII_BMCR_AUTOEN (1 << 12) > > +#define MII_BMCR_FD (1 << 8) > > + > > +#define MII_BMSR_100TX_FD (1 << 14) > > +#define MII_BMSR_100TX_HD (1 << 13) > > +#define MII_BMSR_10T_FD (1 << 12) > > +#define MII_BMSR_10T_HD (1 << 11) > > +#define MII_BMSR_MFPS (1 << 6) > > +#define MII_BMSR_AUTONEG (1 << 3) > > +#define MII_BMSR_LINK_ST (1 << 2) > > + > > +#define MII_ANAR_TXFD (1 << 8) > > +#define MII_ANAR_TX (1 << 7) > > +#define MII_ANAR_10FD (1 << 6) > > +#define MII_ANAR_10 (1 << 5) > > +#define MII_ANAR_CSMACD (1 << 0) > > + > > +#define RTL8201CP_PHYID1 0x0000 > > +#define RTL8201CP_PHYID2 0x8201 > > + > > +/* INT CTL and INT STA registers fields */ > > +#define EMAC_INT_TX_CHAN(x) (1 << (x)) > > +#define EMAC_INT_RX (1 << 8) > > + > > +#define BOARD_PHY_ADDRESS 1 > > This board level configurable? This is the value hardwired on the cubieboard, the only board that uses the Allwinner A10 at the moment in qemu. Should I add a property to the EMAC for changing the phy address? > > > + > > +#define NUM_CHAN 2 > > +#define FIFO_SIZE 2048 > > +#define MAX_RX 12 > > +#define RX_HDR_SIZE 8 > > + > > +#define PHY_TARGET_ADDR(x) (((x) >> 8) & 0xff) > > extract32 (or 16?). Ok, thanks for the review. Regards, Beniamino