| Message ID | 1314752751.84463.YahooMailClassic@web27003.mail.ukl.yahoo.com |
|---|---|
| State | New |
| Headers | show |
This won't even come close to passing checkpatch.pl Regards, Anthony Liguori On 08/30/2011 08:05 PM, bifferos wrote: > > Signed-off-by: Mark Kelly<mark@bifferos.com> > diff --git a/Makefile.objs b/Makefile.objs > index 6991a9f..7d87503 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -240,6 +240,7 @@ hw-obj-$(CONFIG_PCNET_PCI) += pcnet-pci.o > hw-obj-$(CONFIG_PCNET_COMMON) += pcnet.o > hw-obj-$(CONFIG_E1000_PCI) += e1000.o > hw-obj-$(CONFIG_RTL8139_PCI) += rtl8139.o > +hw-obj-$(CONFIG_R6040_PCI) += r6040.o > > hw-obj-$(CONFIG_SMC91C111) += smc91c111.o > hw-obj-$(CONFIG_LAN9118) += lan9118.o > diff --git a/default-configs/pci.mak b/default-configs/pci.mak > index 22bd350..d2ea7a2 100644 > --- a/default-configs/pci.mak > +++ b/default-configs/pci.mak > @@ -10,6 +10,7 @@ CONFIG_PCNET_PCI=y > CONFIG_PCNET_COMMON=y > CONFIG_LSI_SCSI_PCI=y > CONFIG_RTL8139_PCI=y > +CONFIG_R6040_PCI=y > CONFIG_E1000_PCI=y > CONFIG_IDE_CORE=y > CONFIG_IDE_QDEV=y > diff --git a/hw/pci.c b/hw/pci.c > index b904a4e..7e12935 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -1527,6 +1527,7 @@ static const char * const pci_nic_models[] = { > "rtl8139", > "e1000", > "pcnet", > + "r6040", > "virtio", > NULL > }; > @@ -1539,6 +1540,7 @@ static const char * const pci_nic_names[] = { > "rtl8139", > "e1000", > "pcnet", > + "r6040", > "virtio-net-pci", > NULL > }; > diff --git a/hw/r6040.c b/hw/r6040.c > new file mode 100644 > index 0000000..83587e7 > --- /dev/null > +++ b/hw/r6040.c > @@ -0,0 +1,627 @@ > +/* > + Emulation of r6040 ethernet controller found in a number of SoCs. > + Copyright (c) 2011 Mark Kelly, mark@bifferos.com > + > + This has been written using the R8610[1] and ip101a[2] datasheets. > + > + ICs with the embedded controller include R8610, R3210, AMRISC20000 > + and Vortex86SX > + > + The emulation seems good enough to fool Linux 2.6.37.6. It is > + not perfect, but has proven useful. > + > + [1] http://www.sima.com.tw/download/R8610_D06_20051003.pdf > + [2] http://www.icplus.com.tw/pp-IP101A.html > + */ > + > +#include "hw.h" > +#include "pci.h" > +#include "net.h" > +#include "loader.h" > +#include "sysemu.h" > +#include "qemu-timer.h" > + > +/* #define DEBUG_R6040 1 */ > + > + > +#if defined DEBUG_R6040 > +#define DPRINTF(fmt, ...) \ > + do { fprintf(stderr, "R6040: " fmt, ## __VA_ARGS__); } while (0) > +#else > +static inline GCC_FMT_ATTR(1, 2) int DPRINTF(const char *fmt, ...) > +{ > + return 0; > +} > +#endif > + > + > +/* Cast in order of appearance. _W prevfix means it's used to index the > + register word array (RegsW) > + */ > + > +#define MPSCCR_W (0x88 / 2) > + > +#define MAC0_W (0x68 / 2) > +#define MAC1_W (0x6a / 2) > +#define MAC2_W (0x6c / 2) > + > + > +#define RX_START_LOW_W (0x34 / 2) > +#define RX_START_HIGH_W (0x38 / 2) > +#define TX_PKT_COUNT_W (0x5a / 2) > +#define RX_PKT_COUNT_W (0x50 / 2) > + > + > +#define MCR0_W (0x00 / 2) > +#define MCR1_W (0x04 / 2) > +#define BIT_MRST (1<< 0) > + > +#define MTPR_W (0x14 / 2) > +#define MRBSR_W (0x18 / 2) > +#define MISR_W (0x3c / 2) > +#define MIER_W (0x40 / 2) > + > +#define MMDIO_W (0x20 / 2) > +#define MDIO_READ_W (0x24 / 2) > +#define MDIO_WRITE_W (0x28 / 2) > + > +#define MRCNT_W (0x50 / 2) > +#define MTCNT_W (0x5c / 2) > + > + > +#define MDIO_WRITE 0x4000 > +#define MDIO_READ 0x2000 > + > + > +typedef struct R6040State { > + PCIDevice dev; > + NICState *nic; > + NICConf conf; > + > + /* PHY related register sets */ > + uint16_t MID0[3]; > + uint16_t phy_regs[32]; > + uint32_t phy_op_in_progress; > + > + /* Primary IO address space */ > + union { > + uint8_t RegsB[0x100]; /* Byte access */ > + uint16_t RegsW[0x100/2]; /* word access */ > + uint32_t RegsL[0x100/4]; /* DWORD access */ > + }; > + > +} R6040State; > + > + > +/* some inlines to help access above structure */ > +static inline uint32_t TX_START(R6040State *s) > +{ > + uint32_t tmp = s->RegsW[0x2c/2]; > + return tmp | (s->RegsW[0x30/2]<< 16); > +} > + > +static inline void TX_START_SET(R6040State *s, uint32_t start) > +{ > + s->RegsW[0x2c/2] = start& 0xffff; > + s->RegsW[0x30/2] = (start>> 16)& 0xffff; > +} > + > +static inline uint32_t RX_START(R6040State *s) > +{ > + uint32_t tmp = s->RegsW[0x34/2]; > + return tmp | (s->RegsW[0x38/2]<< 16); > +} > + > +static inline void RX_START_SET(R6040State *s, uint32_t start) > +{ > + s->RegsW[0x34/2] = start& 0xffff; > + s->RegsW[0x38/2] = (start>> 16)& 0xffff; > +} > + > + > +static void r6040_update_irq(R6040State *s) > +{ > + uint16_t isr = s->RegsW[MISR_W]& s->RegsW[MIER_W]; > + > + qemu_set_irq(s->dev.irq[0], isr ? 1 : 0); > +} > + > + > +/* Mark auto-neg complete, NIC up. */ > +static void PhysicalLinkUp(void *opaque) > +{ > + R6040State *s = opaque; > + s->phy_regs[1] |= (1<< 2); > +} > + > + > +/* Transmit and receive descriptors are doubled up > + One is a subset of the other anyhow > + */ > +typedef struct descriptor { > + uint16_t DST; > + uint16_t DLEN; > + uint32_t DBP; > + uint32_t DNX; > + uint16_t HIDX; > + uint16_t Reserved1; > + uint16_t Reserved2; > +} descriptor_t; > + > + > +/* Some debugging functions */ > + > +#ifdef DEBUG_R6040 > +static void addr_dump16(const char *name, uint16_t val) > +{ > + DPRINTF("%s: 0x%04x ", name, val); > +} > + > +static void addr_dump32(const char *name, uint32_t val) > +{ > + DPRINTF("%s: 0x%x ", name, val); > +} > + > +static void hex_dump(const uint8_t *data, uint32_t len) > +{ > + uint8_t i; > + DPRINTF("hex: "); > + for (i = 0; i< len; i++) { > + fprintf(stderr, "%02x ", *data); > + if (i&& !(i % 0x20)) { > + fprintf(stderr, "\n"); > + } > + data++; > + } > + fprintf(stderr, "\n"); > +} > + > +static void desc_dump(descriptor_t *d, uint32_t addr) > +{ > + DPRINTF("\nDumping: 0x%x\n", addr); > + addr_dump16("DST", d->DST); > + addr_dump16("DLEN", d->DLEN); > + addr_dump32("DBP", (unsigned long)d->DBP); > + addr_dump32("DNX", (unsigned long)d->DNX); > + addr_dump16("HIDX", d->HIDX); > + printf("\n"); > +} > + > +static void dump_phys_mem(uint32_t addr, int len) > +{ > + uint8_t buffer[1024]; > + cpu_physical_memory_read(addr, buffer, len); > + hex_dump(buffer, len); > +} > + > +static void dump_pci(uint8_t *pci_conf) > +{ > + uint32_t *p = (uint32_t *)pci_conf; > + int i = 0; > + for (i = 0; i< 0x40; i += 4) { > + DPRINTF("Addr: 0x%08x, Data: 0x%08x\n", i, *p); > + p++; > + } > +} > +#endif > + > + > +static const VMStateDescription vmstate_r6040 = { > + .name = "r6040", > + .version_id = 3, > + .minimum_version_id = 2, > + .minimum_version_id_old = 2, > + .fields = (VMStateField[]) { > + VMSTATE_PCI_DEVICE(dev, R6040State), > + VMSTATE_BUFFER(RegsB, R6040State), > + VMSTATE_UINT16_ARRAY(MID0, R6040State, 3), > + VMSTATE_UINT16_ARRAY(phy_regs, R6040State, 32), > + VMSTATE_UINT32(phy_op_in_progress, R6040State), > + VMSTATE_MACADDR(conf.macaddr, R6040State), > + VMSTATE_END_OF_LIST() > + } > +}; > + > + > +static int TryToSendOnePacket(void *opaque) > +{ > + R6040State *s = opaque; > + descriptor_t d; > + uint8_t pkt_buffer[2000]; > + uint32_t tocopy; > + > + cpu_physical_memory_read(TX_START(s), (uint8_t *)&d, sizeof(d)); > + > + if (d.DST& 0x8000) { /* MAC owns it? */ > + tocopy = d.DLEN; > + if (tocopy> sizeof(pkt_buffer)) { > + tocopy = sizeof(pkt_buffer); > + } > + /* copy the packet to send it */ > + cpu_physical_memory_read(d.DBP, pkt_buffer, tocopy); > + > + qemu_send_packet(&s->nic->nc, pkt_buffer, tocopy); > + s->RegsW[TX_PKT_COUNT_W]++; > + > + /* relinquish ownership, we're done with it */ > + d.DST&= ~0x8000; > + > + /* Copy the new version of the descriptor back */ > + cpu_physical_memory_write(TX_START(s), (uint8_t *)&d, sizeof(d)); > + > + /* Advance to the next buffer if packet processed */ > + TX_START_SET(s, d.DNX); > + > + return 1; > + } > + > + return 0; > +} > + > + > +static void r6040_transmit(void *opaque) > +{ > + R6040State *s = opaque; > + int count = 0; > + > + while (TryToSendOnePacket(s)) { > + ++count; > + } > + > + if (count) { > + s->RegsW[MISR_W] |= 0x10; > + r6040_update_irq(s); > + } > +} > + > + > +/* Whether to allow callback returning 1 for yes, can receive */ > +static int r6040_can_receive(VLANClientState *nc) > +{ > + R6040State *s = DO_UPCAST(NICState, nc, nc)->opaque; > + int tmp = (s->RegsW[0]& (1<< 1)) ? 1 : 0; > + return tmp; > +} > + > + > +static int ReceiveOnePacket(void *opaque, const uint8_t *buf, size_t len) > +{ > + R6040State *s = opaque; > + uint32_t tocopy = len+4; /* include checksum */ > + descriptor_t d; > + > + cpu_physical_memory_read(RX_START(s), (uint8_t *)&d, sizeof(descriptor_t)); > + /*desc_dump(&d, 0);*/ > + > + if (d.DST& 0x8000) { /* MAC owned? */ > + > + uint16_t max_buffer = s->RegsW[MRBSR_W]& 0x07fc; > + if (tocopy> max_buffer) { > + tocopy = max_buffer; > + } > + > + cpu_physical_memory_write(d.DBP, buf, tocopy-4); > + > + /* indicate received OK */ > + d.DST |= (1<< 14); > + d.DLEN = tocopy; > + /* relinquish ownership */ > + d.DST&= ~0x8000; > + > + /* Copy the descriptor back */ > + cpu_physical_memory_write(RX_START(s), (uint8_t *)&d, > + sizeof(descriptor_t)); > + > + s->RegsW[RX_PKT_COUNT_W]++; > + > + s->RegsW[MISR_W] |= 1; /* received pkt interrupt */ > + > + r6040_update_irq(s); > + > + RX_START_SET(s, d.DNX); /* advance */ > + > + return 0; > + } > + return -1; > +} > + > + > +/* called on incoming packets */ > +static ssize_t r6040_receive(VLANClientState *nc, const uint8_t *buf, > + size_t len) > +{ > + R6040State *s = DO_UPCAST(NICState, nc, nc)->opaque; > + DPRINTF("Received incoming packet of len %ld\n", len); > + > + if (0 == ReceiveOnePacket(s, buf, len)) { > + return len; /* copied OK */ > + } > + > + return 0; > +} > + > + > +static void r6040_cleanup(VLANClientState *vc) > +{ > + DPRINTF("r6040_cleanup\n"); > +} > + > + > +static inline int BIT_SET(uint16_t old, uint16_t new, uint16_t bit) > +{ > + uint16_t before = (old& (1<< bit)); > + uint16_t after = (new& (1<< bit)); > + if (!before&& after) { > + return 1; > + } > + return 0; > +} > + > + > +static void r6040_ioport_writew(void *opaque, uint32_t addr, uint32_t val) > +{ > + R6040State *s = opaque; > + uint16_t old; > + addr&= 0xff; /* get relative to base address */ > + addr /= 2; /* Get the offset into the word-array */ > + old = s->RegsW[addr]; /* store the old value for future use */ > + > + switch (addr) { > + case MCR0_W: /* 0x00 */ > + if (BIT_SET(old, val, 12)) { > + r6040_transmit(opaque); > + } > + break; > + case MCR1_W: /* 0x04 */ > + if (val& BIT_MRST) { /* reset request incoming */ > + /* reset requested, complete it immediately, set this value to > + default */ > + val = 0x0010; > + } > + break; > + case MTPR_W: /* TX command reg, 0x14 */ > + if (val& 1) { > + r6040_transmit(opaque); > + val&= ~1; > + } > + break; > + case MMDIO_W: /* MDIO control, 0x20 */ > + { > + int phy_exists = ((val& 0x1f00) == 0x100) ? 1 : 0; > + uint16_t *phy = s->phy_regs; > + phy += (val& 0x1f); > + > + if (val& (1<< 13)) { /* read data */ > + if (phy_exists) { > + s->RegsW[MDIO_READ_W] = *phy; > + } else { > + s->RegsW[MDIO_READ_W] = 0xffff; > + } > + } else if (val& (1<< 14)) { /* write data */ > + if (phy_exists) { > + *phy = s->RegsW[MDIO_WRITE_W]; > + } > + } > + > + /* Whether you request to read or write, both bits go high while > + the operation is in progress, e.g. tell it to read, and the > + write-in-progress flag also goes high */ > + val |= 0x6000; /* signal operation has started */ > + s->phy_op_in_progress = 1; > + > + break; > + } > + case MISR_W: /* interrupt status reg (read to clear), 0x3c */ > + return; > + > + case MIER_W: /* interrupt enable register, 0x40 */ > + s->RegsW[MIER_W] = val; > + r6040_update_irq(s); > + return; > + > + case MRCNT_W: /* 0x50 */ > + case MTCNT_W: /* 0x5c */ > + return; /* Can't write to pkt count registers, skip */ > + > + } > + s->RegsW[addr] = val& 0xFFFF; > +} > + > + > + > +static uint32_t r6040_ioport_readw(void *opaque, uint32_t addr) > +{ > + R6040State *s = opaque; > + addr&= 0xff; /* get relative to base address */ > + addr /= 2; /* Get the offset into the word-array */ > + uint32_t tmp = s->RegsW[addr]; /* get the value */ > + > + switch (addr) { > + > + case MMDIO_W: /* MDIO control, 0x20 */ > + { > + /* Clear any in-progress MDIO activity for the next read > + This simulates the polling of the MDIO operation status, > + so the driver code always has to read the register twice > + before it thinks the operation is complete. */ > + if (s->phy_op_in_progress) { > + s->RegsW[addr]&= ~0x6000; > + s->phy_op_in_progress = 0; > + } > + break; > + } > + case MISR_W: /* interrupt status reg (read to clear) 0x3c */ > + s->RegsW[addr] = 0; > + break; > + case MIER_W: /* interrupt enable reg 0x40 */ > + break; > + case MRCNT_W: /* 0x50 */ > + case MTCNT_W: /* 0x5c */ > + s->RegsW[addr] = 0; /* read to clear */ > + break; > + default: > + break; > + } > + return tmp; > +} > + > + > +/* byte and long access are routed via the word operation handlers */ > +static void r6040_ioport_writeb(void *opaque, uint32_t addr, uint32_t val) > +{ > + R6040State *s = opaque; > + addr&= 0xFF; > + val&= 0xFF; > + uint16_t old = s->RegsW[addr/2]; /* get the current value */ > + if (addr& 1) { > + old&= 0xff; > + old |= (val<< 8); > + } else { > + old&= 0xff00; > + old |= val; > + } > + > + r6040_ioport_writew(opaque, addr, old); /* call the word-based version */ > +} > + > +static void r6040_ioport_writel(void *opaque, uint32_t addr, uint32_t val) > +{ > + /* Set the low value */ > + r6040_ioport_writew(opaque, addr, val& 0xffff); > + /* Set the high value */ > + r6040_ioport_writew(opaque, addr+2, (val>> 16)& 0xffff); > +} > + > +static uint32_t r6040_ioport_readb(void *opaque, uint32_t addr) > +{ > + uint32_t tmp = r6040_ioport_readw(opaque, addr& ~1); > + if (addr& 1) { > + return (tmp& 0xff00)>> 8; > + } > + return tmp& 0xff; > +} > + > +static uint32_t r6040_ioport_readl(void *opaque, uint32_t addr) > +{ > + uint32_t tmp = r6040_ioport_readw(opaque, addr); > + return tmp | (r6040_ioport_readw(opaque, addr+2)<< 16); > +} > + > + > +static void r6040_register_ioports(R6040State *s, pcibus_t addr) > +{ > + register_ioport_write(addr, 0x100, 1, r6040_ioport_writeb, s); > + register_ioport_read(addr, 0x100, 1, r6040_ioport_readb, s); > + > + register_ioport_write(addr, 0x100, 2, r6040_ioport_writew, s); > + register_ioport_read(addr, 0x100, 2, r6040_ioport_readw, s); > + > + register_ioport_write(addr, 0x100, 4, r6040_ioport_writel, s); > + register_ioport_read(addr, 0x100, 4, r6040_ioport_readl, s); > +} > + > + > +static void r6040_map(PCIDevice *pci_dev, int region_num, > + pcibus_t addr, pcibus_t size, int type) > +{ > + R6040State *s = DO_UPCAST(R6040State, dev, pci_dev);; > + > + DPRINTF("## Mapping to address %lx\n", addr); > + r6040_register_ioports(s, addr); > +} > + > + > +static NetClientInfo net_r6040_info = { > + .type = NET_CLIENT_TYPE_NIC, > + .size = sizeof(NICState), > + .can_receive = r6040_can_receive, > + .receive = r6040_receive, > + .cleanup = r6040_cleanup, > +}; > + > + > +static int r6040_init_pci(PCIDevice *dev) > +{ > + QEMUTimer *timer; > + > + R6040State *s = DO_UPCAST(R6040State, dev, dev); > + uint8_t *pci_conf; > + > + /* MAC PHYS status change register. Linux driver expects something > + sensible as default and if not will try to set it */ > + s->RegsW[MPSCCR_W] = 0x9f07; > + > + /* Default value for maximum packet size */ > + s->RegsW[MRBSR_W] = 0x600; > + > + /* set the MAC, linux driver reads this when it loads, it is > + normally set by the BIOS, but obviously Qemu BIOS isn't going > + to do that */ > + s->RegsW[MAC0_W] = 0x5452; > + s->RegsW[MAC1_W] = 0x1200; > + s->RegsW[MAC2_W] = 0x5734; > + > + /* Tell Qemu the same thing */ > + s->conf.macaddr.a[0] = s->RegsW[MAC0_W]& 0xff; > + s->conf.macaddr.a[1] = (s->RegsW[MAC0_W]>> 8)& 0xff; > + s->conf.macaddr.a[2] = s->RegsW[MAC1_W]& 0xff; > + s->conf.macaddr.a[3] = (s->RegsW[MAC1_W]>> 8)& 0xff; > + s->conf.macaddr.a[4] = s->RegsW[MAC2_W]& 0xff; > + s->conf.macaddr.a[5] = (s->RegsW[MAC2_W]>> 8)& 0xff; > + > + /* no commands running */ > + s->phy_op_in_progress = 0; > + > + /* PHY auto-neg in progress */ > + s->phy_regs[1] = 0x786d& ~(1<< 2); > + s->phy_regs[2] = 0x0243; > + s->phy_regs[3] = 0x0c54; > + > + pci_conf = (uint8_t *)s->dev.config; > + > + pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; /* header_type */ > + pci_conf[PCI_INTERRUPT_LINE] = 0xa; /* interrupt line */ > + pci_conf[PCI_INTERRUPT_PIN] = 1; /* interrupt pin 0 */ > + > + pci_register_bar(&s->dev, 0, 0x100, PCI_BASE_ADDRESS_SPACE_IO, r6040_map); > + > + s->nic = qemu_new_nic(&net_r6040_info,&s->conf, > + dev->qdev.info->name, dev->qdev.id, s); > + > + qemu_format_nic_info_str(&s->nic->nc, s->conf.macaddr.a); > + > + /* Simulate a delay of a couple of seconds for the link to come up. > + Not required for Linux but very handy for developing BIOS code. > + */ > + timer = qemu_new_timer_ns(vm_clock, PhysicalLinkUp, s); > + qemu_mod_timer(timer, qemu_get_clock_ms(vm_clock) + 1500000000); > + > + /* Register IO port access as well */ > + r6040_register_ioports(s, 0xe800); > + > + return 0; > +} > + > + > +static PCIDeviceInfo r6040_info = { > + .qdev.name = "r6040", > + .qdev.size = sizeof(R6040State), > + .qdev.vmsd =&vmstate_r6040, > + .init = r6040_init_pci, > + .vendor_id = 0x17f3, /* RDC */ > + .device_id = 0x6040, /* r6040 nic */ > + .class_id = PCI_CLASS_NETWORK_ETHERNET, > + .qdev.props = (Property[]) { > + DEFINE_NIC_PROPERTIES(R6040State, conf), > + DEFINE_PROP_END_OF_LIST(), > + } > +}; > + > + > +static void r6040_register(void) > +{ > + pci_qdev_register(&r6040_info); > +} > + > + > +device_init(r6040_register) > >
On Tue, 30 Aug 2011, Anthony Liguori wrote: > This won't even come close to passing checkpatch.pl Have you actually tried? > > Regards, > > Anthony Liguori > > On 08/30/2011 08:05 PM, bifferos wrote: > > > > Signed-off-by: Mark Kelly<mark@bifferos.com> > > diff --git a/Makefile.objs b/Makefile.objs > > index 6991a9f..7d87503 100644 > > --- a/Makefile.objs > > +++ b/Makefile.objs > > @@ -240,6 +240,7 @@ hw-obj-$(CONFIG_PCNET_PCI) += pcnet-pci.o > > hw-obj-$(CONFIG_PCNET_COMMON) += pcnet.o > > hw-obj-$(CONFIG_E1000_PCI) += e1000.o > > hw-obj-$(CONFIG_RTL8139_PCI) += rtl8139.o > > +hw-obj-$(CONFIG_R6040_PCI) += r6040.o > > > > hw-obj-$(CONFIG_SMC91C111) += smc91c111.o > > hw-obj-$(CONFIG_LAN9118) += lan9118.o > > diff --git a/default-configs/pci.mak b/default-configs/pci.mak > > index 22bd350..d2ea7a2 100644 > > --- a/default-configs/pci.mak > > +++ b/default-configs/pci.mak > > @@ -10,6 +10,7 @@ CONFIG_PCNET_PCI=y > > CONFIG_PCNET_COMMON=y > > CONFIG_LSI_SCSI_PCI=y > > CONFIG_RTL8139_PCI=y > > +CONFIG_R6040_PCI=y > > CONFIG_E1000_PCI=y > > CONFIG_IDE_CORE=y > > CONFIG_IDE_QDEV=y > > diff --git a/hw/pci.c b/hw/pci.c > > index b904a4e..7e12935 100644 > > --- a/hw/pci.c > > +++ b/hw/pci.c > > @@ -1527,6 +1527,7 @@ static const char * const pci_nic_models[] = { > > "rtl8139", > > "e1000", > > "pcnet", > > + "r6040", > > "virtio", > > NULL > > }; > > @@ -1539,6 +1540,7 @@ static const char * const pci_nic_names[] = { > > "rtl8139", > > "e1000", > > "pcnet", > > + "r6040", > > "virtio-net-pci", > > NULL > > }; > > diff --git a/hw/r6040.c b/hw/r6040.c > > new file mode 100644 > > index 0000000..83587e7 > > --- /dev/null > > +++ b/hw/r6040.c > > @@ -0,0 +1,627 @@ > > +/* > > + Emulation of r6040 ethernet controller found in a number of SoCs. > > + Copyright (c) 2011 Mark Kelly, mark@bifferos.com This doesn't spell out under which conditions this can be used by people other than Mark Kelly. > > + > > + This has been written using the R8610[1] and ip101a[2] datasheets. > > + > > + ICs with the embedded controller include R8610, R3210, AMRISC20000 > > + and Vortex86SX > > + > > + The emulation seems good enough to fool Linux 2.6.37.6. It is > > + not perfect, but has proven useful. > > + > > + [1] http://www.sima.com.tw/download/R8610_D06_20051003.pdf > > + [2] http://www.icplus.com.tw/pp-IP101A.html > > + */ > > + > > +#include "hw.h" > > +#include "pci.h" > > +#include "net.h" > > +#include "loader.h" > > +#include "sysemu.h" > > +#include "qemu-timer.h" > > + > > +/* #define DEBUG_R6040 1 */ > > + > > + > > +#if defined DEBUG_R6040 > > +#define DPRINTF(fmt, ...) \ > > + do { fprintf(stderr, "R6040: " fmt, ## __VA_ARGS__); } while (0) > > +#else > > +static inline GCC_FMT_ATTR(1, 2) int DPRINTF(const char *fmt, ...) > > +{ > > + return 0; > > +} > > +#endif > > + > > + > > +/* Cast in order of appearance. _W prevfix means it's used to index the prefix (though suffix is even better) [..snip..]
On 08/30/2011 08:30 PM, malc wrote: > On Tue, 30 Aug 2011, Anthony Liguori wrote: > >> This won't even come close to passing checkpatch.pl > > Have you actually tried? Sigh. I was hoping checkpatch.pl was more useful than it appears to be. At any rate, the patch doesn't follow CODING_STYLE. Regards, Anthony Liguori > >> >> Regards, >> >> Anthony Liguori >> >> On 08/30/2011 08:05 PM, bifferos wrote: >>> >>> Signed-off-by: Mark Kelly<mark@bifferos.com> >>> diff --git a/Makefile.objs b/Makefile.objs >>> index 6991a9f..7d87503 100644 >>> --- a/Makefile.objs >>> +++ b/Makefile.objs >>> @@ -240,6 +240,7 @@ hw-obj-$(CONFIG_PCNET_PCI) += pcnet-pci.o >>> hw-obj-$(CONFIG_PCNET_COMMON) += pcnet.o >>> hw-obj-$(CONFIG_E1000_PCI) += e1000.o >>> hw-obj-$(CONFIG_RTL8139_PCI) += rtl8139.o >>> +hw-obj-$(CONFIG_R6040_PCI) += r6040.o >>> >>> hw-obj-$(CONFIG_SMC91C111) += smc91c111.o >>> hw-obj-$(CONFIG_LAN9118) += lan9118.o >>> diff --git a/default-configs/pci.mak b/default-configs/pci.mak >>> index 22bd350..d2ea7a2 100644 >>> --- a/default-configs/pci.mak >>> +++ b/default-configs/pci.mak >>> @@ -10,6 +10,7 @@ CONFIG_PCNET_PCI=y >>> CONFIG_PCNET_COMMON=y >>> CONFIG_LSI_SCSI_PCI=y >>> CONFIG_RTL8139_PCI=y >>> +CONFIG_R6040_PCI=y >>> CONFIG_E1000_PCI=y >>> CONFIG_IDE_CORE=y >>> CONFIG_IDE_QDEV=y >>> diff --git a/hw/pci.c b/hw/pci.c >>> index b904a4e..7e12935 100644 >>> --- a/hw/pci.c >>> +++ b/hw/pci.c >>> @@ -1527,6 +1527,7 @@ static const char * const pci_nic_models[] = { >>> "rtl8139", >>> "e1000", >>> "pcnet", >>> + "r6040", >>> "virtio", >>> NULL >>> }; >>> @@ -1539,6 +1540,7 @@ static const char * const pci_nic_names[] = { >>> "rtl8139", >>> "e1000", >>> "pcnet", >>> + "r6040", >>> "virtio-net-pci", >>> NULL >>> }; >>> diff --git a/hw/r6040.c b/hw/r6040.c >>> new file mode 100644 >>> index 0000000..83587e7 >>> --- /dev/null >>> +++ b/hw/r6040.c >>> @@ -0,0 +1,627 @@ >>> +/* >>> + Emulation of r6040 ethernet controller found in a number of SoCs. >>> + Copyright (c) 2011 Mark Kelly, mark@bifferos.com > > This doesn't spell out under which conditions this can be used by > people other than Mark Kelly. > >>> + >>> + This has been written using the R8610[1] and ip101a[2] datasheets. >>> + >>> + ICs with the embedded controller include R8610, R3210, AMRISC20000 >>> + and Vortex86SX >>> + >>> + The emulation seems good enough to fool Linux 2.6.37.6. It is >>> + not perfect, but has proven useful. >>> + >>> + [1] http://www.sima.com.tw/download/R8610_D06_20051003.pdf >>> + [2] http://www.icplus.com.tw/pp-IP101A.html >>> + */ >>> + >>> +#include "hw.h" >>> +#include "pci.h" >>> +#include "net.h" >>> +#include "loader.h" >>> +#include "sysemu.h" >>> +#include "qemu-timer.h" >>> + >>> +/* #define DEBUG_R6040 1 */ >>> + >>> + >>> +#if defined DEBUG_R6040 >>> +#define DPRINTF(fmt, ...) \ >>> + do { fprintf(stderr, "R6040: " fmt, ## __VA_ARGS__); } while (0) >>> +#else >>> +static inline GCC_FMT_ATTR(1, 2) int DPRINTF(const char *fmt, ...) >>> +{ >>> + return 0; >>> +} >>> +#endif >>> + >>> + >>> +/* Cast in order of appearance. _W prevfix means it's used to index the > > prefix (though suffix is even better) > > [..snip..] >
On Tue, 30 Aug 2011, Anthony Liguori wrote: > On 08/30/2011 08:30 PM, malc wrote: > > On Tue, 30 Aug 2011, Anthony Liguori wrote: > > > > > This won't even come close to passing checkpatch.pl > > > > Have you actually tried? > > Sigh. I was hoping checkpatch.pl was more useful than it appears to be. > > At any rate, the patch doesn't follow CODING_STYLE. > Where? [..snip..]
On 08/31/2011 08:17 AM, malc wrote: > On Tue, 30 Aug 2011, Anthony Liguori wrote: > >> On 08/30/2011 08:30 PM, malc wrote: >>> On Tue, 30 Aug 2011, Anthony Liguori wrote: >>> >>>> This won't even come close to passing checkpatch.pl >>> >>> Have you actually tried? >> >> Sigh. I was hoping checkpatch.pl was more useful than it appears to be. >> >> At any rate, the patch doesn't follow CODING_STYLE. >> > > Where? 3. Naming Variables are lower_case_with_underscores; easy to type and read. Structured type names are in CamelCase; harder to type but standing out. Scalar type names are lower_case_with_underscores_ending_with_a_t, like the POSIX uint64_t and family. Note that this last convention contradicts POSIX and is therefore likely to be changed. When wrapping standard library functions, use the prefix qemu_ to alert readers that they are seeing a wrapped version; otherwise avoid this prefix. Regards, Anthony Liguori > [..snip..] >
--- On Wed, 31/8/11, malc <av1474@comtv.ru> wrote: > From: malc <av1474@comtv.ru> > Subject: Re: [Qemu-devel] [PATCH] Add support for r6040 NIC > To: "Anthony Liguori" <anthony@codemonkey.ws> > Cc: qemu-devel@nongnu.org, "bifferos" <bifferos@yahoo.co.uk> > Date: Wednesday, 31 August, 2011, 14:17 > On Tue, 30 Aug 2011, Anthony Liguori > wrote: > > > On 08/30/2011 08:30 PM, malc wrote: > > > On Tue, 30 Aug 2011, Anthony Liguori wrote: > > > > > > > This won't even come close to passing > checkpatch.pl > > > > > > Have you actually tried? > > > > Sigh. I was hoping checkpatch.pl was more useful > than it appears to be. > > > > At any rate, the patch doesn't follow CODING_STYLE. > > > > Where? My apologies, actually I had a half-hearted look for the coding style, came to this link: http://git.qemu.org/qemu.git/plain/CODING_STYLE Which was dead, and then fell back on the checkpatch.pl script, thinking nobody cared so much about coding styles. I should have looked a bit more carefully. Since then I found this: http://git.savannah.gnu.org/cgit/qemu.git/tree/CODING_STYLE AFAICS the problem is with the naming (part 3), which I can correct tonight (along with the other issues raised)
On Wed, 31 Aug 2011, Anthony Liguori wrote: > On 08/31/2011 08:17 AM, malc wrote: > > On Tue, 30 Aug 2011, Anthony Liguori wrote: > > > > > On 08/30/2011 08:30 PM, malc wrote: > > > > On Tue, 30 Aug 2011, Anthony Liguori wrote: > > > > > > > > > This won't even come close to passing checkpatch.pl > > > > > > > > Have you actually tried? > > > > > > Sigh. I was hoping checkpatch.pl was more useful than it appears to be. > > > > > > At any rate, the patch doesn't follow CODING_STYLE. > > > > > > > Where? > > 3. Naming > > Variables are lower_case_with_underscores; easy to type and read. Structured > type names are in CamelCase; harder to type but standing out. Scalar type > names are lower_case_with_underscores_ending_with_a_t, like the POSIX > uint64_t and family. Note that this last convention contradicts POSIX > and is therefore likely to be changed. Where in the patch this was violated, and do note that fields are not variables. > > When wrapping standard library functions, use the prefix qemu_ to alert > readers that they are seeing a wrapped version; otherwise avoid this prefix. And what ^^^ has to do with anything? > > Regards, > > Anthony Liguori > > > > [..snip..] > > >
On 08/31/2011 08:39 AM, malc wrote: > On Wed, 31 Aug 2011, Anthony Liguori wrote: > >> On 08/31/2011 08:17 AM, malc wrote: >>> On Tue, 30 Aug 2011, Anthony Liguori wrote: >>> >>>> On 08/30/2011 08:30 PM, malc wrote: >>>>> On Tue, 30 Aug 2011, Anthony Liguori wrote: >>>>> >>>>>> This won't even come close to passing checkpatch.pl >>>>> >>>>> Have you actually tried? >>>> >>>> Sigh. I was hoping checkpatch.pl was more useful than it appears to be. >>>> >>>> At any rate, the patch doesn't follow CODING_STYLE. >>>> >>> >>> Where? >> >> 3. Naming >> >> Variables are lower_case_with_underscores; easy to type and read. Structured >> type names are in CamelCase; harder to type but standing out. Scalar type >> names are lower_case_with_underscores_ending_with_a_t, like the POSIX >> uint64_t and family. Note that this last convention contradicts POSIX >> and is therefore likely to be changed. > > Where in the patch this was violated, and do note that fields are not > variables. fields are variables. And the struct names weren't all CamelCase. Regards, Anthony Liguori > >> >> When wrapping standard library functions, use the prefix qemu_ to alert >> readers that they are seeing a wrapped version; otherwise avoid this prefix. > > And what ^^^ has to do with anything? > >> >> Regards, >> >> Anthony Liguori >> >> >>> [..snip..] >>> >> >
On Wed, 31 Aug 2011, Anthony Liguori wrote: > On 08/31/2011 08:39 AM, malc wrote: > > On Wed, 31 Aug 2011, Anthony Liguori wrote: > > > > > On 08/31/2011 08:17 AM, malc wrote: > > > > On Tue, 30 Aug 2011, Anthony Liguori wrote: > > > > > > > > > On 08/30/2011 08:30 PM, malc wrote: > > > > > > On Tue, 30 Aug 2011, Anthony Liguori wrote: > > > > > > > > > > > > > This won't even come close to passing checkpatch.pl > > > > > > > > > > > > Have you actually tried? > > > > > > > > > > Sigh. I was hoping checkpatch.pl was more useful than it appears to > > > > > be. > > > > > > > > > > At any rate, the patch doesn't follow CODING_STYLE. > > > > > > > > > > > > > Where? > > > > > > 3. Naming > > > > > > Variables are lower_case_with_underscores; easy to type and read. > > > Structured > > > type names are in CamelCase; harder to type but standing out. Scalar type > > > names are lower_case_with_underscores_ending_with_a_t, like the POSIX > > > uint64_t and family. Note that this last convention contradicts POSIX > > > and is therefore likely to be changed. > > > > Where in the patch this was violated, and do note that fields are not > > variables. > > fields are variables. And the struct names weren't all CamelCase. c&v please. The only thing i can agree with is descriptor_t other than that patch is just fine. > > Regards, > > Anthony Liguori > > > > > > > > > When wrapping standard library functions, use the prefix qemu_ to alert > > > readers that they are seeing a wrapped version; otherwise avoid this > > > prefix. > > > > And what ^^^ has to do with anything? > > > > > > > > Regards, > > > > > > Anthony Liguori > > > > > > > > > > [..snip..] > > > > > > > > > >
On 08/31/2011 08:51 AM, malc wrote: > On Wed, 31 Aug 2011, Anthony Liguori wrote: > >> On 08/31/2011 08:39 AM, malc wrote: >>> On Wed, 31 Aug 2011, Anthony Liguori wrote: >>> >>>> On 08/31/2011 08:17 AM, malc wrote: >>>>> On Tue, 30 Aug 2011, Anthony Liguori wrote: >>>>> >>>>>> On 08/30/2011 08:30 PM, malc wrote: >>>>>>> On Tue, 30 Aug 2011, Anthony Liguori wrote: >>>>>>> >>>>>>>> This won't even come close to passing checkpatch.pl >>>>>>> >>>>>>> Have you actually tried? >>>>>> >>>>>> Sigh. I was hoping checkpatch.pl was more useful than it appears to >>>>>> be. >>>>>> >>>>>> At any rate, the patch doesn't follow CODING_STYLE. >>>>>> >>>>> >>>>> Where? >>>> >>>> 3. Naming >>>> >>>> Variables are lower_case_with_underscores; easy to type and read. >>>> Structured >>>> type names are in CamelCase; harder to type but standing out. Scalar type >>>> names are lower_case_with_underscores_ending_with_a_t, like the POSIX >>>> uint64_t and family. Note that this last convention contradicts POSIX >>>> and is therefore likely to be changed. >>> >>> Where in the patch this was violated, and do note that fields are not >>> variables. >> >> fields are variables. And the struct names weren't all CamelCase. > > c&v please. The only thing i can agree with is descriptor_t other than > that patch is just fine. Upper case field names are not okay. If you think coding style isn't clear, that's a bug in coding style. Just look at the vast majority of code in the tree. Regards, Anthony Liguori > >> >> Regards, >> >> Anthony Liguori >> >>> >>>> >>>> When wrapping standard library functions, use the prefix qemu_ to alert >>>> readers that they are seeing a wrapped version; otherwise avoid this >>>> prefix. >>> >>> And what ^^^ has to do with anything? >>> >>>> >>>> Regards, >>>> >>>> Anthony Liguori >>>> >>>> >>>>> [..snip..] >>>>> >>>> >>> >> >
On Wed, 31 Aug 2011, Anthony Liguori wrote: > On 08/31/2011 08:51 AM, malc wrote: > > On Wed, 31 Aug 2011, Anthony Liguori wrote: > > > > > On 08/31/2011 08:39 AM, malc wrote: > > > > On Wed, 31 Aug 2011, Anthony Liguori wrote: > > > > > > > > > On 08/31/2011 08:17 AM, malc wrote: > > > > > > On Tue, 30 Aug 2011, Anthony Liguori wrote: > > > > > > > > > > > > > On 08/30/2011 08:30 PM, malc wrote: > > > > > > > > On Tue, 30 Aug 2011, Anthony Liguori wrote: > > > > > > > > > > > > > > > > > This won't even come close to passing checkpatch.pl > > > > > > > > > > > > > > > > Have you actually tried? > > > > > > > > > > > > > > Sigh. I was hoping checkpatch.pl was more useful than it appears > > > > > > > to > > > > > > > be. > > > > > > > > > > > > > > At any rate, the patch doesn't follow CODING_STYLE. > > > > > > > > > > > > > > > > > > > Where? > > > > > > > > > > 3. Naming > > > > > > > > > > Variables are lower_case_with_underscores; easy to type and read. > > > > > Structured > > > > > type names are in CamelCase; harder to type but standing out. Scalar > > > > > type > > > > > names are lower_case_with_underscores_ending_with_a_t, like the POSIX > > > > > uint64_t and family. Note that this last convention contradicts POSIX > > > > > and is therefore likely to be changed. > > > > > > > > Where in the patch this was violated, and do note that fields are not > > > > variables. > > > > > > fields are variables. And the struct names weren't all CamelCase. > > > > c&v please. The only thing i can agree with is descriptor_t other than > > that patch is just fine. > > Upper case field names are not okay. If you think coding style isn't clear, > that's a bug in coding style. Sez hu? Coding style is garbage that should be thrown out of the window. As for looking, yeah, i'm looking at usb with it's lovely hungarian fields, should we stampede to "fix" it? If the one who's going to maintain the code is fine with whatever naming is used so be it. [..snip..]
On 08/31/2011 09:35 AM, malc wrote: > On Wed, 31 Aug 2011, Anthony Liguori wrote: > >> Upper case field names are not okay. If you think coding style isn't clear, >> that's a bug in coding style. > > Sez hu? Coding style is garbage that should be thrown out of the window. > As for looking, yeah, i'm looking at usb with it's lovely hungarian > fields, should we stampede to "fix" it? > > If the one who's going to maintain the code is fine with whatever naming > is used so be it. No. That's how we got into the coding style mess we're in in the first place. There's no benefit to going through and changing existing code but new code needs to be consistent with the vast majority of code in the rest of the tree. It's about overall code base consistency and maintainability. Regards, Anthony Liguori > > [..snip..] >
On Wed, 31 Aug 2011, Anthony Liguori wrote: > On 08/31/2011 09:35 AM, malc wrote: > > On Wed, 31 Aug 2011, Anthony Liguori wrote: > > > > > Upper case field names are not okay. If you think coding style isn't > > > clear, > > > that's a bug in coding style. > > > > Sez hu? Coding style is garbage that should be thrown out of the window. > > As for looking, yeah, i'm looking at usb with it's lovely hungarian > > fields, should we stampede to "fix" it? > > > > If the one who's going to maintain the code is fine with whatever naming > > is used so be it. > > No. That's how we got into the coding style mess we're in in the first > place. boblycat.org/~malc/right.ogg > > There's no benefit to going through and changing existing code but new code > needs to be consistent with the vast majority of code in the rest of the tree. > It's about overall code base consistency and maintainability. > Hand waving, for instance vast majority of the code never used the mandatory braces, the choice was arbitrary.
On Wed, Aug 31, 2011 at 11:06:11AM -0500, Anthony Liguori wrote: > On 08/31/2011 09:35 AM, malc wrote: > >On Wed, 31 Aug 2011, Anthony Liguori wrote: > > > >>Upper case field names are not okay. If you think coding style isn't clear, > >>that's a bug in coding style. > > > >Sez hu? Coding style is garbage that should be thrown out of the window. > >As for looking, yeah, i'm looking at usb with it's lovely hungarian > >fields, should we stampede to "fix" it? > > > >If the one who's going to maintain the code is fine with whatever naming > >is used so be it. > > No. That's how we got into the coding style mess we're in in the > first place. TBH, the codingstyle in QEMU is the least of "problems" we are facing. We've got lack of documentation, lack of tests, lack of contributors, etc, etc. IMO, those bring codingstyle issues into the pretty much neglectable space. I think we should throw out everything from CS beyond the details of spaces and braces. Maybe keep the 80 char limit. We should ofcourse refer to the C and other specs regarding correctness, like the _t thing, but those are not really stylistic issues. Those are bugs. my 5 cents Cheers
On Wed, Aug 31, 2011 at 4:06 PM, Anthony Liguori <anthony@codemonkey.ws> wrote: > On 08/31/2011 09:35 AM, malc wrote: >> >> On Wed, 31 Aug 2011, Anthony Liguori wrote: >> >>> Upper case field names are not okay. If you think coding style isn't >>> clear, >>> that's a bug in coding style. >> >> Sez hu? Coding style is garbage that should be thrown out of the window. >> As for looking, yeah, i'm looking at usb with it's lovely hungarian >> fields, should we stampede to "fix" it? >> >> If the one who's going to maintain the code is fine with whatever naming >> is used so be it. > > No. That's how we got into the coding style mess we're in in the first > place. > > There's no benefit to going through and changing existing code but new code > needs to be consistent with the vast majority of code in the rest of the > tree. It's about overall code base consistency and maintainability. I agree about importance of consistency, though I'd even go further and reformat globally. New code gets introduced based on copying old code so the pain goes on.
On Wed, Aug 31, 2011 at 4:24 PM, malc <av1474@comtv.ru> wrote: > On Wed, 31 Aug 2011, Anthony Liguori wrote: > >> On 08/31/2011 09:35 AM, malc wrote: >> > On Wed, 31 Aug 2011, Anthony Liguori wrote: >> > >> > > Upper case field names are not okay. If you think coding style isn't >> > > clear, >> > > that's a bug in coding style. >> > >> > Sez hu? Coding style is garbage that should be thrown out of the window. >> > As for looking, yeah, i'm looking at usb with it's lovely hungarian >> > fields, should we stampede to "fix" it? >> > >> > If the one who's going to maintain the code is fine with whatever naming >> > is used so be it. >> >> No. That's how we got into the coding style mess we're in in the first >> place. > > boblycat.org/~malc/right.ogg > >> >> There's no benefit to going through and changing existing code but new code >> needs to be consistent with the vast majority of code in the rest of the tree. >> It's about overall code base consistency and maintainability. >> > > Hand waving, for instance vast majority of the code never used the > mandatory braces, the choice was arbitrary. No, mandatory braces are better than the alternative. The choice has been made and it has been mostly upheld.
On Wed, 31 Aug 2011, Blue Swirl wrote: > On Wed, Aug 31, 2011 at 4:24 PM, malc <av1474@comtv.ru> wrote: > > On Wed, 31 Aug 2011, Anthony Liguori wrote: > > > >> On 08/31/2011 09:35 AM, malc wrote: > >> > On Wed, 31 Aug 2011, Anthony Liguori wrote: > >> > > >> > > Upper case field names are not okay. If you think coding style isn't > >> > > clear, > >> > > that's a bug in coding style. > >> > > >> > Sez hu? Coding style is garbage that should be thrown out of the window. > >> > As for looking, yeah, i'm looking at usb with it's lovely hungarian > >> > fields, should we stampede to "fix" it? > >> > > >> > If the one who's going to maintain the code is fine with whatever naming > >> > is used so be it. > >> > >> No. That's how we got into the coding style mess we're in in the first > >> place. > > > > boblycat.org/~malc/right.ogg > > > >> > >> There's no benefit to going through and changing existing code but new code > >> needs to be consistent with the vast majority of code in the rest of the tree. > >> It's about overall code base consistency and maintainability. > >> > > > > Hand waving, for instance vast majority of the code never used the > > mandatory braces, the choice was arbitrary. > > No, mandatory braces are better than the alternative. The choice has > been made and it has been mostly upheld. > I'm not arguing the merit of braces (though i doubt they has any), i'm saying that it was added without looking at the existing code, and fwiw the only piece of code which consistently used braces was audio/* .
On Wed, Aug 31, 2011 at 5:59 PM, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > On Wed, Aug 31, 2011 at 11:06:11AM -0500, Anthony Liguori wrote: >> On 08/31/2011 09:35 AM, malc wrote: >> >On Wed, 31 Aug 2011, Anthony Liguori wrote: >> > >> >>Upper case field names are not okay. If you think coding style isn't clear, >> >>that's a bug in coding style. >> > >> >Sez hu? Coding style is garbage that should be thrown out of the window. >> >As for looking, yeah, i'm looking at usb with it's lovely hungarian >> >fields, should we stampede to "fix" it? >> > >> >If the one who's going to maintain the code is fine with whatever naming >> >is used so be it. >> >> No. That's how we got into the coding style mess we're in in the >> first place. > > TBH, the codingstyle in QEMU is the least of "problems" we are facing. > We've got lack of documentation, lack of tests, lack of contributors, > etc, etc. IMO, those bring codingstyle issues into the pretty much > neglectable space. The issues you mention are real, but problems in those areas do not make the case for consistency invalid. We could also make rules for documentation. For example, must use doxygen decorations for APIs, must add a file under docs/apis/ when adding an API, must convert old APIs when adding new ones etc. Another way would be to downscale the project to match the resources, we should throw away code which is not maintained and then focus on the good parts. > I think we should throw out everything from CS beyond the details of > spaces and braces. Maybe keep the 80 char limit. I disagree, those rules try to make the code consistent. I don't care much what is the standard (could be Linux kernel style for example), but maintaining the style improves readability and maintainability of the code. > We should ofcourse refer to the C and other specs regarding correctness, > like the _t thing, but those are not really stylistic issues. Those are > bugs. > > my 5 cents > > Cheers > >
On 08/31/2011 12:59 PM, Edgar E. Iglesias wrote: > On Wed, Aug 31, 2011 at 11:06:11AM -0500, Anthony Liguori wrote: >> On 08/31/2011 09:35 AM, malc wrote: >>> On Wed, 31 Aug 2011, Anthony Liguori wrote: >>> >>>> Upper case field names are not okay. If you think coding style isn't clear, >>>> that's a bug in coding style. >>> >>> Sez hu? Coding style is garbage that should be thrown out of the window. >>> As for looking, yeah, i'm looking at usb with it's lovely hungarian >>> fields, should we stampede to "fix" it? >>> >>> If the one who's going to maintain the code is fine with whatever naming >>> is used so be it. >> >> No. That's how we got into the coding style mess we're in in the >> first place. > > TBH, the codingstyle in QEMU is the least of "problems" we are facing. > We've got lack of documentation, lack of tests, lack of contributors, > etc, etc. IMO, those bring codingstyle issues into the pretty much > neglectable space. I don't think we lack contributors. Documentation and tests are really about discipline. If we can't even be bothered to maintain consistency in variable naming, do you really expected that we can be disciplined in writing documentation and tests? Is the next argument going to be that every subsystem should be able to have its documentation in it's preferred natural language such that the documentation for the block layer is in Esperanto? Consistent coding style makes the tree a single code base, instead of a bunch of independent islands. This encourages sharing code and ideas across subsystems. Too often, we reproduce the same thing and over again in different subsystems (and even different machine architectures). Regards, Anthony Liguori > > I think we should throw out everything from CS beyond the details of > spaces and braces. Maybe keep the 80 char limit. > > We should ofcourse refer to the C and other specs regarding correctness, > like the _t thing, but those are not really stylistic issues. Those are > bugs. > > my 5 cents > > Cheers >
On Wed, Aug 31, 2011 at 06:46:37PM +0000, Blue Swirl wrote: > On Wed, Aug 31, 2011 at 5:59 PM, Edgar E. Iglesias > <edgar.iglesias@gmail.com> wrote: > > On Wed, Aug 31, 2011 at 11:06:11AM -0500, Anthony Liguori wrote: > >> On 08/31/2011 09:35 AM, malc wrote: > >> >On Wed, 31 Aug 2011, Anthony Liguori wrote: > >> > > >> >>Upper case field names are not okay. If you think coding style isn't clear, > >> >>that's a bug in coding style. > >> > > >> >Sez hu? Coding style is garbage that should be thrown out of the window. > >> >As for looking, yeah, i'm looking at usb with it's lovely hungarian > >> >fields, should we stampede to "fix" it? > >> > > >> >If the one who's going to maintain the code is fine with whatever naming > >> >is used so be it. > >> > >> No. That's how we got into the coding style mess we're in in the > >> first place. > > > > TBH, the codingstyle in QEMU is the least of "problems" we are facing. > > We've got lack of documentation, lack of tests, lack of contributors, > > etc, etc. IMO, those bring codingstyle issues into the pretty much > > neglectable space. > > The issues you mention are real, but problems in those areas do not > make the case for consistency invalid. We could also make rules for > documentation. For example, must use doxygen decorations for APIs, > must add a file under docs/apis/ when adding an API, must convert old > APIs when adding new ones etc. > > Another way would be to downscale the project to match the resources, > we should throw away code which is not maintained and then focus on > the good parts. > > > I think we should throw out everything from CS beyond the details of > > spaces and braces. Maybe keep the 80 char limit. > > I disagree, those rules try to make the code consistent. I don't care > much what is the standard (could be Linux kernel style for example), > but maintaining the style improves readability and maintainability of > the code. I agree with you to some extent, but consistent code is relative. I dont like seeing patches rejected based on details that IMO don't matter. Of course, what doesn't matter to me, may matter to others. So I accept it and go on... :) I guess that my point is that the improvments in readabilty and maintainnability we get for example on dictating camel case here and there but not on other places is so minimal that the focus we loose and time we spend on looking for those, makes it not worth it. At the moment I'm happy with just seeing that CODING_STYLE doesn't grow though. Cheers
On Wed, Aug 31, 2011 at 01:48:34PM -0500, Anthony Liguori wrote: > On 08/31/2011 12:59 PM, Edgar E. Iglesias wrote: > >On Wed, Aug 31, 2011 at 11:06:11AM -0500, Anthony Liguori wrote: > >>On 08/31/2011 09:35 AM, malc wrote: > >>>On Wed, 31 Aug 2011, Anthony Liguori wrote: > >>> > >>>>Upper case field names are not okay. If you think coding style isn't clear, > >>>>that's a bug in coding style. > >>> > >>>Sez hu? Coding style is garbage that should be thrown out of the window. > >>>As for looking, yeah, i'm looking at usb with it's lovely hungarian > >>>fields, should we stampede to "fix" it? > >>> > >>>If the one who's going to maintain the code is fine with whatever naming > >>>is used so be it. > >> > >>No. That's how we got into the coding style mess we're in in the > >>first place. > > > >TBH, the codingstyle in QEMU is the least of "problems" we are facing. > >We've got lack of documentation, lack of tests, lack of contributors, > >etc, etc. IMO, those bring codingstyle issues into the pretty much > >neglectable space. > > I don't think we lack contributors. Documentation and tests are > really about discipline. If we can't even be bothered to maintain > consistency in variable naming, do you really expected that we can > be disciplined in writing documentation and tests? Yes I do. It's not white and black, it's not about making the code completely inconsistent or 100 consistent. It's about find a level of consistency that is acceptable and doesn't cost too much to maintain. > Is the next argument going to be that every subsystem should be able > to have its documentation in it's preferred natural language such > that the documentation for the block layer is in Esperanto? Heh, I think you missunderstood me. I'm talking about details that dont matter. Like, would you like to enforce American english or UK english? More that kind of question. > Consistent coding style makes the tree a single code base, instead > of a bunch of independent islands. This encourages sharing code and > ideas across subsystems. Too often, we reproduce the same thing and > over again in different subsystems (and even different machine > architectures). I dont think the latter has to do with codingstyle consistensy. But I agree with the former. Cheers
On Wed, Aug 31, 2011 at 09:12:06PM +0200, Edgar E. Iglesias wrote: > On Wed, Aug 31, 2011 at 01:48:34PM -0500, Anthony Liguori wrote: > > On 08/31/2011 12:59 PM, Edgar E. Iglesias wrote: > > >On Wed, Aug 31, 2011 at 11:06:11AM -0500, Anthony Liguori wrote: > > >>On 08/31/2011 09:35 AM, malc wrote: > > >>>On Wed, 31 Aug 2011, Anthony Liguori wrote: > > >>> > > >>>>Upper case field names are not okay. If you think coding style isn't clear, > > >>>>that's a bug in coding style. > > >>> > > >>>Sez hu? Coding style is garbage that should be thrown out of the window. > > >>>As for looking, yeah, i'm looking at usb with it's lovely hungarian > > >>>fields, should we stampede to "fix" it? > > >>> > > >>>If the one who's going to maintain the code is fine with whatever naming > > >>>is used so be it. > > >> > > >>No. That's how we got into the coding style mess we're in in the > > >>first place. > > > > > >TBH, the codingstyle in QEMU is the least of "problems" we are facing. > > >We've got lack of documentation, lack of tests, lack of contributors, > > >etc, etc. IMO, those bring codingstyle issues into the pretty much > > >neglectable space. > > > > I don't think we lack contributors. Documentation and tests are > > really about discipline. If we can't even be bothered to maintain > > consistency in variable naming, do you really expected that we can > > be disciplined in writing documentation and tests? > > Yes I do. It's not white and black, it's not about making the code > completely inconsistent or 100 consistent. It's about find a level > of consistency that is acceptable and doesn't cost too much to > maintain. Now if that's my opinion, then the 99999 dollar question is: why am I wasting so much time on discusing it? And btw Bifferos, don't worry. Not all patches you contribute will cause this much controversy :) Cheers
On 08/31/2011 02:12 PM, Edgar E. Iglesias wrote: > On Wed, Aug 31, 2011 at 01:48:34PM -0500, Anthony Liguori wrote: >>> etc, etc. IMO, those bring codingstyle issues into the pretty much >>> neglectable space. >> >> I don't think we lack contributors. Documentation and tests are >> really about discipline. If we can't even be bothered to maintain >> consistency in variable naming, do you really expected that we can >> be disciplined in writing documentation and tests? > > Yes I do. It's not white and black, it's not about making the code > completely inconsistent or 100 consistent. It's about find a level > of consistency that is acceptable and doesn't cost too much to > maintain. I actually agree. I don't like the idea of absolutely enforcing a coding style that demands no white space at the end of a line (if you can't see it, why in the world would you care?). But coding style deviations that make the code look foreign, like using CamelCase for field names, seems important to me. And I respect that other things seem important to other people (even invisible things like trailing white space). So even though I wouldn't want to reject a patch because of coding style in some cases, I think it's important that we do our best to enforce it. Regards, Anthony Liguori
On Wed, Aug 31, 2011 at 7:23 PM, Anthony Liguori <anthony@codemonkey.ws> wrote: > On 08/31/2011 02:12 PM, Edgar E. Iglesias wrote: >> >> On Wed, Aug 31, 2011 at 01:48:34PM -0500, Anthony Liguori wrote: >>>> >>>> etc, etc. IMO, those bring codingstyle issues into the pretty much >>>> neglectable space. >>> >>> I don't think we lack contributors. Documentation and tests are >>> really about discipline. If we can't even be bothered to maintain >>> consistency in variable naming, do you really expected that we can >>> be disciplined in writing documentation and tests? >> >> Yes I do. It's not white and black, it's not about making the code >> completely inconsistent or 100 consistent. It's about find a level >> of consistency that is acceptable and doesn't cost too much to >> maintain. > > I actually agree. I don't like the idea of absolutely enforcing a coding > style that demands no white space at the end of a line (if you can't see it, > why in the world would you care?). Because then the patches for the line could be mangled by e-mail transport. Actually git apply.whitespace=fix handles this nicely. > But coding style deviations that make the code look foreign, like using > CamelCase for field names, seems important to me. > > And I respect that other things seem important to other people (even > invisible things like trailing white space). So even though I wouldn't want > to reject a patch because of coding style in some cases, I think it's > important that we do our best to enforce it. If the rules are enforced in some case but not in others, this is not fair on submitters.
--- On Wed, 31/8/11, Blue Swirl <blauwirbel@gmail.com> wrote: > I agree about importance of consistency, though I'd even go > further and reformat globally. New code gets introduced > based on copying old code so the pain goes on. I think this is the most important point. I've worked on many projects where coding standards were applied, generally the order is: 1) Someone comes up with some new standards 2) They want to dictate the standards but are not prepared to invest any effort in making the existing code base conform to those standards 3) They decide it's too high risk to modify existing code 4) In conclusion they apply the coding standards to new code only 5) Some... time... passes 6) Evangelist(s) responsible for coding standards leave the team *or* Are convinced by other developers that coding standards suck 7) Repeat from 1), or drop coding standards I've seen this pattern pretty much without fail, over the last 20 years of commercial software development. You never reach the standards utopia unless you retrospectively change all existing code. The only way you ever want to do that is if you have either a) Extensive unit tests or b) Balls Preferably both! So what generally happens in experienced teams, is that you either re-work (only) the modules you touch, or you identify the coding standard of the module you're working on or the ones nearby and follow them. Alternatively some developers write coding standards in a rather vague way such that there are no hard and fast rules. They categorise types of problem and their severity rather than using all rules as rejection criteria. Couple of examples come to mind: When developing either driver or device emulation software it's nice to use the same identifiers as the data sheet. By forcing everything to lower case it makes it harder to cross-reference things, and God forbid if the data sheet actually used case to distinguish registers you'd then have to invent identifiers to deal with it. I looked around to see which license I could use, cribbed one from another Qemu file, and then promptly had to reformat it because it broke the line length rule, so I've reformated what was previously a standard license and crafted my own for no other reason that conforming with checkpatch.pl. cheers, Biff.
Blue Swirl <blauwirbel@gmail.com> writes: > On Wed, Aug 31, 2011 at 4:06 PM, Anthony Liguori <anthony@codemonkey.ws> wrote: >> On 08/31/2011 09:35 AM, malc wrote: >>> >>> On Wed, 31 Aug 2011, Anthony Liguori wrote: >>> >>>> Upper case field names are not okay. If you think coding style isn't >>>> clear, >>>> that's a bug in coding style. >>> >>> Sez hu? Coding style is garbage that should be thrown out of the window. >>> As for looking, yeah, i'm looking at usb with it's lovely hungarian >>> fields, should we stampede to "fix" it? >>> >>> If the one who's going to maintain the code is fine with whatever naming >>> is used so be it. >> >> No. That's how we got into the coding style mess we're in in the first >> place. >> >> There's no benefit to going through and changing existing code but new code >> needs to be consistent with the vast majority of code in the rest of the >> tree. It's about overall code base consistency and maintainability. > > I agree about importance of consistency, though I'd even go further > and reformat globally. New code gets introduced based on copying old > code so the pain goes on. If we reformat globally (big if), then we better reformat to a well-established style (such as linux kernel), not to this idiosyncratic QEMU-only style. Because the arguments about sharing and readability apply across projects, too.
On 08/31/2011 04:05 AM, bifferos wrote: > + pci_register_bar(&s->dev, 0, 0x100, PCI_BASE_ADDRESS_SPACE_IO, r6040_map); > + > pci_register_bar() has been converted to the memory API. Please rebase to the latest qemu.git master and read docs/memory.txt and memory.h.
On 08/31/11 18:06, Anthony Liguori wrote: > On 08/31/2011 09:35 AM, malc wrote: >> On Wed, 31 Aug 2011, Anthony Liguori wrote: >> >>> Upper case field names are not okay. If you think coding style isn't >>> clear, >>> that's a bug in coding style. > There's no benefit to going through and changing existing code but new > code needs to be consistent with the vast majority of code in the rest > of the tree. It's about overall code base consistency and maintainability. I don't see the point in being too picky about this. Especially consistency is a tricky thing. As USB has been raised in this thread already: The struct field names in the USBDesc* structs are actually consistent. They are consistent with the naming convention in the USB world, i.e. you'll find those funky names like "wMaxPacketSize" in the USB specs, in the "lsusb -v" output and probably other places too. Which makes alot of sense IMHO and is quite convenient when hacking the USB code. The USB naming convention and the qemu naming convention don't match, so it is impossible to please everybody here ... cheers, Gerd
On 09/01/2011 02:39 AM, Markus Armbruster wrote: > Blue Swirl<blauwirbel@gmail.com> writes: > >> On Wed, Aug 31, 2011 at 4:06 PM, Anthony Liguori<anthony@codemonkey.ws> wrote: >>> On 08/31/2011 09:35 AM, malc wrote: >>>> >>>> On Wed, 31 Aug 2011, Anthony Liguori wrote: >>>> >>>>> Upper case field names are not okay. If you think coding style isn't >>>>> clear, >>>>> that's a bug in coding style. >>>> >>>> Sez hu? Coding style is garbage that should be thrown out of the window. >>>> As for looking, yeah, i'm looking at usb with it's lovely hungarian >>>> fields, should we stampede to "fix" it? >>>> >>>> If the one who's going to maintain the code is fine with whatever naming >>>> is used so be it. >>> >>> No. That's how we got into the coding style mess we're in in the first >>> place. >>> >>> There's no benefit to going through and changing existing code but new code >>> needs to be consistent with the vast majority of code in the rest of the >>> tree. It's about overall code base consistency and maintainability. >> >> I agree about importance of consistency, though I'd even go further >> and reformat globally. New code gets introduced based on copying old >> code so the pain goes on. > > If we reformat globally (big if), I'm very strongly opposed to doing a global reformat. It makes it harder to use things like git blame which makes reviewing code difficult. Following a reasonable policy of using a consistent coding style and only fixing style issues when you touch code for other reasons is well established (this is the kernel policy) and over time will result in a reasonably consistent code base. Regards, Anthony Liguori Regards, Anthony Liguori
On Thu, Sep 1, 2011 at 9:49 PM, Anthony Liguori <anthony@codemonkey.ws> wrote: > On 09/01/2011 02:39 AM, Markus Armbruster wrote: >> >> Blue Swirl<blauwirbel@gmail.com> writes: >> >>> On Wed, Aug 31, 2011 at 4:06 PM, Anthony Liguori<anthony@codemonkey.ws> >>> wrote: >>>> >>>> On 08/31/2011 09:35 AM, malc wrote: >>>>> >>>>> On Wed, 31 Aug 2011, Anthony Liguori wrote: >>>>> >>>>>> Upper case field names are not okay. If you think coding style isn't >>>>>> clear, >>>>>> that's a bug in coding style. >>>>> >>>>> Sez hu? Coding style is garbage that should be thrown out of the >>>>> window. >>>>> As for looking, yeah, i'm looking at usb with it's lovely hungarian >>>>> fields, should we stampede to "fix" it? >>>>> >>>>> If the one who's going to maintain the code is fine with whatever >>>>> naming >>>>> is used so be it. >>>> >>>> No. That's how we got into the coding style mess we're in in the first >>>> place. >>>> >>>> There's no benefit to going through and changing existing code but new >>>> code >>>> needs to be consistent with the vast majority of code in the rest of the >>>> tree. It's about overall code base consistency and maintainability. >>> >>> I agree about importance of consistency, though I'd even go further >>> and reformat globally. New code gets introduced based on copying old >>> code so the pain goes on. >> >> If we reformat globally (big if), > > I'm very strongly opposed to doing a global reformat. It makes it harder to > use things like git blame which makes reviewing code difficult. Only for one commit, the commits before and after would still be accurate. There is a tradeoff between benefit of consistent, better quality code and usefulness of git blame. I'd value consistency higher. Inaccuracy is also relative, the reformatting commit is still a commit. > Following a reasonable policy of using a consistent coding style and only > fixing style issues when you touch code for other reasons is well > established (this is the kernel policy) and over time will result in a > reasonably consistent code base. This assumes that all code gets touched every now and then. But in reality some things may never need any changes and then they remain inconsistent. Then bad practices spread from this unchanged base if it's large enough, just like now. By the way, if we assume the opposite (all code is under change), then some time after global reformat git blame would also be accurate since the reformatting commit would be overwritten by the newer changes.
diff --git a/Makefile.objs b/Makefile.objs index 6991a9f..7d87503 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -240,6 +240,7 @@ hw-obj-$(CONFIG_PCNET_PCI) += pcnet-pci.o hw-obj-$(CONFIG_PCNET_COMMON) += pcnet.o hw-obj-$(CONFIG_E1000_PCI) += e1000.o hw-obj-$(CONFIG_RTL8139_PCI) += rtl8139.o +hw-obj-$(CONFIG_R6040_PCI) += r6040.o hw-obj-$(CONFIG_SMC91C111) += smc91c111.o hw-obj-$(CONFIG_LAN9118) += lan9118.o diff --git a/default-configs/pci.mak b/default-configs/pci.mak index 22bd350..d2ea7a2 100644 --- a/default-configs/pci.mak +++ b/default-configs/pci.mak @@ -10,6 +10,7 @@ CONFIG_PCNET_PCI=y CONFIG_PCNET_COMMON=y CONFIG_LSI_SCSI_PCI=y CONFIG_RTL8139_PCI=y +CONFIG_R6040_PCI=y CONFIG_E1000_PCI=y CONFIG_IDE_CORE=y CONFIG_IDE_QDEV=y diff --git a/hw/pci.c b/hw/pci.c index b904a4e..7e12935 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -1527,6 +1527,7 @@ static const char * const pci_nic_models[] = { "rtl8139", "e1000", "pcnet", + "r6040", "virtio", NULL }; @@ -1539,6 +1540,7 @@ static const char * const pci_nic_names[] = { "rtl8139", "e1000", "pcnet", + "r6040", "virtio-net-pci", NULL }; diff --git a/hw/r6040.c b/hw/r6040.c new file mode 100644 index 0000000..83587e7 --- /dev/null +++ b/hw/r6040.c @@ -0,0 +1,627 @@ +/* + Emulation of r6040 ethernet controller found in a number of SoCs. + Copyright (c) 2011 Mark Kelly, mark@bifferos.com + + This has been written using the R8610[1] and ip101a[2] datasheets. + + ICs with the embedded controller include R8610, R3210, AMRISC20000 + and Vortex86SX + + The emulation seems good enough to fool Linux 2.6.37.6. It is + not perfect, but has proven useful. + + [1] http://www.sima.com.tw/download/R8610_D06_20051003.pdf + [2] http://www.icplus.com.tw/pp-IP101A.html + */ + +#include "hw.h" +#include "pci.h" +#include "net.h" +#include "loader.h" +#include "sysemu.h" +#include "qemu-timer.h" + +/* #define DEBUG_R6040 1 */ + + +#if defined DEBUG_R6040 +#define DPRINTF(fmt, ...) \ + do { fprintf(stderr, "R6040: " fmt, ## __VA_ARGS__); } while (0) +#else +static inline GCC_FMT_ATTR(1, 2) int DPRINTF(const char *fmt, ...) +{ + return 0; +} +#endif + + +/* Cast in order of appearance. _W prevfix means it's used to index the + register word array (RegsW) + */ + +#define MPSCCR_W (0x88 / 2) + +#define MAC0_W (0x68 / 2) +#define MAC1_W (0x6a / 2) +#define MAC2_W (0x6c / 2) + + +#define RX_START_LOW_W (0x34 / 2) +#define RX_START_HIGH_W (0x38 / 2) +#define TX_PKT_COUNT_W (0x5a / 2) +#define RX_PKT_COUNT_W (0x50 / 2) + + +#define MCR0_W (0x00 / 2) +#define MCR1_W (0x04 / 2) +#define BIT_MRST (1 << 0) + +#define MTPR_W (0x14 / 2) +#define MRBSR_W (0x18 / 2) +#define MISR_W (0x3c / 2) +#define MIER_W (0x40 / 2) + +#define MMDIO_W (0x20 / 2) +#define MDIO_READ_W (0x24 / 2) +#define MDIO_WRITE_W (0x28 / 2) + +#define MRCNT_W (0x50 / 2) +#define MTCNT_W (0x5c / 2) + + +#define MDIO_WRITE 0x4000 +#define MDIO_READ 0x2000 + + +typedef struct R6040State { + PCIDevice dev; + NICState *nic; + NICConf conf; + + /* PHY related register sets */ + uint16_t MID0[3]; + uint16_t phy_regs[32]; + uint32_t phy_op_in_progress; + + /* Primary IO address space */ + union { + uint8_t RegsB[0x100]; /* Byte access */ + uint16_t RegsW[0x100/2]; /* word access */ + uint32_t RegsL[0x100/4]; /* DWORD access */ + }; + +} R6040State; + + +/* some inlines to help access above structure */ +static inline uint32_t TX_START(R6040State *s) +{ + uint32_t tmp = s->RegsW[0x2c/2]; + return tmp | (s->RegsW[0x30/2] << 16); +} + +static inline void TX_START_SET(R6040State *s, uint32_t start) +{ + s->RegsW[0x2c/2] = start & 0xffff; + s->RegsW[0x30/2] = (start >> 16) & 0xffff; +} + +static inline uint32_t RX_START(R6040State *s) +{ + uint32_t tmp = s->RegsW[0x34/2]; + return tmp | (s->RegsW[0x38/2] << 16); +} + +static inline void RX_START_SET(R6040State *s, uint32_t start) +{ + s->RegsW[0x34/2] = start & 0xffff; + s->RegsW[0x38/2] = (start >> 16) & 0xffff; +} + + +static void r6040_update_irq(R6040State *s) +{ + uint16_t isr = s->RegsW[MISR_W] & s->RegsW[MIER_W]; + + qemu_set_irq(s->dev.irq[0], isr ? 1 : 0); +} + + +/* Mark auto-neg complete, NIC up. */ +static void PhysicalLinkUp(void *opaque) +{ + R6040State *s = opaque; + s->phy_regs[1] |= (1 << 2); +} + + +/* Transmit and receive descriptors are doubled up + One is a subset of the other anyhow + */ +typedef struct descriptor { + uint16_t DST; + uint16_t DLEN; + uint32_t DBP; + uint32_t DNX; + uint16_t HIDX; + uint16_t Reserved1; + uint16_t Reserved2; +} descriptor_t; + + +/* Some debugging functions */ + +#ifdef DEBUG_R6040 +static void addr_dump16(const char *name, uint16_t val) +{ + DPRINTF("%s: 0x%04x ", name, val); +} + +static void addr_dump32(const char *name, uint32_t val) +{ + DPRINTF("%s: 0x%x ", name, val); +} + +static void hex_dump(const uint8_t *data, uint32_t len) +{ + uint8_t i; + DPRINTF("hex: "); + for (i = 0; i < len; i++) { + fprintf(stderr, "%02x ", *data); + if (i && !(i % 0x20)) { + fprintf(stderr, "\n"); + } + data++; + } + fprintf(stderr, "\n"); +} + +static void desc_dump(descriptor_t *d, uint32_t addr) +{ + DPRINTF("\nDumping: 0x%x\n", addr); + addr_dump16("DST", d->DST); + addr_dump16("DLEN", d->DLEN); + addr_dump32("DBP", (unsigned long)d->DBP); + addr_dump32("DNX", (unsigned long)d->DNX); + addr_dump16("HIDX", d->HIDX); + printf("\n"); +} + +static void dump_phys_mem(uint32_t addr, int len) +{ + uint8_t buffer[1024]; + cpu_physical_memory_read(addr, buffer, len); + hex_dump(buffer, len); +} + +static void dump_pci(uint8_t *pci_conf) +{ + uint32_t *p = (uint32_t *)pci_conf; + int i = 0; + for (i = 0; i < 0x40; i += 4) { + DPRINTF("Addr: 0x%08x, Data: 0x%08x\n", i, *p); + p++; + } +} +#endif + + +static const VMStateDescription vmstate_r6040 = { + .name = "r6040", + .version_id = 3, + .minimum_version_id = 2, + .minimum_version_id_old = 2, + .fields = (VMStateField[]) { + VMSTATE_PCI_DEVICE(dev, R6040State), + VMSTATE_BUFFER(RegsB, R6040State), + VMSTATE_UINT16_ARRAY(MID0, R6040State, 3), + VMSTATE_UINT16_ARRAY(phy_regs, R6040State, 32), + VMSTATE_UINT32(phy_op_in_progress, R6040State), + VMSTATE_MACADDR(conf.macaddr, R6040State), + VMSTATE_END_OF_LIST() + } +}; + + +static int TryToSendOnePacket(void *opaque) +{ + R6040State *s = opaque; + descriptor_t d; + uint8_t pkt_buffer[2000]; + uint32_t tocopy; + + cpu_physical_memory_read(TX_START(s), (uint8_t *)&d, sizeof(d)); + + if (d.DST & 0x8000) { /* MAC owns it? */ + tocopy = d.DLEN; + if (tocopy > sizeof(pkt_buffer)) { + tocopy = sizeof(pkt_buffer); + } + /* copy the packet to send it */ + cpu_physical_memory_read(d.DBP, pkt_buffer, tocopy); + + qemu_send_packet(&s->nic->nc, pkt_buffer, tocopy); + s->RegsW[TX_PKT_COUNT_W]++; + + /* relinquish ownership, we're done with it */ + d.DST &= ~0x8000; + + /* Copy the new version of the descriptor back */ + cpu_physical_memory_write(TX_START(s), (uint8_t *)&d, sizeof(d)); + + /* Advance to the next buffer if packet processed */ + TX_START_SET(s, d.DNX); + + return 1; + } + + return 0; +} + + +static void r6040_transmit(void *opaque) +{ + R6040State *s = opaque; + int count = 0; + + while (TryToSendOnePacket(s)) { + ++count; + } + + if (count) { + s->RegsW[MISR_W] |= 0x10; + r6040_update_irq(s); + } +} + + +/* Whether to allow callback returning 1 for yes, can receive */ +static int r6040_can_receive(VLANClientState *nc) +{ + R6040State *s = DO_UPCAST(NICState, nc, nc)->opaque; + int tmp = (s->RegsW[0] & (1 << 1)) ? 1 : 0; + return tmp; +} + + +static int ReceiveOnePacket(void *opaque, const uint8_t *buf, size_t len) +{ + R6040State *s = opaque; + uint32_t tocopy = len+4; /* include checksum */ + descriptor_t d; + + cpu_physical_memory_read(RX_START(s), (uint8_t *)&d, sizeof(descriptor_t)); + /*desc_dump(&d, 0);*/ + + if (d.DST & 0x8000) { /* MAC owned? */ + + uint16_t max_buffer = s->RegsW[MRBSR_W] & 0x07fc; + if (tocopy > max_buffer) { + tocopy = max_buffer; + } + + cpu_physical_memory_write(d.DBP, buf, tocopy-4); + + /* indicate received OK */ + d.DST |= (1 << 14); + d.DLEN = tocopy; + /* relinquish ownership */ + d.DST &= ~0x8000; + + /* Copy the descriptor back */ + cpu_physical_memory_write(RX_START(s), (uint8_t *)&d, + sizeof(descriptor_t)); + + s->RegsW[RX_PKT_COUNT_W]++; + + s->RegsW[MISR_W] |= 1; /* received pkt interrupt */ + + r6040_update_irq(s); + + RX_START_SET(s, d.DNX); /* advance */ + + return 0; + } + return -1; +} + + +/* called on incoming packets */ +static ssize_t r6040_receive(VLANClientState *nc, const uint8_t *buf, + size_t len) +{ + R6040State *s = DO_UPCAST(NICState, nc, nc)->opaque; + DPRINTF("Received incoming packet of len %ld\n", len); + + if (0 == ReceiveOnePacket(s, buf, len)) { + return len; /* copied OK */ + } + + return 0; +} + + +static void r6040_cleanup(VLANClientState *vc) +{ + DPRINTF("r6040_cleanup\n"); +} + + +static inline int BIT_SET(uint16_t old, uint16_t new, uint16_t bit) +{ + uint16_t before = (old & (1 << bit)); + uint16_t after = (new & (1 << bit)); + if (!before && after) { + return 1; + } + return 0; +} + + +static void r6040_ioport_writew(void *opaque, uint32_t addr, uint32_t val) +{ + R6040State *s = opaque; + uint16_t old; + addr &= 0xff; /* get relative to base address */ + addr /= 2; /* Get the offset into the word-array */ + old = s->RegsW[addr]; /* store the old value for future use */ + + switch (addr) { + case MCR0_W: /* 0x00 */ + if (BIT_SET(old, val, 12)) { + r6040_transmit(opaque); + } + break; + case MCR1_W: /* 0x04 */ + if (val & BIT_MRST) { /* reset request incoming */ + /* reset requested, complete it immediately, set this value to + default */ + val = 0x0010; + } + break; + case MTPR_W: /* TX command reg, 0x14 */ + if (val & 1) { + r6040_transmit(opaque); + val &= ~1; + } + break; + case MMDIO_W: /* MDIO control, 0x20 */ + { + int phy_exists = ((val & 0x1f00) == 0x100) ? 1 : 0; + uint16_t *phy = s->phy_regs; + phy += (val & 0x1f); + + if (val & (1 << 13)) { /* read data */ + if (phy_exists) { + s->RegsW[MDIO_READ_W] = *phy; + } else { + s->RegsW[MDIO_READ_W] = 0xffff; + } + } else if (val & (1 << 14)) { /* write data */ + if (phy_exists) { + *phy = s->RegsW[MDIO_WRITE_W]; + } + } + + /* Whether you request to read or write, both bits go high while + the operation is in progress, e.g. tell it to read, and the + write-in-progress flag also goes high */ + val |= 0x6000; /* signal operation has started */ + s->phy_op_in_progress = 1; + + break; + } + case MISR_W: /* interrupt status reg (read to clear), 0x3c */ + return; + + case MIER_W: /* interrupt enable register, 0x40 */ + s->RegsW[MIER_W] = val; + r6040_update_irq(s); + return; + + case MRCNT_W: /* 0x50 */ + case MTCNT_W: /* 0x5c */ + return; /* Can't write to pkt count registers, skip */ + + } + s->RegsW[addr] = val & 0xFFFF; +} + + + +static uint32_t r6040_ioport_readw(void *opaque, uint32_t addr) +{ + R6040State *s = opaque; + addr &= 0xff; /* get relative to base address */ + addr /= 2; /* Get the offset into the word-array */ + uint32_t tmp = s->RegsW[addr]; /* get the value */ + + switch (addr) { + + case MMDIO_W: /* MDIO control, 0x20 */ + { + /* Clear any in-progress MDIO activity for the next read + This simulates the polling of the MDIO operation status, + so the driver code always has to read the register twice + before it thinks the operation is complete. */ + if (s->phy_op_in_progress) { + s->RegsW[addr] &= ~0x6000; + s->phy_op_in_progress = 0; + } + break; + } + case MISR_W: /* interrupt status reg (read to clear) 0x3c */ + s->RegsW[addr] = 0; + break; + case MIER_W: /* interrupt enable reg 0x40 */ + break; + case MRCNT_W: /* 0x50 */ + case MTCNT_W: /* 0x5c */ + s->RegsW[addr] = 0; /* read to clear */ + break; + default: + break; + } + return tmp; +} + + +/* byte and long access are routed via the word operation handlers */ +static void r6040_ioport_writeb(void *opaque, uint32_t addr, uint32_t val) +{ + R6040State *s = opaque; + addr &= 0xFF; + val &= 0xFF; + uint16_t old = s->RegsW[addr/2]; /* get the current value */ + if (addr & 1) { + old &= 0xff; + old |= (val << 8); + } else { + old &= 0xff00; + old |= val; + } + + r6040_ioport_writew(opaque, addr, old); /* call the word-based version */ +} + +static void r6040_ioport_writel(void *opaque, uint32_t addr, uint32_t val) +{ + /* Set the low value */ + r6040_ioport_writew(opaque, addr, val & 0xffff); + /* Set the high value */ + r6040_ioport_writew(opaque, addr+2, (val >> 16) & 0xffff); +} + +static uint32_t r6040_ioport_readb(void *opaque, uint32_t addr) +{ + uint32_t tmp = r6040_ioport_readw(opaque, addr & ~1); + if (addr & 1) { + return (tmp & 0xff00) >> 8; + } + return tmp & 0xff; +} + +static uint32_t r6040_ioport_readl(void *opaque, uint32_t addr) +{ + uint32_t tmp = r6040_ioport_readw(opaque, addr); + return tmp | (r6040_ioport_readw(opaque, addr+2) << 16); +} + + +static void r6040_register_ioports(R6040State *s, pcibus_t addr) +{ + register_ioport_write(addr, 0x100, 1, r6040_ioport_writeb, s); + register_ioport_read(addr, 0x100, 1, r6040_ioport_readb, s); + + register_ioport_write(addr, 0x100, 2, r6040_ioport_writew, s); + register_ioport_read(addr, 0x100, 2, r6040_ioport_readw, s); + + register_ioport_write(addr, 0x100, 4, r6040_ioport_writel, s); + register_ioport_read(addr, 0x100, 4, r6040_ioport_readl, s); +} + + +static void r6040_map(PCIDevice *pci_dev, int region_num, + pcibus_t addr, pcibus_t size, int type) +{ + R6040State *s = DO_UPCAST(R6040State, dev, pci_dev);; + + DPRINTF("## Mapping to address %lx\n", addr); + r6040_register_ioports(s, addr); +} + + +static NetClientInfo net_r6040_info = { + .type = NET_CLIENT_TYPE_NIC, + .size = sizeof(NICState), + .can_receive = r6040_can_receive, + .receive = r6040_receive, + .cleanup = r6040_cleanup, +}; + + +static int r6040_init_pci(PCIDevice *dev) +{ + QEMUTimer *timer; + + R6040State *s = DO_UPCAST(R6040State, dev, dev); + uint8_t *pci_conf; + + /* MAC PHYS status change register. Linux driver expects something + sensible as default and if not will try to set it */ + s->RegsW[MPSCCR_W] = 0x9f07; + + /* Default value for maximum packet size */ + s->RegsW[MRBSR_W] = 0x600; + + /* set the MAC, linux driver reads this when it loads, it is + normally set by the BIOS, but obviously Qemu BIOS isn't going + to do that */ + s->RegsW[MAC0_W] = 0x5452; + s->RegsW[MAC1_W] = 0x1200; + s->RegsW[MAC2_W] = 0x5734; + + /* Tell Qemu the same thing */ + s->conf.macaddr.a[0] = s->RegsW[MAC0_W] & 0xff; + s->conf.macaddr.a[1] = (s->RegsW[MAC0_W] >> 8) & 0xff; + s->conf.macaddr.a[2] = s->RegsW[MAC1_W] & 0xff; + s->conf.macaddr.a[3] = (s->RegsW[MAC1_W] >> 8) & 0xff; + s->conf.macaddr.a[4] = s->RegsW[MAC2_W] & 0xff; + s->conf.macaddr.a[5] = (s->RegsW[MAC2_W] >> 8) & 0xff; + + /* no commands running */ + s->phy_op_in_progress = 0; + + /* PHY auto-neg in progress */ + s->phy_regs[1] = 0x786d & ~(1 << 2); + s->phy_regs[2] = 0x0243; + s->phy_regs[3] = 0x0c54; + + pci_conf = (uint8_t *)s->dev.config; + + pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; /* header_type */ + pci_conf[PCI_INTERRUPT_LINE] = 0xa; /* interrupt line */ + pci_conf[PCI_INTERRUPT_PIN] = 1; /* interrupt pin 0 */ + + pci_register_bar(&s->dev, 0, 0x100, PCI_BASE_ADDRESS_SPACE_IO, r6040_map); + + s->nic = qemu_new_nic(&net_r6040_info, &s->conf, + dev->qdev.info->name, dev->qdev.id, s); + + qemu_format_nic_info_str(&s->nic->nc, s->conf.macaddr.a); + + /* Simulate a delay of a couple of seconds for the link to come up. + Not required for Linux but very handy for developing BIOS code. + */ + timer = qemu_new_timer_ns(vm_clock, PhysicalLinkUp, s); + qemu_mod_timer(timer, qemu_get_clock_ms(vm_clock) + 1500000000); + + /* Register IO port access as well */ + r6040_register_ioports(s, 0xe800); + + return 0; +} + + +static PCIDeviceInfo r6040_info = { + .qdev.name = "r6040", + .qdev.size = sizeof(R6040State), + .qdev.vmsd = &vmstate_r6040, + .init = r6040_init_pci, + .vendor_id = 0x17f3, /* RDC */ + .device_id = 0x6040, /* r6040 nic */ + .class_id = PCI_CLASS_NETWORK_ETHERNET, + .qdev.props = (Property[]) { + DEFINE_NIC_PROPERTIES(R6040State, conf), + DEFINE_PROP_END_OF_LIST(), + } +}; + + +static void r6040_register(void) +{ + pci_qdev_register(&r6040_info); +} + + +device_init(r6040_register)
Signed-off-by: Mark Kelly <mark@bifferos.com>