From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38115) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z0Q7E-0003Qd-Bo for qemu-devel@nongnu.org; Thu, 04 Jun 2015 04:03:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z0Q7B-0008Sz-L7 for qemu-devel@nongnu.org; Thu, 04 Jun 2015 04:03:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51199) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z0Q7B-0008Ss-Ej for qemu-devel@nongnu.org; Thu, 04 Jun 2015 04:03:01 -0400 Message-ID: <55700630.7090006@redhat.com> Date: Thu, 04 Jun 2015 10:02:56 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1433351328-23326-1-git-send-email-pbonzini@redhat.com> <1433351328-23326-9-git-send-email-pbonzini@redhat.com> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 08/23] pflash_cfi01: change to new-style MMIO accessors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: "Michael S. Tsirkin" , Laszlo Ersek , "qemu-devel@nongnu.org Developers" , Gerd Hoffmann On 04/06/2015 08:19, Peter Crosthwaite wrote: > On Wed, Jun 3, 2015 at 10:08 AM, Paolo Bonzini wrote: >> This is a required step to implement read_with_attrs and write_with_attrs. >> >> Signed-off-by: Paolo Bonzini >> --- >> hw/block/pflash_cfi01.c | 96 ++++++------------------------------------------- > > Nice stats. > >> 1 file changed, 10 insertions(+), 86 deletions(-) >> >> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c >> index 7507a15..0b3667a 100644 >> --- a/hw/block/pflash_cfi01.c >> +++ b/hw/block/pflash_cfi01.c >> @@ -650,101 +650,25 @@ static void pflash_write(pflash_t *pfl, hwaddr offset, >> } >> >> >> -static uint32_t pflash_readb_be(void *opaque, hwaddr addr) >> -{ >> - return pflash_read(opaque, addr, 1, 1); >> -} >> - >> -static uint32_t pflash_readb_le(void *opaque, hwaddr addr) >> -{ >> - return pflash_read(opaque, addr, 1, 0); >> -} >> - >> -static uint32_t pflash_readw_be(void *opaque, hwaddr addr) >> +static uint64_t pflash_mem_read(void *opaque, hwaddr addr, unsigned len) >> { >> pflash_t *pfl = opaque; >> + bool be = !!(pfl->features & (1 << PFLASH_BE)); > > !!() not needed. Otherwise I don't like magic bool-ification... Is there a coding style item that forbids this idiom in bool assignments? Paolo > Reviewed-by: Peter Crosthwaite > >> >> - return pflash_read(pfl, addr, 2, 1); >> + return pflash_read(pfl, addr, len, be); >> } >> >> -static uint32_t pflash_readw_le(void *opaque, hwaddr addr) >> +static void pflash_mem_write(void *opaque, hwaddr addr, uint64_t value, unsigned len) >> { >> pflash_t *pfl = opaque; >> + bool be = !!(pfl->features & (1 << PFLASH_BE)); >> >> - return pflash_read(pfl, addr, 2, 0); >> + pflash_write(pfl, addr, value, len, be); >> } >> >> -static uint32_t pflash_readl_be(void *opaque, hwaddr addr) >> -{ >> - pflash_t *pfl = opaque; >> - >> - return pflash_read(pfl, addr, 4, 1); >> -} >> - >> -static uint32_t pflash_readl_le(void *opaque, hwaddr addr) >> -{ >> - pflash_t *pfl = opaque; >> - >> - return pflash_read(pfl, addr, 4, 0); >> -} >> - >> -static void pflash_writeb_be(void *opaque, hwaddr addr, >> - uint32_t value) >> -{ >> - pflash_write(opaque, addr, value, 1, 1); >> -} >> - >> -static void pflash_writeb_le(void *opaque, hwaddr addr, >> - uint32_t value) >> -{ >> - pflash_write(opaque, addr, value, 1, 0); >> -} >> - >> -static void pflash_writew_be(void *opaque, hwaddr addr, >> - uint32_t value) >> -{ >> - pflash_t *pfl = opaque; >> - >> - pflash_write(pfl, addr, value, 2, 1); >> -} >> - >> -static void pflash_writew_le(void *opaque, hwaddr addr, >> - uint32_t value) >> -{ >> - pflash_t *pfl = opaque; >> - >> - pflash_write(pfl, addr, value, 2, 0); >> -} >> - >> -static void pflash_writel_be(void *opaque, hwaddr addr, >> - uint32_t value) >> -{ >> - pflash_t *pfl = opaque; >> - >> - pflash_write(pfl, addr, value, 4, 1); >> -} >> - >> -static void pflash_writel_le(void *opaque, hwaddr addr, >> - uint32_t value) >> -{ >> - pflash_t *pfl = opaque; >> - >> - pflash_write(pfl, addr, value, 4, 0); >> -} >> - >> -static const MemoryRegionOps pflash_cfi01_ops_be = { >> - .old_mmio = { >> - .read = { pflash_readb_be, pflash_readw_be, pflash_readl_be, }, >> - .write = { pflash_writeb_be, pflash_writew_be, pflash_writel_be, }, >> - }, >> - .endianness = DEVICE_NATIVE_ENDIAN, >> -}; >> - >> -static const MemoryRegionOps pflash_cfi01_ops_le = { >> - .old_mmio = { >> - .read = { pflash_readb_le, pflash_readw_le, pflash_readl_le, }, >> - .write = { pflash_writeb_le, pflash_writew_le, pflash_writel_le, }, >> - }, >> +static const MemoryRegionOps pflash_cfi01_ops = { >> + .read = pflash_mem_read, >> + .write = pflash_mem_write, >> .endianness = DEVICE_NATIVE_ENDIAN, >> }; >> >> @@ -775,7 +699,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp) >> >> memory_region_init_rom_device( >> &pfl->mem, OBJECT(dev), >> - pfl->features & (1 << PFLASH_BE) ? &pflash_cfi01_ops_be : &pflash_cfi01_ops_le, >> + &pflash_cfi01_ops, >> pfl, >> pfl->name, total_len, &local_err); >> if (local_err) { >> -- >> 2.4.1 >> >> >>