Received: from eggs.gnu.org ([2001:4830:134:3::10]:60785) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YzjzA-0007Ju-NR for qemu-devel@nongnu.org; Tue, 02 Jun 2015 07:03:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yzjz9-0004zs-70 for qemu-devel@nongnu.org; Tue, 02 Jun 2015 07:03:56 -0400 Received: from hall.aurel32.net ([2001:bc8:30d7:101::1]:59808) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yzjz8-0004zI-Us for qemu-devel@nongnu.org; Tue, 02 Jun 2015 07:03:55 -0400 Date: Tue, 2 Jun 2015 13:03:54 +0200 From: Aurelien Jarno Message-ID: <20150602110354.GI26298@aurel32.net> References: <1432729200-5322-1-git-send-email-hpoussin@reactos.org> <1432729200-5322-10-git-send-email-hpoussin@reactos.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <1432729200-5322-10-git-send-email-hpoussin@reactos.org> Sender: Aurelien Jarno Subject: Re: [Qemu-devel] [PATCH v2 09/17] net/dp8393x: do not use old_mmio accesses List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?iso-8859-15?Q?Herv=E9?= Poussineau Cc: Leon Alrae , qemu-devel@nongnu.org On 2015-05-27 14:19, Herv=E9 Poussineau wrote: > Signed-off-by: Herv=E9 Poussineau > --- > hw/net/dp8393x.c | 114 ++++++++++++++-----------------------------------= ------ > 1 file changed, 29 insertions(+), 85 deletions(-) >=20 > diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c > index 093f0cc..5cc1e6b 100644 > --- a/hw/net/dp8393x.c > +++ b/hw/net/dp8393x.c > @@ -473,8 +473,10 @@ static void do_command(dp8393xState *s, uint16_t com= mand) > do_load_cam(s); > } > =20 > -static uint16_t read_register(dp8393xState *s, int reg) > +static uint64_t dp8393x_read(void *opaque, hwaddr addr, unsigned int siz= e) > { > + dp8393xState *s =3D opaque; > + int reg =3D addr >> s->it_shift; > uint16_t val =3D 0; > =20 > switch (reg) { > @@ -503,14 +505,18 @@ static uint16_t read_register(dp8393xState *s, int = reg) > return val; > } > =20 > -static void write_register(dp8393xState *s, int reg, uint16_t val) > +static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data, > + unsigned int size) > { > - DPRINTF("write 0x%04x to reg %s\n", val, reg_names[reg]); > + dp8393xState *s =3D opaque; > + int reg =3D addr >> s->it_shift; > + > + DPRINTF("write 0x%04x to reg %s\n", (uint16_t)data, reg_names[reg]); > =20 > switch (reg) { > /* Command register */ > case SONIC_CR: > - do_command(s, val); > + do_command(s, data); > break; > /* Prevent write to read-only registers */ > case SONIC_CAP2: > @@ -523,36 +529,36 @@ static void write_register(dp8393xState *s, int reg= , uint16_t val) > /* Accept write to some registers only when in reset mode */ > case SONIC_DCR: > if (s->regs[SONIC_CR] & SONIC_CR_RST) { > - s->regs[reg] =3D val & 0xbfff; > + s->regs[reg] =3D data & 0xbfff; > } else { > DPRINTF("writing to DCR invalid\n"); > } > break; > case SONIC_DCR2: > if (s->regs[SONIC_CR] & SONIC_CR_RST) { > - s->regs[reg] =3D val & 0xf017; > + s->regs[reg] =3D data & 0xf017; > } else { > DPRINTF("writing to DCR2 invalid\n"); > } > break; > /* 12 lower bytes are Read Only */ > case SONIC_TCR: > - s->regs[reg] =3D val & 0xf000; > + s->regs[reg] =3D data & 0xf000; > break; > /* 9 lower bytes are Read Only */ > case SONIC_RCR: > - s->regs[reg] =3D val & 0xffe0; > + s->regs[reg] =3D data & 0xffe0; > break; > /* Ignore most significant bit */ > case SONIC_IMR: > - s->regs[reg] =3D val & 0x7fff; > + s->regs[reg] =3D data & 0x7fff; > dp8393x_update_irq(s); > break; > /* Clear bits by writing 1 to them */ > case SONIC_ISR: > - val &=3D s->regs[reg]; > - s->regs[reg] &=3D ~val; > - if (val & SONIC_ISR_RBE) { > + data &=3D s->regs[reg]; > + s->regs[reg] &=3D ~data; > + if (data & SONIC_ISR_RBE) { > do_read_rra(s); > } > dp8393x_update_irq(s); > @@ -562,17 +568,17 @@ static void write_register(dp8393xState *s, int reg= , uint16_t val) > case SONIC_REA: > case SONIC_RRP: > case SONIC_RWP: > - s->regs[reg] =3D val & 0xfffe; > + s->regs[reg] =3D data & 0xfffe; > break; > /* Invert written value for some registers */ > case SONIC_CRCT: > case SONIC_FAET: > case SONIC_MPT: > - s->regs[reg] =3D val ^ 0xffff; > + s->regs[reg] =3D data ^ 0xffff; > break; > /* All other registers have no special contrainst */ > default: > - s->regs[reg] =3D val; > + s->regs[reg] =3D data; > } > =20 > if (reg =3D=3D SONIC_WT0 || reg =3D=3D SONIC_WT1) { > @@ -580,6 +586,14 @@ static void write_register(dp8393xState *s, int reg,= uint16_t val) > } > } > =20 > +static const MemoryRegionOps dp8393x_ops =3D { > + .read =3D dp8393x_read, > + .write =3D dp8393x_write, > + .impl.min_access_size =3D 2, > + .impl.max_access_size =3D 2, > + .endianness =3D DEVICE_NATIVE_ENDIAN, > +}; > + > static void dp8393x_watchdog(void *opaque) > { > dp8393xState *s =3D opaque; > @@ -597,76 +611,6 @@ static void dp8393x_watchdog(void *opaque) > dp8393x_update_irq(s); > } > =20 > -static uint32_t dp8393x_readw(void *opaque, hwaddr addr) > -{ > - dp8393xState *s =3D opaque; > - int reg; > - > - if ((addr & ((1 << s->it_shift) - 1)) !=3D 0) { > - return 0; > - } > - > - reg =3D addr >> s->it_shift; > - return read_register(s, reg); > -} > - > -static uint32_t dp8393x_readb(void *opaque, hwaddr addr) > -{ > - uint16_t v =3D dp8393x_readw(opaque, addr & ~0x1); > - return (v >> (8 * (addr & 0x1))) & 0xff; > -} > - > -static uint32_t dp8393x_readl(void *opaque, hwaddr addr) > -{ > - uint32_t v; > - v =3D dp8393x_readw(opaque, addr); > - v |=3D dp8393x_readw(opaque, addr + 2) << 16; > - return v; > -} > - > -static void dp8393x_writew(void *opaque, hwaddr addr, uint32_t val) > -{ > - dp8393xState *s =3D opaque; > - int reg; > - > - if ((addr & ((1 << s->it_shift) - 1)) !=3D 0) { > - return; > - } > - > - reg =3D addr >> s->it_shift; > - > - write_register(s, reg, (uint16_t)val); > -} > - > -static void dp8393x_writeb(void *opaque, hwaddr addr, uint32_t val) > -{ > - uint16_t old_val =3D dp8393x_readw(opaque, addr & ~0x1); > - > - switch (addr & 3) { > - case 0: > - val =3D val | (old_val & 0xff00); > - break; > - case 1: > - val =3D (val << 8) | (old_val & 0x00ff); > - break; > - } > - dp8393x_writew(opaque, addr & ~0x1, val); > -} > - > -static void dp8393x_writel(void *opaque, hwaddr addr, uint32_t val) > -{ > - dp8393x_writew(opaque, addr, val & 0xffff); > - dp8393x_writew(opaque, addr + 2, (val >> 16) & 0xffff); > -} > - > -static const MemoryRegionOps dp8393x_ops =3D { > - .old_mmio =3D { > - .read =3D { dp8393x_readb, dp8393x_readw, dp8393x_readl, }, > - .write =3D { dp8393x_writeb, dp8393x_writew, dp8393x_writel, }, > - }, > - .endianness =3D DEVICE_NATIVE_ENDIAN, > -}; > - > static int nic_can_receive(NetClientState *nc) > { > dp8393xState *s =3D qemu_get_nic_opaque(nc); Nice cleanup :) Reviewed-by: Aurelien Jarno --=20 Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net